From: Naman Munet Date: Tue, 17 Sep 2024 06:59:37 +0000 (+0530) Subject: mgr/dashboard: multisite sync policy improvements X-Git-Tag: v20.0.0~984^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=af437b4e7f13e448a93583ee22c625cb1c74aae7;p=ceph.git mgr/dashboard: multisite sync policy improvements https://tracker.ceph.com/issues/68097 Changes for this PR includes: 1) Populating the destination zones select option with a set of options to choose from, for flow and pipe so that user can't enter any invalid zone name 2) Provided zone option as 'All Zones (*)' in pipe, if user want to select all zones for source and destination zones 3) We are hiding the UniqueId column on sync policy table as we do not want to show it as this column is introduced just to uniquely identify a row in the table and should not be displayed to users as it is part of the internal logic to work. Signed-off-by: Naman Munet --- diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.html index 50b0bf321bf2..5f13a0aedb1f 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.html @@ -2,109 +2,84 @@ {{ action | titlecase }} {{ groupType | upperFirst }} Flow - -
-
+ + +
+
+ +
+ +
+
+
+ +
+ + The bucket with chosen name does not exist. +
+
+
+ for="zones"> + Zones + + Flow need to be associated with atleast one zone + +
- +
+ +
- +
- - The bucket with chosen name does not exist. +
- -
- -
- -
-
- - -
- -
- - This field is required. -
-
-
- -
- - This field is required. -
+
+ +
+
- -
-
- -
- - - +
+ +
+
+ +
+ + + {{name?.split('_').join(' ')}} selection is required! + + + + This field is required. + diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.spec.ts index fb864bac4570..2efdbfb8c9db 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.spec.ts @@ -6,14 +6,22 @@ import { PipesModule } from '~/app/shared/pipes/pipes.module'; import { HttpClientTestingModule } from '@angular/common/http/testing'; import { ReactiveFormsModule } from '@angular/forms'; import { CommonModule } from '@angular/common'; +import { RgwMultisiteService } from '~/app/shared/api/rgw-multisite.service'; +import { of } from 'rxjs'; enum FlowType { symmetrical = 'symmetrical', directional = 'directional' } + +class MultisiteServiceMock { + createEditSyncFlow = jest.fn().mockReturnValue(of(null)); +} + describe('RgwMultisiteSyncFlowModalComponent', () => { let component: RgwMultisiteSyncFlowModalComponent; let fixture: ComponentFixture; + let multisiteServiceMock: MultisiteServiceMock; beforeEach(async () => { await TestBed.configureTestingModule({ @@ -25,10 +33,11 @@ describe('RgwMultisiteSyncFlowModalComponent', () => { ReactiveFormsModule, CommonModule ], - providers: [NgbActiveModal] + providers: [NgbActiveModal, { provide: RgwMultisiteService, useClass: MultisiteServiceMock }] }).compileComponents(); fixture = TestBed.createComponent(RgwMultisiteSyncFlowModalComponent); + multisiteServiceMock = (TestBed.inject(RgwMultisiteService) as unknown) as MultisiteServiceMock; component = fixture.componentInstance; component.groupType = FlowType.symmetrical; fixture.detectChanges(); @@ -37,4 +46,56 @@ describe('RgwMultisiteSyncFlowModalComponent', () => { it('should create', () => { expect(component).toBeTruthy(); }); + + it('should assign zone value', () => { + let zonesAdded: string[] = []; + let selectedZone = ['zone2-zg1-realm1']; + const spy = jest.spyOn(component, 'assignZoneValue').mockReturnValue(selectedZone); + const res = component.assignZoneValue(zonesAdded, selectedZone); + expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith(zonesAdded, selectedZone); + expect(res).toEqual(selectedZone); + }); + + it('should call createEditSyncFlow for creating/editing symmetrical sync flow', () => { + component.editing = false; + component.currentFormGroupContext.patchValue({ + flow_id: 'symmetrical', + group_id: 'new', + zones: { added: ['zone1-zg1-realm1'], removed: [] } + }); + component.zones.data.selected = ['zone1-zg1-realm1']; + const spy = jest.spyOn(component, 'submit'); + const putDataSpy = jest + .spyOn(multisiteServiceMock, 'createEditSyncFlow') + .mockReturnValue(of(null)); + component.submit(); + expect(spy).toHaveBeenCalled(); + expect(putDataSpy).toHaveBeenCalled(); + expect(putDataSpy).toHaveBeenCalledWith(component.currentFormGroupContext.getRawValue()); + }); + + it('should call createEditSyncFlow for creating/editing directional sync flow', () => { + component.editing = false; + component.groupType = FlowType.directional; + component.ngOnInit(); + fixture.detectChanges(); + component.currentFormGroupContext.patchValue({ + flow_id: 'directional', + group_id: 'new', + source_zone: { added: ['zone1-zg1-realm1'], removed: [] }, + destination_zone: { added: ['zone2-zg1-realm1'], removed: [] } + }); + const spy = jest.spyOn(component, 'submit'); + const putDataSpy = jest + .spyOn(multisiteServiceMock, 'createEditSyncFlow') + .mockReturnValue(of(null)); + component.submit(); + expect(spy).toHaveBeenCalled(); + expect(putDataSpy).toHaveBeenCalled(); + expect(putDataSpy).toHaveBeenCalledWith({ + ...component.currentFormGroupContext.getRawValue(), + zones: { added: [], removed: [] } + }); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.ts index bf243f880d9b..1ab0705f53b2 100755 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-flow-modal/rgw-multisite-sync-flow-modal.component.ts @@ -35,8 +35,6 @@ export class RgwMultisiteSyncFlowModalComponent implements OnInit { flowType = FlowType; icons = Icons; zones = new ZoneData(false, 'Filter Zones'); - sourceZone: string; - destinationZone: string; constructor( public activeModal: NgbActiveModal, @@ -122,6 +120,11 @@ export class RgwMultisiteSyncFlowModalComponent implements OnInit { }); } + onChangeZoneDropdown(zoneType: string, event: Event) { + const selectedVal = (event.target as HTMLSelectElement).value; + this.currentFormGroupContext.get(zoneType).setValue(selectedVal); + } + commonFormControls(flowType: FlowType) { return { bucket_name: new UntypedFormControl(this.groupExpandedRow?.bucket), diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.spec.ts index 30fd3e42c0ec..369658d7d427 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.spec.ts @@ -7,10 +7,17 @@ import { PipesModule } from '~/app/shared/pipes/pipes.module'; import { ReactiveFormsModule } from '@angular/forms'; import { CommonModule } from '@angular/common'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { of } from 'rxjs'; +import { RgwMultisiteService } from '~/app/shared/api/rgw-multisite.service'; + +class MultisiteServiceMock { + createEditSyncPipe = jest.fn().mockReturnValue(of(null)); +} describe('RgwMultisiteSyncPipeModalComponent', () => { let component: RgwMultisiteSyncPipeModalComponent; let fixture: ComponentFixture; + let multisiteServiceMock: MultisiteServiceMock; beforeEach(async () => { await TestBed.configureTestingModule({ @@ -22,10 +29,11 @@ describe('RgwMultisiteSyncPipeModalComponent', () => { ReactiveFormsModule, CommonModule ], - providers: [NgbActiveModal] + providers: [NgbActiveModal, { provide: RgwMultisiteService, useClass: MultisiteServiceMock }] }).compileComponents(); fixture = TestBed.createComponent(RgwMultisiteSyncPipeModalComponent); + multisiteServiceMock = (TestBed.inject(RgwMultisiteService) as unknown) as MultisiteServiceMock; component = fixture.componentInstance; fixture.detectChanges(); }); @@ -33,4 +41,54 @@ describe('RgwMultisiteSyncPipeModalComponent', () => { it('should create', () => { expect(component).toBeTruthy(); }); + + it('should replace `*` with `All Zones (*)`', () => { + let zones = ['*', 'zone1-zg1-realm1', 'zone2-zg1-realm1']; + let mockReturnVal = ['All Zones (*)', 'zone1-zg1-realm1', 'zone2-zg1-realm1']; + const spy = jest.spyOn(component, 'replaceAsteriskWithString').mockReturnValue(mockReturnVal); + const res = component.replaceAsteriskWithString(zones); + expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith(zones); + expect(res).toEqual(mockReturnVal); + }); + + it('should replace `All Zones (*)` with `*`', () => { + let zones = ['All Zones (*)', 'zone1-zg1-realm1', 'zone2-zg1-realm1']; + let mockReturnVal = ['*', 'zone1-zg1-realm1', 'zone2-zg1-realm1']; + const spy = jest.spyOn(component, 'replaceWithAsterisk').mockReturnValue(mockReturnVal); + const res = component.replaceWithAsterisk(zones); + expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith(zones); + expect(res).toEqual(mockReturnVal); + }); + + it('should assign zone value', () => { + let zonesAdded: string[] = []; + let selectedZone = ['zone2-zg1-realm1']; + const spy = jest.spyOn(component, 'assignZoneValue').mockReturnValue(selectedZone); + const res = component.assignZoneValue(zonesAdded, selectedZone); + expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith(zonesAdded, selectedZone); + expect(res).toEqual(selectedZone); + }); + + it('should call createEditSyncPipe for creating/editing sync pipe', () => { + component.editing = false; + component.pipeForm.patchValue({ + pipe_id: 'pipe1', + group_id: 'new', + source_bucket: '', + source_zones: { added: ['zone1-zg1-realm1'], removed: [] }, + destination_bucket: '', + destination_zones: { added: ['zone2-zg1-realm1'], removed: [] } + }); + component.sourceZones.data.selected = ['zone1-zg1-realm1']; + component.destZones.data.selected = ['zone2-zg1-realm1']; + const spy = jest.spyOn(component, 'submit'); + const putDataSpy = jest.spyOn(multisiteServiceMock, 'createEditSyncPipe'); + component.submit(); + expect(spy).toHaveBeenCalled(); + expect(putDataSpy).toHaveBeenCalled(); + expect(putDataSpy).toHaveBeenCalledWith(component.pipeForm.getRawValue()); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.ts index da4475fe1cdd..2f41dbd23c84 100755 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-pipe-modal/rgw-multisite-sync-pipe-modal.component.ts @@ -17,6 +17,8 @@ import { NotificationService } from '~/app/shared/services/notification.service' import { ZoneData } from '../models/rgw-multisite-zone-selector'; import { SucceededActionLabelsI18n } from '~/app/shared/constants/app.constants'; +const ALL_ZONES = $localize`All zones (*)`; + @Component({ selector: 'cd-rgw-multisite-sync-pipe-modal', templateUrl: './rgw-multisite-sync-pipe-modal.component.html', @@ -29,7 +31,7 @@ export class RgwMultisiteSyncPipeModalComponent implements OnInit { action: string; editing: boolean; sourceZones = new ZoneData(false, 'Filter Zones'); - destZones = new ZoneData(true, 'Filter or Add Zones'); + destZones = new ZoneData(false, 'Filter Zones'); icons = Icons; constructor( @@ -42,6 +44,14 @@ export class RgwMultisiteSyncPipeModalComponent implements OnInit { ) {} ngOnInit(): void { + if (this.pipeSelectedRow) { + this.pipeSelectedRow.source.zones = this.replaceAsteriskWithString( + this.pipeSelectedRow.source.zones + ); + this.pipeSelectedRow.dest.zones = this.replaceAsteriskWithString( + this.pipeSelectedRow.dest.zones + ); + } this.editing = this.action === 'create' ? false : true; this.pipeForm = new CdFormGroup({ pipe_id: new UntypedFormControl('', { @@ -80,10 +90,12 @@ export class RgwMultisiteSyncPipeModalComponent implements OnInit { .subscribe((zonegroupData: any) => { if (zonegroupData && zonegroupData?.zones?.length > 0) { let zones: any[] = []; + zones.push(new SelectOption(false, ALL_ZONES, '')); zonegroupData.zones.forEach((zone: any) => { zones.push(new SelectOption(false, zone.name, '')); }); - this.sourceZones.data.available = [...zones]; + this.sourceZones.data.available = JSON.parse(JSON.stringify(zones)); + this.destZones.data.available = JSON.parse(JSON.stringify(zones)); if (this.editing) { this.pipeForm.get('pipe_id').disable(); this.sourceZones.data.selected = [...this.pipeSelectedRow.source.zones]; @@ -92,7 +104,6 @@ export class RgwMultisiteSyncPipeModalComponent implements OnInit { this.pipeSelectedRow.dest.zones.forEach((zone: string) => { availableDestZone.push(new SelectOption(true, zone, '')); }); - this.destZones.data.available = availableDestZone; this.pipeForm.patchValue({ pipe_id: this.pipeSelectedRow.id, source_zones: this.pipeSelectedRow.source.zones, @@ -105,6 +116,14 @@ export class RgwMultisiteSyncPipeModalComponent implements OnInit { }); } + replaceWithAsterisk(zones: string[]) { + return zones.map((str) => str.replace(ALL_ZONES, '*')); + } + + replaceAsteriskWithString(zones: string[]) { + return zones.map((str) => str.replace('*', ALL_ZONES)); + } + onZoneSelection(zoneType: string) { if (zoneType === 'source_zones') { this.pipeForm.patchValue({ @@ -122,7 +141,9 @@ export class RgwMultisiteSyncPipeModalComponent implements OnInit { } assignZoneValue(zone: string[], selectedZone: string[]) { - return zone.length > 0 ? zone : selectedZone; + return zone.length > 0 + ? this.replaceWithAsterisk(zone) + : this.replaceWithAsterisk(selectedZone); } submit() { @@ -159,6 +180,9 @@ export class RgwMultisiteSyncPipeModalComponent implements OnInit { sourceZones.added = this.assignZoneValue(sourceZones.added, this.sourceZones.data.selected); destZones.added = this.assignZoneValue(destZones.added, this.destZones.data.selected); + sourceZones.removed = this.replaceWithAsterisk(sourceZones.removed); + destZones.removed = this.replaceWithAsterisk(destZones.removed); + this.rgwMultisiteService .createEditSyncPipe({ ...this.pipeForm.getRawValue(), diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-policy/rgw-multisite-sync-policy.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-policy/rgw-multisite-sync-policy.component.ts index 53602b2f7b98..ee261db5042c 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-policy/rgw-multisite-sync-policy.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-multisite-sync-policy/rgw-multisite-sync-policy.component.ts @@ -59,6 +59,7 @@ export class RgwMultisiteSyncPolicyComponent extends ListWithDetails implements this.columns = [ { prop: 'uniqueId', + isInvisible: true, isHidden: true }, { diff --git a/src/pybind/mgr/dashboard/services/rgw_client.py b/src/pybind/mgr/dashboard/services/rgw_client.py index 1f9fa74f4256..00b052c2d167 100755 --- a/src/pybind/mgr/dashboard/services/rgw_client.py +++ b/src/pybind/mgr/dashboard/services/rgw_client.py @@ -2174,7 +2174,8 @@ class RgwMultisite: except SubprocessError as error: raise DashboardException(error, http_status_code=500, component='rgw') - if source_zones['removed'] or destination_zones['removed']: + if ((source_zones['removed'] and '*' not in source_zones['added']) + or (destination_zones['removed'] and '*' not in destination_zones['added'])): self.remove_sync_pipe(group_id, pipe_id, source_zones['removed'], destination_zones['removed'], destination_bucket, bucket_name)