From a2f983074019bc7ecf48dd4313ea333ceccf1214 Mon Sep 17 00:00:00 2001 From: Ivo Almeida Date: Tue, 19 Mar 2024 12:44:40 +0000 Subject: [PATCH] mgr/dashboard: fix repeat frequency validation Fixed snap schedule repeat frequency validation Fixes: https://tracker.ceph.com/issues/64980 Signed-off-by: Ivo Almeida (cherry picked from commit 92b40f91c07086406103c865af7c645b99d43806) --- ...fs-snapshotschedule-form.component.spec.ts | 23 ++- .../cephfs-snapshotschedule-form.component.ts | 162 ++++++++++-------- .../api/cephfs-snapshot-schedule.service.ts | 9 +- 3 files changed, 111 insertions(+), 83 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.spec.ts index 6a9fbcb942a68..dedbd3bafc1ae 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.spec.ts @@ -1,5 +1,11 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { + ComponentFixture, + TestBed, + discardPeriodicTasks, + fakeAsync, + tick +} from '@angular/core/testing'; import { CephfsSnapshotscheduleFormComponent } from './cephfs-snapshotschedule-form.component'; import { @@ -13,12 +19,12 @@ import { RouterTestingModule } from '@angular/router/testing'; import { ReactiveFormsModule } from '@angular/forms'; import { FormHelper, configureTestBed } from '~/testing/unit-test-helper'; import { CephfsSnapshotScheduleService } from '~/app/shared/api/cephfs-snapshot-schedule.service'; +import { of } from 'rxjs'; describe('CephfsSnapshotscheduleFormComponent', () => { let component: CephfsSnapshotscheduleFormComponent; let fixture: ComponentFixture; let formHelper: FormHelper; - let createSpy: jasmine.Spy; configureTestBed({ declarations: [CephfsSnapshotscheduleFormComponent], @@ -40,7 +46,6 @@ describe('CephfsSnapshotscheduleFormComponent', () => { component.fsName = 'test_fs'; component.ngOnInit(); formHelper = new FormHelper(component.snapScheduleForm); - createSpy = spyOn(TestBed.inject(CephfsSnapshotScheduleService), 'create').and.stub(); fixture.detectChanges(); }); @@ -53,7 +58,12 @@ describe('CephfsSnapshotscheduleFormComponent', () => { expect(nativeEl.querySelector('cd-modal')).not.toBe(null); }); - it('should submit the form', () => { + it('should submit the form', fakeAsync(() => { + const createSpy = spyOn(TestBed.inject(CephfsSnapshotScheduleService), 'create').and.stub(); + const checkScheduleExistsSpy = spyOn( + TestBed.inject(CephfsSnapshotScheduleService), + 'checkScheduleExists' + ).and.returnValue(of(false)); const input = { directory: '/test', startDate: { @@ -73,7 +83,10 @@ describe('CephfsSnapshotscheduleFormComponent', () => { formHelper.setMultipleValues(input); component.snapScheduleForm.get('directory').setValue('/test'); component.submit(); + tick(400); + expect(checkScheduleExistsSpy).toHaveBeenCalled(); expect(createSpy).toHaveBeenCalled(); - }); + discardPeriodicTasks(); + })); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.ts index a34226c563ba3..61743caa72805 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-snapshotschedule-form/cephfs-snapshotschedule-form.component.ts @@ -115,6 +115,10 @@ export class CephfsSnapshotscheduleFormComponent extends CdForm implements OnIni this.subvolume = value?.split?.('/')?.[3]; }), filter(() => !!this.subvolume && !!this.subvolumeGroup), + tap(() => { + this.isSubvolume = !!this.subvolume && !!this.subvolumeGroup; + this.snapScheduleForm.get('repeatFrequency').setErrors(null); + }), mergeMap(() => this.subvolumeService .exists( @@ -282,84 +286,92 @@ export class CephfsSnapshotscheduleFormComponent extends CdForm implements OnIni } submit() { - if (this.snapScheduleForm.invalid) { - this.snapScheduleForm.setErrors({ cdSubmitButton: true }); - return; - } - - const values = this.snapScheduleForm.value as SnapshotScheduleFormValue; - - if (this.isEdit) { - const retentionPoliciesToAdd = (this.snapScheduleForm.get( - 'retentionPolicies' - ) as FormArray).controls - ?.filter( - (ctrl) => - !ctrl.get('retentionInterval').disabled && !ctrl.get('retentionFrequency').disabled - ) - .map((ctrl) => ({ - retentionInterval: ctrl.get('retentionInterval').value, - retentionFrequency: ctrl.get('retentionFrequency').value - })); - - const updateObj = { - fs: this.fsName, - path: this.path, - subvol: this.subvol, - group: this.group, - retention_to_add: this.parseRetentionPolicies(retentionPoliciesToAdd) || null, - retention_to_remove: this.parseRetentionPolicies(this.retentionPoliciesToRemove) || null - }; - - this.taskWrapper - .wrapTaskAroundCall({ - task: new FinishedTask('cephfs/snapshot/schedule/' + URLVerbs.EDIT, { - path: this.path - }), - call: this.snapScheduleService.update(updateObj) - }) - .subscribe({ - error: () => { - this.snapScheduleForm.setErrors({ cdSubmitButton: true }); - }, - complete: () => { - this.activeModal.close(); + this.validateSchedule()(this.snapScheduleForm).subscribe({ + next: () => { + if (this.snapScheduleForm.invalid) { + this.snapScheduleForm.setErrors({ cdSubmitButton: true }); + return; + } + + const values = this.snapScheduleForm.value as SnapshotScheduleFormValue; + + if (this.isEdit) { + const retentionPoliciesToAdd = (this.snapScheduleForm.get( + 'retentionPolicies' + ) as FormArray).controls + ?.filter( + (ctrl) => + !ctrl.get('retentionInterval').disabled && !ctrl.get('retentionFrequency').disabled + ) + .map((ctrl) => ({ + retentionInterval: ctrl.get('retentionInterval').value, + retentionFrequency: ctrl.get('retentionFrequency').value + })); + + const updateObj = { + fs: this.fsName, + path: this.path, + subvol: this.subvol, + group: this.group, + retention_to_add: this.parseRetentionPolicies(retentionPoliciesToAdd) || null, + retention_to_remove: this.parseRetentionPolicies(this.retentionPoliciesToRemove) || null + }; + + this.taskWrapper + .wrapTaskAroundCall({ + task: new FinishedTask('cephfs/snapshot/schedule/' + URLVerbs.EDIT, { + path: this.path + }), + call: this.snapScheduleService.update(updateObj) + }) + .subscribe({ + error: () => { + this.snapScheduleForm.setErrors({ cdSubmitButton: true }); + }, + complete: () => { + this.activeModal.close(); + } + }); + } else { + const snapScheduleObj = { + fs: this.fsName, + path: values.directory, + snap_schedule: this.parseSchedule(values?.repeatInterval, values?.repeatFrequency), + start: this.parseDatetime(values?.startDate, values?.startTime) + }; + + const retentionPoliciesValues = this.parseRetentionPolicies(values?.retentionPolicies); + + if (retentionPoliciesValues) { + snapScheduleObj['retention_policy'] = retentionPoliciesValues; } - }); - } else { - const snapScheduleObj = { - fs: this.fsName, - path: values.directory, - snap_schedule: this.parseSchedule(values?.repeatInterval, values?.repeatFrequency), - start: this.parseDatetime(values?.startDate, values?.startTime) - }; - const retentionPoliciesValues = this.parseRetentionPolicies(values?.retentionPolicies); - - if (retentionPoliciesValues) snapScheduleObj['retention_policy'] = retentionPoliciesValues; + if (this.isSubvolume) { + snapScheduleObj['subvol'] = this.subvolume; + } - if (this.isSubvolume) snapScheduleObj['subvol'] = this.subvolume; + if (this.isSubvolume && !this.isDefaultSubvolumeGroup) { + snapScheduleObj['group'] = this.subvolumeGroup; + } - if (this.isSubvolume && !this.isDefaultSubvolumeGroup) { - snapScheduleObj['group'] = this.subvolumeGroup; + this.taskWrapper + .wrapTaskAroundCall({ + task: new FinishedTask('cephfs/snapshot/schedule/' + URLVerbs.CREATE, { + path: snapScheduleObj.path + }), + call: this.snapScheduleService.create(snapScheduleObj) + }) + .subscribe({ + error: () => { + this.snapScheduleForm.setErrors({ cdSubmitButton: true }); + }, + complete: () => { + this.activeModal.close(); + } + }); + } } - - this.taskWrapper - .wrapTaskAroundCall({ - task: new FinishedTask('cephfs/snapshot/schedule/' + URLVerbs.CREATE, { - path: snapScheduleObj.path - }), - call: this.snapScheduleService.create(snapScheduleObj) - }) - .subscribe({ - error: () => { - this.snapScheduleForm.setErrors({ cdSubmitButton: true }); - }, - complete: () => { - this.activeModal.close(); - } - }); - } + }); } validateSchedule() { @@ -379,11 +391,13 @@ export class CephfsSnapshotscheduleFormComponent extends CdForm implements OnIni directory?.value, this.fsName, repeatInterval?.value, - repeatFrequency?.value + repeatFrequency?.value, + this.isSubvolume ) .pipe( map((exists: boolean) => { if (exists) { + repeatFrequency?.markAsDirty(); repeatFrequency?.setErrors({ notUnique: true }, { emitEvent: true }); } else { repeatFrequency?.setErrors(null); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-snapshot-schedule.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-snapshot-schedule.service.ts index ade935a9299a4..855f3939496b6 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-snapshot-schedule.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-snapshot-schedule.service.ts @@ -73,13 +73,14 @@ export class CephfsSnapshotScheduleService { path: string, fs: string, interval: number, - frequency: RepeatFrequency + frequency: RepeatFrequency, + isSubvolume = false ): Observable { return this.getSnapshotScheduleList(path, fs, false).pipe( map((response) => { - const index = response.findIndex( - (x) => x.path === path && x.schedule === `${interval}${frequency}` - ); + const index = response + .filter((x) => (isSubvolume ? x.path.startsWith(path) : x.path === path)) + .findIndex((x) => x.schedule === `${interval}${frequency}`); return index > -1; }), catchError(() => { -- 2.39.5