From 8ac655963755820c9bab3953a4195fbc916f7072 Mon Sep 17 00:00:00 2001 From: Volker Theile Date: Thu, 5 Sep 2019 16:06:57 +0200 Subject: [PATCH] mgr/dashboard: Handle always-on Ceph Manager modules correctly Currently always-on modules are not marked as enabled in the WebUI and can be disabled. This PR will fix that. Note, this PR will NOT implement code that will prevent a developer from trying to disable an always-on module through the REST API. The Mgr Python extension will throw an adequate exception. This PR will also do: * Remove old code fragments from a previous Mgr Module management UI that is obsolete now. * Cleanup code in BaseMgrModule code. Fixes: https://tracker.ceph.com/issues/41648 Signed-off-by: Volker Theile --- src/mgr/BaseMgrModule.cc | 13 +++++++-- .../mgr/dashboard/controllers/mgr_modules.py | 15 ++++++----- .../mgr-module-list.component.spec.ts | 27 +++++++++++++++++-- .../mgr-module-list.component.ts | 21 ++++++++++++--- .../telemetry/telemetry.component.html | 0 src/pybind/mgr/mgr_module.py | 9 +++++++ 6 files changed, 71 insertions(+), 14 deletions(-) delete mode 100644 src/pybind/mgr/dashboard/frontend/src/app/core/mgr-modules/telemetry/telemetry.component.html diff --git a/src/mgr/BaseMgrModule.cc b/src/mgr/BaseMgrModule.cc index e69d28c3cfda0..812e23d9fa67c 100644 --- a/src/mgr/BaseMgrModule.cc +++ b/src/mgr/BaseMgrModule.cc @@ -574,7 +574,13 @@ ceph_get_version(BaseMgrModule *self, PyObject *args) } static PyObject * -ceph_get_context(BaseMgrModule *self, PyObject *args) +ceph_get_release_name(BaseMgrModule *self, PyObject *args) +{ + return PyString_FromString(ceph_release_to_str()); +} + +static PyObject * +ceph_get_context(BaseMgrModule *self) { return self->py_modules->get_context(); } @@ -1064,9 +1070,12 @@ PyMethodDef BaseMgrModule_methods[] = { {"_ceph_cluster_log", (PyCFunction)ceph_cluster_log, METH_VARARGS, "Emit a cluster log message"}, - {"_ceph_get_version", (PyCFunction)ceph_get_version, METH_VARARGS, + {"_ceph_get_version", (PyCFunction)ceph_get_version, METH_NOARGS, "Get the ceph version of this process"}, + {"_ceph_get_release_name", (PyCFunction)ceph_get_release_name, METH_NOARGS, + "Get the ceph release name of this process"}, + {"_ceph_get_context", (PyCFunction)ceph_get_context, METH_NOARGS, "Get a CephContext* in a python capsule"}, diff --git a/src/pybind/mgr/dashboard/controllers/mgr_modules.py b/src/pybind/mgr/dashboard/controllers/mgr_modules.py index 358ae8be56abe..6e3cb5a3ee180 100644 --- a/src/pybind/mgr/dashboard/controllers/mgr_modules.py +++ b/src/pybind/mgr/dashboard/controllers/mgr_modules.py @@ -21,18 +21,19 @@ class MgrModules(RESTController): """ result = [] mgr_map = mgr.get('mgr_map') + always_on_modules = mgr_map['always_on_modules'][mgr.release_name] for module_config in mgr_map['available_modules']: - if module_config['name'] not in self.ignore_modules: + module_name = module_config['name'] + if module_name not in self.ignore_modules: + always_on = module_name in always_on_modules + enabled = module_name in mgr_map['modules'] or always_on result.append({ - 'name': module_config['name'], - 'enabled': False, + 'name': module_name, + 'enabled': enabled, + 'always_on': always_on, 'options': self._convert_module_options( module_config['module_options']) }) - for name in mgr_map['modules']: - if name not in self.ignore_modules: - obj = find_object_in_list('name', name, result) - obj['enabled'] = True return result def get(self, module_name): diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts index 7c10a1700ed88..15cc3034909a8 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts @@ -99,7 +99,8 @@ describe('MgrModuleListComponent', () => { spyOn(mgrModuleService, 'list').and.returnValues(observableThrowError('z'), observableOf([])); component.selection.selected.push({ name: 'foo', - enabled: false + enabled: false, + always_on: false }); component.selection.update(); component.updateModuleState(); @@ -118,7 +119,8 @@ describe('MgrModuleListComponent', () => { spyOn(mgrModuleService, 'list').and.returnValue(observableOf([])); component.selection.selected.push({ name: 'bar', - enabled: true + enabled: true, + always_on: false }); component.selection.update(); component.updateModuleState(); @@ -130,5 +132,26 @@ describe('MgrModuleListComponent', () => { expect(component.blockUI.stop).toHaveBeenCalled(); expect(component.table.refreshBtn).toHaveBeenCalled(); })); + + it('should not disable module (1)', () => { + component.selection.selected = [ + { + name: 'dashboard' + } + ]; + component.selection.update(); + expect(component.isTableActionDisabled('enabled')).toBeTruthy(); + }); + + it('should not disable module (2)', () => { + component.selection.selected = [ + { + name: 'bar', + always_on: true + } + ]; + component.selection.update(); + expect(component.isTableActionDisabled('enabled')).toBeTruthy(); + }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts index 892df1f52ad40..47635095d2cef 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts @@ -82,6 +82,7 @@ export class MgrModuleListComponent { permission: 'update', click: () => this.updateModuleState(), disable: () => this.isTableActionDisabled('disabled'), + disableDesc: () => this.getTableActionDisabledDesc(), icon: Icons.stop } ]; @@ -112,17 +113,31 @@ export class MgrModuleListComponent { if (!this.selection.hasSelection) { return true; } + const selected = this.selection.first(); // Make sure the user can't modify the run state of the 'Dashboard' module. // This check is only done in the UI because the REST API should still be // able to do so. - if (this.selection.first().name === 'dashboard') { + if (selected.name === 'dashboard') { + return true; + } + // Always-on modules can't be disabled. + if (selected.always_on) { return true; } switch (state) { case 'enabled': - return this.selection.first().enabled; + return selected.enabled; case 'disabled': - return !this.selection.first().enabled; + return !selected.enabled; + } + } + + getTableActionDisabledDesc(): string | undefined { + if (this.selection.hasSelection) { + const selected = this.selection.first(); + if (selected.always_on) { + return this.i18n('This Manager module is always on.'); + } } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/mgr-modules/telemetry/telemetry.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/mgr-modules/telemetry/telemetry.component.html deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index b34bc523c926f..e6b74ef72f733 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -584,6 +584,15 @@ class MgrModule(ceph_module.BaseMgrModule): def version(self): return self._version + @property + def release_name(self): + """ + Get the release name of the Ceph version, e.g. 'nautilus' or 'octopus'. + :return: Returns the release name of the Ceph version in lower case. + :rtype: str + """ + return self._ceph_get_release_name() + def get_context(self): """ :return: a Python capsule containing a C++ CephContext pointer -- 2.47.3