From 31a3023ad6014b3530d2f2ff7f33594a18cb6fbb Mon Sep 17 00:00:00 2001 From: Devika Babrekar Date: Mon, 15 Jun 2026 16:47:16 +0530 Subject: [PATCH] mgr/dashboard:EC Profile creation form simplification and improvement Fixes: https://tracker.ceph.com/issues/77369 Signed-off-by: Devika Babrekar Conflicts: src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/erasure-code-profile-form/erasure-code-profile-form-modal.component.html -removed Directory field, as it is no longer needed in form --- ...ure-code-profile-form-modal.component.html | 82 +--------- ...-code-profile-form-modal.component.spec.ts | 116 ++------------ ...asure-code-profile-form-modal.component.ts | 89 ++--------- .../pool/pool-form/pool-form.component.html | 3 +- .../pool-form/pool-form.component.spec.ts | 142 ++++++++++++++++-- .../pool/pool-form/pool-form.component.ts | 126 ++++++++++++++-- .../table-key-value.component.spec.ts | 15 ++ .../table-key-value.component.ts | 5 +- 8 files changed, 287 insertions(+), 291 deletions(-) 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 99a6fa77b0a..2653128e402 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 @@ -36,7 +36,8 @@ id="name" type="text" formControlName="name" - placeholder="Add issue title" + placeholder="Profile name" + i18n-placeholder /> @@ -612,71 +613,6 @@ } - - @if ([PLUGIN.JERASURE, PLUGIN.ISA, PLUGIN.CLAY].includes(plugin)) { -
- - @for (technique of techniques; track technique) { - - } - -
- } - - - @if (plugin === PLUGIN.JERASURE) { -
- - - - @if (form.showError('packetSize', formDir, 'min')) { - - Must be equal to or greater than 1. - - } - -
- } - - - @if ([PLUGIN.JERASURE, PLUGIN.ISA, PLUGIN.CLAY].includes(plugin)) { -
- - - -
- } -
- - -
- - Directory - - -
{ let fixtureHelper: FixtureHelper; let data: { plugins: string[]; names: string[]; nodes: CrushNode[] }; - const expectTechnique = (current: string) => - expect(component.form.getValue('technique')).toBe(current); - - const expectTechniques = (techniques: string[], current: string) => { - expect(component.techniques).toEqual(techniques); - expectTechnique(current); - }; - const expectRequiredControls = (controlNames: string[]) => { controlNames.forEach((name) => { const value = component.form.getValue(name); @@ -160,7 +152,7 @@ describe('ErasureCodeProfileFormModalComponent', () => { const showDefaults = (plugin: string) => { formHelper.setValue('plugin', plugin); fixtureHelper.expectIdElementsVisible( - ['name', 'plugin', 'k', 'm', 'crushFailureDomain', 'crushDeviceClass', 'directory'], + ['name', 'plugin', 'k', 'm', 'crushFailureDomain', 'crushDeviceClass'], true ); }; @@ -170,18 +162,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { showDefaults('isa'); }); - it('should change technique to default if not available in other plugin', () => { - formHelper.setValue('plugin', 'jerasure'); - expectTechnique('reed_sol_van'); - formHelper.setValue('technique', 'blaum_roth'); - expectTechnique('blaum_roth'); - formHelper.setValue('plugin', 'isa'); - expectTechnique('reed_sol_van'); - formHelper.setValue('plugin', 'clay'); - formHelper.expectValidChange('scalar_mds', 'shec'); - expectTechnique('single'); - }); - describe(`for 'jerasure' plugin (default)`, () => { beforeEach(() => { formHelper.setValue('plugin', 'jerasure'); @@ -191,27 +171,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { expectRequiredControls(['k', 'm']); }); - it(`should show 'packetSize' and 'technique'`, () => { - fixture.detectChanges(); - expect(component.form.get('packetSize')).toBeTruthy(); - expect(component.form.get('technique')).toBeTruthy(); - }); - - it('should show available techniques', () => { - expectTechniques( - [ - 'reed_sol_van', - 'reed_sol_r6_op', - 'cauchy_orig', - 'cauchy_good', - 'liberation', - 'blaum_roth', - 'liber8tion' - ], - 'reed_sol_van' - ); - }); - it(`should not show any other plugin specific form control`, () => { fixtureHelper.expectIdElementsVisible( ['c', 'l', 'crushLocality', 'd', 'scalar_mds'], @@ -237,18 +196,9 @@ describe('ErasureCodeProfileFormModalComponent', () => { expectRequiredControls(['k', 'm']); }); - it(`should show 'technique'`, () => { - fixture.detectChanges(); - expect(component.form.get('technique')).toBeTruthy(); - }); - - it('should show available techniques', () => { - expectTechniques(['reed_sol_van', 'cauchy'], 'reed_sol_van'); - }); - it(`should not show any other plugin specific form control`, () => { fixtureHelper.expectIdElementsVisible( - ['c', 'l', 'crushLocality', 'packetSize', 'd', 'scalar_mds'], + ['c', 'l', 'crushLocality', 'd', 'scalar_mds'], false ); }); @@ -284,7 +234,7 @@ describe('ErasureCodeProfileFormModalComponent', () => { it(`should not show any other plugin specific form control`, () => { fixture.detectChanges(); // Be tolerant to layout differences; verify core LRC-hidden fields - ['c', 'packetSize', 'd', 'scalar_mds'].forEach((id) => { + ['c', 'd', 'scalar_mds'].forEach((id) => { expect(fixture.debugElement.query(By.css(`#${id}`))).toBeNull(); }); }); @@ -427,8 +377,8 @@ describe('ErasureCodeProfileFormModalComponent', () => { it(`should not show any other plugin specific form control`, () => { fixture.detectChanges(); - // Technique can be present in some layouts; focus on SHEC-specific hidden fields - ['l', 'crushLocality', 'packetSize', 'd', 'scalar_mds'].forEach((id) => { + // Focus on SHEC-specific hidden fields + ['l', 'crushLocality', 'd', 'scalar_mds'].forEach((id) => { expect(fixture.debugElement.query(By.css(`#${id}`))).toBeNull(); }); }); @@ -495,7 +445,7 @@ describe('ErasureCodeProfileFormModalComponent', () => { }); it(`should not show any other plugin specific form control`, () => { - fixtureHelper.expectIdElementsVisible(['l', 'crushLocality', 'packetSize', 'c'], false); + fixtureHelper.expectIdElementsVisible(['l', 'crushLocality', 'c'], false); }); it('should show default values for d and scalar_mds', () => { @@ -515,23 +465,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { ); }); - it('should have specific techniques for scalar_mds jerasure', () => { - expectTechniques( - ['reed_sol_van', 'reed_sol_r6_op', 'cauchy_orig', 'cauchy_good', 'liber8tion'], - 'reed_sol_van' - ); - }); - - it('should have specific techniques for scalar_mds isa', () => { - formHelper.expectValidChange('scalar_mds', 'isa'); - expectTechniques(['reed_sol_van', 'cauchy'], 'reed_sol_van'); - }); - - it('should have specific techniques for scalar_mds shec', () => { - formHelper.expectValidChange('scalar_mds', 'shec'); - expectTechniques(['single', 'multiple'], 'single'); - }); - describe('Validity of d', () => { beforeEach(() => { // Don't automatically change d - the only way to get d invalid @@ -640,10 +573,7 @@ describe('ErasureCodeProfileFormModalComponent', () => { beforeEach(() => { ecp = new ErasureCodeProfile(); submittedEcp = new ErasureCodeProfile(); - submittedEcp['crush-root'] = 'default'; submittedEcp['crush-failure-domain'] = CrushFailureDomains.Host; - submittedEcp['packetsize'] = 2048; - submittedEcp['technique'] = 'reed_sol_van'; const taskWrapper = TestBed.inject(TaskWrapperService); spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough(); @@ -671,15 +601,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { expect(ecpService.create).not.toHaveBeenCalled(); }); - it('should be able to create a profile with m, k, name, directory and packetSize', () => { - ecpChange('m', 3); - ecpChange('directory', '/different/ecp/path'); - formHelper.setMultipleValues(ecp, true); - formHelper.setValue('packetSize', 8192, true); - ecpChange('packetsize', 8192); - testCreation(); - }); - it('should not send the profile with unsupported fields', () => { formHelper.setMultipleValues(ecp, true); formHelper.setValue('crushLocality', 'osd', true); @@ -693,7 +614,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { 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', () => { @@ -701,8 +621,7 @@ describe('ErasureCodeProfileFormModalComponent', () => { testCreation(); }); - it('should send profile with plugin, name, failure domain and technique only', () => { - ecpChange('technique', 'cauchy'); + it('should send profile with plugin, name and failure domain only', () => { formHelper.setMultipleValues(ecp, true); formHelper.setValue('crushFailureDomain', 'osd', true); formHelper.setValue('crushDeviceClass', 'ssd', true); @@ -710,12 +629,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { submittedEcp['crush-device-class'] = 'ssd'; testCreation(); }); - - it('should not send the profile with unsupported fields', () => { - formHelper.setMultipleValues(ecp, true); - formHelper.setValue('packetSize', 'osd', true); - testCreation(); - }); }); describe(`'lrc' usage`, () => { @@ -725,8 +638,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { 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', () => { @@ -734,15 +645,10 @@ describe('ErasureCodeProfileFormModalComponent', () => { testCreation(); }); - it('should send profile with all required fields and crush root and locality', () => { + it('should send profile with all required fields and crush locality', () => { ecpChange('l', '6'); formHelper.setMultipleValues(ecp, true); - formHelper.setValue( - 'crushRoot', - component.buckets.find((bucket) => bucket.name === 'mix-host'), - true - ); - submittedEcp['crush-root'] = 'mix-host'; + formHelper.setValue('crushFailureDomain', 'osd-rack', true); submittedEcp['crush-failure-domain'] = 'osd-rack'; formHelper.setValue('crushLocality', 'osd-rack', true); submittedEcp['crush-locality'] = 'osd-rack'; @@ -763,8 +669,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { 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', () => { @@ -796,7 +700,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { submittedEcp.m = 2; submittedEcp.d = 5; submittedEcp.scalar_mds = 'jerasure'; - delete submittedEcp.packetsize; }); it('should be able to create a profile with only plugin and name', () => { @@ -828,7 +731,6 @@ describe('ErasureCodeProfileFormModalComponent', () => { ecpChange('scalar_mds', 'shec'); formHelper.setMultipleValues(ecp, true); submittedEcp.scalar_mds = 'shec'; - submittedEcp.technique = 'single'; 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 448175b4553..6c917abda3f 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 @@ -6,7 +6,13 @@ import { Output, ViewChild } from '@angular/core'; -import { AbstractControl, FormGroupDirective, ValidatorFn, Validators } from '@angular/forms'; +import { + AbstractControl, + FormControl, + FormGroupDirective, + ValidatorFn, + Validators +} from '@angular/forms'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; @@ -52,13 +58,12 @@ export class ErasureCodeProfileFormModalComponent form: CdFormGroup; plugins: string[]; names: string[]; - techniques: string[]; action: string; resource: string; dCalc: boolean; lrcGroups: number; lrcMultiK: number; - selectedCrushRoot: CrushNode; + private rootControl = new FormControl(null); public CrushFailureDomains = CrushFailureDomains; @@ -125,13 +130,7 @@ export class ErasureCodeProfileFormModalComponent CdValidators.number(false) ] ], - crushRoot: null, // Will be preselected crushDeviceClass: '', // Will be preselected - directory: '', - // Only for 'jerasure', 'clay' and 'isa' use - technique: 'reed_sol_van', - // Only for 'jerasure' use - packetSize: [2048], // Only for 'lrc' use l: [ 3, // Will be overwritten with plugin defaults @@ -171,7 +170,6 @@ export class ErasureCodeProfileFormModalComponent this.form.get('l').updateValueAndValidity({ emitEvent: false }); }); this.form.get('plugin').valueChanges.subscribe((plugin) => this.onPluginChange(plugin)); - this.form.get('scalar_mds').valueChanges.subscribe(() => this.setClayDefaultsForScalar()); this.form.get('crushFailureDomain').valueChanges.subscribe(() => { this.form.get('crushNumFailureDomains').updateValueAndValidity(); this.form.get('crushOsdsPerFailureDomain').updateValueAndValidity(); @@ -344,19 +342,9 @@ export class ErasureCodeProfileFormModalComponent } private setJerasureDefaults() { - this.techniques = [ - 'reed_sol_van', - 'reed_sol_r6_op', - 'cauchy_orig', - 'cauchy_good', - 'liberation', - 'blaum_roth', - 'liber8tion' - ]; this.setDefaults({ k: 4, - m: 2, - technique: 'reed_sol_van' + m: 2 }); } @@ -374,11 +362,9 @@ export class ErasureCodeProfileFormModalComponent * if they are not set, therefore it's fine to mark them as required in order to get * strange values that weren't set. */ - this.techniques = ['reed_sol_van', 'cauchy']; this.setDefaults({ k: 7, - m: 3, - technique: 'reed_sol_van' + m: 3 }); } @@ -410,28 +396,6 @@ export class ErasureCodeProfileFormModalComponent // d: 5, <- Will be automatically update to 5 scalar_mds: this.PLUGIN.JERASURE }); - this.setClayDefaultsForScalar(); - } - - private setClayDefaultsForScalar() { - const plugin = this.form.getValue('scalar_mds'); - let defaultTechnique = 'reed_sol_van'; - if (plugin === this.PLUGIN.JERASURE) { - this.techniques = [ - 'reed_sol_van', - 'reed_sol_r6_op', - 'cauchy_orig', - 'cauchy_good', - 'liber8tion' - ]; - } else if (plugin === this.PLUGIN.ISA) { - this.techniques = ['reed_sol_van', 'cauchy']; - } else { - // this.PLUGIN.SHEC - defaultTechnique = 'single'; - this.techniques = ['single', 'multiple']; - } - this.setDefaults({ technique: defaultTechnique }); } private setDefaults(defaults: object) { @@ -446,7 +410,6 @@ export class ErasureCodeProfileFormModalComponent */ const overwrite = control.pristine || - (controlName === 'technique' && !this.techniques.includes(value)) || (controlName === 'k' && [4, 7].includes(value)) || (controlName === 'm' && [2, 3].includes(value)); if (overwrite) { @@ -463,36 +426,18 @@ export class ErasureCodeProfileFormModalComponent this.ecpService .getInfo() .subscribe( - ({ - plugins, - names, - directory, - nodes - }: { - plugins: string[]; - names: string[]; - directory: string; - nodes: CrushNode[]; - }) => { + ({ plugins, names, nodes }: { plugins: string[]; names: string[]; nodes: CrushNode[] }) => { this.initCrushNodeSelection( nodes, - this.form.get('crushRoot'), + this.rootControl, this.form.get('crushFailureDomain'), this.form.get('crushDeviceClass'), false ); this.plugins = plugins; this.names = names; - this.form.silentSet('directory', directory); this.preValidateNumericInputFields(); - - setTimeout(() => { - const selectElement = document.getElementById('crushRoot') as any; - if (selectElement) { - selectElement.value = this.form.get('crushRoot').value; - } - this.cdr.detectChanges(); - }, 0); + this.cdr.detectChanges(); if (this.plugins.includes(this.PLUGIN.ISA)) { this.setIsaDefaults(); @@ -532,16 +477,14 @@ export class ErasureCodeProfileFormModalComponent this.form.setErrors({ cdSubmitButton: true }); }, complete: () => { - this.closeModal(); this.submitAction.emit(profile); + this.closeModal(); } }); } private createJson() { const pluginControls = { - technique: [this.PLUGIN.ISA, this.PLUGIN.JERASURE, this.PLUGIN.CLAY], - packetSize: [this.PLUGIN.JERASURE], l: [this.PLUGIN.LRC], crushLocality: [this.PLUGIN.LRC], c: [this.PLUGIN.SHEC], @@ -568,13 +511,11 @@ export class ErasureCodeProfileFormModalComponent crushFailureDomain: 'crush-failure-domain', crushNumFailureDomains: 'crush-num-failure-domains', crushOsdsPerFailureDomain: 'crush-osds-per-failure-domain', - crushRoot: 'crush-root', crushDeviceClass: 'crush-device-class', - packetSize: 'packetsize', crushLocality: 'crush-locality' }; const value = this.form.getValue(name); - ecp[differentApiAttributes[name] || name] = name === 'crushRoot' ? value.name : value; + ecp[differentApiAttributes[name] || name] = value; } onCrushFailureDomainChane() { 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 8543484f9ef..2b2b09d6dfd 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 @@ -536,7 +536,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 c6779becfb2..928205b8479 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 @@ -1064,21 +1064,116 @@ describe('PoolFormComponent', () => { fixtureHelper.expectIdElementsVisible(['erasureProfile', 'ecp-info-block'], true); }); + it('should order profile details consistently with the creation form', () => { + component.selectedEcp = { + name: 'ecp1', + plugin: 'lrc', + k: 4, + m: 2, + l: 3, + 'crush-failure-domain': 'host', + 'crush-locality': 'rack' + } as ErasureCodeProfile; + expect(component.getEcpProfileDetails()).toEqual([ + ['Plugin', 'lrc'], + ['Data chunks (k)', 4], + ['Coding chunks (m)', 2], + ['Locality (l)', 3], + ['Crush failure domain', 'host'], + ['Crush locality', 'rack'] + ]); + }); + + it('should only show plugin-eligible fields with values in profile details', () => { + component.selectedEcp = { + name: 'ecp1', + plugin: 'shec', + k: 4, + m: 3, + c: 2, + l: 3, + d: 5, + scalar_mds: 'jerasure', + 'crush-failure-domain': 'host', + 'crush-num-failure-domains': 0, + 'crush-osds-per-failure-domain': 0 + } as ErasureCodeProfile; + expect(component.getEcpProfileDetails()).toEqual([ + ['Plugin', 'shec'], + ['Data chunks (k)', 4], + ['Coding chunks (m)', 3], + ['Durability estimator (c)', 2], + ['Crush failure domain', 'host'] + ]); + }); + it('should select the newly created profile', () => { spyOn(ecpService, 'list').and.callFake(() => of(infoReturn.erasure_code_profiles)); expect(form.getValue('erasureProfile')).toBe('ecp1'); const name = 'awesomeProfile'; const modalCdsService = TestBed.inject(ModalCdsService); - // Mock the show method to return a mock component with submitAction + const ecp2 = new ErasureCodeProfile(); + ecp2.name = name; + ecp2.plugin = 'isa'; + ecp2.k = 4; + ecp2.m = 2; spyOn(modalCdsService, 'show').and.returnValue({ - submitAction: of({ name }) + submitAction: of(ecp2) } as any); + infoReturn.erasure_code_profiles.push(ecp2); + component.addErasureCodeProfile(); + expect(form.getValue('erasureProfile')).toBe(name); + expect(component.selectedEcp).toEqual(ecp2); + }); + + it('should select the newly created profile before it appears in the listing', () => { + const name = 'pendingProfile'; const ecp2 = new ErasureCodeProfile(); ecp2.name = name; - infoReturn.erasure_code_profiles.push(ecp2); + ecp2.plugin = 'jerasure'; + ecp2.k = 4; + ecp2.m = 2; + spyOn(ecpService, 'list').and.callFake(() => of([infoReturn.erasure_code_profiles[0]])); + const modalCdsService = TestBed.inject(ModalCdsService); + spyOn(modalCdsService, 'show').and.returnValue({ + submitAction: of(ecp2) + } as any); + component.addErasureCodeProfile(); + expect(form.getValue('erasureProfile')).toBe(name); + expect(component.selectedEcp).toEqual(ecp2); + expect(component.ecProfiles.some((profile) => profile.name === name)).toBe(true); + }); + + it('should update the dropdown immediately without waiting for the listing API', () => { + const name = 'instantProfile'; + const ecp2 = new ErasureCodeProfile(); + ecp2.name = name; + ecp2.plugin = 'isa'; + ecp2.k = 4; + ecp2.m = 2; + let emitList: (value: ErasureCodeProfile[]) => void; + spyOn(ecpService, 'list').and.returnValue( + new Observable((subscriber) => { + emitList = (profiles) => { + subscriber.next(profiles); + subscriber.complete(); + }; + }) + ); + const modalCdsService = TestBed.inject(ModalCdsService); + spyOn(modalCdsService, 'show').and.returnValue({ + submitAction: of(ecp2) + } as any); + component.addErasureCodeProfile(); - // Form stores erasureProfile as string name, not full object + + expect(component.ecProfiles.some((profile) => profile.name === name)).toBe(true); + expect(form.getValue('erasureProfile')).toBe(name); + expect(component.selectedEcp).toEqual(ecp2); + + emitList([infoReturn.erasure_code_profiles[0], ecp2]); expect(form.getValue('erasureProfile')).toBe(name); + expect(component.ecProfiles.some((profile) => profile.name === name)).toBe(true); }); describe('ecp deletion', () => { @@ -1087,6 +1182,16 @@ describe('PoolFormComponent', () => { let modalSpy: jasmine.Spy; let submitActionObservable: () => Observable; + const mockDeletionModalShow = () => { + const modalCdsService = TestBed.inject(ModalCdsService); + const existingShowSpy = modalCdsService.show as jasmine.Spy; + modalSpy = existingShowSpy?.and ? existingShowSpy : spyOn(modalCdsService, 'show'); + modalSpy.and.callFake((_deletionClass: any, config: any) => { + submitActionObservable = config.submitActionObservable; + return {} as any; + }); + }; + const callEcpDeletion = () => { component.deleteErasureCodeProfile(); submitActionObservable().subscribe(); @@ -1101,21 +1206,29 @@ describe('PoolFormComponent', () => { }; beforeEach(() => { - const modalCdsService = TestBed.inject(ModalCdsService); - modalSpy = spyOn(modalCdsService, 'show').and.callFake( - (_deletionClass: any, config: any) => { - submitActionObservable = config.submitActionObservable; - return {} as any; - } - ); - deleteSpy = spyOn(ecpService, 'delete').and.callFake((name: string) => { + mockDeletionModalShow(); + + const listSpy = ecpService.list as jasmine.Spy; + if (listSpy?.and) { + listSpy.and.callFake(() => of(infoReturn.erasure_code_profiles)); + } + + const existingDeleteSpy = ecpService.delete as jasmine.Spy; + deleteSpy = existingDeleteSpy?.and ? existingDeleteSpy : spyOn(ecpService, 'delete'); + deleteSpy.and.callFake((name: string) => { const profiles = infoReturn.erasure_code_profiles; const index = _.findIndex(profiles, (profile) => profile.name === name); profiles.splice(index, 1); return of({ status: 202 }); }); + taskWrapper = TestBed.inject(TaskWrapperService); - spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough(); + const wrapSpy = taskWrapper.wrapTaskAroundCall as jasmine.Spy; + if (wrapSpy?.and) { + wrapSpy.and.callThrough(); + } else { + spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough(); + } const ecp2 = new ErasureCodeProfile(); ecp2.name = 'someEcpName'; @@ -1124,6 +1237,9 @@ describe('PoolFormComponent', () => { const ecp3 = new ErasureCodeProfile(); ecp3.name = 'aDifferentEcpName'; infoReturn.erasure_code_profiles.push(ecp3); + + component.ecProfiles = [...infoReturn.erasure_code_profiles]; + component.form.get('erasureProfile').enable({ emitEvent: false }); }); it('should delete two different erasure code profiles', () => { 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 a38fb0583bb..5f1e32eccd4 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 @@ -57,6 +57,11 @@ interface MonitorResponse { }; } +interface EcpDetailField { + label: string; + plugins?: string[]; +} + @Component({ selector: 'cd-pool-form', templateUrl: './pool-form.component.html', @@ -65,6 +70,20 @@ interface MonitorResponse { }) export class PoolFormComponent extends CdForm implements OnInit { private static readonly DEFAULT_RULE_NAME = 'replicated_rule'; + private static readonly ECP_DETAIL_FIELDS = new Map([ + ['plugin', { label: $localize`Plugin` }], + ['k', { label: $localize`Data chunks (k)` }], + ['m', { label: $localize`Coding chunks (m)` }], + ['c', { label: $localize`Durability estimator (c)`, plugins: ['shec'] }], + ['d', { label: $localize`Helper chunks (d)`, plugins: ['clay'] }], + ['l', { label: $localize`Locality (l)`, plugins: ['lrc'] }], + ['crush-failure-domain', { label: $localize`Crush failure domain` }], + ['crush-num-failure-domains', { label: $localize`Crush num failure domain` }], + ['crush-osds-per-failure-domain', { label: $localize`Crush osds per failure domain` }], + ['crush-locality', { label: $localize`Crush locality`, plugins: ['lrc'] }], + ['scalar_mds', { label: $localize`Scalar mds`, plugins: ['clay'] }], + ['crush-device-class', { label: $localize`Crush device class` }] + ]); @ViewChild('crushInfoTabs') crushInfoTabs: NgbNav; @ViewChild('ecpInfoTabs') ecpInfoTabs: NgbNav; @@ -254,10 +273,10 @@ export class PoolFormComponent extends CdForm implements OnInit { this.initEcp(info.erasure_code_profiles); } - private initEcp(ecProfiles: ErasureCodeProfile[]) { - this.setListControlStatus('erasureProfile', ecProfiles); - this.ecProfiles = ecProfiles; - this.erasureProfileChange(); + private initEcp(ecProfiles: ErasureCodeProfile[], selectedName?: string) { + this.ecProfiles = [...ecProfiles]; + this.setListControlStatus('erasureProfile', ecProfiles, selectedName); + this.erasureProfileChange(selectedName); } /** @@ -268,12 +287,14 @@ export class PoolFormComponent extends CdForm implements OnInit { * If more than one rule or profile exists the listing will be enabled, * otherwise disabled. */ - private setListControlStatus(controlName: string, arr: any[]) { + private setListControlStatus(controlName: string, arr: any[], selectedName?: string) { const control = this.form.get(controlName); const value = control.value; if (controlName === 'erasureProfile') { - if (arr.length === 1 && (!value || !_.isEqual(value, arr[0]))) { + if (selectedName && arr.some((profile) => profile.name === selectedName)) { + control.setValue(selectedName); + } else if (arr.length === 1 && (!value || !_.isEqual(value, arr[0]))) { control.setValue(arr[0].name); } else if (arr.length === 0 && value) { control.setValue(null); @@ -713,14 +734,16 @@ export class PoolFormComponent extends CdForm implements OnInit { } addErasureCodeProfile() { - this.addModal(ErasureCodeProfileFormModalComponent, (name) => this.reloadECPs(name)); + this.addModal(ErasureCodeProfileFormModalComponent, (profile: ErasureCodeProfile) => + this.reloadECPs(profile) + ); } - private addModal(modalComponent: Type, reload: (name: string) => void) { + private addModal(modalComponent: Type, reload: (item: any) => void) { this.hideOpenTooltips(); const modalRef = this.modalService.show(modalComponent); modalRef.submitAction.subscribe((item: any) => { - reload(item.name); + reload(item); }); } @@ -729,24 +752,53 @@ export class PoolFormComponent extends CdForm implements OnInit { this.data.erasureInfo = false; } - private reloadECPs(profileName?: string) { + private reloadECPs(profileOrName?: ErasureCodeProfile | string) { + const profileName = typeof profileOrName === 'string' ? profileOrName : profileOrName?.name; + const createdProfile = typeof profileOrName === 'object' ? profileOrName : undefined; + + if (createdProfile) { + this.applyCreatedEcp(createdProfile); + } + this.reloadList({ newItemName: profileName, getInfo: () => this.ecpService.list(), - initInfo: (profiles) => this.initEcp(profiles), - findNewItem: () => this.ecProfiles.find((p: ErasureCodeProfile) => p.name === profileName), + initInfo: (profiles: ErasureCodeProfile[]) => { + if (createdProfile && !profiles.some((profile) => profile.name === createdProfile.name)) { + profiles = [...profiles, createdProfile]; + } + this.initEcp(profiles, profileName); + }, + findNewItem: () => + this.ecProfiles.find((profile: ErasureCodeProfile) => profile.name === profileName) || + createdProfile, controlName: 'erasureProfile', nameAttribute: 'name' }); } + private applyCreatedEcp(profile: ErasureCodeProfile) { + let profiles = [...(this.ecProfiles || [])]; + if (!profiles.some((existing) => existing.name === profile.name)) { + profiles = [...profiles, profile]; + } + this.ecProfiles = profiles; + + const control = this.form.get('erasureProfile'); + control?.enable({ emitEvent: false }); + control?.setValue(profile.name, { emitEvent: true }); + this.erasureProfileChange(profile.name); + this.cdr.detectChanges(); + } + private reloadList({ newItemName, getInfo, initInfo, findNewItem, controlName, - nameAttribute + nameAttribute, + afterSelect }: { newItemName: string; getInfo: () => Observable; @@ -754,6 +806,7 @@ export class PoolFormComponent extends CdForm implements OnInit { findNewItem: () => any; controlName: string; nameAttribute?: string; + afterSelect?: (value: any) => void; }) { if (this.modalSubscription) { this.modalSubscription.unsubscribe(); @@ -761,13 +814,18 @@ export class PoolFormComponent extends CdForm implements OnInit { getInfo().subscribe((items: any) => { initInfo(items); if (!newItemName) { + this.cdr.detectChanges(); return; } const item = findNewItem(); if (item) { + const control = this.form.get(controlName); const value = nameAttribute ? item[nameAttribute] : item; - this.form.get(controlName)?.setValue(value); + control?.enable({ emitEvent: false }); + control?.setValue(value, { emitEvent: true }); + afterSelect?.(value); } + this.cdr.detectChanges(); }); } @@ -841,7 +899,7 @@ export class PoolFormComponent extends CdForm implements OnInit { } addCrushRule() { - this.addModal(CrushRuleFormModalComponent, (name) => this.reloadCrushRules(name)); + this.addModal(CrushRuleFormModalComponent, (item) => this.reloadCrushRules(item.name)); } private reloadCrushRules(ruleName?: string) { @@ -1099,6 +1157,44 @@ export class PoolFormComponent extends CdForm implements OnInit { this.form.get('name').updateValueAndValidity({ emitEvent: false, onlySelf: true }); } + getEcpProfileDetails(): [string, any][] { + if (!this.selectedEcp) { + return []; + } + const plugin = this.selectedEcp.plugin; + const details: [string, any][] = []; + + PoolFormComponent.ECP_DETAIL_FIELDS.forEach(({ label, plugins }, key) => { + if (!this.isEcpDetailFieldEligible(plugins, plugin)) { + return; + } + const value = this.selectedEcp[key]; + if (!this.isEcpDetailValueVisible(key, value)) { + return; + } + details.push([label, value]); + }); + + return details; + } + + private isEcpDetailFieldEligible(plugins: string[] | undefined, plugin: string): boolean { + return !plugins || plugins.includes(plugin); + } + + private isEcpDetailValueVisible(key: string, value: any): boolean { + if (value === undefined || value === null || value === '') { + return false; + } + if ( + (key === 'crush-num-failure-domains' || key === 'crush-osds-per-failure-domain') && + value === 0 + ) { + return false; + } + return true; + } + erasureProfileChange(selectedName?: string) { if (!this.ecProfiles || this.ecProfiles.length === 0) { return; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.spec.ts index 62caa51315c..2d9d9da1780 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.spec.ts @@ -65,6 +65,21 @@ describe('TableKeyValueComponent', () => { expect(component.tableData).toEqual([{ key: 'b', value: 2 }]); }); + it('should preserve the order of array data when requested', () => { + component.data = [ + ['Plugin', 'isa'], + ['Data chunks (k)', 4], + ['Coding chunks (m)', 2] + ]; + component.preserveOrder = true; + component.ngOnInit(); + expect(component.tableData).toEqual([ + { key: 'Plugin', value: 'isa' }, + { key: 'Data chunks (k)', value: 4 }, + { key: 'Coding chunks (m)', value: 2 } + ]); + }); + it('should remove items with objects as values', () => { component.data = [ [3, 'something'], diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.ts index 1c24e1218dc..f2e74ea3219 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-key-value/table-key-value.component.ts @@ -60,6 +60,8 @@ export class TableKeyValueComponent implements OnInit, OnChanges { showMultiLineText = false; // If true, the value field will use a template that supports multi line text @Input() multilineTextKeys: string[]; // If set, the value field will use a template that supports multi line text for this key + @Input() + preserveOrder = false; // If set, the classAddingTpl is used to enable different css for different values @Input() @@ -139,7 +141,8 @@ export class TableKeyValueComponent implements OnInit, OnChanges { return item; }) .filter((i) => i.value !== null); - return _.sortBy(this.renderObjects ? this.insertFlattenObjects(result) : result, 'key'); + const processed = this.renderObjects ? this.insertFlattenObjects(result) : result; + return this.preserveOrder ? processed : _.sortBy(processed, 'key'); } private makePairsFromArray(data: any[]): KeyValueItem[] { -- 2.47.3