mgr/dashboard: fix adding/removing host errors

Send a HostSpec instance to the Orchestrator when adding a host.

Also, to be consistent with other components:
- Reword from Add/Remove hosts to Create/Delete hosts
- Display a modal when there is no Orchestrator backend enabled

Fixes: https://tracker.ceph.com/issues/44664
Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>

diff --git a/src/pybind/mgr/dashboard/controllers/host.py b/src/pybind/mgr/dashboard/controllers/host.py
index c609db0930df0b06158f97a79ed0a200ec5efd99..3a90389bba578851e1a7fcbdb0095b99a9507165 100644 (file)
--- a/src/pybind/mgr/dashboard/controllers/host.py
+++ b/src/pybind/mgr/dashboard/controllers/host.py
@@ -73,7 +73,7 @@ class Host(RESTController):
 
     @raise_if_no_orchestrator
     @handle_orchestrator_error('host')
-    @host_task('add', {'hostname': '{hostname}'})
+    @host_task('create', {'hostname': '{hostname}'})
     def create(self, hostname):
         orch_client = OrchClient.instance()
         self._check_orchestrator_host_op(orch_client, hostname, True)
@@ -81,7 +81,7 @@ class Host(RESTController):
 
     @raise_if_no_orchestrator
     @handle_orchestrator_error('host')
-    @host_task('remove', {'hostname': '{hostname}'})
+    @host_task('delete', {'hostname': '{hostname}'})
     def delete(self, hostname):
         orch_client = OrchClient.instance()
         self._check_orchestrator_host_op(orch_client, hostname, False)

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts b/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts
index 24fdf9ea8f11b4502a062fd3c2403a04fe8d1340..99208d37c6fa6f8af189a86d9a4da2abb0009e40 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts
@@ -88,9 +88,9 @@ const routes: Routes = [
         children: [
           { path: '', component: HostsComponent },
           {
-            path: URLVerbs.ADD,
+            path: URLVerbs.CREATE,
             component: HostFormComponent,
-            data: { breadcrumbs: ActionLabels.ADD }
+            data: { breadcrumbs: ActionLabels.CREATE }
           }
         ]
       },

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.ts
index 59473514e4e749476b3d81a9e4ade33510bc6b67..bfa2987107a4ceddb2a658a6821a08fb99b4a0a2 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/host-form/host-form.component.ts
@@ -29,7 +29,7 @@ export class HostFormComponent implements OnInit {
     private taskWrapper: TaskWrapperService
   ) {
     this.resource = this.i18n('host');
-    this.action = this.actionLabels.ADD;
+    this.action = this.actionLabels.CREATE;
     this.createForm();
   }
 
@@ -59,10 +59,10 @@ export class HostFormComponent implements OnInit {
     const hostname = this.hostForm.get('hostname').value;
     this.taskWrapper
       .wrapTaskAroundCall({
-        task: new FinishedTask('host/' + URLVerbs.ADD, {
+        task: new FinishedTask('host/' + URLVerbs.CREATE, {
           hostname: hostname
         }),
-        call: this.hostService.add(hostname)
+        call: this.hostService.create(hostname)
       })
       .subscribe(
         undefined,

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts
index 6af6eff886f679c74cb39335a79660badf34a99d..d6812be0d69d80809195846cc7aeeb5a75ff28e6 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts
@@ -1,10 +1,10 @@
 import { Component, OnInit, TemplateRef, ViewChild } from '@angular/core';
+import { Router } from '@angular/router';
 
 import { I18n } from '@ngx-translate/i18n-polyfill';
 import { BsModalRef, BsModalService } from 'ngx-bootstrap/modal';
 
 import { HostService } from '../../../shared/api/host.service';
-import { OrchestratorService } from '../../../shared/api/orchestrator.service';
 import { CriticalConfirmationModalComponent } from '../../../shared/components/critical-confirmation-modal/critical-confirmation-modal.component';
 import { ActionLabelsI18n } from '../../../shared/constants/app.constants';
 import { Icons } from '../../../shared/enum/icons.enum';
@@ -16,6 +16,7 @@ import { FinishedTask } from '../../../shared/models/finished-task';
 import { Permissions } from '../../../shared/models/permissions';
 import { CephShortVersionPipe } from '../../../shared/pipes/ceph-short-version.pipe';
 import { AuthStorageService } from '../../../shared/services/auth-storage.service';
+import { DepCheckerService } from '../../../shared/services/dep-checker.service';
 import { TaskWrapperService } from '../../../shared/services/task-wrapper.service';
 import { URLBuilderService } from '../../../shared/services/url-builder.service';
 
@@ -32,7 +33,6 @@ export class HostsComponent implements OnInit {
   columns: Array<CdTableColumn> = [];
   hosts: Array<object> = [];
   isLoadingHosts = false;
-  orchestratorAvailable = false;
   cdParams = { fromLink: '/hosts' };
   tableActions: CdTableAction[];
   selection = new CdTableSelection();
@@ -50,25 +50,37 @@ export class HostsComponent implements OnInit {
     private actionLabels: ActionLabelsI18n,
     private modalService: BsModalService,
     private taskWrapper: TaskWrapperService,
-    private orchService: OrchestratorService
+    private router: Router,
+    private depCheckerService: DepCheckerService
   ) {
     this.permissions = this.authStorageService.getPermissions();
     this.tableActions = [
       {
-        name: this.actionLabels.ADD,
+        name: this.actionLabels.CREATE,
         permission: 'create',
         icon: Icons.add,
-        routerLink: () => this.urlBuilder.getAdd(),
-        disable: () => !this.orchestratorAvailable,
-        disableDesc: () => this.getDisableDesc()
+        click: () => {
+          this.depCheckerService.checkOrchestratorOrModal(
+            this.actionLabels.CREATE,
+            this.i18n('Host'),
+            () => {
+              this.router.navigate([this.urlBuilder.getCreate()]);
+            }
+          );
+        }
       },
       {
-        name: this.actionLabels.REMOVE,
+        name: this.actionLabels.DELETE,
         permission: 'delete',
         icon: Icons.destroy,
-        click: () => this.deleteHostModal(),
-        disable: () => !this.orchestratorAvailable || !this.selection.hasSelection,
-        disableDesc: () => this.getDisableDesc()
+        click: () => {
+          this.depCheckerService.checkOrchestratorOrModal(
+            this.actionLabels.DELETE,
+            this.i18n('Host'),
+            () => this.deleteHostModal()
+          );
+        },
+        disable: () => !this.selection.hasSelection
       }
     ];
   }
@@ -93,10 +105,6 @@ export class HostsComponent implements OnInit {
         pipe: this.cephShortVersionPipe
       }
     ];
-
-    this.orchService.status().subscribe((data: { available: boolean }) => {
-      this.orchestratorAvailable = data.available;
-    });
   }
 
   updateSelection(selection: CdTableSelection) {
@@ -109,11 +117,11 @@ export class HostsComponent implements OnInit {
       initialState: {
         itemDescription: 'Host',
         itemNames: [hostname],
-        actionDescription: 'remove',
+        actionDescription: 'delete',
         submitActionObservable: () =>
           this.taskWrapper.wrapTaskAroundCall({
-            task: new FinishedTask('host/remove', { hostname: hostname }),
-            call: this.hostService.remove(hostname)
+            task: new FinishedTask('host/delete', { hostname: hostname }),
+            call: this.hostService.delete(hostname)
           })
       }
     });
@@ -153,12 +161,4 @@ export class HostsComponent implements OnInit {
       }
     );
   }
-
-  getDisableDesc() {
-    if (!this.orchestratorAvailable) {
-      return this.i18n('Host operation is disabled because orchestrator is unavailable');
-    }
-
-    return undefined;
-  }
 }

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 5fb991c6747a2ac0408151b05a596e9394ae3817..60e1d3e97ea5d898dc373a73f35e96537b5b08d3 100644 (file)
--- 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
@@ -22,11 +22,11 @@ export class HostService {
     return this.http.get(this.baseURL);
   }
 
-  add(hostname: string) {
+  create(hostname: string) {
     return this.http.post(this.baseURL, { hostname: hostname }, { observe: 'response' });
   }
 
-  remove(hostname: string) {
+  delete(hostname: string) {
     return this.http.delete(`${this.baseURL}/${hostname}`, { observe: 'response' });
   }
 

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts
index 6b107e668be3cf8d2af34d44e07c3c2366a2f2ab..3047a287db329ad297e8b6c63eb3cbb54fb62233 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts
@@ -162,8 +162,10 @@ export class TaskMessageService {
 
   messages = {
     // Host tasks
-    'host/add': this.newTaskMessage(this.commonOperations.add, (metadata) => this.host(metadata)),
-    'host/remove': this.newTaskMessage(this.commonOperations.remove, (metadata) =>
+    'host/create': this.newTaskMessage(this.commonOperations.create, (metadata) =>
+      this.host(metadata)
+    ),
+    'host/delete': this.newTaskMessage(this.commonOperations.delete, (metadata) =>
       this.host(metadata)
     ),
     // OSD tasks

diff --git a/src/pybind/mgr/dashboard/services/exception.py b/src/pybind/mgr/dashboard/services/exception.py
index 63249eb8ccf60283bb3f99830fd1dd7e2dd913e8..26b6ee4b19dd89fa31ce50e1f62258574374ce9a 100644 (file)
--- a/src/pybind/mgr/dashboard/services/exception.py
+++ b/src/pybind/mgr/dashboard/services/exception.py
@@ -8,6 +8,7 @@ import six
 
 import cherrypy
 
+from orchestrator import OrchestratorError
 import rbd
 import rados
 
@@ -131,6 +132,5 @@ def handle_send_command_error(component):
 def handle_orchestrator_error(component):
     try:
         yield
-    except RuntimeError as e:
-        # how to catch remote error e.g. NotImplementedError ?
+    except OrchestratorError as e:
         raise DashboardException(e, component=component)

diff --git a/src/pybind/mgr/dashboard/services/orchestrator.py b/src/pybind/mgr/dashboard/services/orchestrator.py
index 400055c8620c15c7ca8453339f296c5e94c6fda8..ab3b83fa431c245cc2846f1dd06bbe3dfed17bec 100644 (file)
--- a/src/pybind/mgr/dashboard/services/orchestrator.py
+++ b/src/pybind/mgr/dashboard/services/orchestrator.py
@@ -7,6 +7,7 @@ from typing import List, Optional
 from orchestrator import InventoryFilter, DeviceLightLoc, Completion
 from orchestrator import ServiceDescription, DaemonDescription
 from orchestrator import OrchestratorClientMixin, raise_if_exception, OrchestratorError
+from orchestrator import HostSpec
 from .. import mgr
 from ..tools import wraps
 
@@ -55,19 +56,19 @@ class ResourceManager(object):
 
 class HostManger(ResourceManager):
     @wait_api_result
-    def list(self):
+    def list(self) -> List[HostSpec]:
         return self.api.get_hosts()
 
-    def get(self, hostname):
-        hosts = [host for host in self.list() if host.name == hostname]
+    def get(self, hostname: str) -> Optional[HostSpec]:
+        hosts = [host for host in self.list() if host.hostname == hostname]
         return hosts[0] if hosts else None
 
     @wait_api_result
-    def add(self, hostname):
-        return self.api.add_host(hostname)
+    def add(self, hostname: str):
+        return self.api.add_host(HostSpec(hostname))
 
     @wait_api_result
-    def remove(self, hostname):
+    def remove(self, hostname: str):
         return self.api.remove_host(hostname)