From: Patrick Nawracay <pnawracay@suse.com>
Date: Tue, 18 Jun 2019 11:57:46 +0000 (+0200)
Subject: mgr/dashboard: Interlock `fast-diff` and `object-map`
X-Git-Tag: v15.1.0~2314^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dfeba0b1399f13bca9f9c2fe2a77812a33f1e2bd;p=ceph.git

mgr/dashboard: Interlock `fast-diff` and `object-map`

Fixes: http://tracker.ceph.com/issues/39451

Signed-off-by: Patrick Nawracay <pnawracay@suse.com>
---

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-feature.interface.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-feature.interface.ts
new file mode 100644
index 000000000000..c12975f05f1f
--- /dev/null
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-feature.interface.ts
@@ -0,0 +1,9 @@
+export interface RbdImageFeature {
+  desc: string;
+  allowEnable: boolean;
+  allowDisable: boolean;
+  requires?: string;
+  interlockedWith?: string;
+  key?: string;
+  initDisabled?: boolean;
+}
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.html
index b567ddd96eae..2a458b87fa47 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.html
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.html
@@ -184,7 +184,7 @@
 
         <!-- Features -->
         <div class="form-group"
-             [ngClass]="{'has-error': (formDir.submitted || featuresFormGroups.dirty) && featuresFormGroups.invalid}"
+             [ngClass]="{'has-error': (formDir.submitted || rbdForm.get('features').dirty) && rbdForm.get('features').invalid}"
              formGroupName="features">
           <label i18n
                  class="col-sm-3 control-label"
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts
index 9d15d10cfe69..39a0125cd27c 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts
@@ -1,13 +1,14 @@
 import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { ReactiveFormsModule } from '@angular/forms';
-import { ActivatedRoute } from '@angular/router';
+import { ActivatedRoute, Router } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
 import { TooltipModule } from 'ngx-bootstrap/tooltip';
 
 import { ToastModule } from 'ng2-toastr';
 
 import { By } from '@angular/platform-browser';
+import { of } from 'rxjs';
 import { ActivatedRouteStub } from '../../../../testing/activated-route-stub';
 import { configureTestBed, i18nProviders } from '../../../../testing/unit-test-helper';
 import { RbdService } from '../../../shared/api/rbd.service';
@@ -21,6 +22,9 @@ describe('RbdFormComponent', () => {
   let fixture: ComponentFixture<RbdFormComponent>;
   let activatedRoute: ActivatedRouteStub;
 
+  const queryNativeElement = (cssSelector) =>
+    fixture.debugElement.query(By.css(cssSelector)).nativeElement;
+
   configureTestBed({
     imports: [
       HttpClientTestingModule,
@@ -36,7 +40,8 @@ describe('RbdFormComponent', () => {
         provide: ActivatedRoute,
         useValue: new ActivatedRouteStub({ pool: 'foo', name: 'bar', snap: undefined })
       },
-      i18nProviders
+      i18nProviders,
+      RbdService
     ]
   });
 
@@ -78,10 +83,91 @@ describe('RbdFormComponent', () => {
   describe('test image configuration component', () => {
     it('is visible', () => {
       fixture.detectChanges();
-      expect(
-        fixture.debugElement.query(By.css('cd-rbd-configuration-form')).nativeElement.parentElement
-          .hidden
-      ).toBe(false);
+      expect(queryNativeElement('cd-rbd-configuration-form').parentElement.hidden).toBe(false);
+    });
+  });
+
+  describe('tests for feature flags', () => {
+    let deepFlatten, layering, exclusiveLock, objectMap, journaling, fastDiff;
+    const defaultFeatures = [
+      // Supposed to be enabled by default
+      'deep-flatten',
+      'exclusive-lock',
+      'fast-diff',
+      'layering',
+      'object-map'
+    ];
+    const allFeatureNames = [
+      'deep-flatten',
+      'layering',
+      'exclusive-lock',
+      'object-map',
+      'journaling',
+      'fast-diff'
+    ];
+    const setFeatures = (features) => {
+      component.features = features;
+      component.featuresList = component.objToArray(features);
+      component.createForm();
+    };
+    const getFeatureNativeElements = () => allFeatureNames.map((f) => queryNativeElement(`#${f}`));
+
+    it('should convert feature flags correctly in the constructor', () => {
+      setFeatures({
+        one: { desc: 'one', allowEnable: true, allowDisable: true },
+        two: { desc: 'two', allowEnable: true, allowDisable: true },
+        three: { desc: 'three', allowEnable: true, allowDisable: true }
+      });
+      expect(component.featuresList).toEqual([
+        { desc: 'one', key: 'one', allowDisable: true, allowEnable: true },
+        { desc: 'two', key: 'two', allowDisable: true, allowEnable: true },
+        { desc: 'three', key: 'three', allowDisable: true, allowEnable: true }
+      ]);
+    });
+
+    describe('test create form flags', () => {
+      beforeEach(() => {
+        const rbdService = TestBed.get(RbdService);
+        spyOn(rbdService, 'defaultFeatures').and.returnValue(of(defaultFeatures));
+        component.router = { url: '/block/rbd/create' } as Router;
+        fixture.detectChanges();
+        [
+          deepFlatten,
+          layering,
+          exclusiveLock,
+          objectMap,
+          journaling,
+          fastDiff
+        ] = getFeatureNativeElements();
+      });
+
+      it('should initialize the checkboxes correctly', () => {
+        expect(deepFlatten.disabled).toBe(false);
+        expect(layering.disabled).toBe(false);
+        expect(exclusiveLock.disabled).toBe(false);
+        expect(objectMap.disabled).toBe(false);
+        expect(journaling.disabled).toBe(false);
+        expect(fastDiff.disabled).toBe(false);
+
+        expect(deepFlatten.checked).toBe(true);
+        expect(layering.checked).toBe(true);
+        expect(exclusiveLock.checked).toBe(true);
+        expect(objectMap.checked).toBe(true);
+        expect(journaling.checked).toBe(false);
+        expect(fastDiff.checked).toBe(true);
+      });
+
+      it('should disable features if their requirements are not met (exclusive-lock)', () => {
+        exclusiveLock.click(); // unchecks exclusive-lock
+        expect(objectMap.disabled).toBe(true);
+        expect(journaling.disabled).toBe(true);
+        expect(fastDiff.disabled).toBe(true);
+      });
+
+      it('should disable features if their requirements are not met (object-map)', () => {
+        objectMap.click(); // unchecks object-map
+        expect(fastDiff.disabled).toBe(true);
+      });
     });
   });
 });
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts
index 8bda29553c72..0e82e8f6200a 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts
@@ -20,6 +20,7 @@ import { DimlessBinaryPipe } from '../../../shared/pipes/dimless-binary.pipe';
 import { AuthStorageService } from '../../../shared/services/auth-storage.service';
 import { FormatterService } from '../../../shared/services/formatter.service';
 import { TaskWrapperService } from '../../../shared/services/task-wrapper.service';
+import { RbdImageFeature } from './rbd-feature.interface';
 import { RbdFormCloneRequestModel } from './rbd-form-clone-request.model';
 import { RbdFormCopyRequestModel } from './rbd-form-copy-request.model';
 import { RbdFormCreateRequestModel } from './rbd-form-create-request.model';
@@ -35,13 +36,6 @@ import { RbdFormResponseModel } from './rbd-form-response.model';
 export class RbdFormComponent implements OnInit {
   poolPermission: Permission;
   rbdForm: CdFormGroup;
-  featuresFormGroups: CdFormGroup;
-  deepFlattenFormControl: FormControl;
-  layeringFormControl: FormControl;
-  exclusiveLockFormControl: FormControl;
-  objectMapFormControl: FormControl;
-  journalingFormControl: FormControl;
-  fastDiffFormControl: FormControl;
   getDirtyConfigurationValues: (
     includeLocalField?: boolean,
     localField?: RbdConfigurationSourceField
@@ -51,8 +45,8 @@ export class RbdFormComponent implements OnInit {
   allPools: Array<string> = null;
   dataPools: Array<string> = null;
   allDataPools: Array<string> = null;
-  features: any;
-  featuresList = [];
+  features: { [key: string]: RbdImageFeature };
+  featuresList: RbdImageFeature[] = [];
   initializeConfigData = new EventEmitter<{
     initialData: RbdConfigurationEntry[];
     sourceType: RbdConfigurationSourceField;
@@ -92,14 +86,14 @@ export class RbdFormComponent implements OnInit {
   constructor(
     private authStorageService: AuthStorageService,
     private route: ActivatedRoute,
-    private router: Router,
     private poolService: PoolService,
     private rbdService: RbdService,
     private formatter: FormatterService,
     private taskWrapper: TaskWrapperService,
     private dimlessBinaryPipe: DimlessBinaryPipe,
     private i18n: I18n,
-    public actionLabels: ActionLabelsI18n
+    public actionLabels: ActionLabelsI18n,
+    public router: Router
   ) {
     this.poolPermission = this.authStorageService.getPermissions().pool;
     this.resource = this.i18n('RBD');
@@ -126,44 +120,34 @@ export class RbdFormComponent implements OnInit {
         desc: this.i18n('Object map (requires exclusive-lock)'),
         requires: 'exclusive-lock',
         allowEnable: true,
-        allowDisable: true
+        allowDisable: true,
+        initDisabled: true
       },
       journaling: {
         desc: this.i18n('Journaling (requires exclusive-lock)'),
         requires: 'exclusive-lock',
         allowEnable: true,
-        allowDisable: true
+        allowDisable: true,
+        initDisabled: true
       },
       'fast-diff': {
-        desc: this.i18n('Fast diff (requires object-map)'),
+        desc: this.i18n('Fast diff (interlocked with object-map)'),
         requires: 'object-map',
         allowEnable: true,
-        allowDisable: true
+        allowDisable: true,
+        interlockedWith: 'object-map',
+        initDisabled: true
       }
     };
+    this.featuresList = this.objToArray(this.features);
     this.createForm();
-    for (const key of Object.keys(this.features)) {
-      const listItem = this.features[key];
-      listItem.key = key;
-      this.featuresList.push(listItem);
-    }
+  }
+
+  objToArray(obj: { [key: string]: any }) {
+    return _.map(obj, (o, key) => Object.assign(o, { key: key }));
   }
 
   createForm() {
-    this.deepFlattenFormControl = new FormControl(false);
-    this.layeringFormControl = new FormControl(false);
-    this.exclusiveLockFormControl = new FormControl(false);
-    this.objectMapFormControl = new FormControl({ value: false, disabled: true });
-    this.journalingFormControl = new FormControl({ value: false, disabled: true });
-    this.fastDiffFormControl = new FormControl({ value: false, disabled: true });
-    this.featuresFormGroups = new CdFormGroup({
-      'deep-flatten': this.deepFlattenFormControl,
-      layering: this.layeringFormControl,
-      'exclusive-lock': this.exclusiveLockFormControl,
-      'object-map': this.objectMapFormControl,
-      journaling: this.journalingFormControl,
-      'fast-diff': this.fastDiffFormControl
-    });
     this.rbdForm = new CdFormGroup(
       {
         parent: new FormControl(''),
@@ -179,7 +163,12 @@ export class RbdFormComponent implements OnInit {
           updateOn: 'blur'
         }),
         obj_size: new FormControl(this.defaultObjectSize),
-        features: this.featuresFormGroups,
+        features: new CdFormGroup(
+          this.featuresList.reduce((acc, e) => {
+            acc[e.key] = new FormControl({ value: false, disabled: !!e.initDisabled });
+            return acc;
+          }, {})
+        ),
         stripingUnit: new FormControl(null),
         stripingCount: new FormControl(null, {
           updateOn: 'blur'
@@ -241,6 +230,7 @@ export class RbdFormComponent implements OnInit {
         });
       });
     } else {
+      // New image
       this.rbdService.defaultFeatures().subscribe((defaultFeatures: Array<string>) => {
         this.setFeatures(defaultFeatures);
       });
@@ -277,23 +267,11 @@ export class RbdFormComponent implements OnInit {
           }
         });
     }
-    this.deepFlattenFormControl.valueChanges.subscribe((value) => {
-      this.watchDataFeatures('deep-flatten', value);
-    });
-    this.layeringFormControl.valueChanges.subscribe((value) => {
-      this.watchDataFeatures('layering', value);
-    });
-    this.exclusiveLockFormControl.valueChanges.subscribe((value) => {
-      this.watchDataFeatures('exclusive-lock', value);
-    });
-    this.objectMapFormControl.valueChanges.subscribe((value) => {
-      this.watchDataFeatures('object-map', value);
-    });
-    this.journalingFormControl.valueChanges.subscribe((value) => {
-      this.watchDataFeatures('journaling', value);
-    });
-    this.fastDiffFormControl.valueChanges.subscribe((value) => {
-      this.watchDataFeatures('fast-diff', value);
+    _.each(this.features, (feature) => {
+      this.rbdForm
+        .get('features')
+        .get(feature.key)
+        .valueChanges.subscribe((value) => this.featureFormUpdate(feature.key, value));
     });
   }
 
@@ -376,41 +354,65 @@ export class RbdFormComponent implements OnInit {
     };
   }
 
+  protected getDependendChildFeatures(featureKey: string) {
+    return _.filter(this.features, (f) => f.requires === featureKey) || [];
+  }
+
   deepBoxCheck(key, checked) {
-    _.forIn(this.features, (details, feature) => {
-      if (details.requires === key) {
-        if (checked) {
-          this.rbdForm.get(feature).enable();
-        } else {
-          this.rbdForm.get(feature).disable();
-          this.rbdForm.get(feature).setValue(checked);
-          this.watchDataFeatures(feature, checked);
-          this.deepBoxCheck(feature, checked);
-        }
+    const childFeatures = this.getDependendChildFeatures(key);
+    childFeatures.forEach((feature) => {
+      const featureControl = this.rbdForm.get(feature.key);
+      if (checked) {
+        featureControl.enable({ emitEvent: false });
+      } else {
+        featureControl.disable({ emitEvent: false });
+        featureControl.setValue(false, { emitEvent: false });
+        this.deepBoxCheck(feature.key, checked);
       }
-      if (this.mode === this.rbdFormMode.editing && this.rbdForm.get(feature).enabled) {
-        if (this.response.features_name.indexOf(feature) !== -1 && !details.allowDisable) {
-          this.rbdForm.get(feature).disable();
-        } else if (this.response.features_name.indexOf(feature) === -1 && !details.allowEnable) {
-          this.rbdForm.get(feature).disable();
+
+      const featureFormGroup = this.rbdForm.get('features');
+      if (this.mode === this.rbdFormMode.editing && featureFormGroup.get(feature.key).enabled) {
+        if (this.response.features_name.indexOf(feature.key) !== -1 && !feature.allowDisable) {
+          featureFormGroup.get(feature.key).disable();
+        } else if (
+          this.response.features_name.indexOf(feature.key) === -1 &&
+          !feature.allowEnable
+        ) {
+          featureFormGroup.get(feature.key).disable();
         }
       }
     });
   }
 
+  interlockCheck(key, checked) {
+    if (checked) {
+      _.filter(this.features, (f) => f.interlockedWith === key).forEach((f) =>
+        this.rbdForm.get(f.key).setValue(true, { emitEvent: false })
+      );
+    } else {
+      if (feature.interlockedWith) {
+        // Don't skip emitting the event here, as it prevents `fast-diff` from
+        // becoming disabled when manually unchecked.  This is because it
+        // triggers an update on `object-map` and if `object-map` doesn't emit,
+        // `fast-diff` will not be automatically disabled.
+        this.rbdForm
+          .get('features')
+          .get(feature.interlockedWith)
+          .setValue(false);
+      }
+    }
+  }
+
   featureFormUpdate(key, checked) {
     if (checked) {
       const required = this.features[key].requires;
       if (required && !this.rbdForm.getValue(required)) {
-        this.rbdForm.get(key).setValue(false);
+        this.rbdForm.get(`features.${key}`).setValue(false);
         return;
       }
     }
     this.deepBoxCheck(key, checked);
-  }
-
-  watchDataFeatures(key, checked) {
-    this.featureFormUpdate(key, checked);
+    this.interlockCheck(key, checked);
   }
 
   setFeatures(features: Array<string>) {
@@ -419,7 +421,7 @@ export class RbdFormComponent implements OnInit {
       if (features.indexOf(feature.key) !== -1) {
         featuresControl.get(feature.key).setValue(true);
       }
-      this.watchDataFeatures(feature.key, featuresControl.get(feature.key).value);
+      this.featureFormUpdate(feature.key, featuresControl.get(feature.key).value);
     });
   }