From 8f61fc4919eabfa769cc3448d5a0aa7efca3666d Mon Sep 17 00:00:00 2001 From: Tatjana Dehler Date: Thu, 17 Jan 2019 17:13:26 +0100 Subject: [PATCH] mgr/dashboard: consider config option default values Consider config option default values as a fallback if no values are defined by the user when detecting a profile. Fixes: https://tracker.ceph.com/issues/37683 Signed-off-by: Tatjana Dehler --- .../osd-recv-speed-modal.component.spec.ts | 121 +++++++++++------- .../osd-recv-speed-modal.component.ts | 50 ++++---- 2 files changed, 105 insertions(+), 66 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.spec.ts index bee7768fd78..efee6abe23d 100755 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.spec.ts @@ -26,10 +26,39 @@ describe('OsdRecvSpeedModalComponent', () => { providers: [BsModalRef, i18nProviders] }); + let configOptions = []; + beforeEach(() => { fixture = TestBed.createComponent(OsdRecvSpeedModalComponent); component = fixture.componentInstance; fixture.detectChanges(); + + configOptions = [ + { + name: 'osd_max_backfills', + desc: '', + type: 'uint', + default: 1 + }, + { + name: 'osd_recovery_max_active', + desc: '', + type: 'uint', + default: 3 + }, + { + name: 'osd_recovery_max_single_start', + desc: '', + type: 'uint', + default: 1 + }, + { + name: 'osd_recovery_sleep', + desc: 'Time in seconds to sleep before next recovery or backfill op', + type: 'float', + default: 0 + } + ]; }); it('should create', () => { @@ -90,7 +119,7 @@ describe('OsdRecvSpeedModalComponent', () => { }); }); - describe('getStoredPriority', () => { + describe('detectPriority', () => { const configOptionsLow = { osd_max_backfills: 1, osd_recovery_max_active: 1, @@ -126,61 +155,84 @@ describe('OsdRecvSpeedModalComponent', () => { }; it('should return priority "low" if the config option values have been set accordingly', () => { - component.getStoredPriority(configOptionsLow, (priority) => { + component.detectPriority(configOptionsLow, (priority) => { expect(priority.name).toBe('low'); }); expect(component.osdRecvSpeedForm.getValue('customizePriority')).toBeFalsy(); }); it('should return priority "default" if the config option values have been set accordingly', () => { - component.getStoredPriority(configOptionsDefault, (priority) => { + component.detectPriority(configOptionsDefault, (priority) => { expect(priority.name).toBe('default'); }); expect(component.osdRecvSpeedForm.getValue('customizePriority')).toBeFalsy(); }); it('should return priority "high" if the config option values have been set accordingly', () => { - component.getStoredPriority(configOptionsHigh, (priority) => { + component.detectPriority(configOptionsHigh, (priority) => { expect(priority.name).toBe('high'); }); expect(component.osdRecvSpeedForm.getValue('customizePriority')).toBeFalsy(); }); it('should return priority "custom" if the config option values do not match any priority', () => { - component.getStoredPriority(configOptionsCustom, (priority) => { + component.detectPriority(configOptionsCustom, (priority) => { expect(priority.name).toBe('custom'); }); expect(component.osdRecvSpeedForm.getValue('customizePriority')).toBeTruthy(); }); it('should return no priority if the config option values are incomplete', () => { - component.getStoredPriority(configOptionsIncomplete, (priority) => { + component.detectPriority(configOptionsIncomplete, (priority) => { expect(priority.name).toBeNull(); }); expect(component.osdRecvSpeedForm.getValue('customizePriority')).toBeFalsy(); }); }); - describe('setDescription', () => { - const configOptions = [ - { - name: 'osd_max_backfills', - desc: '' - }, - { - name: 'osd_recovery_max_active', - desc: '' - }, - { - name: 'osd_recovery_max_single_start', - desc: '' - }, - { - name: 'osd_recovery_sleep', - desc: 'Time in seconds to sleep before next recovery or backfill op' - } - ]; + describe('getCurrentValues', () => { + it('should return default values if no value has been set by the user', () => { + const currentValues = component.getCurrentValues(configOptions); + configOptions.forEach((configOption) => { + const configOptionValue = currentValues.values[configOption.name]; + expect(configOptionValue).toBe(configOption.default); + }); + }); + it('should return the values set by the user if they exist', () => { + configOptions.forEach((configOption) => { + configOption['value'] = [{ section: 'osd', value: 7 }]; + }); + + const currentValues = component.getCurrentValues(configOptions); + Object.values(currentValues.values).forEach((configValue) => { + expect(configValue).toBe(7); + }); + }); + + it('should return the default value if one is missing', () => { + for (let i = 1; i < configOptions.length; i++) { + configOptions[i]['value'] = [{ section: 'osd', value: 7 }]; + } + + const currentValues = component.getCurrentValues(configOptions); + Object.entries(currentValues.values).forEach(([configName, configValue]) => { + if (configName === 'osd_max_backfills') { + expect(configValue).toBe(1); + } else { + expect(configValue).toBe(7); + } + }); + }); + + it('should return nothing if neither value nor default value is given', () => { + configOptions[0].default = null; + const currentValues = component.getCurrentValues(configOptions); + expect(currentValues.values).not.toContain('osd_max_backfills'); + }); + }); + + describe('setDescription', () => { it('should set the description if one is given', () => { component.setDescription(configOptions); Object.keys(component.priorityAttrs).forEach((configOptionName) => { @@ -196,25 +248,6 @@ describe('OsdRecvSpeedModalComponent', () => { }); describe('setValidators', () => { - const configOptions = [ - { - name: 'osd_max_backfills', - type: 'uint' - }, - { - name: 'osd_recovery_max_active', - type: 'uint' - }, - { - name: 'osd_recovery_max_single_start', - type: 'uint' - }, - { - name: 'osd_recovery_sleep', - type: 'float' - } - ]; - it('should set needed validators for config option', () => { component.setValidators(configOptions); configOptions.forEach((configOption) => { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.ts index 12dc31a0dcc..651600edcb8 100755 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-recv-speed-modal/osd-recv-speed-modal.component.ts @@ -84,23 +84,11 @@ export class OsdRecvSpeedModalComponent implements OnInit { observableForkJoin(observables) .pipe( mergeMap((configOptions) => { - const result = { values: {}, configOptions: [] }; - configOptions.forEach((configOption) => { - result.configOptions.push(configOption); - - if (configOption && 'value' in configOption) { - configOption.value.forEach((value) => { - if (value['section'] === 'osd') { - result.values[configOption.name] = Number(value.value); - } - }); - } - }); - return of(result); + return of(this.getCurrentValues(configOptions)); }) ) .subscribe((resp) => { - this.getStoredPriority(resp.values, (priority) => { + this.detectPriority(resp.values, (priority) => { this.setPriority(priority); }); this.setDescription(resp.configOptions); @@ -108,7 +96,7 @@ export class OsdRecvSpeedModalComponent implements OnInit { }); } - getStoredPriority(configOptionValues: any, callbackFn: Function) { + detectPriority(configOptionValues: any, callbackFn: Function) { const priority = _.find(this.priorities, (p) => { return _.isEqual(p.values, configOptionValues); }); @@ -129,6 +117,24 @@ export class OsdRecvSpeedModalComponent implements OnInit { return callbackFn(this.priorities[0]); } + getCurrentValues(configOptions: any) { + const currentValues = { values: {}, configOptions: [] }; + configOptions.forEach((configOption) => { + currentValues.configOptions.push(configOption); + + if ('value' in configOption) { + configOption.value.forEach((value) => { + if (value.section === 'osd') { + currentValues.values[configOption.name] = Number(value.value); + } + }); + } else if ('default' in configOption && configOption.default !== null) { + currentValues.values[configOption.name] = Number(configOption.default); + } + }); + return currentValues; + } + setDescription(configOptions: Array) { configOptions.forEach((configOption) => { if (configOption.desc !== '') { @@ -181,11 +187,12 @@ export class OsdRecvSpeedModalComponent implements OnInit { } onCustomizePriorityChange() { + const values = {}; + Object.keys(this.priorityAttrs).forEach((configOptionName) => { + values[configOptionName] = this.osdRecvSpeedForm.getValue(configOptionName); + }); + if (this.osdRecvSpeedForm.getValue('customizePriority')) { - const values = {}; - Object.keys(this.priorityAttrs).forEach((configOptionName) => { - values[configOptionName] = this.osdRecvSpeedForm.getValue(configOptionName); - }); const customPriority = { name: 'custom', text: this.i18n('Custom'), @@ -193,10 +200,9 @@ export class OsdRecvSpeedModalComponent implements OnInit { }; this.setPriority(customPriority); } else { - Object.keys(this.priorityAttrs).forEach((configOptionName) => { - this.osdRecvSpeedForm.get(configOptionName).reset(); + this.detectPriority(values, (priority) => { + this.setPriority(priority); }); - this.setPriority(this.priorities[0]); } }