From: Nizamudeen A Date: Tue, 15 Jun 2021 08:47:58 +0000 (+0530) Subject: mgr/dashboard: Fix 500 error while exiting out of maintenance X-Git-Tag: v17.1.0~1624^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F41856%2Fhead;p=ceph.git 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 --- diff --git a/qa/tasks/mgr/dashboard/test_host.py b/qa/tasks/mgr/dashboard/test_host.py index 935e9d71314d..124fff8d1544 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', version='0.1') + self._post('/api/host?hostname=foo', {'status': ''}, version='0.1') 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 d27da0a7aa17..fddeb2e5d7f0 100644 --- a/src/pybind/mgr/dashboard/controllers/host.py +++ b/src/pybind/mgr/dashboard/controllers/host.py @@ -254,6 +254,18 @@ def get_inventories(hosts: Optional[List[str]] = None, return inventory_hosts +@allow_empty_body +def add_host(hostname: str, addr: Optional[str] = None, + labels: Optional[List[str]] = None, + status: Optional[str] = None): + orch_client = OrchClient.instance() + host = Host() + host.check_orchestrator_host_op(orch_client, hostname) + orch_client.hosts.add(hostname, addr, labels) + if status == 'maintenance': + orch_client.hosts.enter_maintenance(hostname) + + @ApiController('/host', Scope.HOSTS) @ControllerDoc("Get Host Details", "Host") class Host(RESTController): @@ -278,7 +290,7 @@ class Host(RESTController): 'hostname': (str, 'Hostname'), 'addr': (str, 'Network Address'), 'labels': ([str], 'Host Labels'), - 'status': (str, 'Status of the Host') + 'status': (str, 'Host Status') }, responses={200: None, 204: None}) @RESTController.MethodMap(version='0.1') @@ -286,10 +298,7 @@ class Host(RESTController): addr: Optional[str] = None, labels: Optional[List[str]] = None, 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, addr, labels, status) - create._cp_config = {'tools.json_in.force': False} # pylint: disable=W0212 + add_host(hostname, addr, labels, status) @raise_if_no_orchestrator([OrchFeature.HOST_LIST, OrchFeature.HOST_DELETE]) @handle_orchestrator_error('host') @@ -297,10 +306,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 @@ -308,16 +317,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 fad0fa7270fc..dbb834ea8c82 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 @@ -68,7 +68,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/frontend/src/app/shared/api/host.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts index df643afb0ee5..7f8956baa263 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts @@ -28,7 +28,7 @@ export class HostService { create(hostname: string, addr: string, labels: string[], status: string) { return this.http.post( this.baseURL, - { hostname: hostname, addr, labels, status: status }, + { hostname: hostname, addr: addr, labels: labels, status: status }, { observe: 'response', headers: { Accept: 'application/vnd.ceph.api.v0.1+json' } } ); } diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index 7973832c7328..921dadb0471d 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -3251,7 +3251,7 @@ paths: type: string type: array status: - description: Status of the Host + 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 d564c4699408..44309a849391 100644 --- a/src/pybind/mgr/dashboard/services/orchestrator.py +++ b/src/pybind/mgr/dashboard/services/orchestrator.py @@ -63,8 +63,8 @@ class HostManger(ResourceManager): return hosts[0] if hosts else None @wait_api_result - def add(self, hostname: str, addr: str, labels: List[str], status: str): - return self.api.add_host(HostSpec(hostname, addr=addr, labels=labels, status=status)) + def add(self, hostname: str, addr: str, labels: List[str]): + return self.api.add_host(HostSpec(hostname, addr=addr, labels=labels)) @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 e7a09a3a7107..6d719a0fc922 100644 --- a/src/pybind/mgr/dashboard/tests/test_host.py +++ b/src/pybind/mgr/dashboard/tests/test_host.py @@ -126,6 +126,19 @@ class HostControllerTest(ControllerTestCase): self.assertIn('status', self.json_body()) self.assertIn('addr', 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', + 'addr': '192.0.2.0', + 'labels': 'mon', + 'status': 'maintenance' + } + self._post(self.URL_HOST, payload, version='0.1') + self.assertStatus(201) + mock_add_host.assert_called() + def test_set_labels(self): mgr.list_servers.return_value = [] orch_hosts = [