From 5627919c016785a27666b1992d336f8ff378a072 Mon Sep 17 00:00:00 2001 From: Tiago Melo Date: Mon, 3 Feb 2020 16:14:13 -0100 Subject: [PATCH] mgr/dashboard: Allow removing RBD with snapshots Fixes: https://tracker.ceph.com/issues/36404 Signed-off-by: Tiago Melo --- qa/tasks/mgr/dashboard/helper.py | 2 +- qa/tasks/mgr/dashboard/test_rbd.py | 28 +++++-- src/pybind/mgr/dashboard/controllers/rbd.py | 80 +++++++------------ .../dashboard/controllers/rbd_mirroring.py | 4 +- .../block/rbd-list/rbd-list.component.html | 20 ++++- .../block/rbd-list/rbd-list.component.spec.ts | 61 ++++++++++++++ .../ceph/block/rbd-list/rbd-list.component.ts | 49 +++++++++++- .../rbd-trash-move-modal.component.html | 7 ++ .../rbd-trash-move-modal.component.ts | 3 + .../services/task-message.service.spec.ts | 1 + .../shared/services/task-message.service.ts | 3 + src/pybind/mgr/dashboard/services/rbd.py | 53 +++++++++++- 12 files changed, 242 insertions(+), 69 deletions(-) diff --git a/qa/tasks/mgr/dashboard/helper.py b/qa/tasks/mgr/dashboard/helper.py index 24d0f455da7..15b1fccb69e 100644 --- a/qa/tasks/mgr/dashboard/helper.py +++ b/qa/tasks/mgr/dashboard/helper.py @@ -261,7 +261,7 @@ class DashboardTestCase(MgrTestCase): @classmethod def _task_request(cls, method, url, data, timeout): res = cls._request(url, method, data) - cls._assertIn(cls._resp.status_code, [200, 201, 202, 204, 400, 403]) + cls._assertIn(cls._resp.status_code, [200, 201, 202, 204, 400, 403, 404]) if cls._resp.status_code == 403: return None diff --git a/qa/tasks/mgr/dashboard/test_rbd.py b/qa/tasks/mgr/dashboard/test_rbd.py index 4623873a61d..a3ef00b1b7f 100644 --- a/qa/tasks/mgr/dashboard/test_rbd.py +++ b/qa/tasks/mgr/dashboard/test_rbd.py @@ -459,9 +459,9 @@ class RbdTest(DashboardTestCase): def test_delete_non_existent_image(self): res = self.remove_image('rbd', None, 'i_dont_exist') - self.assertStatus(400) - self.assertEqual(res, {u'code': u'2', "status": 400, "component": "rbd", - "detail": "[errno 2] RBD image not found (error removing image)", + self.assertStatus(404) + self.assertEqual(res, {u'code': 404, "status": 404, "component": None, + "detail": "(404, 'Image not found')", 'task': {'name': 'rbd/delete', 'metadata': {'image_spec': 'rbd/i_dont_exist'}}}) @@ -491,6 +491,22 @@ class RbdTest(DashboardTestCase): self.remove_image('rbd', None, 'delete_me') self.assertStatus(204) + def test_image_delete_with_snapshot(self): + self.create_image('rbd', None, 'delete_me', 2**30) + self.assertStatus(201) + self.create_snapshot('rbd', None, 'delete_me', 'snap1') + self.assertStatus(201) + self.create_snapshot('rbd', None, 'delete_me', 'snap2') + self.assertStatus(201) + + img = self.get_image('rbd', None, 'delete_me') + self.assertStatus(200) + self._validate_image(img, name='delete_me', size=2**30) + self.assertEqual(len(img['snapshots']), 2) + + self.remove_image('rbd', None, 'delete_me') + self.assertStatus(204) + def test_image_rename(self): self.create_image('rbd', None, 'edit_img', 2**30) self.assertStatus(201) @@ -672,14 +688,10 @@ class RbdTest(DashboardTestCase): res = self.remove_image('rbd', None, 'cimg') self.assertStatus(400) self.assertIn('code', res) - self.assertEqual(res['code'], '39') + self.assertEqual(res['code'], '16') self.remove_image('rbd', None, 'cimg-clone') self.assertStatus(204) - self.update_snapshot('rbd', None, 'cimg', 'snap1', None, False) - self.assertStatus(200) - self.remove_snapshot('rbd', None, 'cimg', 'snap1') - self.assertStatus(204) self.remove_image('rbd', None, 'cimg') self.assertStatus(204) diff --git a/src/pybind/mgr/dashboard/controllers/rbd.py b/src/pybind/mgr/dashboard/controllers/rbd.py index 7ea98360240..6bfff994d22 100644 --- a/src/pybind/mgr/dashboard/controllers/rbd.py +++ b/src/pybind/mgr/dashboard/controllers/rbd.py @@ -7,8 +7,6 @@ import math from functools import partial from datetime import datetime -import cherrypy - import rbd from . import ApiController, RESTController, Task, UpdatePermission, \ @@ -17,8 +15,8 @@ from .. import mgr from ..exceptions import DashboardException from ..security import Scope from ..services.ceph_service import CephService -from ..services.rbd import RbdConfiguration, RbdService, format_bitmask, format_features,\ - parse_image_spec +from ..services.rbd import RbdConfiguration, RbdService, RbdSnapshotService, \ + format_bitmask, format_features, parse_image_spec, rbd_call, rbd_image_call from ..tools import ViewCache, str_to_bool from ..services.exception import handle_rados_error, handle_rbd_error, \ serialize_dashboard_exception @@ -34,20 +32,6 @@ def RbdTask(name, metadata, wait_for): # noqa: N802 return composed_decorator -def _rbd_call(pool_name, namespace, func, *args, **kwargs): - with mgr.rados.open_ioctx(pool_name) as ioctx: - ioctx.set_namespace(namespace if namespace is not None else '') - func(ioctx, *args, **kwargs) - - -def _rbd_image_call(pool_name, namespace, image_name, func, *args, **kwargs): - def _ioctx_func(ioctx, image_name, func, *args, **kwargs): - with rbd.Image(ioctx, image_name) as img: - func(ioctx, img, *args, **kwargs) - - return _rbd_call(pool_name, namespace, _ioctx_func, image_name, func, *args, **kwargs) - - def _sort_features(features, enable=True): """ Sorts image features according to feature dependencies: @@ -101,14 +85,7 @@ class Rbd(RESTController): @handle_rbd_error() @handle_rados_error('pool') def get(self, image_spec): - pool_name, namespace, image_name = parse_image_spec(image_spec) - ioctx = mgr.rados.open_ioctx(pool_name) - if namespace: - ioctx.set_namespace(namespace) - try: - return RbdService.rbd_image(ioctx, pool_name, namespace, image_name) - except rbd.ImageNotFound: - raise cherrypy.HTTPError(404) + return RbdService.get_image(image_spec) @RbdTask('create', {'pool_name': '{pool_name}', 'namespace': '{namespace}', 'image_name': '{name}'}, 2.0) @@ -134,13 +111,19 @@ class Rbd(RESTController): RbdConfiguration(pool_ioctx=ioctx, namespace=namespace, image_name=name).set_configuration(configuration) - _rbd_call(pool_name, namespace, _create) + rbd_call(pool_name, namespace, _create) @RbdTask('delete', ['{image_spec}'], 2.0) def delete(self, image_spec): pool_name, namespace, image_name = parse_image_spec(image_spec) + + image = RbdService.get_image(image_spec) + snapshots = image['snapshots'] + for snap in snapshots: + RbdSnapshotService.remove_snapshot(image_spec, snap['name'], snap['is_protected']) + rbd_inst = rbd.RBD() - return _rbd_call(pool_name, namespace, rbd_inst.remove, image_name) + return rbd_call(pool_name, namespace, rbd_inst.remove, image_name) @RbdTask('edit', ['{image_spec}', '{name}'], 4.0) def set(self, image_spec, name=None, size=None, features=None, configuration=None): @@ -179,7 +162,7 @@ class Rbd(RESTController): RbdConfiguration(pool_ioctx=ioctx, image_name=image_name).set_configuration( configuration) - return _rbd_image_call(pool_name, namespace, image_name, _edit) + return rbd_image_call(pool_name, namespace, image_name, _edit) @RbdTask('copy', {'src_image_spec': '{image_spec}', @@ -210,9 +193,9 @@ class Rbd(RESTController): RbdConfiguration(pool_ioctx=d_ioctx, image_name=dest_image_name).set_configuration( configuration) - return _rbd_call(dest_pool_name, dest_namespace, _copy) + return rbd_call(dest_pool_name, dest_namespace, _copy) - return _rbd_image_call(pool_name, namespace, image_name, _src_copy) + return rbd_image_call(pool_name, namespace, image_name, _src_copy) @RbdTask('flatten', ['{image_spec}'], 2.0) @RESTController.Resource('POST') @@ -223,7 +206,7 @@ class Rbd(RESTController): image.flatten() pool_name, namespace, image_name = parse_image_spec(image_spec) - return _rbd_image_call(pool_name, namespace, image_name, _flatten) + return rbd_image_call(pool_name, namespace, image_name, _flatten) @RESTController.Collection('GET') def default_features(self): @@ -239,7 +222,7 @@ class Rbd(RESTController): """ pool_name, namespace, image_name = parse_image_spec(image_spec) rbd_inst = rbd.RBD() - return _rbd_call(pool_name, namespace, rbd_inst.trash_move, image_name, delay) + return rbd_call(pool_name, namespace, rbd_inst.trash_move, image_name, delay) @ApiController('/block/image/{image_spec}/snap', Scope.RBD_IMAGE) @@ -255,18 +238,13 @@ class RbdSnapshot(RESTController): def _create_snapshot(ioctx, img, snapshot_name): img.create_snap(snapshot_name) - return _rbd_image_call(pool_name, namespace, image_name, _create_snapshot, - snapshot_name) + return rbd_image_call(pool_name, namespace, image_name, _create_snapshot, + snapshot_name) @RbdTask('snap/delete', ['{image_spec}', '{snapshot_name}'], 2.0) def delete(self, image_spec, snapshot_name): - def _remove_snapshot(ioctx, img, snapshot_name): - img.remove_snap(snapshot_name) - - pool_name, namespace, image_name = parse_image_spec(image_spec) - return _rbd_image_call(pool_name, namespace, image_name, _remove_snapshot, - snapshot_name) + return RbdSnapshotService.remove_snapshot(image_spec, snapshot_name) @RbdTask('snap/edit', ['{image_spec}', '{snapshot_name}'], 4.0) @@ -284,7 +262,7 @@ class RbdSnapshot(RESTController): img.unprotect_snap(snapshot_name) pool_name, namespace, image_name = parse_image_spec(image_spec) - return _rbd_image_call(pool_name, namespace, image_name, _edit, snapshot_name) + return rbd_image_call(pool_name, namespace, image_name, _edit, snapshot_name) @RbdTask('snap/rollback', ['{image_spec}', '{snapshot_name}'], 5.0) @@ -295,7 +273,7 @@ class RbdSnapshot(RESTController): img.rollback_to_snap(snapshot_name) pool_name, namespace, image_name = parse_image_spec(image_spec) - return _rbd_image_call(pool_name, namespace, image_name, _rollback, snapshot_name) + return rbd_image_call(pool_name, namespace, image_name, _rollback, snapshot_name) @RbdTask('clone', {'parent_image_spec': '{image_spec}', @@ -330,9 +308,9 @@ class RbdSnapshot(RESTController): RbdConfiguration(pool_ioctx=ioctx, image_name=child_image_name).set_configuration( configuration) - return _rbd_call(child_pool_name, child_namespace, _clone) + return rbd_call(child_pool_name, child_namespace, _clone) - _rbd_call(pool_name, namespace, _parent_clone) + rbd_call(pool_name, namespace, _parent_clone) @ApiController('/block/image/trash', Scope.RBD_IMAGE) @@ -391,8 +369,8 @@ class RbdTrash(RESTController): for pool in pools: for image in pool['value']: if image['deferment_end_time'] < now: - _rbd_call(pool['pool_name'], image['namespace'], - self.rbd_inst.trash_remove, image['id'], 0) + rbd_call(pool['pool_name'], image['namespace'], + self.rbd_inst.trash_remove, image['id'], 0) @RbdTask('trash/restore', ['{image_id_spec}', '{new_image_name}'], 2.0) @RESTController.Resource('POST') @@ -400,8 +378,8 @@ class RbdTrash(RESTController): def restore(self, image_id_spec, new_image_name): """Restore an image from trash.""" pool_name, namespace, image_id = parse_image_spec(image_id_spec) - return _rbd_call(pool_name, namespace, self.rbd_inst.trash_restore, image_id, - new_image_name) + return rbd_call(pool_name, namespace, self.rbd_inst.trash_restore, image_id, + new_image_name) @RbdTask('trash/remove', ['{image_id_spec}'], 2.0) def delete(self, image_id_spec, force=False): @@ -410,8 +388,8 @@ class RbdTrash(RESTController): But an actively in-use by clones or has snapshots can not be removed. """ pool_name, namespace, image_id = parse_image_spec(image_id_spec) - return _rbd_call(pool_name, namespace, self.rbd_inst.trash_remove, image_id, - int(str_to_bool(force))) + return rbd_call(pool_name, namespace, self.rbd_inst.trash_remove, image_id, + int(str_to_bool(force))) @ApiController('/block/pool/{pool_name}/namespace', Scope.RBD_IMAGE) diff --git a/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py b/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py index 497f8c3d72c..02728903085 100644 --- a/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py +++ b/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py @@ -13,11 +13,11 @@ import rbd from . import ApiController, Endpoint, Task, BaseController, ReadPermission, \ UpdatePermission, RESTController -from .rbd import _rbd_call from .. import mgr from ..security import Scope from ..services.ceph_service import CephService +from ..services.rbd import rbd_call from ..tools import ViewCache from ..services.exception import handle_rados_error, handle_rbd_error, \ serialize_dashboard_exception @@ -403,7 +403,7 @@ class RbdMirroringPoolMode(RESTController): rbd.RBD().mirror_mode_set(ioctx, mode_enum) _reset_view_cache() - return _rbd_call(pool_name, None, _edit, mirror_mode) + return rbd_call(pool_name, None, _edit, mirror_mode) @ApiController('/block/mirroring/pool/{pool_name}/bootstrap', diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.html index 0058b10df17..e539e5c081d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.html @@ -28,7 +28,8 @@ - {{ value.pool_name }}/{{ value.pool_namespace }}/{{ value.image_name }}@{{ value.snap_name }} + {{ value.pool_name }}/{{ value.pool_namespace }}/{{ value.image_name }}@{{ value.snap_name }} - @@ -41,3 +42,20 @@ {{ value.parent }} to child {{ value.child }}. + + + + diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts index b4b67710f33..5647abcf29e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts @@ -99,6 +99,67 @@ describe('RbdListComponent', () => { }); }); + describe('handling of deletion', () => { + beforeEach(() => { + fixture.detectChanges(); + }); + + it('should check if there are no snapshots', () => { + component.selection.add({ + id: '-1', + name: 'rbd1', + pool_name: 'rbd' + }); + expect(component.hasSnapshots()).toBeFalsy(); + }); + + it('should check if there are snapshots', () => { + component.selection.add({ + id: '-1', + name: 'rbd1', + pool_name: 'rbd', + snapshots: [{}, {}] + }); + expect(component.hasSnapshots()).toBeTruthy(); + }); + + it('should get delete disable description', () => { + component.selection.add({ + id: '-1', + name: 'rbd1', + pool_name: 'rbd', + snapshots: [ + { + children: [{}] + } + ] + }); + expect(component.getDeleteDisableDesc()).toBe( + 'This RBD has cloned snapshots. Please delete related RBDs before deleting this RBD.' + ); + }); + + it('should list all protected snapshots', () => { + component.selection.add({ + id: '-1', + name: 'rbd1', + pool_name: 'rbd', + snapshots: [ + { + name: 'snap1', + is_protected: false + }, + { + name: 'snap2', + is_protected: true + } + ] + }); + + expect(component.listProtectedSnapshots()).toEqual(['snap2']); + }); + }); + describe('handling of executing tasks', () => { let images: RbdModel[]; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts index 56187a69e51..aaea7eb8905 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts @@ -51,6 +51,8 @@ export class RbdListComponent implements OnInit { nameTpl: TemplateRef; @ViewChild('flattenTpl', { static: true }) flattenTpl: TemplateRef; + @ViewChild('deleteTpl', { static: true }) + deleteTpl: TemplateRef; permission: Permission; tableActions: CdTableAction[]; @@ -131,7 +133,12 @@ export class RbdListComponent implements OnInit { permission: 'delete', icon: Icons.destroy, click: () => this.deleteRbdModal(), - name: this.actionLabels.DELETE + name: this.actionLabels.DELETE, + disable: (selection: CdTableSelection) => + !this.selection.first() || + !this.selection.hasSingleSelection || + this.hasClonedSnapshots(selection.first()), + disableDesc: () => this.getDeleteDisableDesc() }; const copyAction: CdTableAction = { permission: 'create', @@ -327,6 +334,11 @@ export class RbdListComponent implements OnInit { initialState: { itemDescription: 'RBD', itemNames: [imageSpec], + bodyTemplate: this.deleteTpl, + bodyContext: { + hasSnapshots: this.hasSnapshots(), + snapshots: this.listProtectedSnapshots() + }, submitActionObservable: () => this.taskWrapper.wrapTaskAroundCall({ task: new FinishedTask('rbd/delete', { @@ -342,7 +354,8 @@ export class RbdListComponent implements OnInit { const initialState = { poolName: this.selection.first().pool_name, namespace: this.selection.first().namespace, - imageName: this.selection.first().name + imageName: this.selection.first().name, + hasSnapshots: this.hasSnapshots() }; this.modalRef = this.modalService.show(RbdTrashMoveModalComponent, { initialState }); } @@ -387,4 +400,36 @@ export class RbdListComponent implements OnInit { this.modalRef = this.modalService.show(ConfirmationModalComponent, { initialState }); } + + hasSnapshots() { + const snapshots = this.selection.first()['snapshots'] || []; + return snapshots.length > 0; + } + + hasClonedSnapshots(image: object) { + const snapshots = image['snapshots'] || []; + return snapshots.some((snap: object) => snap['children'] && snap['children'].length > 0); + } + + listProtectedSnapshots() { + const first = this.selection.first(); + const snapshots = first['snapshots']; + return snapshots.reduce((accumulator: string[], snap: object) => { + if (snap['is_protected']) { + accumulator.push(snap['name']); + } + return accumulator; + }, []); + } + + getDeleteDisableDesc(): string { + const first = this.selection.first(); + if (first && this.hasClonedSnapshots(first)) { + return this.i18n( + 'This RBD has cloned snapshots. Please delete related RBDs before deleting this RBD.' + ); + } + + return ''; + } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-trash-move-modal/rbd-trash-move-modal.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-trash-move-modal/rbd-trash-move-modal.component.html index c08f8861340..cb1ee7f89dc 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-trash-move-modal/rbd-trash-move-modal.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-trash-move-modal/rbd-trash-move-modal.component.html @@ -9,6 +9,13 @@ [formGroup]="moveForm" novalidate>