From ed237aa2030d1c05664337f36bb032172a4d5d1c Mon Sep 17 00:00:00 2001 From: Tiago Melo Date: Wed, 7 Oct 2020 23:01:28 +0000 Subject: [PATCH] mgr/dashboard: Disable RBD clone action when conditions are not met Disable clone action when RBD snapshot is not protected and clone format version is 1. Fixes: https://tracker.ceph.com/issues/37873 Signed-off-by: Tiago Melo --- qa/tasks/mgr/dashboard/test_rbd.py | 53 ++++++++++++++++++- src/pybind/mgr/dashboard/controllers/rbd.py | 15 ++++++ .../rbd-snapshot-actions.model.ts | 14 ++++- .../rbd-snapshot-list.component.spec.ts | 29 ++++++++++ .../rbd-snapshot-list.component.ts | 6 ++- .../src/app/shared/api/rbd.service.spec.ts | 6 +++ .../src/app/shared/api/rbd.service.ts | 4 ++ src/pybind/mgr/dashboard/openapi.yaml | 20 +++++++ 8 files changed, 144 insertions(+), 3 deletions(-) diff --git a/qa/tasks/mgr/dashboard/test_rbd.py b/qa/tasks/mgr/dashboard/test_rbd.py index 48119383bf3c0..c6150dd06df27 100644 --- a/qa/tasks/mgr/dashboard/test_rbd.py +++ b/qa/tasks/mgr/dashboard/test_rbd.py @@ -9,7 +9,7 @@ from .helper import DashboardTestCase, JObj, JLeaf, JList class RbdTest(DashboardTestCase): - AUTH_ROLES = ['pool-manager', 'block-manager'] + AUTH_ROLES = ['pool-manager', 'block-manager', 'cluster-manager'] @classmethod def create_pool(cls, name, pg_num, pool_type, application='rbd'): @@ -754,6 +754,57 @@ class RbdTest(DashboardTestCase): self.assertEqual(default_features, [ 'deep-flatten', 'exclusive-lock', 'fast-diff', 'layering', 'object-map']) + def test_clone_format_version(self): + config_name = 'rbd_default_clone_format' + def _get_config_by_name(conf_name): + data = self._get('/api/cluster_conf/{}'.format(conf_name)) + if 'value' in data: + return data['value'] + return None + + # with rbd_default_clone_format = auto + clone_format_version = self._get('/api/block/image/clone_format_version') + self.assertEqual(clone_format_version, 1) + self.assertStatus(200) + + # with rbd_default_clone_format = 1 + value = [{'section': "global", 'value': "1"}] + self._post('/api/cluster_conf', { + 'name': config_name, + 'value': value + }) + self.wait_until_equal( + lambda: _get_config_by_name(config_name), + value, + timeout=60) + clone_format_version = self._get('/api/block/image/clone_format_version') + self.assertEqual(clone_format_version, 1) + self.assertStatus(200) + + # with rbd_default_clone_format = 2 + value = [{'section': "global", 'value': "2"}] + self._post('/api/cluster_conf', { + 'name': config_name, + 'value': value + }) + self.wait_until_equal( + lambda: _get_config_by_name(config_name), + value, + timeout=60) + clone_format_version = self._get('/api/block/image/clone_format_version') + self.assertEqual(clone_format_version, 2) + self.assertStatus(200) + + value = [] + self._post('/api/cluster_conf', { + 'name': config_name, + 'value': value + }) + self.wait_until_equal( + lambda: _get_config_by_name(config_name), + None, + timeout=60) + def test_image_with_namespace(self): self.create_namespace('rbd', 'ns') self.create_image('rbd', 'ns', 'test', 10240) diff --git a/src/pybind/mgr/dashboard/controllers/rbd.py b/src/pybind/mgr/dashboard/controllers/rbd.py index bd72e80bc9b36..57fe06a00a65a 100644 --- a/src/pybind/mgr/dashboard/controllers/rbd.py +++ b/src/pybind/mgr/dashboard/controllers/rbd.py @@ -237,6 +237,21 @@ class Rbd(RESTController): rbd_default_features = mgr.get('config')['rbd_default_features'] return format_bitmask(int(rbd_default_features)) + @RESTController.Collection('GET') + def clone_format_version(self): + """Return the RBD clone format version. + """ + rbd_default_clone_format = mgr.get('config')['rbd_default_clone_format'] + if rbd_default_clone_format != 'auto': + return int(rbd_default_clone_format) + osd_map = mgr.get_osdmap().dump() + min_compat_client = osd_map.get('min_compat_client', '') + require_min_compat_client = osd_map.get('require_min_compat_client', '') + if max(min_compat_client, require_min_compat_client) < 'mimic': + return 1 + + return 2 + @RbdTask('trash/move', ['{image_spec}'], 2.0) @RESTController.Resource('POST') @allow_empty_body diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts index 396bd45eb0c5a..2b22da09aab65 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts @@ -1,3 +1,5 @@ +import { RbdService } from 'app/shared/api/rbd.service'; + import { ActionLabelsI18n } from '../../../shared/constants/app.constants'; import { Icons } from '../../../shared/enum/icons.enum'; import { CdTableAction } from '../../../shared/models/cd-table-action'; @@ -14,7 +16,13 @@ export class RbdSnapshotActionsModel { deleteSnap: CdTableAction; ordering: CdTableAction[]; - constructor(actionLabels: ActionLabelsI18n, featuresName: string[]) { + cloneFormatVersion = 1; + + constructor(actionLabels: ActionLabelsI18n, featuresName: string[], rbdService: RbdService) { + rbdService.cloneFormatVersion().subscribe((version: number) => { + this.cloneFormatVersion = version; + }); + this.create = { permission: 'create', icon: Icons.add, @@ -87,6 +95,10 @@ export class RbdSnapshotActionsModel { return $localize`Parent image must support Layering`; } + if (this.cloneFormatVersion === 1 && !selection.first().is_protected) { + return $localize`Snapshot must be protected in order to clone.`; + } + return false; } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts index 1ea292e0b4ffa..30e68261b2833 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts @@ -31,6 +31,7 @@ import { SummaryService } from '../../../shared/services/summary.service'; import { TaskListService } from '../../../shared/services/task-list.service'; import { RbdSnapshotFormModalComponent } from '../rbd-snapshot-form/rbd-snapshot-form-modal.component'; import { RbdTabsComponent } from '../rbd-tabs/rbd-tabs.component'; +import { RbdSnapshotActionsModel } from './rbd-snapshot-actions.model'; import { RbdSnapshotListComponent } from './rbd-snapshot-list.component'; import { RbdSnapshotModel } from './rbd-snapshot.model'; @@ -277,4 +278,32 @@ describe('RbdSnapshotListComponent', () => { } }); }); + + describe('clone button disable state', () => { + let actions: RbdSnapshotActionsModel; + + beforeEach(() => { + fixture.detectChanges(); + const rbdService = TestBed.inject(RbdService); + const actionLabelsI18n = TestBed.inject(ActionLabelsI18n); + actions = new RbdSnapshotActionsModel(actionLabelsI18n, [], rbdService); + }); + + it('should be disabled with version 1 and protected false', () => { + const selection = new CdTableSelection([{ name: 'someName', is_protected: false }]); + const disableDesc = actions.getCloneDisableDesc(selection, ['layering']); + expect(disableDesc).toBe('Snapshot must be protected in order to clone.'); + }); + + it.each([ + [1, true], + [2, true], + [2, false] + ])('should be enabled with version %d and protected %s', (version, is_protected) => { + actions.cloneFormatVersion = version; + const selection = new CdTableSelection([{ name: 'someName', is_protected: is_protected }]); + const disableDesc = actions.getCloneDisableDesc(selection, ['layering']); + expect(disableDesc).toBe(false); + }); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts index d2d7378a8f586..d14136c7c888d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts @@ -130,7 +130,11 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { ngOnChanges() { const imageSpec = new ImageSpec(this.poolName, this.namespace, this.rbdName); - const actions = new RbdSnapshotActionsModel(this.actionLabels, this.featuresName); + const actions = new RbdSnapshotActionsModel( + this.actionLabels, + this.featuresName, + this.rbdService + ); actions.create.click = () => this.openCreateSnapshotModal(); actions.rename.click = () => this.openEditSnapshotModal(); actions.protect.click = () => this.toggleProtection(); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.spec.ts index 72af80b977d9f..dae61e7e19254 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.spec.ts @@ -80,6 +80,12 @@ describe('RbdService', () => { expect(req.request.method).toBe('GET'); }); + it('should call cloneFormatVersion', () => { + service.cloneFormatVersion().subscribe(); + const req = httpTesting.expectOne('api/block/image/clone_format_version'); + expect(req.request.method).toBe('GET'); + }); + it('should call createSnapshot', () => { service.createSnapshot(new ImageSpec('poolName', null, 'rbdName'), 'snapshotName').subscribe(); const req = httpTesting.expectOne('api/block/image/poolName%2FrbdName/snap'); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.ts index d65c2356f5690..5482f09312286 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rbd.service.ts @@ -75,6 +75,10 @@ export class RbdService { return this.http.get('api/block/image/default_features'); } + cloneFormatVersion() { + return this.http.get('api/block/image/clone_format_version'); + } + createSnapshot(imageSpec: ImageSpec, @cdEncodeNot snapshotName: string) { const request = { snapshot_name: snapshotName diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index bba9a054a30ee..f93dfe5f669cd 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -226,6 +226,26 @@ paths: - jwt: [] tags: - Rbd + /api/block/image/clone_format_version: + get: + description: "Return the RBD clone format version.\n " + parameters: [] + responses: + '200': + description: OK + '400': + description: Operation exception. Please check the response body for details. + '401': + description: Unauthenticated access. Please login first. + '403': + description: Unauthorized access. Please check your permissions. + '500': + description: Unexpected error. Please check the response body for the stack + trace. + security: + - jwt: [] + tags: + - Rbd /api/block/image/default_features: get: parameters: [] -- 2.39.5