Merge pull request ceph#53182 from rhcs-dashboard/fix-subvolume-rm-sn…

…apshots mgr/dashboard: subvolume rm with snapshots Reviewed-by: Aashish Sharma <[email protected]> Reviewed-by: Nizamudeen A <[email protected]>
ssdohammer-sl · Aug 31, 2023 · 9051031 · 9051031
2 parents a8ad05f + 453fbcb
commit 9051031
Show file tree

Hide file tree

Showing 12 changed files with 100 additions and 28 deletions.
diff --git a/src/pybind/mgr/dashboard/controllers/cephfs.py b/src/pybind/mgr/dashboard/controllers/cephfs.py
@@ -14,7 +14,7 @@
 from ..services.ceph_service import CephService
 from ..services.cephfs import CephFS as CephFS_
 from ..services.exception import handle_cephfs_error
-from ..tools import ViewCache
+from ..tools import ViewCache, str_to_bool
 from . import APIDoc, APIRouter, DeletePermission, Endpoint, EndpointDoc, \
     RESTController, UIRouter, UpdatePermission, allow_empty_body
 
@@ -722,14 +722,17 @@ def set(self, vol_name: str, subvol_name: str, size: str):
 
         return f'Subvolume {subvol_name} updated successfully'
 
-    def delete(self, vol_name: str, subvol_name: str):
+    def delete(self, vol_name: str, subvol_name: str, retain_snapshots: bool = False):
+        params = {'vol_name': vol_name, 'sub_name': subvol_name}
+        retain_snapshots = str_to_bool(retain_snapshots)
+        if retain_snapshots:
+            params['retain_snapshots'] = 'True'
         error_code, _, err = mgr.remote(
-            'volumes', '_cmd_fs_subvolume_rm', None, {
-                'vol_name': vol_name, 'sub_name': subvol_name})
+            'volumes', '_cmd_fs_subvolume_rm', None, params)
         if error_code != 0:
-            raise RuntimeError(
-                f'Failed to delete subvolume {subvol_name}: {err}'
-            )
+            raise DashboardException(
+                msg=f'Failed to remove subvolume {subvol_name}: {err}',
+                component='cephfs')
         return f'Subvolume {subvol_name} removed successfully'
 
 

diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/common/forms-helper.feature.po.ts b/src/pybind/mgr/dashboard/frontend/cypress/e2e/common/forms-helper.feature.po.ts
@@ -57,7 +57,7 @@ And('I click on submit button', () => {
  * by ticking the 'Are you sure?' box.
  */
 Then('I check the tick box in modal', () => {
-  cy.get('cd-modal .custom-control-label').click();
+  cy.get('cd-modal input#confirmation').click();
 });
 
 And('I confirm to {string}', (action: string) => {

diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/filesystems/subvolumes.e2e-spec.feature b/src/pybind/mgr/dashboard/frontend/cypress/e2e/filesystems/subvolumes.e2e-spec.feature
@@ -38,7 +38,7 @@ Feature: CephFS Subvolume management
         When I select a row "test_subvolume" in the expanded row
         And I click on "Remove" button from the table actions in the expanded row
         And I check the tick box in modal
-        And I click on "Remove subvolume" button
+        And I click on "Remove Subvolume" button
         Then I should not see a row with "test_subvolume" in the expanded row
 
     Scenario: Remove CephFS Volume

diff --git a/...d/frontend/src/app/ceph/cephfs/cephfs-subvolume-list/cephfs-subvolume-list.component.html b/...d/frontend/src/app/ceph/cephfs/cephfs-subvolume-list/cephfs-subvolume-list.component.html
@@ -74,3 +74,30 @@
             *ngIf="row.info.pool_namespace"
             [tooltipText]="row.info.pool_namespace"></cd-label>
 </ng-template>
+
+<ng-template #removeTmpl
+             let-form="form">
+  <ng-container [formGroup]="form">
+    <ng-container formGroupName="child">
+      <cd-alert-panel *ngIf="errorMessage.length > 1"
+                      type="error">
+              {{errorMessage}}
+      </cd-alert-panel>
+      <div class="form-group">
+        <div class="custom-control custom-checkbox">
+          <input type="checkbox"
+                 class="custom-control-input"
+                 name="retainSnapshots"
+                 id="retainSnapshots"
+                 formControlName="retainSnapshots">
+          <label class="custom-control-label"
+                 for="retainSnapshots"
+                 i18n>Retain snapshots <cd-helper>The subvolume can be removed retaining
+                  existing snapshots using this option.
+                   If snapshots are retained, the subvolume is considered empty for all
+                    operations not involving the retained snapshots.</cd-helper></label>
+        </div>
+      </div>
+    </ng-container>
+  </ng-container>
+</ng-template>
diff --git a/...rontend/src/app/ceph/cephfs/cephfs-subvolume-list/cephfs-subvolume-list.component.spec.ts b/...rontend/src/app/ceph/cephfs/cephfs-subvolume-list/cephfs-subvolume-list.component.spec.ts
@@ -5,6 +5,7 @@ import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { SharedModule } from '~/app/shared/shared.module';
 import { ToastrModule } from 'ngx-toastr';
 import { RouterTestingModule } from '@angular/router/testing';
+import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
 
 describe('CephfsSubvolumeListComponent', () => {
   let component: CephfsSubvolumeListComponent;
@@ -13,7 +14,8 @@ describe('CephfsSubvolumeListComponent', () => {
   beforeEach(async () => {
     await TestBed.configureTestingModule({
       declarations: [CephfsSubvolumeListComponent],
-      imports: [HttpClientTestingModule, SharedModule, ToastrModule.forRoot(), RouterTestingModule]
+      imports: [HttpClientTestingModule, SharedModule, ToastrModule.forRoot(), RouterTestingModule],
+      providers: [NgbActiveModal]
     }).compileComponents();
   });
 

diff --git a/...ard/frontend/src/app/ceph/cephfs/cephfs-subvolume-list/cephfs-subvolume-list.component.ts b/...ard/frontend/src/app/ceph/cephfs/cephfs-subvolume-list/cephfs-subvolume-list.component.ts
@@ -1,4 +1,4 @@
-import { Component, Input, OnChanges, OnInit, ViewChild } from '@angular/core';
+import { Component, Input, OnChanges, OnInit, TemplateRef, ViewChild } from '@angular/core';
 import { Observable, ReplaySubject, of } from 'rxjs';
 import { catchError, shareReplay, switchMap } from 'rxjs/operators';
 import { CephfsSubvolumeService } from '~/app/shared/api/cephfs-subvolume.service';
@@ -14,16 +14,20 @@ import { ModalService } from '~/app/shared/services/modal.service';
 import { CephfsSubvolumeFormComponent } from '../cephfs-subvolume-form/cephfs-subvolume-form.component';
 import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
 import { Permissions } from '~/app/shared/models/permissions';
-import { CriticalConfirmationModalComponent } from '~/app/shared/components/critical-confirmation-modal/critical-confirmation-modal.component';
 import { TaskWrapperService } from '~/app/shared/services/task-wrapper.service';
 import { FinishedTask } from '~/app/shared/models/finished-task';
+import { NgbModalRef } from '@ng-bootstrap/ng-bootstrap';
+import { FormControl } from '@angular/forms';
+import { CdFormGroup } from '~/app/shared/forms/cd-form-group';
+import { CdForm } from '~/app/shared/forms/cd-form';
+import { CriticalConfirmationModalComponent } from '~/app/shared/components/critical-confirmation-modal/critical-confirmation-modal.component';
 
 @Component({
   selector: 'cd-cephfs-subvolume-list',
   templateUrl: './cephfs-subvolume-list.component.html',
   styleUrls: ['./cephfs-subvolume-list.component.scss']
 })
-export class CephfsSubvolumeListComponent implements OnInit, OnChanges {
+export class CephfsSubvolumeListComponent extends CdForm implements OnInit, OnChanges {
   @ViewChild('quotaUsageTpl', { static: true })
   quotaUsageTpl: any;
 
@@ -39,15 +43,22 @@ export class CephfsSubvolumeListComponent implements OnInit, OnChanges {
   @ViewChild('quotaSizeTpl', { static: true })
   quotaSizeTpl: any;
 
+  @ViewChild('removeTmpl', { static: true })
+  removeTmpl: TemplateRef<any>;
+
   @Input() fsName: string;
   @Input() pools: any[];
 
   columns: CdTableColumn[] = [];
   tableActions: CdTableAction[];
   context: CdTableFetchDataContext;
   selection = new CdTableSelection();
+  removeForm: CdFormGroup;
   icons = Icons;
   permissions: Permissions;
+  modalRef: NgbModalRef;
+  errorMessage: string = '';
+  selectedName: string = '';
 
   subVolumes$: Observable<CephfsSubvolume[]>;
   subject = new ReplaySubject<CephfsSubvolume[]>();
@@ -59,6 +70,7 @@ export class CephfsSubvolumeListComponent implements OnInit, OnChanges {
     private authStorageService: AuthStorageService,
     private taskWrapper: TaskWrapperService
   ) {
+    super();
     this.permissions = this.authStorageService.getPermissions();
   }
 
@@ -174,16 +186,34 @@ export class CephfsSubvolumeListComponent implements OnInit, OnChanges {
   }
 
   removeSubVolumeModal() {
-    const name = this.selection.first().name;
-    this.modalService.show(CriticalConfirmationModalComponent, {
-      itemDescription: 'subvolume',
-      itemNames: [name],
-      actionDescription: 'remove',
-      submitActionObservable: () =>
-        this.taskWrapper.wrapTaskAroundCall({
-          task: new FinishedTask('cephfs/subvolume/remove', { subVolumeName: name }),
-          call: this.cephfsSubVolume.remove(this.fsName, name)
-        })
+    this.removeForm = new CdFormGroup({
+      retainSnapshots: new FormControl(false)
+    });
+    this.errorMessage = '';
+    this.selectedName = this.selection.first().name;
+    this.modalRef = this.modalService.show(CriticalConfirmationModalComponent, {
+      actionDescription: 'Remove',
+      itemNames: [this.selectedName],
+      itemDescription: 'Subvolume',
+      childFormGroup: this.removeForm,
+      childFormGroupTemplate: this.removeTmpl,
+      submitAction: () =>
+        this.taskWrapper
+          .wrapTaskAroundCall({
+            task: new FinishedTask('cephfs/subvolume/remove', { subVolumeName: this.selectedName }),
+            call: this.cephfsSubVolume.remove(
+              this.fsName,
+              this.selectedName,
+              this.removeForm.getValue('retainSnapshots')
+            )
+          })
+          .subscribe({
+            complete: () => this.modalRef.close(),
+            error: (error) => {
+              this.modalRef.componentInstance.stopLoadingSpinner();
+              this.errorMessage = error.error.detail;
+            }
+          })
     });
   }
 }
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-subvolume.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-subvolume.service.spec.ts
@@ -35,7 +35,9 @@ describe('CephfsSubvolumeService', () => {
 
   it('should call remove', () => {
     service.remove('testFS', 'testSubvol').subscribe();
-    const req = httpTesting.expectOne('api/cephfs/subvolume/testFS?subvol_name=testSubvol');
+    const req = httpTesting.expectOne(
+      'api/cephfs/subvolume/testFS?subvol_name=testSubvol&retain_snapshots=false'
+    );
     expect(req.request.method).toBe('DELETE');
   });
 });
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-subvolume.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs-subvolume.service.ts
@@ -51,10 +51,11 @@ export class CephfsSubvolumeService {
     });
   }
 
-  remove(fsName: string, subVolumeName: string) {
+  remove(fsName: string, subVolumeName: string, retainSnapshots: boolean = false) {
     return this.http.delete(`${this.baseURL}/${fsName}`, {
       params: {
-        subvol_name: subVolumeName
+        subvol_name: subVolumeName,
+        retain_snapshots: retainSnapshots
       },
       observe: 'response'
     });

diff --git a/.../shared/components/critical-confirmation-modal/critical-confirmation-modal.component.html b/.../shared/components/critical-confirmation-modal/critical-confirmation-modal.component.html
@@ -43,7 +43,7 @@
       </div>
       <div class="modal-footer">
         <cd-form-button-panel (submitActionEvent)="callSubmitAction()"
-                              (backActionEvent)="callBackAction()"
+                              (backActionEvent)="backAction ? callBackAction() : hideModal()"
                               [form]="deletionForm"
                               [submitText]="(actionDescription | titlecase) + ' ' + itemDescription"></cd-form-button-panel>
       </div>

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.html b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.html
@@ -347,7 +347,8 @@
   <span data-toggle="tooltip"
         [title]="value"
         class="font-monospace">{{ value | path }}
-    <cd-copy-2-clipboard-button [source]="value"
+    <cd-copy-2-clipboard-button *ngIf="value"
+                                [source]="value"
                                 [byId]="false"
                                 [showIconOnly]="true">
     </cd-copy-2-clipboard-button>

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/path.pipe.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/path.pipe.ts
@@ -5,6 +5,7 @@ import { Pipe, PipeTransform } from '@angular/core';
 })
 export class PathPipe implements PipeTransform {
   transform(value: unknown): string {
+    if (!value) return '';
     const splittedPath = value.toString().split('/');
 
     if (splittedPath[0] === '') {

diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml
@@ -1990,6 +1990,11 @@ paths:
         required: true
         schema:
           type: string
+      - default: false
+        in: query
+        name: retain_snapshots
+        schema:
+          type: boolean
       responses:
         '202':
           content: