From 481ad5590285f76a2c1d4515ffff6c35692b8459 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/frontend/src/app/ceph/block/rbd-details/rbd-details.component.html src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts src/pybind/mgr/dashboard/services/rbd.py src/pybind/mgr/dashboard/tests/test_rbd_service.py Resolved various conflicts because octopus and master diverged a lot. --- .../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 | 30 ++++++ .../ceph/block/rbd-list/rbd-list.component.ts | 30 ++++-- src/pybind/mgr/dashboard/services/rbd.py | 37 +++++-- .../mgr/dashboard/tests/test_rbd_service.py | 99 ++++++++++++++++++- 8 files changed, 217 insertions(+), 15 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 b42094d1d0b1b..960b6111ca9b7 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 - + @@ -137,6 +137,10 @@ + + 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 e69de29bb2d1d..bee73b7bddf1b 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 @@ +@import 'defaults.scss'; + +.warn { + color: $color-bright-yellow; +} 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 87f8e78f994fe..a89abf450309c 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 @@ -306,4 +306,34 @@ describe('RbdListComponent', () => { } }); }); + + const getActionDisable = (name: string) => + component.tableActions.find((o) => o.name === name).disable; + + 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.moveTrash || false); + }; + + 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 da53bb3fd549a..fccc5e660082a 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 '../../../shared/components/confirmat import { CriticalConfirmationModalComponent } from '../../../shared/components/critical-confirmation-modal/critical-confirmation-modal.component'; import { ActionLabelsI18n } from '../../../shared/constants/app.constants'; import { TableComponent } from '../../../shared/datatable/table/table.component'; -import { CellTemplate } from '../../../shared/enum/cell-template.enum'; import { Icons } from '../../../shared/enum/icons.enum'; import { ViewCacheStatus } from '../../../shared/enum/view-cache-status.enum'; import { CdTableAction } from '../../../shared/models/cd-table-action'; @@ -54,6 +53,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[]; @@ -62,6 +63,7 @@ export class RbdListComponent extends ListWithDetails implements OnInit { retries: number; viewCacheStatusList: any[]; selection = new CdTableSelection(); + icons = Icons; modalRef: BsModalRef; @@ -131,7 +133,9 @@ export class RbdListComponent extends ListWithDetails implements OnInit { permission: 'update', icon: Icons.edit, routerLink: () => this.urlBuilder.getEdit(getImageUri()), - name: this.actionLabels.EDIT + name: this.actionLabels.EDIT, + disable: (selection: CdTableSelection) => + !selection.hasSingleSelection || this.getRemovingStatusDesc(selection) }; const deleteAction: CdTableAction = { permission: 'delete', @@ -144,7 +148,9 @@ export class RbdListComponent extends ListWithDetails implements OnInit { permission: 'create', canBePrimary: (selection: CdTableSelection) => selection.hasSingleSelection, disable: (selection: CdTableSelection) => - !selection.hasSingleSelection || selection.first().cdExecuting, + !selection.hasSingleSelection || + selection.first().cdExecuting || + this.getRemovingStatusDesc(selection), icon: Icons.copy, routerLink: () => `/block/rbd/copy/${getImageUri()}`, name: this.actionLabels.COPY @@ -152,7 +158,10 @@ export class RbdListComponent extends ListWithDetails implements OnInit { const flattenAction: CdTableAction = { permission: 'update', disable: (selection: CdTableSelection) => - !selection.hasSingleSelection || selection.first().cdExecuting || !selection.first().parent, + !selection.hasSingleSelection || + selection.first().cdExecuting || + !selection.first().parent || + this.getRemovingStatusDesc(selection), icon: Icons.flatten, click: () => this.flattenRbdModal(), name: this.actionLabels.FLATTEN @@ -165,7 +174,8 @@ export class RbdListComponent extends ListWithDetails implements OnInit { disable: (selection: CdTableSelection) => !selection.first() || !selection.hasSingleSelection || - selection.first().image_format === RBDImageFormat.V1 + selection.first().image_format === RBDImageFormat.V1 || + this.getRemovingStatusDesc(selection) }; this.tableActions = [ addAction, @@ -183,7 +193,7 @@ export class RbdListComponent extends ListWithDetails implements OnInit { name: this.i18n('Name'), prop: 'name', flexGrow: 2, - cellTransformation: CellTemplate.executing + cellTemplate: this.removingStatTpl }, { name: this.i18n('Pool'), @@ -441,4 +451,12 @@ export class RbdListComponent extends ListWithDetails implements OnInit { this.hasClonedSnapshots(selection.first()) ); } + + getRemovingStatusDesc(selection: CdTableSelection): string | boolean { + const first = selection.first(); + if (first && first.source && first.source === 'REMOVING') { + return this.i18n(`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 4e628342282eb..944e479046ee7 100644 --- a/src/pybind/mgr/dashboard/services/rbd.py +++ b/src/pybind/mgr/dashboard/services/rbd.py @@ -2,6 +2,7 @@ # pylint: disable=unused-argument from __future__ import absolute_import +import errno import six import cherrypy @@ -334,14 +335,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): @@ -356,13 +373,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 caee4029180f5..6989c9080c90b 100644 --- a/src/pybind/mgr/dashboard/tests/test_rbd_service.py +++ b/src/pybind/mgr/dashboard/tests/test_rbd_service.py @@ -3,12 +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 get_image_spec, parse_image_spec, RbdConfiguration +from .. import mgr +from ..services.rbd import get_image_spec, parse_image_spec, RbdConfiguration, RbdService + + +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 +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