From: Kiefer Chang Date: Wed, 18 Mar 2020 07:21:35 +0000 (+0800) Subject: mgr/dashboard: fix adding/removing host errors X-Git-Tag: v15.2.0~15^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4f2db7e60e7ef761d84b67b6d5d21933e7caec40;p=ceph.git 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 --- diff --git a/src/pybind/mgr/dashboard/controllers/host.py b/src/pybind/mgr/dashboard/controllers/host.py index c609db0930df..3a90389bba57 100644 --- 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 24fdf9ea8f11..99208d37c6fa 100644 --- 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 59473514e4e7..bfa2987107a4 100644 --- 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 6af6eff886f6..d6812be0d69d 100644 --- 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 = []; hosts: Array = []; 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 5fb991c6747a..60e1d3e97ea5 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 @@ -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 6b107e668be3..3047a287db32 100644 --- 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 63249eb8ccf6..26b6ee4b19dd 100644 --- 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 400055c8620c..ab3b83fa431c 100644 --- 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)