From af437b4e7f13e448a93583ee22c625cb1c74aae7 Mon Sep 17 00:00:00 2001 From: Naman Munet Date: Tue, 17 Sep 2024 12:29:37 +0530 Subject: [PATCH] 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 --- ...w-multisite-sync-flow-modal.component.html | 185 +++++++++--------- ...ultisite-sync-flow-modal.component.spec.ts | 63 +++++- ...rgw-multisite-sync-flow-modal.component.ts | 7 +- ...ultisite-sync-pipe-modal.component.spec.ts | 60 +++++- ...rgw-multisite-sync-pipe-modal.component.ts | 32 ++- .../rgw-multisite-sync-policy.component.ts | 1 + .../mgr/dashboard/services/rgw_client.py | 3 +- 7 files changed, 248 insertions(+), 103 deletions(-) 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 50b0bf321bf..5f13a0aedb1 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 fb864bac457..2efdbfb8c9d 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 bf243f880d9..1ab0705f53b 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 30fd3e42c0e..369658d7d42 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 da4475fe1cd..2f41dbd23c8 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 53602b2f7b9..ee261db5042 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 1f9fa74f425..00b052c2d16 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)