From: Stephan Müller Date: Thu, 21 Feb 2019 16:40:17 +0000 (+0100) Subject: mgr/dashboard: Pgs will update as expected X-Git-Tag: v14.1.1~125^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0d3d16724cb2d11619185bda7f5c90643c7d0892;p=ceph.git 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 --- 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 ba85d2a000e1..b54d287aaf4d 100644 --- 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> { 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 0353855cab62..0a0e6a45ac2f 100644 --- 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('_', ' '),