mgr/dashboard: Pgs will update as expected

The problem was that PGs jumped over the next calculated number when
pressing "up".

Fixes: https://tracker.ceph.com/issues/38382
Signed-off-by: Stephan Müller <smueller@suse.com>

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
index ba85d2a000e15027e8468e89433814248142bd5d..b54d287aaf4d117d7bf835cec254f4b58fbdabd9 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
@@ -87,8 +87,7 @@
                      min="1"
                      type="number"
                      (focus)="externalPgChange = false"
-                     (keyup)="pgKeyUp($event)"
-                     (blur)="pgUpdate()"
+                     (blur)="alignPgs()"
                      required>
               <span class="help-block"
                     *ngIf="form.showError('pgNum', formDir, 'required')"

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
index 2c861cb26062adcbaf46cea13843738e0e4cd1c5..c61ce23423960f82dd19a48127897b1cd720bc00 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
@@ -37,9 +37,7 @@ describe('PoolFormComponent', () => {
   let ecpService: ErasureCodeProfileService;
 
   const setPgNum = (pgs): AbstractControl => {
-    formHelper.setValue('poolType', 'erasure');
     const control = formHelper.setValue('pgNum', pgs);
-    fixture.detectChanges();
     fixture.debugElement.query(By.css('#pgNum')).nativeElement.dispatchEvent(new Event('blur'));
     return control;
   };
@@ -212,7 +210,7 @@ describe('PoolFormComponent', () => {
 
   describe('pool form validation', () => {
     beforeEach(() => {
-      component.ngOnInit();
+      fixture.detectChanges();
     });
 
     it('is invalid at the beginning all sub forms are valid', () => {
@@ -244,38 +242,24 @@ describe('PoolFormComponent', () => {
       formHelper.expectValidChange('poolType', 'replicated');
     });
 
-    it('validates pgNum in creation mode', () => {
+    it('validates that pgNum is required creation mode', () => {
       formHelper.expectError(form.get('pgNum'), 'required');
-      formHelper.setValue('poolType', 'erasure');
-      formHelper.expectValid(setPgNum(-28));
-      expect(form.getValue('pgNum')).toBe(1);
-      formHelper.expectValid(setPgNum(15));
-      expect(form.getValue('pgNum')).toBe(16);
-    });
-
-    it('increases pgNum by the power of two for if the value has changed by one', () => {
-      setPgNum('16');
-      expect(setPgNum(17).value).toBe(32);
-      expect(setPgNum(31).value).toBe(16);
-    });
-
-    it('not increases pgNum by more than one but lower than the next pg update change', () => {
-      setPgNum('16');
-      expect(setPgNum('18').value).toBe(16);
-      expect(setPgNum('14').value).toBe(16);
     });
 
     it('validates pgNum in edit mode', () => {
       component.data.pool = new Pool('test');
       component.data.pool.pg_num = 16;
       component.editing = true;
-      component.ngOnInit();
+      component.ngOnInit(); // Switches form into edit mode
+      formHelper.setValue('poolType', 'erasure');
+      fixture.detectChanges();
       formHelper.expectError(setPgNum('8'), 'noDecrease');
     });
 
     it('is valid if pgNum, poolType and name are valid', () => {
       formHelper.setValue('name', 'some-name');
       formHelper.setValue('poolType', 'erasure');
+      fixture.detectChanges();
       setPgNum(1);
       expect(form.valid).toBeTruthy();
     });
@@ -588,16 +572,84 @@ describe('PoolFormComponent', () => {
   });
 
   describe('pg number changes', () => {
+    const setPgs = (pgs) => {
+      formHelper.setValue('pgNum', pgs);
+      fixture.debugElement.query(By.css('#pgNum')).nativeElement.dispatchEvent(new Event('blur'));
+    };
+
+    const testPgUpdate = (pgs, jump, returnValue) => {
+      if (pgs) {
+        setPgs(pgs);
+      }
+      if (jump) {
+        setPgs(form.getValue('pgNum') + jump);
+      }
+      expect(form.getValue('pgNum')).toBe(returnValue);
+    };
+
     beforeEach(() => {
       formHelper.setValue('crushRule', {
         min_size: 1,
         max_size: 20
       });
-      component.ngOnInit();
-      // triggers pgUpdate
+      formHelper.setValue('poolType', 'erasure');
+      fixture.detectChanges();
       setPgNum(256);
     });
 
+    it('updates by value', () => {
+      testPgUpdate(10, undefined, 8);
+      testPgUpdate(22, undefined, 16);
+      testPgUpdate(26, undefined, 32);
+      testPgUpdate(200, undefined, 256);
+      testPgUpdate(300, undefined, 256);
+      testPgUpdate(350, undefined, 256);
+    });
+
+    it('updates by jump -> a magnitude of the power of 2', () => {
+      testPgUpdate(undefined, 1, 512);
+      testPgUpdate(undefined, -1, 256);
+    });
+
+    it('returns 1 as minimum for false numbers', () => {
+      testPgUpdate(-26, undefined, 1);
+      testPgUpdate(0, undefined, 1);
+      testPgUpdate(0, -1, 1);
+      testPgUpdate(undefined, -20, 1);
+    });
+
+    it('changes the value and than jumps', () => {
+      testPgUpdate(230, 1, 512);
+      testPgUpdate(3500, -1, 2048);
+    });
+
+    describe('pg power jump', () => {
+      it('should jump correctly at the beginning', () => {
+        testPgUpdate(1, -1, 1);
+        testPgUpdate(1, 1, 2);
+        testPgUpdate(2, -1, 1);
+        testPgUpdate(2, 1, 4);
+        testPgUpdate(4, -1, 2);
+        testPgUpdate(4, 1, 8);
+        testPgUpdate(4, 1, 8);
+      });
+
+      it('increments pg power if difference to the current number is 1', () => {
+        testPgUpdate(undefined, 1, 512);
+        testPgUpdate(undefined, 1, 1024);
+        testPgUpdate(undefined, 1, 2048);
+        testPgUpdate(undefined, 1, 4096);
+      });
+
+      it('decrements pg power if difference to the current number is -1', () => {
+        testPgUpdate(undefined, -1, 128);
+        testPgUpdate(undefined, -1, 64);
+        testPgUpdate(undefined, -1, 32);
+        testPgUpdate(undefined, -1, 16);
+        testPgUpdate(undefined, -1, 8);
+      });
+    });
+
     describe('pgCalc', () => {
       const PGS = 1;
 
@@ -683,68 +735,6 @@ describe('PoolFormComponent', () => {
         testPgCalc(test);
       });
     });
-
-    describe('pgUpdate', () => {
-      const testPgUpdate = (pgs, jump, returnValue) => {
-        component['pgUpdate'](pgs, jump);
-        expect(form.getValue('pgNum')).toBe(returnValue);
-      };
-
-      it('updates by value', () => {
-        testPgUpdate(10, undefined, 8);
-        testPgUpdate(22, undefined, 16);
-        testPgUpdate(26, undefined, 32);
-      });
-
-      it('updates by jump -> a magnitude of the power of 2', () => {
-        testPgUpdate(undefined, 1, 512);
-        testPgUpdate(undefined, -1, 256);
-        testPgUpdate(undefined, -2, 64);
-        testPgUpdate(undefined, -10, 1);
-      });
-
-      it('returns 1 as minimum for false numbers', () => {
-        testPgUpdate(-26, undefined, 1);
-        testPgUpdate(0, undefined, 1);
-        testPgUpdate(undefined, -20, 1);
-      });
-
-      it('uses by value and jump', () => {
-        testPgUpdate(330, 0, 256);
-        testPgUpdate(230, 2, 1024);
-        testPgUpdate(230, 3, 2048);
-      });
-    });
-
-    describe('pgKeyUp', () => {
-      const testPgKeyUp = (keyName, returnValue) => {
-        component.pgKeyUp({ key: keyName });
-        expect(form.getValue('pgNum')).toBe(returnValue);
-      };
-
-      it('does nothing with unrelated keys', () => {
-        testPgKeyUp('0', 256);
-        testPgKeyUp(',', 256);
-        testPgKeyUp('a', 256);
-        testPgKeyUp('Space', 256);
-        testPgKeyUp('ArrowLeft', 256);
-        testPgKeyUp('ArrowRight', 256);
-      });
-
-      it('increments by jump with plus or ArrowUp', () => {
-        testPgKeyUp('ArrowUp', 512);
-        testPgKeyUp('ArrowUp', 1024);
-        testPgKeyUp('+', 2048);
-        testPgKeyUp('+', 4096);
-      });
-
-      it('decrement by jump with minus or ArrowDown', () => {
-        testPgKeyUp('ArrowDown', 128);
-        testPgKeyUp('ArrowDown', 64);
-        testPgKeyUp('-', 32);
-        testPgKeyUp('-', 16);
-      });
-    });
   });
 
   describe('crushRule', () => {
@@ -1018,7 +1008,7 @@ describe('PoolFormComponent', () => {
     describe('after ngOnInit', () => {
       beforeEach(() => {
         component.editing = true;
-        component.ngOnInit();
+        fixture.detectChanges();
       });
 
       it('disabled inputs', () => {

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
index 0353855cab62b74d5773a0eb9da6e4a39119cd0a..0a0e6a45ac2ffc0ac3341b8edf52bfc15b158d33 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
@@ -237,12 +237,29 @@ export class PoolFormComponent implements OnInit {
   private listenToChangesDuringAddEdit() {
     this.form.get('pgNum').valueChanges.subscribe((pgs) => {
       const change = pgs - this.data.pgs;
-      if (Math.abs(change) === 1) {
-        this.pgUpdate(undefined, change);
+      if (Math.abs(change) !== 1 || pgs === 2) {
+        this.data.pgs = pgs;
+        return;
       }
+      this.doPgPowerJump(change as 1 | -1);
     });
   }
 
+  private doPgPowerJump(jump: 1 | -1) {
+    const power = this.calculatePgPower() + jump;
+    this.setPgs(jump === -1 ? Math.round(power) : Math.floor(power));
+  }
+
+  private calculatePgPower(pgs = this.form.getValue('pgNum')): number {
+    return Math.log(pgs) / Math.log(2);
+  }
+
+  private setPgs(power: number) {
+    const pgs = Math.pow(2, power < 0 ? 0 : power); // Set size the nearest accurate size.
+    this.data.pgs = pgs;
+    this.form.silentSet('pgNum', pgs);
+  }
+
   private listenToChangesDuringAdd() {
     this.form.get('poolType').valueChanges.subscribe((poolType) => {
       this.form.get('size').updateValueAndValidity();
@@ -350,7 +367,7 @@ export class PoolFormComponent implements OnInit {
       return;
     }
     const oldValue = this.data.pgs;
-    this.pgUpdate(pgs);
+    this.alignPgs(pgs);
     const newValue = this.data.pgs;
     if (!this.externalPgChange) {
       this.externalPgChange = oldValue !== newValue;
@@ -373,21 +390,8 @@ export class PoolFormComponent implements OnInit {
     }
   }
 
-  private pgUpdate(pgs?, jump?) {
-    pgs = _.isNumber(pgs) ? pgs : this.form.getValue('pgNum');
-    if (pgs < 1) {
-      pgs = 1;
-    }
-    let power = Math.round(Math.log(pgs) / Math.log(2));
-    if (_.isNumber(jump)) {
-      power += jump;
-    }
-    if (power < 0) {
-      power = 0;
-    }
-    pgs = Math.pow(2, power); // Set size the nearest accurate size.
-    this.data.pgs = pgs;
-    this.form.silentSet('pgNum', pgs);
+  private alignPgs(pgs = this.form.getValue('pgNum')) {
+    this.setPgs(Math.round(this.calculatePgPower(pgs < 1 ? 1 : pgs)));
   }
 
   private setComplexValidators() {
@@ -468,15 +472,6 @@ export class PoolFormComponent implements OnInit {
     return this.form.getValue('mode') && this.form.get('mode').value.toLowerCase() !== 'none';
   }
 
-  pgKeyUp($e) {
-    const key = $e.key;
-    const included = (arr: string[]): number => (arr.indexOf(key) !== -1 ? 1 : 0);
-    const jump = included(['ArrowUp', '+']) - included(['ArrowDown', '-']);
-    if (jump) {
-      this.pgUpdate(undefined, jump);
-    }
-  }
-
   describeCrushStep(step: CrushStep) {
     return [
       step.op.replace('_', ' '),