Merge pull request #59820 from rhcs-dashboard/rgw-multisite-sync-policy-improvements

mgr/dashboard: multisite sync policy improvements 

Reviewed-by: afreen23 <NOT@FOUND>
Reviewed-by: Nizamudeen A <nia@redhat.com>

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 @@
   <ng-container i18n="form title"
                 class="modal-title">{{ action | titlecase }} {{ groupType | upperFirst }} Flow</ng-container>
 
-    <ng-container class="modal-content">
-      <form name="flowForm"
-            #frm="ngForm"
-            [formGroup]="currentFormGroupContext"
-            novalidate>
-        <div class="modal-body">
+  <ng-container class="modal-content">
+    <form name="flowForm"
+          #frm="ngForm"
+          [formGroup]="currentFormGroupContext"
+          novalidate>
+      <div class="modal-body">
+        <div class="form-group row">
+          <label class="cd-col-form-label required"
+                 for="flow_id"
+                 i18n>Name</label>
+          <div class="cd-col-form-input">
+            <input class="form-control"
+                   type="text"
+                   placeholder="Flow Name..."
+                   id="flow_id"
+                   name="flow_id"
+                   formControlName="flow_id"/>
+          </div>
+        </div>
+        <div class="form-group row">
+          <label class="cd-col-form-label"
+                 for="bucket"
+                 i18n>Bucket Name</label>
+          <div class="cd-col-form-input">
+            <input id="bucket"
+                   name="bucket"
+                   class="form-control"
+                   type="text"
+                   i18n-placeholder
+                   placeholder="Bucket Name..."
+                   formControlName="bucket_name"/>
+            <span class="invalid-feedback"
+                  *ngIf="currentFormGroupContext.showError('bucket_name', frm, 'bucketNameNotAllowed')"
+                  i18n>The bucket with chosen name does not exist.</span>
+          </div>
+        </div>
+        <ng-container *ngIf="groupType == flowType.symmetrical; else directionalFlow">
           <div class="form-group row">
             <label class="cd-col-form-label required"
-                   for="flow_id"
-                   i18n>Name</label>
+                   for="zones">
+              <ng-container i18n>Zones</ng-container>
+              <cd-helper>
+                <span i18n>Flow need to be associated with atleast one zone</span>
+              </cd-helper>
+            </label>
             <div class="cd-col-form-input">
-              <input class="form-control"
-                     type="text"
-                     placeholder="Flow Name..."
-                     id="flow_id"
-                     name="flow_id"
-                     formControlName="flow_id"/>
+              <ng-container *ngTemplateOutlet="zoneMultiSelect;context: { name: 'zones', zone: zones }"></ng-container>
+            </div>
+          </div>
+        </ng-container>
+        <ng-template #directionalFlow>
+          <div class="form-group row">
+            <label class="cd-col-form-label required"
+                   for="source_zone"
+                   i18n>Source Zone
+            </label>
+            <div class="cd-col-form-input">
+              <ng-container *ngTemplateOutlet="sourceAndDestZone;context: { name: 'source_zone', zones: zones }"></ng-container>
             </div>
           </div>
           <div class="form-group row">
-            <label class="cd-col-form-label"
-                   for="bucket"
-                   i18n>Bucket Name</label>
+            <label class="cd-col-form-label required"
+                   for="destination_zone"
+                   i18n>Destination Zone</label>
             <div class="cd-col-form-input">
-              <input id="bucket"
-                     name="bucket"
-                     class="form-control"
-                     type="text"
-                     i18n-placeholder
-                     placeholder="Bucket Name..."
-                     formControlName="bucket_name"/>
-              <span class="invalid-feedback"
-                    *ngIf="currentFormGroupContext.showError('bucket_name', frm, 'bucketNameNotAllowed')"
-                    i18n>The bucket with chosen name does not exist.</span>
+              <ng-container *ngTemplateOutlet="sourceAndDestZone;context: { name: 'destination_zone', zones: zones }"></ng-container>
             </div>
           </div>
-          <ng-container *ngIf="groupType == flowType.symmetrical; else directionalFlow">
-            <div class="form-group row">
-              <label class="cd-col-form-label required"
-                     for="zones">
-                <ng-container i18n>Zones</ng-container>
-                <cd-helper>
-                  <span i18n>Flow need to be associated with atleast one zone</span>
-                </cd-helper>
-              </label>
-              <div class="cd-col-form-input">
-                <ng-container *ngTemplateOutlet="zoneMultiSelect;context: { name: 'zones', zone: zones }"></ng-container>
-              </div>
-            </div>
-          </ng-container>
-          <ng-template #directionalFlow>
-            <div class="form-group row">
-              <label class="cd-col-form-label required"
-                     for="source_zone"
-                     i18n>Source Zone
-              </label>
-              <div class="cd-col-form-input">
-                <select id="sourceZone"
-                        name="sourceZone"
-                        class="form-select"
-                        formControlName="source_zone"
-                        [autofocus]="editing">
-                <option i18n
-                        *ngIf="zones.data.available.length == 0"
-                        [ngValue]="null">Loading...</option>
-                <option i18n
-                        *ngIf="zones.data.available.length > 0"
-                        [ngValue]="null">-- Select source zone --</option>
-                <option *ngFor="let sourceZone of zones.data.available"
-                        [value]="sourceZone.name">{{ sourceZone.name }}</option>
-              </select>
-              <span class="invalid-feedback"
-                    *ngIf="currentFormGroupContext.showError('source_zone', frm, 'required')"
-                    i18n>This field is required.</span>
-              </div>
-            </div>
-            <div class="form-group row">
-              <label class="cd-col-form-label required"
-                     for="destination_zone"
-                     i18n>Destination Zone</label>
-              <div class="cd-col-form-input">
-                <input id="destination_zone"
-                       name="destination_zone"
-                       class="form-control"
-                       type="text"
-                       i18n-placeholder
-                       placeholder="Destination Zone..."
-                       formControlName="destination_zone"/>
-                <span class="invalid-feedback"
-                      *ngIf="currentFormGroupContext.showError('destination_zone', frm, 'required')"
-                      i18n>This field is required.</span>
-              </div>
-            </div>
-          </ng-template>
-        </div>
-        <div class="modal-footer">
-          <cd-form-button-panel (submitActionEvent)="submit()"
-                                [form]="currentFormGroupContext"
-                                [submitText]="(action | titlecase) + ' ' + (groupType | upperFirst) + ' ' + 'Flow'"></cd-form-button-panel>
-        </div>
-      </form>
-    </ng-container>
-  </cd-modal>
+        </ng-template>
+      </div>
+      <div class="modal-footer">
+        <cd-form-button-panel (submitActionEvent)="submit()"
+                              [form]="currentFormGroupContext"
+                              [submitText]="(action | titlecase) + ' ' + (groupType | upperFirst) + ' ' + 'Flow'"></cd-form-button-panel>
+      </div>
+    </form>
+  </ng-container>
+</cd-modal>
 
 <ng-template #zoneMultiSelect
              let-name="name"
@@ -128,3 +103,25 @@
         i18n>{{name?.split('_').join(' ')}} selection is required!
   </span>
 </ng-template>
+
+<ng-template #sourceAndDestZone
+             let-name="name"
+             let-zones="zones">
+  <select [id]="name"
+          [name]="name"
+          class="form-select"
+          (change)="onChangeZoneDropdown(name, $event)"
+          [autofocus]="editing">
+  <option i18n
+          *ngIf="zones.data.available.length == 0"
+          [ngValue]="null">Loading...</option>
+  <option i18n
+          *ngIf="zones.data.available.length > 0"
+          [ngValue]="null">-- Select {{name.split('_').join(' ')}} --</option>
+  <option *ngFor="let destinationZone of zones.data.available"
+          [value]="destinationZone.name">{{ destinationZone.name }}</option>
+  </select>
+  <span class="invalid-feedback"
+        *ngIf="currentFormGroupContext.showError(name, frm, 'required')"
+        i18n>This field is required.</span>
+</ng-template>

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<RgwMultisiteSyncFlowModalComponent>;
+  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: [] }
+    });
+  });
 });

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),

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<RgwMultisiteSyncPipeModalComponent>;
+  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());
+  });
 });

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(),

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
       },
       {

src/pybind/mgr/dashboard/services/rgw_client.py

@@ -2201,7 +2201,8 @@ class RgwMultisite:
             if not bucket_name and update_period:
                 self.update_period()
 
-        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)