From d83c277ac1861df31d2a39d16e20c7bebbea676e Mon Sep 17 00:00:00 2001 From: Tatjana Dehler Date: Thu, 27 May 2021 11:46:50 +0200 Subject: [PATCH] mgr/dashboard: show partially deleted RBDs An RBD might be partially deleted if the deletion process has been started but was interrupted. In this case return the RBD as part of the RBD list and mark it as partially deleted. Fixes: https://tracker.ceph.com/issues/48603 Signed-off-by: Tatjana Dehler --- .../rbd-details/rbd-details.component.html | 6 +- .../app/ceph/block/rbd-form/rbd-form.model.ts | 3 + .../block/rbd-list/rbd-list.component.html | 22 +++++ .../block/rbd-list/rbd-list.component.scss | 5 + .../block/rbd-list/rbd-list.component.spec.ts | 43 ++++++-- .../ceph/block/rbd-list/rbd-list.component.ts | 23 ++++- src/pybind/mgr/dashboard/services/rbd.py | 37 +++++-- .../mgr/dashboard/tests/test_rbd_service.py | 98 ++++++++++++++++++- 8 files changed, 214 insertions(+), 23 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-details/rbd-details.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-details/rbd-details.component.html index 17188a1b2de..7503863e0fb 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-details/rbd-details.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-details/rbd-details.component.html @@ -2,7 +2,7 @@ Only available for RBD images with fast-diff enabled - +
    + + Information can not be displayed for RBD in status 'Removing'. + + + + + + + {{ value }} + + + ({{ row.cdExecuting }}) + + + diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.scss b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.scss index e69de29bb2d..4cfa4e8dadd 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.scss +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.scss @@ -0,0 +1,5 @@ +@use './src/styles/vendor/variables' as vv; + +.warn { + color: vv.$warning; +} 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 7e90907b01e..ed898a2fcf2 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 @@ -301,12 +301,12 @@ describe('RbdListComponent', () => { const getActionDisable = (name: string) => component.tableActions.find((o) => o.name === name).disable; - const testActions = (selection: any, expected: string | boolean) => { - expect(getActionDisable('Edit')(selection)).toBe(expected); - expect(getActionDisable('Delete')(selection)).toBe(expected); - expect(getActionDisable('Copy')(selection)).toBe(expected); + const testActions = (selection: any, expected: { [action: string]: string | boolean }) => { + expect(getActionDisable('Edit')(selection)).toBe(expected.edit || false); + expect(getActionDisable('Delete')(selection)).toBe(expected.delete || false); + expect(getActionDisable('Copy')(selection)).toBe(expected.copy || false); expect(getActionDisable('Flatten')(selection)).toBeTruthy(); - expect(getActionDisable('Move to Trash')(selection)).toBe(expected); + expect(getActionDisable('Move to Trash')(selection)).toBe(expected.moveTrash || false); }; it('should test TableActions with valid/invalid image name', () => { @@ -317,7 +317,7 @@ describe('RbdListComponent', () => { snapshots: [] } ]; - testActions(component.selection, false); + testActions(component.selection, {}); component.selection.selected = [ { @@ -326,9 +326,32 @@ describe('RbdListComponent', () => { snapshots: [] } ]; - testActions( - component.selection, - `This RBD image has an invalid name and can't be managed by ceph.` - ); + const message = `This RBD image has an invalid name and can't be managed by ceph.`; + const expected = { + edit: message, + delete: message, + copy: message, + moveTrash: message + }; + testActions(component.selection, expected); + }); + + it('should disable edit, copy, flatten and move action if RBD is in status `Removing`', () => { + component.selection.selected = [ + { + name: 'foobar', + pool_name: 'rbd', + snapshots: [], + source: 'REMOVING' + } + ]; + + const message = `Action not possible for an RBD in status 'Removing'`; + const expected = { + edit: message, + copy: message, + moveTrash: message + }; + testActions(component.selection, expected); }); }); 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 4df87596133..8bc297da62f 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 @@ -10,7 +10,6 @@ import { ConfirmationModalComponent } from '~/app/shared/components/confirmation import { CriticalConfirmationModalComponent } from '~/app/shared/components/critical-confirmation-modal/critical-confirmation-modal.component'; import { ActionLabelsI18n } from '~/app/shared/constants/app.constants'; import { TableComponent } from '~/app/shared/datatable/table/table.component'; -import { CellTemplate } from '~/app/shared/enum/cell-template.enum'; import { Icons } from '~/app/shared/enum/icons.enum'; import { ViewCacheStatus } from '~/app/shared/enum/view-cache-status.enum'; import { CdTableAction } from '~/app/shared/models/cd-table-action'; @@ -55,6 +54,8 @@ export class RbdListComponent extends ListWithDetails implements OnInit { flattenTpl: TemplateRef; @ViewChild('deleteTpl', { static: true }) deleteTpl: TemplateRef; + @ViewChild('removingStatTpl', { static: true }) + removingStatTpl: TemplateRef; permission: Permission; tableActions: CdTableAction[]; @@ -63,6 +64,7 @@ export class RbdListComponent extends ListWithDetails implements OnInit { retries: number; tableStatus = new TableStatusViewCache(); selection = new CdTableSelection(); + icons = Icons; modalRef: NgbModalRef; @@ -132,7 +134,8 @@ export class RbdListComponent extends ListWithDetails implements OnInit { icon: Icons.edit, routerLink: () => this.urlBuilder.getEdit(getImageUri()), name: this.actionLabels.EDIT, - disable: this.getInvalidNameDisable + disable: (selection: CdTableSelection) => + this.getRemovingStatusDesc(selection) || this.getInvalidNameDisable(selection) }; const deleteAction: CdTableAction = { permission: 'delete', @@ -145,7 +148,9 @@ export class RbdListComponent extends ListWithDetails implements OnInit { permission: 'create', canBePrimary: (selection: CdTableSelection) => selection.hasSingleSelection, disable: (selection: CdTableSelection) => - this.getInvalidNameDisable(selection) || !!selection.first().cdExecuting, + this.getRemovingStatusDesc(selection) || + this.getInvalidNameDisable(selection) || + !!selection.first().cdExecuting, icon: Icons.copy, routerLink: () => `/block/rbd/copy/${getImageUri()}`, name: this.actionLabels.COPY @@ -153,6 +158,7 @@ export class RbdListComponent extends ListWithDetails implements OnInit { const flattenAction: CdTableAction = { permission: 'update', disable: (selection: CdTableSelection) => + this.getRemovingStatusDesc(selection) || this.getInvalidNameDisable(selection) || selection.first().cdExecuting || !selection.first().parent, @@ -166,6 +172,7 @@ export class RbdListComponent extends ListWithDetails implements OnInit { click: () => this.trashRbdModal(), name: this.actionLabels.TRASH, disable: (selection: CdTableSelection) => + this.getRemovingStatusDesc(selection) || this.getInvalidNameDisable(selection) || selection.first().image_format === RBDImageFormat.V1 }; @@ -185,7 +192,7 @@ export class RbdListComponent extends ListWithDetails implements OnInit { name: $localize`Name`, prop: 'name', flexGrow: 2, - cellTransformation: CellTemplate.executing + cellTemplate: this.removingStatTpl }, { name: $localize`Pool`, @@ -456,4 +463,12 @@ export class RbdListComponent extends ListWithDetails implements OnInit { return !selection.first() || !selection.hasSingleSelection; } + + getRemovingStatusDesc(selection: CdTableSelection): string | boolean { + const first = selection.first(); + if (first?.source === 'REMOVING') { + return $localize`Action not possible for an RBD in status 'Removing'`; + } + return false; + } } diff --git a/src/pybind/mgr/dashboard/services/rbd.py b/src/pybind/mgr/dashboard/services/rbd.py index 0356de8f5f4..4506938abc3 100644 --- a/src/pybind/mgr/dashboard/services/rbd.py +++ b/src/pybind/mgr/dashboard/services/rbd.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # pylint: disable=unused-argument +import errno import cherrypy import rbd @@ -330,14 +331,30 @@ class RbdService(object): return stat @classmethod - def _rbd_image_names(cls, ioctx): + def _rbd_image_refs(cls, ioctx): rbd_inst = rbd.RBD() - return rbd_inst.list(ioctx) + return rbd_inst.list2(ioctx) @classmethod def _rbd_image_stat(cls, ioctx, pool_name, namespace, image_name): return cls._rbd_image(ioctx, pool_name, namespace, image_name) + @classmethod + def _rbd_image_stat_removing(cls, ioctx, pool_name, namespace, image_id): + rbd_inst = rbd.RBD() + img = rbd_inst.trash_get(ioctx, image_id) + img_spec = get_image_spec(pool_name, namespace, image_id) + + if img['source'] == 'REMOVING': + img['unique_id'] = img_spec + img['pool_name'] = pool_name + img['namespace'] = namespace + img['deletion_time'] = "{}Z".format(img['deletion_time'].isoformat()) + img['deferment_end_time'] = "{}Z".format(img['deferment_end_time'].isoformat()) + return img + raise rbd.ImageNotFound('No image {} in status `REMOVING` found.'.format(img_spec), + errno=errno.ENOENT) + @classmethod @ViewCache() def rbd_pool_list(cls, pool_name, namespace=None): @@ -352,13 +369,19 @@ class RbdService(object): namespaces.append('') for current_namespace in namespaces: ioctx.set_namespace(current_namespace) - names = cls._rbd_image_names(ioctx) - for name in names: + image_refs = cls._rbd_image_refs(ioctx) + for image_ref in image_refs: try: - stat = cls._rbd_image_stat(ioctx, pool_name, current_namespace, name) + stat = cls._rbd_image_stat( + ioctx, pool_name, current_namespace, image_ref['name']) except rbd.ImageNotFound: - # may have been removed in the meanwhile - continue + # Check if the RBD has been deleted partially. This happens for example if + # the deletion process of the RBD has been started and was interrupted. + try: + stat = cls._rbd_image_stat_removing( + ioctx, pool_name, current_namespace, image_ref['id']) + except rbd.ImageNotFound: + continue result.append(stat) return result diff --git a/src/pybind/mgr/dashboard/tests/test_rbd_service.py b/src/pybind/mgr/dashboard/tests/test_rbd_service.py index 1772ac8cf1c..345f360b607 100644 --- a/src/pybind/mgr/dashboard/tests/test_rbd_service.py +++ b/src/pybind/mgr/dashboard/tests/test_rbd_service.py @@ -2,13 +2,22 @@ # pylint: disable=dangerous-default-value,too-many-public-methods import unittest +from datetime import datetime +from unittest.mock import MagicMock try: import mock except ImportError: import unittest.mock as mock -from ..services.rbd import RbdConfiguration, get_image_spec, parse_image_spec +from .. import mgr +from ..services.rbd import RbdConfiguration, RbdService, get_image_spec, parse_image_spec + + +class ImageNotFoundStub(Exception): + def __init__(self, message, errno=None): + super(ImageNotFoundStub, self).__init__( + 'RBD image not found (%s)' % message, errno) class RbdServiceTest(unittest.TestCase): @@ -43,3 +52,90 @@ class RbdServiceTest(unittest.TestCase): self.assertEqual(config.list(), []) config = RbdConfiguration('good-pool') self.assertEqual(config.list(), [1, 2, 3]) + + @mock.patch('dashboard.services.rbd.rbd.RBD') + def test_rbd_image_stat_removing(self, rbd_mock): + time = datetime.utcnow() + rbd_inst_mock = rbd_mock.return_value + rbd_inst_mock.trash_get.return_value = { + 'id': '3c1a5ee60a88', + 'name': 'test_rbd', + 'source': 'REMOVING', + 'deletion_time': time, + 'deferment_end_time': time + } + + ioctx_mock = MagicMock() + + # pylint: disable=protected-access + rbd = RbdService._rbd_image_stat_removing(ioctx_mock, 'test_pool', '', '3c1a5ee60a88') + self.assertEqual(rbd, { + 'id': '3c1a5ee60a88', + 'unique_id': 'test_pool/3c1a5ee60a88', + 'name': 'test_rbd', + 'source': 'REMOVING', + 'deletion_time': '{}Z'.format(time.isoformat()), + 'deferment_end_time': '{}Z'.format(time.isoformat()), + 'pool_name': 'test_pool', + 'namespace': '' + }) + + @mock.patch('dashboard.services.rbd.rbd.ImageNotFound', new_callable=lambda: ImageNotFoundStub) + @mock.patch('dashboard.services.rbd.rbd.RBD') + def test_rbd_image_stat_filter_source_user(self, rbd_mock, _): + rbd_inst_mock = rbd_mock.return_value + rbd_inst_mock.trash_get.return_value = { + 'id': '3c1a5ee60a88', + 'name': 'test_rbd', + 'source': 'USER' + } + + ioctx_mock = MagicMock() + with self.assertRaises(ImageNotFoundStub) as ctx: + # pylint: disable=protected-access + RbdService._rbd_image_stat_removing(ioctx_mock, 'test_pool', '', '3c1a5ee60a88') + self.assertIn('No image test_pool/3c1a5ee60a88 in status `REMOVING` found.', + str(ctx.exception)) + + @mock.patch('dashboard.services.rbd.rbd.ImageNotFound', new_callable=lambda: ImageNotFoundStub) + @mock.patch('dashboard.services.rbd.RbdService._rbd_image_stat_removing') + @mock.patch('dashboard.services.rbd.RbdService._rbd_image_stat') + @mock.patch('dashboard.services.rbd.RbdService._rbd_image_refs') + @mock.patch('dashboard.services.rbd.rbd.RBD') + def test_rbd_pool_list(self, rbd_mock, rbd_image_ref_mock, rbd_image_stat_mock, + rbd_image_stat_removing_mock, _): + time = datetime.utcnow() + + ioctx_mock = MagicMock() + mgr.rados = MagicMock() + mgr.rados.open_ioctx.return_value = ioctx_mock + + rbd_inst_mock = rbd_mock.return_value + rbd_inst_mock.namespace_list.return_value = [] + rbd_image_ref_mock.return_value = [{'name': 'test_rbd', 'id': '3c1a5ee60a88'}] + + rbd_image_stat_mock.side_effect = mock.Mock(side_effect=ImageNotFoundStub( + 'RBD image not found test_pool/3c1a5ee60a88')) + + rbd_image_stat_removing_mock.return_value = { + 'id': '3c1a5ee60a88', + 'unique_id': 'test_pool/3c1a5ee60a88', + 'name': 'test_rbd', + 'source': 'REMOVING', + 'deletion_time': '{}Z'.format(time.isoformat()), + 'deferment_end_time': '{}Z'.format(time.isoformat()), + 'pool_name': 'test_pool', + 'namespace': '' + } + + rbd_pool_list = RbdService.rbd_pool_list('test_pool') + self.assertEqual(rbd_pool_list, (0, [{ + 'id': '3c1a5ee60a88', + 'unique_id': 'test_pool/3c1a5ee60a88', + 'name': 'test_rbd', + 'source': 'REMOVING', + 'deletion_time': '{}Z'.format(time.isoformat()), + 'deferment_end_time': '{}Z'.format(time.isoformat()), + 'pool_name': 'test_pool', + 'namespace': '' + }])) -- 2.47.3