From 855d45d939f691996b3e1666ddb8dd9b8aff4e51 Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Tue, 15 Jun 2021 14:17:58 +0530 Subject: [PATCH] mgr/dashboard: Fix 500 error while exiting out of maintenance When you add a host in maintenance mode and then exit the maintenance mode, a 500 server error will popup which will interrupt the whole exit maintenance process and leave the host in an unknown/offline state. It happened when I was setting the status of the host through the HostSpec(). With this change, I am using the enter_maintenance api of the orch to enable the maintenance. Fixes: https://tracker.ceph.com/issues/51218 Signed-off-by: Nizamudeen A (cherry picked from commit ea89b60e3f5e73a653eebf5cdcc3227acb7b2e62) Conflicts: src/pybind/mgr/dashboard/controllers/host.py - Removed the addr and label variables since its not backported yet to pacific - Removed Versioning since for pacific this endpoint is not yet versioned to 0.1 src/pybind/mgr/dashboard/services/orchestrator.py - Removed the addr and label variables since its not backported yet to pacific src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts - Didn't included this file because the changes in there are irrelevant in pacific src/pybind/mgr/dashboard/openapi.yaml - Regenerated the openapi file --- qa/tasks/mgr/dashboard/test_host.py | 2 +- src/pybind/mgr/dashboard/controllers/host.py | 32 +++++++++++++------ .../host-form/host-form.component.spec.ts | 2 +- src/pybind/mgr/dashboard/openapi.yaml | 2 ++ .../mgr/dashboard/services/orchestrator.py | 4 +-- src/pybind/mgr/dashboard/tests/test_host.py | 11 +++++++ 6 files changed, 39 insertions(+), 14 deletions(-) diff --git a/qa/tasks/mgr/dashboard/test_host.py b/qa/tasks/mgr/dashboard/test_host.py index 49bb33533cd7..b642ad07e4ef 100644 --- a/qa/tasks/mgr/dashboard/test_host.py +++ b/qa/tasks/mgr/dashboard/test_host.py @@ -146,7 +146,7 @@ class HostControllerTest(DashboardTestCase): class HostControllerNoOrchestratorTest(DashboardTestCase): def test_host_create(self): - self._post('/api/host?hostname=foo') + self._post('/api/host?hostname=foo', {'status': ''}) self.assertStatus(503) self.assertError(code='orchestrator_status_unavailable', component='orchestrator') diff --git a/src/pybind/mgr/dashboard/controllers/host.py b/src/pybind/mgr/dashboard/controllers/host.py index 590925ca58bf..3efcccae2681 100644 --- a/src/pybind/mgr/dashboard/controllers/host.py +++ b/src/pybind/mgr/dashboard/controllers/host.py @@ -255,6 +255,16 @@ def get_inventories(hosts: Optional[List[str]] = None, return inventory_hosts +@allow_empty_body +def add_host(hostname: str, status: Optional[str] = None): + orch_client = OrchClient.instance() + host = Host() + host.check_orchestrator_host_op(orch_client, hostname) + orch_client.hosts.add(hostname) + if status == 'maintenance': + orch_client.hosts.enter_maintenance(hostname) + + @ApiController('/host', Scope.HOSTS) @ControllerDoc("Get Host Details", "Host") class Host(RESTController): @@ -274,12 +284,15 @@ class Host(RESTController): @raise_if_no_orchestrator([OrchFeature.HOST_LIST, OrchFeature.HOST_CREATE]) @handle_orchestrator_error('host') @host_task('create', {'hostname': '{hostname}'}) + @EndpointDoc('', + parameters={ + 'hostname': (str, 'Hostname'), + 'status': (str, 'Host Status') + }, + responses={200: None, 204: None}) def create(self, hostname: str, status: Optional[str] = None): # pragma: no cover - requires realtime env - orch_client = OrchClient.instance() - self._check_orchestrator_host_op(orch_client, hostname, True) - orch_client.hosts.add(hostname, status) - create._cp_config = {'tools.json_in.force': False} # pylint: disable=W0212 + add_host(hostname, status) @raise_if_no_orchestrator([OrchFeature.HOST_LIST, OrchFeature.HOST_DELETE]) @handle_orchestrator_error('host') @@ -287,10 +300,10 @@ class Host(RESTController): @allow_empty_body def delete(self, hostname): # pragma: no cover - requires realtime env orch_client = OrchClient.instance() - self._check_orchestrator_host_op(orch_client, hostname, False) + self.check_orchestrator_host_op(orch_client, hostname, False) orch_client.hosts.remove(hostname) - def _check_orchestrator_host_op(self, orch_client, hostname, add_host=True): # pragma:no cover + def check_orchestrator_host_op(self, orch_client, hostname, add=True): # pragma:no cover """Check if we can adding or removing a host with orchestrator :param orch_client: Orchestrator client @@ -298,16 +311,15 @@ class Host(RESTController): :raise DashboardException """ host = orch_client.hosts.get(hostname) - if add_host and host: + if add and host: raise DashboardException( code='orchestrator_add_existed_host', msg='{} is already in orchestrator'.format(hostname), component='orchestrator') - if not add_host and not host: + if not add and not host: raise DashboardException( code='orchestrator_remove_nonexistent_host', - msg='Remove a non-existent host {} from orchestrator'.format( - hostname), + msg='Remove a non-existent host {} from orchestrator'.format(hostname), component='orchestrator') @RESTController.Resource('GET') diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.spec.ts index d9135197e85a..5e722bd3dcd4 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.spec.ts @@ -39,7 +39,7 @@ describe('HostFormComponent', () => { }); it('should select maintenance mode', () => { - component.hostForm.get('maintenance').setValue('maintenance'); + component.hostForm.get('maintenance').setValue(true); fixture.detectChanges(); component.submit(); expect(component.status).toBe('maintenance'); diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index ba2b26bf0dbe..20d544b6bf1a 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -3240,8 +3240,10 @@ paths: schema: properties: hostname: + description: Hostname type: string status: + description: Host Status type: string required: - hostname diff --git a/src/pybind/mgr/dashboard/services/orchestrator.py b/src/pybind/mgr/dashboard/services/orchestrator.py index 7dfdfc2920f8..1b472932c933 100644 --- a/src/pybind/mgr/dashboard/services/orchestrator.py +++ b/src/pybind/mgr/dashboard/services/orchestrator.py @@ -64,8 +64,8 @@ class HostManger(ResourceManager): return hosts[0] if hosts else None @wait_api_result - def add(self, hostname: str, status: str): - return self.api.add_host(HostSpec(hostname, status=status)) + def add(self, hostname: str): + return self.api.add_host(HostSpec(hostname)) @wait_api_result def remove(self, hostname: str): diff --git a/src/pybind/mgr/dashboard/tests/test_host.py b/src/pybind/mgr/dashboard/tests/test_host.py index 6093ba8f4314..93b543ee4a92 100644 --- a/src/pybind/mgr/dashboard/tests/test_host.py +++ b/src/pybind/mgr/dashboard/tests/test_host.py @@ -122,6 +122,17 @@ class HostControllerTest(ControllerTestCase): self.assertStatus(200) self.assertIn('labels', self.json_body()) + @mock.patch('dashboard.controllers.host.add_host') + def test_add_host(self, mock_add_host): + with patch_orch(True): + payload = { + 'hostname': 'node0', + 'status': 'maintenance' + } + self._post(self.URL_HOST, payload) + self.assertStatus(201) + mock_add_host.assert_called() + def test_set_labels(self): mgr.list_servers.return_value = [] orch_hosts = [ -- 2.47.3