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 <nia@redhat.com>
(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

diff --git a/qa/tasks/mgr/dashboard/test_host.py b/qa/tasks/mgr/dashboard/test_host.py
index 49bb33533cd7108a787ac2f5b8bce6f2df89ab5f..b642ad07e4ef3a5f621a3756153acf58c2eacc0f 100644 (file)
--- 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 590925ca58bf9cdef6c748f8dc3ff6c551d7553a..3efcccae2681125d40a714f30cd5103119302648 100644 (file)
--- 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 d9135197e85aa61191b832d5bedf6815f92b2459..5e722bd3dcd405992e46fc5852e4dc267e387567 100644 (file)
--- 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 ba2b26bf0dbe8284759280498a8aae113df43c84..20d544b6bf1ae9ac28b09ec2bd29d0418cd0a08f 100644 (file)
--- 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 7dfdfc2920f83d77f48cab6093530c10a82b0a40..1b472932c933cfac7721cff9f7091bcf7bb9d01e 100644 (file)
--- 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 6093ba8f4314b09eb79c62fb1d0461ff61d48c3e..93b543ee4a922bf22b2c3e067525b38076a7daeb 100644 (file)
--- 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 = [