From f22c2b26375f4b49a3aa7b9cd2ccc5f36b1a6f07 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Stephan=20M=C3=BCller?= Date: Tue, 7 Apr 2020 16:02:28 +0200 Subject: [PATCH] mgr/dashboard: Use of crush node class in ECP modal MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Now the crush node class preselects root, failure domain and device class in the erasure code profile modal. Like for crush rule, now also if you try to delete an used ECP you can't and the info box will show you, what pool is using the profile. Fixes: https://tracker.ceph.com/issues/44621 Signed-off-by: Stephan Müller (cherry picked from commit 94a32d1a12e6fdfb31262a40fe6f210fe2c4c958) --- .../dashboard/test_erasure_code_profile.py | 3 +- qa/tasks/mgr/dashboard/test_pool.py | 1 + .../controllers/erasure_code_profile.py | 12 +- src/pybind/mgr/dashboard/controllers/pool.py | 14 +- ...ure-code-profile-form-modal.component.html | 32 ++- ...-code-profile-form-modal.component.spec.ts | 137 +++++++++--- ...asure-code-profile-form-modal.component.ts | 90 ++++---- .../pool/pool-form/pool-form.component.html | 30 ++- .../pool-form/pool-form.component.spec.ts | 151 +++++++++++-- .../pool/pool-form/pool-form.component.ts | 205 +++++++++++++----- .../src/app/shared/models/pool-form-info.ts | 1 + 11 files changed, 488 insertions(+), 188 deletions(-) diff --git a/qa/tasks/mgr/dashboard/test_erasure_code_profile.py b/qa/tasks/mgr/dashboard/test_erasure_code_profile.py index 111e37c7e3e66..12e061777fc28 100644 --- a/qa/tasks/mgr/dashboard/test_erasure_code_profile.py +++ b/qa/tasks/mgr/dashboard/test_erasure_code_profile.py @@ -102,9 +102,8 @@ class ECPTest(DashboardTestCase): self._get('/ui-api/erasure_code_profile/info') self.assertSchemaBody(JObj({ 'names': JList(six.string_types), - 'failure_domains': JList(six.string_types), 'plugins': JList(six.string_types), - 'devices': JList(six.string_types), 'directory': six.string_types, + 'nodes': JList(JObj({}, allow_unknown=True)) })) diff --git a/qa/tasks/mgr/dashboard/test_pool.py b/qa/tasks/mgr/dashboard/test_pool.py index 969318d2a94b3..2a49dff1ac131 100644 --- a/qa/tasks/mgr/dashboard/test_pool.py +++ b/qa/tasks/mgr/dashboard/test_pool.py @@ -414,4 +414,5 @@ class PoolTest(DashboardTestCase): 'pg_autoscale_modes': JList(six.string_types), 'erasure_code_profiles': JList(JObj({}, allow_unknown=True)), 'used_rules': JObj({}, allow_unknown=True), + 'used_profiles': JObj({}, allow_unknown=True), })) diff --git a/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py b/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py index ca63ba286a4b1..c4cc867220d6c 100644 --- a/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py +++ b/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py @@ -11,10 +11,10 @@ from .. import mgr @ApiController('/erasure_code_profile', Scope.POOL) class ErasureCodeProfile(RESTController): - ''' + """ create() supports additional key-value arguments that are passed to the ECP plugin. - ''' + """ def list(self): return CephService.get_erasure_code_profiles() @@ -40,15 +40,15 @@ class ErasureCodeProfileUi(ErasureCodeProfile): @Endpoint() @ReadPermission def info(self): - '''Used for profile creation and editing''' + """ + Used for profile creation and editing + """ config = mgr.get('config') - osd_map_crush = mgr.get('osd_map_crush') return { # Because 'shec' is experimental it's not included 'plugins': config['osd_erasure_code_plugins'].split() + ['shec'], 'directory': config['erasure_code_dir'], - 'devices': list({device['class'] for device in osd_map_crush['devices']}), - 'failure_domains': [domain['name'] for domain in osd_map_crush['types']], + 'nodes': mgr.get('osd_map_tree')['nodes'], 'names': [name for name, _ in mgr.get('osd_map').get('erasure_code_profiles', {}).items()] } diff --git a/src/pybind/mgr/dashboard/controllers/pool.py b/src/pybind/mgr/dashboard/controllers/pool.py index 275c59c44a920..945a82a177a49 100644 --- a/src/pybind/mgr/dashboard/controllers/pool.py +++ b/src/pybind/mgr/dashboard/controllers/pool.py @@ -229,16 +229,23 @@ class PoolUi(Pool): for o in options if o['name'] == conf_name][0] + profiles = CephService.get_erasure_code_profiles() used_rules = {} + used_profiles = {} pool_names = [] for p in self._pool_list(): name = p['pool_name'] - rule = p['crush_rule'] pool_names.append(name) + rule = p['crush_rule'] if rule in used_rules: used_rules[rule].append(name) else: used_rules[rule] = [name] + profile = p['erasure_code_profile'] + if profile in used_profiles: + used_profiles[profile].append(name) + else: + used_profiles[profile] = [name] mgr_config = mgr.get('config') return { @@ -252,6 +259,7 @@ class PoolUi(Pool): "compression_modes": get_config_option_enum('bluestore_compression_mode'), "pg_autoscale_default_mode": mgr_config['osd_pool_default_pg_autoscale_mode'], "pg_autoscale_modes": get_config_option_enum('osd_pool_default_pg_autoscale_mode'), - "erasure_code_profiles": CephService.get_erasure_code_profiles(), - "used_rules": used_rules + "erasure_code_profiles": profiles, + "used_rules": used_rules, + "used_profiles": used_profiles, } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.html index 985f6a3fa551d..89b383e898781 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.html @@ -168,9 +168,9 @@ - @@ -192,12 +192,12 @@ - - @@ -253,12 +253,18 @@
- +
@@ -275,12 +281,14 @@ name="crushDeviceClass" formControlName="crushDeviceClass"> + i18n>Let Ceph decide + Available OSDs: {{deviceCount}} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.spec.ts index 0d4ce97a21016..1886ebd730fea 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.spec.ts @@ -15,6 +15,7 @@ import { i18nProviders } from '../../../../testing/unit-test-helper'; import { ErasureCodeProfileService } from '../../../shared/api/erasure-code-profile.service'; +import { CrushNode } from '../../../shared/models/crush-node'; import { ErasureCodeProfile } from '../../../shared/models/erasure-code-profile'; import { TaskWrapperService } from '../../../shared/services/task-wrapper.service'; import { PoolModule } from '../pool.module'; @@ -28,6 +29,20 @@ describe('ErasureCodeProfileFormModalComponent', () => { let fixtureHelper: FixtureHelper; let data: {}; + // Object contains mock functions + const mock = { + node: ( + name: string, + id: number, + type: string, + type_id: number, + children?: number[], + device_class?: string + ): CrushNode => { + return { name, type, type_id, id, children, device_class }; + } + }; + configureTestBed({ imports: [ HttpClientTestingModule, @@ -46,10 +61,44 @@ describe('ErasureCodeProfileFormModalComponent', () => { formHelper = new FormHelper(component.form); ecpService = TestBed.get(ErasureCodeProfileService); data = { - failure_domains: ['host', 'osd'], plugins: ['isa', 'jerasure', 'shec', 'lrc'], names: ['ecp1', 'ecp2'], - devices: ['ssd', 'hdd'] + /** + * Create the following test crush map: + * > default + * --> ssd-host + * ----> 3x osd with ssd + * --> mix-host + * ----> hdd-rack + * ------> 2x osd-rack with hdd + * ----> ssd-rack + * ------> 2x osd-rack with ssd + */ + nodes: [ + // Root node + mock.node('default', -1, 'root', 11, [-2, -3]), + // SSD host + mock.node('ssd-host', -2, 'host', 1, [1, 0, 2]), + mock.node('osd.0', 0, 'osd', 0, undefined, 'ssd'), + mock.node('osd.1', 1, 'osd', 0, undefined, 'ssd'), + mock.node('osd.2', 2, 'osd', 0, undefined, 'ssd'), + // SSD and HDD mixed devices host + mock.node('mix-host', -3, 'host', 1, [-4, -5]), + // HDD rack + mock.node('hdd-rack', -4, 'rack', 3, [3, 4, 5, 6, 7]), + mock.node('osd2.0', 3, 'osd-rack', 0, undefined, 'hdd'), + mock.node('osd2.1', 4, 'osd-rack', 0, undefined, 'hdd'), + mock.node('osd2.2', 5, 'osd-rack', 0, undefined, 'hdd'), + mock.node('osd2.3', 6, 'osd-rack', 0, undefined, 'hdd'), + mock.node('osd2.4', 7, 'osd-rack', 0, undefined, 'hdd'), + // SSD rack + mock.node('ssd-rack', -5, 'rack', 3, [8, 9, 10, 11, 12]), + mock.node('osd3.0', 8, 'osd-rack', 0, undefined, 'ssd'), + mock.node('osd3.1', 9, 'osd-rack', 0, undefined, 'ssd'), + mock.node('osd3.2', 10, 'osd-rack', 0, undefined, 'ssd'), + mock.node('osd3.3', 11, 'osd-rack', 0, undefined, 'ssd'), + mock.node('osd3.4', 12, 'osd-rack', 0, undefined, 'ssd') + ] }; spyOn(ecpService, 'getInfo').and.callFake(() => of(data)); fixture.detectChanges(); @@ -189,15 +238,27 @@ describe('ErasureCodeProfileFormModalComponent', () => { describe('submission', () => { let ecp: ErasureCodeProfile; + let submittedEcp: ErasureCodeProfile; const testCreation = () => { fixture.detectChanges(); component.onSubmit(); - expect(ecpService.create).toHaveBeenCalledWith(ecp); + expect(ecpService.create).toHaveBeenCalledWith(submittedEcp); + }; + + const ecpChange = (attribute: string, value: string | number) => { + ecp[attribute] = value; + submittedEcp[attribute] = value; }; beforeEach(() => { ecp = new ErasureCodeProfile(); + submittedEcp = new ErasureCodeProfile(); + submittedEcp['crush-root'] = 'default'; + submittedEcp['crush-failure-domain'] = 'osd-rack'; + submittedEcp['packetsize'] = 2048; + submittedEcp['technique'] = 'reed_sol_van'; + const taskWrapper = TestBed.get(TaskWrapperService); spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough(); spyOn(ecpService, 'create').and.stub(); @@ -205,37 +266,35 @@ describe('ErasureCodeProfileFormModalComponent', () => { describe(`'jerasure' usage`, () => { beforeEach(() => { - ecp.name = 'jerasureProfile'; + submittedEcp['plugin'] = 'jerasure'; + ecpChange('name', 'jerasureProfile'); + submittedEcp.k = 4; + submittedEcp.m = 2; }); it('should be able to create a profile with only required fields', () => { formHelper.setMultipleValues(ecp, true); - ecp.k = 4; - ecp.m = 2; testCreation(); }); it(`does not create with missing 'k' or invalid form`, () => { - ecp.k = 0; + ecpChange('k', 0); formHelper.setMultipleValues(ecp, true); component.onSubmit(); expect(ecpService.create).not.toHaveBeenCalled(); }); it('should be able to create a profile with m, k, name, directory and packetSize', () => { - ecp.m = 3; - ecp.directory = '/different/ecp/path'; + ecpChange('m', 3); + ecpChange('directory', '/different/ecp/path'); formHelper.setMultipleValues(ecp, true); - ecp.k = 4; formHelper.setValue('packetSize', 8192, true); - ecp.packetsize = 8192; + ecpChange('packetsize', 8192); testCreation(); }); it('should not send the profile with unsupported fields', () => { formHelper.setMultipleValues(ecp, true); - ecp.k = 4; - ecp.m = 2; formHelper.setValue('crushLocality', 'osd', true); testCreation(); }); @@ -243,8 +302,11 @@ describe('ErasureCodeProfileFormModalComponent', () => { describe(`'isa' usage`, () => { beforeEach(() => { - ecp.name = 'isaProfile'; - ecp.plugin = 'isa'; + ecpChange('name', 'isaProfile'); + ecpChange('plugin', 'isa'); + submittedEcp.k = 7; + submittedEcp.m = 3; + delete submittedEcp.packetsize; }); it('should be able to create a profile with only plugin and name', () => { @@ -253,10 +315,11 @@ describe('ErasureCodeProfileFormModalComponent', () => { }); it('should send profile with plugin, name, failure domain and technique only', () => { - ecp.technique = 'cauchy'; + ecpChange('technique', 'cauchy'); formHelper.setMultipleValues(ecp, true); formHelper.setValue('crushFailureDomain', 'osd', true); - ecp['crush-failure-domain'] = 'osd'; + submittedEcp['crush-failure-domain'] = 'osd'; + submittedEcp['crush-device-class'] = 'ssd'; testCreation(); }); @@ -269,35 +332,32 @@ describe('ErasureCodeProfileFormModalComponent', () => { describe(`'lrc' usage`, () => { beforeEach(() => { - ecp.name = 'lreProfile'; - ecp.plugin = 'lrc'; + ecpChange('name', 'lrcProfile'); + ecpChange('plugin', 'lrc'); + submittedEcp.k = 4; + submittedEcp.m = 2; + submittedEcp.l = 3; + delete submittedEcp.packetsize; + delete submittedEcp.technique; }); it('should be able to create a profile with only required fields', () => { formHelper.setMultipleValues(ecp, true); - ecp.k = 4; - ecp.m = 2; - ecp.l = 3; testCreation(); }); it('should send profile with all required fields and crush root and locality', () => { - ecp.l = 8; + ecpChange('l', '6'); formHelper.setMultipleValues(ecp, true); - ecp.k = 4; - ecp.m = 2; - formHelper.setValue('crushLocality', 'osd', true); - formHelper.setValue('crushRoot', 'rack', true); - ecp['crush-locality'] = 'osd'; - ecp['crush-root'] = 'rack'; + formHelper.setValue('crushRoot', component.buckets[2], true); + submittedEcp['crush-root'] = 'mix-host'; + formHelper.setValue('crushLocality', 'osd-rack', true); + submittedEcp['crush-locality'] = 'osd-rack'; testCreation(); }); it('should not send the profile with unsupported fields', () => { formHelper.setMultipleValues(ecp, true); - ecp.k = 4; - ecp.m = 2; - ecp.l = 3; formHelper.setValue('c', 4, true); testCreation(); }); @@ -305,8 +365,13 @@ describe('ErasureCodeProfileFormModalComponent', () => { describe(`'shec' usage`, () => { beforeEach(() => { - ecp.name = 'shecProfile'; - ecp.plugin = 'shec'; + ecpChange('name', 'shecProfile'); + ecpChange('plugin', 'shec'); + submittedEcp.k = 4; + submittedEcp.m = 3; + submittedEcp.c = 2; + delete submittedEcp.packetsize; + delete submittedEcp.technique; }); it('should be able to create a profile with only plugin and name', () => { @@ -315,10 +380,10 @@ describe('ErasureCodeProfileFormModalComponent', () => { }); it('should send profile with plugin, name, c and crush device class only', () => { - ecp.c = 4; + ecpChange('c', '3'); formHelper.setMultipleValues(ecp, true); formHelper.setValue('crushDeviceClass', 'ssd', true); - ecp['crush-device-class'] = 'ssd'; + submittedEcp['crush-device-class'] = 'ssd'; testCreation(); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.ts index 6a62a5c87a563..aed3493fc4d09 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.ts @@ -5,10 +5,12 @@ import { I18n } from '@ngx-translate/i18n-polyfill'; import { BsModalRef } from 'ngx-bootstrap/modal'; import { ErasureCodeProfileService } from '../../../shared/api/erasure-code-profile.service'; +import { CrushNodeSelectionClass } from '../../../shared/classes/crush.node.selection.class'; import { ActionLabelsI18n } from '../../../shared/constants/app.constants'; import { CdFormBuilder } from '../../../shared/forms/cd-form-builder'; import { CdFormGroup } from '../../../shared/forms/cd-form-group'; import { CdValidators } from '../../../shared/forms/cd-validators'; +import { CrushNode } from '../../../shared/models/crush-node'; import { ErasureCodeProfile } from '../../../shared/models/erasure-code-profile'; import { FinishedTask } from '../../../shared/models/finished-task'; import { TaskWrapperService } from '../../../shared/services/task-wrapper.service'; @@ -18,19 +20,12 @@ import { TaskWrapperService } from '../../../shared/services/task-wrapper.servic templateUrl: './erasure-code-profile-form-modal.component.html', styleUrls: ['./erasure-code-profile-form-modal.component.scss'] }) -export class ErasureCodeProfileFormModalComponent implements OnInit { +export class ErasureCodeProfileFormModalComponent extends CrushNodeSelectionClass + implements OnInit { @Output() submitAction = new EventEmitter(); - form: CdFormGroup; - failureDomains: string[]; - plugins: string[]; - names: string[]; - techniques: string[]; - requiredControls: string[] = []; - devices: string[] = []; tooltips = this.ecpService.formTooltips; - PLUGIN = { LRC: 'lrc', // Locally Repairable Erasure Code SHEC: 'shec', // Shingled Erasure Code @@ -38,6 +33,11 @@ export class ErasureCodeProfileFormModalComponent implements OnInit { ISA: 'isa' // Intel Storage Acceleration }; plugin = this.PLUGIN.JERASURE; + + form: CdFormGroup; + plugins: string[]; + names: string[]; + techniques: string[]; action: string; resource: string; @@ -49,6 +49,7 @@ export class ErasureCodeProfileFormModalComponent implements OnInit { private i18n: I18n, public actionLabels: ActionLabelsI18n ) { + super(); this.action = this.actionLabels.CREATE; this.resource = this.i18n('EC Profile'); this.createForm(); @@ -175,27 +176,52 @@ export class ErasureCodeProfileFormModalComponent implements OnInit { .getInfo() .subscribe( ({ - failure_domains, plugins, names, directory, - devices + nodes }: { - failure_domains: string[]; plugins: string[]; names: string[]; directory: string; - devices: string[]; + nodes: CrushNode[]; }) => { - this.failureDomains = failure_domains; + this.initCrushNodeSelection( + nodes, + this.form.get('crushRoot'), + this.form.get('crushFailureDomain'), + this.form.get('crushDeviceClass') + ); this.plugins = plugins; this.names = names; - this.devices = devices; this.form.silentSet('directory', directory); } ); } + onSubmit() { + if (this.form.invalid) { + this.form.setErrors({ cdSubmitButton: true }); + return; + } + const profile = this.createJson(); + this.taskWrapper + .wrapTaskAroundCall({ + task: new FinishedTask('ecp/create', { name: profile.name }), + call: this.ecpService.create(profile) + }) + .subscribe( + undefined, + () => { + this.form.setErrors({ cdSubmitButton: true }); + }, + () => { + this.bsModalRef.hide(); + this.submitAction.emit(profile); + } + ); + } + private createJson() { const pluginControls = { technique: [this.PLUGIN.ISA, this.PLUGIN.JERASURE], @@ -209,13 +235,9 @@ export class ErasureCodeProfileFormModalComponent implements OnInit { Object.keys(this.form.controls) .filter((name) => { const pluginControl = pluginControls[name]; - const control = this.form.get(name); + const value = this.form.getValue(name); const usable = (pluginControl && pluginControl.includes(plugin)) || !pluginControl; - return ( - usable && - (control.dirty || this.requiredControls.includes(name)) && - this.form.getValue(name) - ); + return usable && value && value !== ''; }) .forEach((name) => { this.extendJson(name, ecp); @@ -231,29 +253,7 @@ export class ErasureCodeProfileFormModalComponent implements OnInit { packetSize: 'packetsize', crushLocality: 'crush-locality' }; - ecp[differentApiAttributes[name] || name] = this.form.getValue(name); - } - - onSubmit() { - if (this.form.invalid) { - this.form.setErrors({ cdSubmitButton: true }); - return; - } - const profile = this.createJson(); - this.taskWrapper - .wrapTaskAroundCall({ - task: new FinishedTask('ecp/create', { name: profile.name }), - call: this.ecpService.create(profile) - }) - .subscribe( - undefined, - () => { - this.form.setErrors({ cdSubmitButton: true }); - }, - () => { - this.bsModalRef.hide(); - this.submitAction.emit(profile); - } - ); + const value = this.form.getValue(name); + ecp[differentApiAttributes[name] || name] = name === 'crushRoot' ? value.name : value; } } 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 3ccfd2a024be6..67fba23f16456 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 @@ -236,6 +236,10 @@