From: Kiefer Chang Date: Tue, 26 May 2020 08:22:01 +0000 (+0800) Subject: mgr/dashboard: allow preserving OSD IDs when deleting OSDs X-Git-Tag: v16.1.0~1976^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=fecafd8325cce31b37903a5e7e1c9badbf719d89;p=ceph.git mgr/dashboard: allow preserving OSD IDs when deleting OSDs Ask users if they want to preserve OSD ID(s) for replacement after OSD deletion in confirmation modal. Fixes: https://tracker.ceph.com/issues/38234 Signed-off-by: Kiefer Chang --- diff --git a/src/pybind/mgr/dashboard/controllers/osd.py b/src/pybind/mgr/dashboard/controllers/osd.py index 4327f2e92fa9..d9090d2bbff4 100644 --- a/src/pybind/mgr/dashboard/controllers/osd.py +++ b/src/pybind/mgr/dashboard/controllers/osd.py @@ -157,16 +157,28 @@ class Osd(RESTController): @raise_if_no_orchestrator @handle_orchestrator_error('osd') @osd_task('delete', {'svc_id': '{svc_id}'}) - def delete(self, svc_id, force=None): + def delete(self, svc_id, preserve_id=None, force=None): + replace = False + check = False + try: + if preserve_id is not None: + replace = str_to_bool(preserve_id) + if force is not None: + check = not str_to_bool(force) + except ValueError: + raise DashboardException( + component='osd', http_status_code=400, msg='Invalid parameter(s)') + orch = OrchClient.instance() - if not force: + if check: logger.info('Check for removing osd.%s...', svc_id) check = self._check_delete([svc_id]) if not check['safe']: logger.error('Unable to remove osd.%s: %s', svc_id, check['message']) raise DashboardException(component='osd', msg=check['message']) - logger.info('Start removing osd.%s...', svc_id) - orch.osds.remove([svc_id]) + + logger.info('Start removing osd.%s (replace: %s)...', svc_id, replace) + orch.osds.remove([svc_id], replace) while True: removal_osds = orch.osds.removing_status() logger.info('Current removing OSDs %s', removal_osds) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html index d1a5fb653dd0..761a46e80e7a 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html @@ -80,3 +80,24 @@ [usedBytes]="row.stats.stat_bytes_used"> + + + + +
+
+ + +
+
+ + + diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts index 850cdf66f327..43fef70189a2 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts @@ -1,4 +1,5 @@ import { Component, OnInit, TemplateRef, ViewChild } from '@angular/core'; +import { FormControl } from '@angular/forms'; import { Router } from '@angular/router'; import { I18n } from '@ngx-translate/i18n-polyfill'; @@ -16,6 +17,7 @@ import { TableComponent } from '../../../../shared/datatable/table/table.compone import { CellTemplate } from '../../../../shared/enum/cell-template.enum'; import { Icons } from '../../../../shared/enum/icons.enum'; import { NotificationType } from '../../../../shared/enum/notification-type.enum'; +import { CdFormGroup } from '../../../../shared/forms/cd-form-group'; import { CdTableAction } from '../../../../shared/models/cd-table-action'; import { CdTableColumn } from '../../../../shared/models/cd-table-column'; import { CdTableSelection } from '../../../../shared/models/cd-table-selection'; @@ -54,6 +56,8 @@ export class OsdListComponent extends ListWithDetails implements OnInit { reweightBodyTpl: TemplateRef; @ViewChild('safeToDestroyBodyTpl') safeToDestroyBodyTpl: TemplateRef; + @ViewChild('deleteOsdExtraTpl') + deleteOsdExtraTpl: TemplateRef; permissions: Permissions; tableActions: CdTableAction[]; @@ -217,33 +221,7 @@ export class OsdListComponent extends ListWithDetails implements OnInit { { name: this.actionLabels.DELETE, permission: 'delete', - click: () => { - this.depCheckerService.checkOrchestratorOrModal( - this.actionLabels.DELETE, - this.i18n('OSD'), - () => { - this.showCriticalConfirmationModal( - this.i18n('delete'), - this.i18n('OSD'), - this.i18n('deleted'), - (ids: number[]) => { - return this.osdService.safeToDelete(JSON.stringify(ids)); - }, - 'is_safe_to_delete', - (id: number) => { - this.selection = new CdTableSelection(); - return this.taskWrapper.wrapTaskAroundCall({ - task: new FinishedTask('osd/' + URLVerbs.DELETE, { - svc_id: id - }), - call: this.osdService.delete(id, true) - }); - }, - true - ); - } - ); - }, + click: () => this.delete(), disable: () => !this.hasOsdSelected, icon: Icons.destroy } @@ -474,6 +452,40 @@ export class OsdListComponent extends ListWithDetails implements OnInit { }); } + delete() { + const deleteFormGroup = new CdFormGroup({ + preserve: new FormControl(false) + }); + + this.depCheckerService.checkOrchestratorOrModal( + this.actionLabels.DELETE, + this.i18n('OSD'), + () => { + this.showCriticalConfirmationModal( + this.i18n('delete'), + this.i18n('OSD'), + this.i18n('deleted'), + (ids: number[]) => { + return this.osdService.safeToDelete(JSON.stringify(ids)); + }, + 'is_safe_to_delete', + (id: number) => { + this.selection = new CdTableSelection(); + return this.taskWrapper.wrapTaskAroundCall({ + task: new FinishedTask('osd/' + URLVerbs.DELETE, { + svc_id: id + }), + call: this.osdService.delete(id, deleteFormGroup.value.preserve, true) + }); + }, + true, + deleteFormGroup, + this.deleteOsdExtraTpl + ); + } + ); + } + /** * Perform check first and display a critical confirmation modal. * @param {string} actionDescription name of the action. @@ -482,8 +494,9 @@ export class OsdListComponent extends ListWithDetails implements OnInit { * @param {Function} check the function is called to check if the action is safe. * @param {string} checkKey the safe indicator's key in the check response. * @param {Function} action the action function. - * @param {boolean} oneshot if true, action function is called with all items as parameter. - * Otherwise, multiple action functions with individual items are sent. + * @param {boolean} taskWrapped if true, hide confirmation modal after action + * @param {CdFormGroup} childFormGroup additional child form group to be passed to confirmation modal + * @param {TemplateRef} childFormGroupTemplate template for additional child form group */ showCriticalConfirmationModal( actionDescription: string, @@ -492,7 +505,9 @@ export class OsdListComponent extends ListWithDetails implements OnInit { check: (ids: number[]) => Observable, checkKey: string, action: (id: number | number[]) => Observable, - taskWrapped: boolean = false + taskWrapped: boolean = false, + childFormGroup?: CdFormGroup, + childFormGroupTemplate?: TemplateRef ): void { check(this.getSelectedOsdIds()).subscribe((result) => { const modalRef = this.modalService.show(CriticalConfirmationModalComponent, { @@ -505,6 +520,8 @@ export class OsdListComponent extends ListWithDetails implements OnInit { message: result.message, actionDescription: templateItemDescription }, + childFormGroup: childFormGroup, + childFormGroupTemplate: childFormGroupTemplate, submitAction: () => { const observable = observableForkJoin( this.getSelectedOsdIds().map((osd: any) => action.call(this.osdService, osd)) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.spec.ts index 67a40b2091b5..4c8e7eeedeb4 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.spec.ts @@ -55,6 +55,13 @@ describe('OsdService', () => { expect(req.request.body).toEqual(post_data); }); + it('should call delete', () => { + const id = 1; + service.delete(id, true, true).subscribe(); + const req = httpTesting.expectOne(`api/osd/${id}?preserve_id=true&force=true`); + expect(req.request.method).toBe('DELETE'); + }); + it('should call getList', () => { service.getList().subscribe(); const req = httpTesting.expectOne('api/osd'); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.ts index 8d6369a21623..0bd2f370a9b1 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/osd.service.ts @@ -1,4 +1,4 @@ -import { HttpClient, HttpParams } from '@angular/common/http'; +import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { I18n } from '@ngx-translate/i18n-polyfill'; @@ -137,10 +137,12 @@ export class OsdService { return this.http.post(`${this.path}/${id}/destroy`, null); } - delete(id: number, force?: boolean) { - const options = force ? { params: new HttpParams().set('force', 'true') } : {}; - options['observe'] = 'response'; - return this.http.delete(`${this.path}/${id}`, options); + delete(id: number, preserveId?: boolean, force?: boolean) { + const params = { + preserve_id: preserveId ? 'true' : 'false', + force: force ? 'true' : 'false' + }; + return this.http.delete(`${this.path}/${id}`, { observe: 'response', params: params }); } safeToDestroy(ids: string) { diff --git a/src/pybind/mgr/dashboard/services/orchestrator.py b/src/pybind/mgr/dashboard/services/orchestrator.py index a01db2574e85..cb394b8816eb 100644 --- a/src/pybind/mgr/dashboard/services/orchestrator.py +++ b/src/pybind/mgr/dashboard/services/orchestrator.py @@ -109,8 +109,8 @@ class OsdManager(ResourceManager): return self.api.apply_drivegroups(drive_group_specs) @wait_api_result - def remove(self, osd_ids): - return self.api.remove_osds(osd_ids) + def remove(self, osd_ids, replace=False, force=False): + return self.api.remove_osds(osd_ids, replace, force) @wait_api_result def removing_status(self):