From 5eac11a01470449e7f499793c61f9cd9511ac020 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 (cherry picked from commit d83c277ac1861df31d2a39d16e20c7bebbea676e) Conflicts: src/pybind/mgr/dashboard/services/rbd.py Fixed minor imports conflict. --- .../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 | 38 +++++-- .../mgr/dashboard/tests/test_rbd_service.py | 98 ++++++++++++++++++- 8 files changed, 215 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 17188a1b2de2..7503863e0fb2 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 e69de29bb2d1..4cfa4e8daddb 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 7e90907b01e4..ed898a2fcf2f 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 4df87596133c..8bc297da62fd 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 083631915575..07fc2d783a48 100644 --- a/src/pybind/mgr/dashboard/services/rbd.py +++ b/src/pybind/mgr/dashboard/services/rbd.py @@ -2,6 +2,8 @@ # pylint: disable=unused-argument from __future__ import absolute_import +import errno + import cherrypy import rbd @@ -331,14 +333,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): @@ -353,13 +371,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 99cf27d82145..20a968321244 100644 --- a/src/pybind/mgr/dashboard/tests/test_rbd_service.py +++ b/src/pybind/mgr/dashboard/tests/test_rbd_service.py @@ -3,13 +3,22 @@ from __future__ import absolute_import 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): @@ -44,3 +53,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