From: Stephan Müller Date: Mon, 4 May 2020 12:45:52 +0000 (+0200) Subject: mgr/dashboard: Use right size in pool form X-Git-Tag: wip-pdonnell-testing-20200918.022351~1093^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=c76926bcd2fc9ed737b7848239b814f7b75fddc9;p=ceph-ci.git mgr/dashboard: Use right size in pool form Currently the max size is determined by the number of OSDs, which is compared with the maximum of the current crush rule. The problem with that is, that this is wrong for every crush rule that doesn't have OSDs as failure domain and that don't have the root of the cluster set as root of the crush rule. Now the crush map will be used to determine how many failure domains are really available in the cluster and how many can really be used in the end. This number now defines the maximum size you can enter. The crush detail view will now the new attribute usable_size and hide the redundant information steps, ruleset, type and rule_name. Fixes: https://tracker.ceph.com/issues/44620 Signed-off-by: Stephan Müller --- diff --git a/qa/tasks/mgr/dashboard/test_pool.py b/qa/tasks/mgr/dashboard/test_pool.py index 1fd414d103e..c2c25bc1aaf 100644 --- a/qa/tasks/mgr/dashboard/test_pool.py +++ b/qa/tasks/mgr/dashboard/test_pool.py @@ -416,4 +416,5 @@ class PoolTest(DashboardTestCase): 'erasure_code_profiles': JList(JObj({}, allow_unknown=True)), 'used_rules': JObj({}, allow_unknown=True), 'used_profiles': JObj({}, allow_unknown=True), + 'nodes': JList(JObj({}, allow_unknown=True)), })) diff --git a/src/pybind/mgr/dashboard/controllers/pool.py b/src/pybind/mgr/dashboard/controllers/pool.py index 945a82a177a..3e9db8e9c95 100644 --- a/src/pybind/mgr/dashboard/controllers/pool.py +++ b/src/pybind/mgr/dashboard/controllers/pool.py @@ -262,4 +262,5 @@ class PoolUi(Pool): "erasure_code_profiles": profiles, "used_rules": used_rules, "used_profiles": used_profiles, + 'nodes': mgr.get('osd_map_tree')['nodes'] } 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 c49c710e13f..2a3cef16347 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 @@ -142,7 +142,7 @@ The size specified is out of range. A value from - {{ getMinSize() }} to {{ getMaxSize() }} is valid. + {{ getMinSize() }} to {{ getMaxSize() }} is usable. @@ -343,7 +343,8 @@ - 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 d172c7c4563..9d91e08dd6a 100644 --- 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 @@ -18,6 +18,7 @@ import { FixtureHelper, FormHelper, i18nProviders, + Mocks, modalServiceShow } from '../../../../testing/unit-test-helper'; import { NotFoundComponent } from '../../../core/not-found/not-found.component'; @@ -27,7 +28,6 @@ import { PoolService } from '../../../shared/api/pool.service'; import { CriticalConfirmationModalComponent } from '../../../shared/components/critical-confirmation-modal/critical-confirmation-modal.component'; import { SelectBadgesComponent } from '../../../shared/components/select-badges/select-badges.component'; import { CdFormGroup } from '../../../shared/forms/cd-form-group'; -import { CrushRule } from '../../../shared/models/crush-rule'; import { ErasureCodeProfile } from '../../../shared/models/erasure-code-profile'; import { Permission } from '../../../shared/models/permissions'; import { PoolFormInfo } from '../../../shared/models/pool-form-info'; @@ -38,7 +38,7 @@ import { PoolModule } from '../pool.module'; import { PoolFormComponent } from './pool-form.component'; describe('PoolFormComponent', () => { - const OSDS = 8; + let OSDS = 15; let formHelper: FormHelper; let fixtureHelper: FixtureHelper; let component: PoolFormComponent; @@ -65,44 +65,6 @@ describe('PoolFormComponent', () => { expect(form.getValue('pgNum')).toBe(returnValue); }; - const createCrushRule = ({ - id = 0, - name = 'somePoolName', - min = 1, - max = 10, - type = 'replicated' - }: { - max?: number; - min?: number; - id?: number; - name?: string; - type?: string; - }) => { - const typeNumber = type === 'erasure' ? 3 : 1; - const rule = new CrushRule(); - rule.max_size = max; - rule.min_size = min; - rule.rule_id = id; - rule.ruleset = typeNumber; - rule.rule_name = name; - rule.steps = [ - { - item_name: 'default', - item: -1, - op: 'take' - }, - { - num: 0, - type: 'osd', - op: 'choose_firstn' - }, - { - op: 'emit' - } - ]; - return rule; - }; - const expectValidSubmit = ( pool: any, taskName = 'pool/create', @@ -136,12 +98,12 @@ describe('PoolFormComponent', () => { compression_algorithms: ['snappy'], compression_modes: ['none', 'passive'], crush_rules_replicated: [ - createCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }), - createCrushRule({ id: 1, min: 3, max: 18, name: 'rep2', type: 'replicated' }), - createCrushRule({ id: 2, min: 1, max: 9, name: 'used_rule', type: 'replicated' }) + Mocks.getCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }), + Mocks.getCrushRule({ id: 1, min: 3, max: 18, name: 'rep2', type: 'replicated' }), + Mocks.getCrushRule({ id: 2, min: 1, max: 9, name: 'used_rule', type: 'replicated' }) ], crush_rules_erasure: [ - createCrushRule({ id: 3, min: 1, max: 1, name: 'ecp1', type: 'erasure' }) + Mocks.getCrushRule({ id: 3, min: 1, max: 1, name: 'ecp1', type: 'erasure' }) ], erasure_code_profiles: [ecp1], pg_autoscale_default_mode: 'off', @@ -151,7 +113,8 @@ describe('PoolFormComponent', () => { }, used_profiles: { ecp1: ['some.other.pool.uses.it'] - } + }, + nodes: Mocks.generateSimpleCrushMap(3, 5) }; }; @@ -333,15 +296,20 @@ describe('PoolFormComponent', () => { }); it('validates size', () => { + component.info.nodes = Mocks.getCrushMap(); formHelper.setValue('poolType', 'replicated'); formHelper.expectValid('size'); - formHelper.setValue('crushRule', { - min_size: 2, - max_size: 6 - }); + formHelper.setValue('crushRule', Mocks.getCrushRule({ min: 2, max: 6 })); // 3 OSDs usable formHelper.expectErrorChange('size', 1, 'min'); - formHelper.expectErrorChange('size', 8, 'max'); - formHelper.expectValidChange('size', 6); + formHelper.expectErrorChange('size', 4, 'max'); // More than usable + formHelper.expectValidChange('size', 3); + + formHelper.setValue( + 'crushRule', + Mocks.getCrushRule({ min: 1, max: 2, failureDomain: 'osd-rack' }) // 4 OSDs usable + ); + formHelper.expectErrorChange('size', 4, 'max'); // More than rule allows + formHelper.expectValidChange('size', 2); }); it('validates compression mode default value', () => { @@ -455,9 +423,6 @@ describe('PoolFormComponent', () => { describe('pool type changes', () => { beforeEach(() => { component.ngOnInit(); - createCrushRule({ id: 3, min: 1, max: 1, name: 'ep1', type: 'erasure' }); - createCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }); - createCrushRule({ id: 1, min: 3, max: 18, name: 'rep2', type: 'replicated' }); }); it('should have a default replicated size of 3', () => { @@ -516,7 +481,7 @@ describe('PoolFormComponent', () => { it('disables rule field if only one rule exists which is used in the disabled field', () => { infoReturn.crush_rules_replicated = [ - createCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }) + Mocks.getCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }) ]; setUpPoolComponent(); formHelper.setValue('poolType', 'replicated'); @@ -546,10 +511,7 @@ describe('PoolFormComponent', () => { describe('getMaxSize and getMinSize', () => { const setCrushRule = ({ min, max }: { min?: number; max?: number }) => { - formHelper.setValue('crushRule', { - min_size: min, - max_size: max - }); + formHelper.setValue('crushRule', Mocks.getCrushRule({ min, max })); }; it('returns 0 if osd count is 0', () => { @@ -570,23 +532,31 @@ describe('PoolFormComponent', () => { expect(component.getMaxSize()).toBe(6); }); - it('returns 1 as minimum and the osd count as maximum if no crush rule is available', () => { + it('returns 1 as minimum and 3 as maximum if no crush rule is available', () => { expect(component.getMinSize()).toBe(1); - expect(component.getMaxSize()).toBe(OSDS); + expect(component.getMaxSize()).toBe(3); }); it('returns the osd count as maximum if the rule maximum exceeds it', () => { setCrushRule({ max: 100 }); - expect(component.getMaxSize()).toBe(OSDS); + expect(component.getMaxSize()).toBe(15); }); it('should return the osd count as minimum if its lower the the rule minimum', () => { - setCrushRule({ min: 10 }); - expect(component.getMinSize()).toBe(10); + setCrushRule({ min: 20 }); + expect(component.getMinSize()).toBe(20); const control = form.get('crushRule'); expect(control.invalid).toBe(true); formHelper.expectError(control, 'tooFewOsds'); }); + + it('should get the right maximum if the device type is defined', () => { + formHelper.setValue( + 'crushRule', + Mocks.getCrushRule({ min: 1, max: 5, itemName: 'default~ssd' }) + ); + expect(form.getValue('crushRule').usable_size).toBe(5); + }); }); describe('application metadata', () => { @@ -712,6 +682,7 @@ describe('PoolFormComponent', () => { describe('pgCalc', () => { const PGS = 1; + OSDS = 8; const getValidCase = () => ({ type: 'replicated', @@ -818,7 +789,7 @@ describe('PoolFormComponent', () => { } }; }); - infoReturn.crush_rules_replicated.push(createCrushRule({ id: 8, name })); + infoReturn.crush_rules_replicated.push(Mocks.getCrushRule({ id: 8, name })); component.addCrushRule(); expect(form.getValue('crushRule').rule_name).toBe(name); }); @@ -1331,7 +1302,7 @@ describe('PoolFormComponent', () => { pool.quota_max_bytes = 1024 * 1024 * 1024; pool.quota_max_objects = 3000; - createCrushRule({ name: 'someRule' }); + Mocks.getCrushRule({ name: 'someRule' }); spyOn(poolService, 'get').and.callFake(() => of(pool)); }); 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 ab5b26a5742..8bdc4f06622 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 @@ -12,6 +12,7 @@ import { Observable, Subscription } from 'rxjs'; import { CrushRuleService } from '../../../shared/api/crush-rule.service'; import { ErasureCodeProfileService } from '../../../shared/api/erasure-code-profile.service'; import { PoolService } from '../../../shared/api/pool.service'; +import { CrushNodeSelectionClass } from '../../../shared/classes/crush.node.selection.class'; import { CriticalConfirmationModalComponent } from '../../../shared/components/critical-confirmation-modal/critical-confirmation-modal.component'; import { SelectOption } from '../../../shared/components/select/select-option.model'; import { ActionLabelsI18n, URLVerbs } from '../../../shared/constants/app.constants'; @@ -340,6 +341,7 @@ export class PoolFormComponent extends CdForm implements OnInit { if (!rule) { return; } + this.setCorrectMaxSize(rule); this.crushRuleIsUsedBy(rule.rule_name); this.replicatedRuleChange(); this.pgCalc(); @@ -431,17 +433,16 @@ export class PoolFormComponent extends CdForm implements OnInit { } getMaxSize(): number { - if (!this.info || this.info.osd_count < 1) { + const rule = this.form.getValue('crushRule'); + if (!this.info) { return 0; } - const osds: number = this.info.osd_count; - if (this.form.getValue('crushRule')) { - const max: number = this.form.get('crushRule').value.max_size; - if (max < osds) { - return max; - } + if (!rule) { + const osds = this.info.osd_count; + const defaultSize = 3; + return Math.min(osds, defaultSize); } - return osds; + return rule.usable_size; } private pgCalc() { @@ -462,6 +463,19 @@ export class PoolFormComponent extends CdForm implements OnInit { } } + private setCorrectMaxSize(rule: CrushRule = this.form.getValue('crushRule')) { + if (!rule) { + return; + } + const domains = CrushNodeSelectionClass.searchFailureDomains( + this.info.nodes, + rule.steps[0].item_name + ); + const currentDomain = domains[rule.steps[1].type]; + const usable = currentDomain ? currentDomain.length : rule.max_size; + rule.usable_size = Math.min(usable, rule.max_size); + } + private replicatedPgCalc(pgs: number): number { const sizeControl = this.form.get('size'); const size = sizeControl.value; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts index c7c6d56ca0b..83c1db6b6df 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts @@ -2,6 +2,7 @@ import { CrushStep } from './crush-step'; export class CrushRule { max_size: number; + usable_size?: number; min_size: number; rule_id: number; rule_name: string; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts index 4eb09e67c3c..c5cc0bb6d83 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts @@ -1,3 +1,4 @@ +import { CrushNode } from './crush-node'; import { CrushRule } from './crush-rule'; import { ErasureCodeProfile } from './erasure-code-profile'; @@ -15,4 +16,5 @@ export class PoolFormInfo { erasure_code_profiles: ErasureCodeProfile[]; used_rules: { [rule_name: string]: string[] }; used_profiles: { [profile_name: string]: string[] }; + nodes: CrushNode[]; }