From: Sage Weil Date: Tue, 29 Jun 2021 21:01:23 +0000 (-0400) Subject: mgr/dashboard: remove max/min_size and ruleset X-Git-Tag: v17.1.0~1398^2~6 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4d91a3d4936fbe099a82b56426d5d7c18e05d9de;p=ceph.git mgr/dashboard: remove max/min_size and ruleset Signed-off-by: Sage Weil Signed-off-by: Avan Thakkar --- diff --git a/qa/tasks/mgr/dashboard/test_crush_rule.py b/qa/tasks/mgr/dashboard/test_crush_rule.py index 1e37553b24a..aa2250b1d2a 100644 --- a/qa/tasks/mgr/dashboard/test_crush_rule.py +++ b/qa/tasks/mgr/dashboard/test_crush_rule.py @@ -10,11 +10,8 @@ class CrushRuleTest(DashboardTestCase): AUTH_ROLES = ['pool-manager'] rule_schema = JObj(sub_elems={ - 'max_size': int, - 'min_size': int, 'rule_id': int, 'rule_name': str, - 'ruleset': int, 'steps': JList(JObj({}, allow_unknown=True)) }, allow_unknown=True) @@ -24,7 +21,7 @@ class CrushRuleTest(DashboardTestCase): self._post('/api/crush_rule', data) self.assertStatus(201) # Makes sure rule exists - rule = self._get('/api/crush_rule/{}'.format(name)) + rule = self._get('/api/crush_rule/{}'.format(name), version='2.0') self.assertStatus(200) self.assertSchemaBody(self.rule_schema) self.assertEqual(rule['rule_name'], name) @@ -34,12 +31,12 @@ class CrushRuleTest(DashboardTestCase): @DashboardTestCase.RunAs('test', 'test', ['rgw-manager']) def test_read_access_permissions(self): - self._get('/api/crush_rule') + self._get('/api/crush_rule', version='2.0') self.assertStatus(403) @DashboardTestCase.RunAs('test', 'test', ['read-only']) def test_write_access_permissions(self): - self._get('/api/crush_rule') + self._get('/api/crush_rule', version='2.0') self.assertStatus(200) data = {'name': 'some_rule', 'root': 'default', 'failure_domain': 'osd'} self._post('/api/crush_rule', data) @@ -54,7 +51,7 @@ class CrushRuleTest(DashboardTestCase): cls._ceph_cmd(['osd', 'crush', 'rule', 'rm', 'another_rule']) def test_list(self): - self._get('/api/crush_rule') + self._get('/api/crush_rule', version='2.0') self.assertStatus(200) self.assertSchemaBody(JList(self.rule_schema)) diff --git a/src/pybind/mgr/dashboard/controllers/crush_rule.py b/src/pybind/mgr/dashboard/controllers/crush_rule.py index 13986f38413..57f05fd39e0 100644 --- a/src/pybind/mgr/dashboard/controllers/crush_rule.py +++ b/src/pybind/mgr/dashboard/controllers/crush_rule.py @@ -11,10 +11,7 @@ from . import ApiController, ControllerDoc, Endpoint, EndpointDoc, \ LIST_SCHEMA = { "rule_id": (int, 'Rule ID'), "rule_name": (str, 'Rule Name'), - "ruleset": (int, 'RuleSet related to the rule'), "type": (int, 'Type of Rule'), - "min_size": (int, 'Minimum size of Rule'), - "max_size": (int, 'Maximum size of Rule'), 'steps': ([{str}], 'Steps included in the rule') } @@ -24,9 +21,11 @@ LIST_SCHEMA = { class CrushRule(RESTController): @EndpointDoc("List Crush Rule Configuration", responses={200: LIST_SCHEMA}) + @RESTController.MethodMap(version='2.0') def list(self): return mgr.get('osd_map_crush')['rules'] + @RESTController.MethodMap(version='2.0') def get(self, name): rules = mgr.get('osd_map_crush')['rules'] for r in rules: 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 dd45a461645..5b8b1459ea6 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 @@ -369,7 +369,7 @@ i18n>Crush rule 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 1d58a1778ce..87b791b6903 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 @@ -105,13 +105,11 @@ describe('PoolFormComponent', () => { compression_algorithms: ['snappy'], compression_modes: ['none', 'passive'], crush_rules_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: [ - Mocks.getCrushRule({ id: 3, min: 1, max: 1, name: 'ecp1', type: 'erasure' }) + Mocks.getCrushRule({ id: 0, name: 'rep1', type: 'replicated' }), + Mocks.getCrushRule({ id: 1, name: 'rep2', type: 'replicated' }), + Mocks.getCrushRule({ id: 2, name: 'used_rule', type: 'replicated' }) ], + crush_rules_erasure: [Mocks.getCrushRule({ id: 3, name: 'ecp1', type: 'erasure' })], erasure_code_profiles: [ecp1], pg_autoscale_default_mode: 'off', pg_autoscale_modes: ['off', 'warn', 'on'], @@ -289,7 +287,6 @@ describe('PoolFormComponent', () => { formHelper.expectValidChange('poolType', 'replicated'); form.get('crushRule').updateValueAndValidity(); formHelper.expectError('crushRule', 'required'); // As multiple rules exist - formHelper.expectErrorChange('crushRule', { min_size: 20 }, 'tooFewOsds'); }); it('validates crushRule with no crush rules', () => { @@ -303,17 +300,6 @@ describe('PoolFormComponent', () => { component.info.nodes = Mocks.getCrushMap(); formHelper.setValue('poolType', 'replicated'); formHelper.expectValid('size'); - formHelper.setValue('crushRule', Mocks.getCrushRule({ min: 2, max: 6 })); // 3 OSDs usable - formHelper.expectErrorChange('size', 1, 'min'); - 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 if warning is displayed when size is 1', () => { @@ -460,13 +446,13 @@ describe('PoolFormComponent', () => { it('should set size to maximum if size exceeds maximum', () => { formHelper.setValue('crushRule', component.info.crush_rules_replicated[0]); - expect(form.getValue('size')).toBe(4); + expect(form.getValue('size')).toBe(10); }); it('should set size to minimum if size is lower than minimum', () => { formHelper.setValue('size', -1); formHelper.setValue('crushRule', component.info.crush_rules_replicated[0]); - expect(form.getValue('size')).toBe(2); + expect(form.getValue('size')).toBe(1); }); }); @@ -496,7 +482,7 @@ describe('PoolFormComponent', () => { it('disables rule field if only one rule exists which is used in the disabled field', () => { infoReturn.crush_rules_replicated = [ - Mocks.getCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }) + Mocks.getCrushRule({ id: 0, name: 'rep1', type: 'replicated' }) ]; setUpPoolComponent(); formHelper.setValue('poolType', 'replicated'); @@ -525,10 +511,6 @@ describe('PoolFormComponent', () => { }); describe('getMaxSize and getMinSize', () => { - const setCrushRule = ({ min, max }: { min?: number; max?: number }) => { - formHelper.setValue('crushRule', Mocks.getCrushRule({ min, max })); - }; - it('returns 0 if osd count is 0', () => { component.info.osd_count = 0; expect(component.getMinSize()).toBe(0); @@ -541,36 +523,22 @@ describe('PoolFormComponent', () => { expect(component.getMaxSize()).toBe(0); }); - it('returns minimum and maximum of rule', () => { - setCrushRule({ min: 2, max: 6 }); - expect(component.getMinSize()).toBe(2); - expect(component.getMaxSize()).toBe(6); - }); - it('returns 1 as minimum and 3 as maximum if no crush rule is available', () => { expect(component.getMinSize()).toBe(1); 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(15); - }); - it('should return the osd count as minimum if its lower the the rule minimum', () => { - setCrushRule({ min: 20 }); - expect(component.getMinSize()).toBe(20); + component.info.osd_count = 0; + formHelper.setValue('crushRule', component.info.crush_rules_replicated[0]); 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); + formHelper.setValue('crushRule', Mocks.getCrushRule({ itemName: 'default~ssd' })); + expect(form.getValue('crushRule').usable_size).toBe(10); }); }); 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 a3cec385485..ae380cf7400 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 @@ -82,6 +82,7 @@ export class PoolFormComponent extends CdForm implements OnInit { pgAutoscaleModes: string[]; crushUsage: string[] = undefined; // Will only be set if a rule is used by some pool ecpUsage: string[] = undefined; // Will only be set if a rule is used by some pool + crushRuleMaxSize = 10; private modalSubscription: Subscription; @@ -155,7 +156,7 @@ export class PoolFormComponent extends CdForm implements OnInit { validators: [ CdValidators.custom( 'tooFewOsds', - (rule: any) => this.info && rule && this.info.osd_count < rule.min_size + (rule: any) => this.info && rule && this.info.osd_count < 1 ), CdValidators.custom( 'required', @@ -422,10 +423,6 @@ export class PoolFormComponent extends CdForm implements OnInit { if (!this.info || this.info.osd_count < 1) { return 0; } - const rule = this.form.getValue('crushRule'); - if (rule) { - return rule.min_size; - } return 1; } @@ -469,8 +466,8 @@ export class PoolFormComponent extends CdForm implements OnInit { 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); + const usable = currentDomain ? currentDomain.length : this.crushRuleMaxSize; + rule.usable_size = Math.min(usable, this.crushRuleMaxSize); } private replicatedPgCalc(pgs: number): number { 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 83c1db6b6df..e1e31ed20fa 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 @@ -1,12 +1,10 @@ import { CrushStep } from './crush-step'; export class CrushRule { - max_size: number; usable_size?: number; - min_size: number; rule_id: number; + type: number; rule_name: string; - ruleset: number; steps: CrushStep[]; } diff --git a/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts b/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts index 66033f0330f..574919d08ec 100644 --- a/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts +++ b/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts @@ -546,26 +546,19 @@ export class Mocks { static getCrushRule({ id = 0, name = 'somePoolName', - min = 1, - max = 10, type = 'replicated', failureDomain = 'osd', itemName = 'default' // This string also sets the device type - "default~ssd" <- ssd usage only }: { - max?: number; - min?: number; id?: number; name?: string; type?: string; failureDomain?: string; itemName?: string; }): CrushRule { - const typeNumber = type === 'erasure' ? 3 : 1; const rule = new CrushRule(); - rule.max_size = max; - rule.min_size = min; + rule.type = type === 'erasure' ? 3 : 1; rule.rule_id = id; - rule.ruleset = typeNumber; rule.rule_name = name; rule.steps = [ { diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index 921dadb0471..ff2dea0e99a 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -2308,24 +2308,15 @@ paths: responses: '200': content: - application/vnd.ceph.api.v1.0+json: + application/vnd.ceph.api.v2.0+json: schema: properties: - max_size: - description: Maximum size of Rule - type: integer - min_size: - description: Minimum size of Rule - type: integer rule_id: description: Rule ID type: integer rule_name: description: Rule Name type: string - ruleset: - description: RuleSet related to the rule - type: integer steps: description: Steps included in the rule items: @@ -2337,10 +2328,7 @@ paths: required: - rule_id - rule_name - - ruleset - type - - min_size - - max_size - steps type: object description: OK @@ -2444,7 +2432,7 @@ paths: responses: '200': content: - application/vnd.ceph.api.v1.0+json: + application/vnd.ceph.api.v2.0+json: type: object description: OK '400':