From: Fabrizio D'Angelo Date: Fri, 26 Jun 2020 00:09:34 +0000 (-0400) Subject: mgr/dashboard: Consolidate Osd mark endpoints X-Git-Tag: v16.1.0~1030^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ff92dd4776027a353e4030f4d7c2d13a54704986;p=ceph.git mgr/dashboard: Consolidate Osd mark endpoints Instead of 4 different GET requests, there is now one PUT request that parses the desired action in the request body. A description was added to describe the usage. * Osd flag endpoints were grouped with Osd endpoints * A few mypy issues were also resolved Fixes: https://tracker.ceph.com/issues/46181 Signed-off-by: Avan Thakkar (cherry picked from commit f67d5aa8fa30859d15d314ef5fbb3d4cda87cd5b) (cherry picked from commit dcc2ceb0a0ce30f67aee8d8096d6e1714d3e56e7) (cherry picked from commit 5ee7f8c9b731fe249daeec6f114cf6e9c998af66) --- diff --git a/qa/tasks/mgr/dashboard/test_osd.py b/qa/tasks/mgr/dashboard/test_osd.py index 5eea93a9eff1..1beede1011e9 100644 --- a/qa/tasks/mgr/dashboard/test_osd.py +++ b/qa/tasks/mgr/dashboard/test_osd.py @@ -19,7 +19,7 @@ class OsdTest(DashboardTestCase): cls.mgr_cluster.mon_manager.raw_cluster_cmd(*cmd) def tearDown(self): - self._post('/api/osd/0/mark_in') + self._put('/api/osd/0/mark', data={'action': 'in'}) @DashboardTestCase.RunAs('test', 'test', ['block-manager']) def test_access_permissions(self): @@ -72,14 +72,14 @@ class OsdTest(DashboardTestCase): self.assertStatus(200) def test_mark_out_and_in(self): - self._post('/api/osd/0/mark_out') + self._put('/api/osd/0/mark', data={'action': 'out'}) self.assertStatus(200) - self._post('/api/osd/0/mark_in') + self._put('/api/osd/0/mark', data={'action': 'in'}) self.assertStatus(200) def test_mark_down(self): - self._post('/api/osd/0/mark_down') + self._put('/api/osd/0/mark', data={'action': 'down'}) self.assertStatus(200) def test_reweight(self): @@ -121,7 +121,7 @@ class OsdTest(DashboardTestCase): self.assertStatus(400) # Lost - self._post('/api/osd/5/mark_lost') + self._put('/api/osd/5/mark', data={'action': 'lost'}) self.assertStatus(200) # Destroy self._post('/api/osd/5/destroy') diff --git a/src/pybind/mgr/dashboard/controllers/osd.py b/src/pybind/mgr/dashboard/controllers/osd.py index 75e0d565b72f..c41aea7c9c70 100644 --- a/src/pybind/mgr/dashboard/controllers/osd.py +++ b/src/pybind/mgr/dashboard/controllers/osd.py @@ -4,7 +4,7 @@ import json import logging import time -from ceph.deployment.drive_group import DriveGroupSpec, DriveGroupValidationError +from ceph.deployment.drive_group import DriveGroupSpec, DriveGroupValidationError # type: ignore from mgr_util import get_most_recent_rate from . import ApiController, RESTController, Endpoint, Task @@ -18,10 +18,8 @@ from ..services.ceph_service import CephService, SendCommandError from ..services.exception import handle_send_command_error, handle_orchestrator_error from ..services.orchestrator import OrchClient, OrchFeature from ..tools import str_to_bool -try: - from typing import Dict, List, Any, Union # noqa: F401 pylint: disable=unused-import -except ImportError: # pragma: no cover - pass # For typing only + +from typing import Any, Dict, List, Union # pylint: disable=C0411 logger = logging.getLogger('controllers.osd') @@ -44,7 +42,7 @@ def osd_task(name, metadata, wait_for=2.0): @ApiController('/osd', Scope.OSD) -@ControllerDoc("Get OSD Details", "OSD") +@ControllerDoc('OSD management API', 'OSD') class Osd(RESTController): def list(self): osds = self.get_osd_map() @@ -162,7 +160,7 @@ class Osd(RESTController): @osd_task('delete', {'svc_id': '{svc_id}'}) def delete(self, svc_id, preserve_id=None, force=None): # pragma: no cover replace = False - check = False + check: Union[Dict[str, Any], bool] = False try: if preserve_id is not None: replace = str_to_bool(preserve_id) @@ -197,20 +195,25 @@ class Osd(RESTController): api_scrub = "osd deep-scrub" if str_to_bool(deep) else "osd scrub" CephService.send_command("mon", api_scrub, who=svc_id) - @RESTController.Resource('POST') - @allow_empty_body - def mark_out(self, svc_id): - CephService.send_command('mon', 'osd out', ids=[svc_id]) - - @RESTController.Resource('POST') - @allow_empty_body - def mark_in(self, svc_id): - CephService.send_command('mon', 'osd in', ids=[svc_id]) - - @RESTController.Resource('POST') - @allow_empty_body - def mark_down(self, svc_id): - CephService.send_command('mon', 'osd down', ids=[svc_id]) + @RESTController.Resource('PUT') + @EndpointDoc("Mark OSD flags (out, in, down, lost, ...)", + parameters={'svc_id': (str, 'SVC ID')}) + def mark(self, svc_id, action): + """ + Note: osd must be marked `down` before marking lost. + """ + valid_actions = ['out', 'in', 'down', 'lost'] + args = {'srv_type': 'mon', 'prefix': 'osd ' + action} + if action.lower() in valid_actions: + if action == 'lost': + args['id'] = int(svc_id) + args['yes_i_really_mean_it'] = True + else: + args['ids'] = [svc_id] + + CephService.send_command(**args) + else: + logger.error("Invalid OSD mark action: %s attempted on SVC_ID: %s", action, svc_id) @RESTController.Resource('POST') @allow_empty_body @@ -234,18 +237,6 @@ class Osd(RESTController): id=int(svc_id), weight=float(weight)) - @RESTController.Resource('POST') - @allow_empty_body - def mark_lost(self, svc_id): - """ - Note: osd must be marked `down` before marking lost. - """ - CephService.send_command( - 'mon', - 'osd lost', - id=int(svc_id), - yes_i_really_mean_it=True) - def _create_bare(self, data): """Create a OSD container that has no associated device. @@ -361,7 +352,7 @@ class Osd(RESTController): @ApiController('/osd/flags', Scope.OSD) -@ControllerDoc("OSD Flags controller Management API", "OsdFlagsController") +@ControllerDoc(group='OSD') class OsdFlagsController(RESTController): @staticmethod def _osd_flags(): 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 e96a13ae3066..831a022e1d73 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 @@ -101,20 +101,23 @@ describe('OsdService', () => { it('should mark the OSD out', () => { service.markOut(1).subscribe(); - const req = httpTesting.expectOne('api/osd/1/mark_out'); - expect(req.request.method).toBe('POST'); + const req = httpTesting.expectOne('api/osd/1/mark'); + expect(req.request.method).toBe('PUT'); + expect(req.request.body).toEqual({ action: 'out' }); }); it('should mark the OSD in', () => { service.markIn(1).subscribe(); - const req = httpTesting.expectOne('api/osd/1/mark_in'); - expect(req.request.method).toBe('POST'); + const req = httpTesting.expectOne('api/osd/1/mark'); + expect(req.request.method).toBe('PUT'); + expect(req.request.body).toEqual({ action: 'in' }); }); it('should mark the OSD down', () => { service.markDown(1).subscribe(); - const req = httpTesting.expectOne('api/osd/1/mark_down'); - expect(req.request.method).toBe('POST'); + const req = httpTesting.expectOne('api/osd/1/mark'); + expect(req.request.method).toBe('PUT'); + expect(req.request.body).toEqual({ action: 'down' }); }); it('should reweight an OSD', () => { @@ -133,8 +136,9 @@ describe('OsdService', () => { it('should mark an OSD lost', () => { service.markLost(1).subscribe(); - const req = httpTesting.expectOne('api/osd/1/mark_lost'); - expect(req.request.method).toBe('POST'); + const req = httpTesting.expectOne('api/osd/1/mark'); + expect(req.request.method).toBe('PUT'); + expect(req.request.body).toEqual({ action: 'lost' }); }); it('should purge an 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 847b788ee5c6..2e698a220689 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 @@ -103,15 +103,15 @@ export class OsdService { } markOut(id: number) { - return this.http.post(`${this.path}/${id}/mark_out`, null); + return this.http.put(`${this.path}/${id}/mark`, { action: 'out' }); } markIn(id: number) { - return this.http.post(`${this.path}/${id}/mark_in`, null); + return this.http.put(`${this.path}/${id}/mark`, { action: 'in' }); } markDown(id: number) { - return this.http.post(`${this.path}/${id}/mark_down`, null); + return this.http.put(`${this.path}/${id}/mark`, { action: 'down' }); } reweight(id: number, weight: number) { @@ -123,7 +123,7 @@ export class OsdService { } markLost(id: number) { - return this.http.post(`${this.path}/${id}/mark_lost`, null); + return this.http.put(`${this.path}/${id}/mark`, { action: 'lost' }); } purge(id: number) { diff --git a/src/pybind/mgr/dashboard/tests/test_osd.py b/src/pybind/mgr/dashboard/tests/test_osd.py index 5bbaf39afb7e..e1ccd08a5382 100644 --- a/src/pybind/mgr/dashboard/tests/test_osd.py +++ b/src/pybind/mgr/dashboard/tests/test_osd.py @@ -1,15 +1,10 @@ # -*- coding: utf-8 -*- -from __future__ import absolute_import - import uuid from contextlib import contextmanager -try: - import mock -except ImportError: - from unittest import mock -from ceph.deployment.drive_group import DeviceSelection, DriveGroupSpec -from ceph.deployment.service_spec import PlacementSpec +from unittest import mock +from ceph.deployment.drive_group import DeviceSelection, DriveGroupSpec # type: ignore +from ceph.deployment.service_spec import PlacementSpec # type: ignore from . import ControllerTestCase from ..controllers.osd import Osd @@ -17,18 +12,15 @@ from ..tools import NotificationQueue, TaskManager from .. import mgr from .helper import update_dict -try: - from typing import List, Dict, Any # pylint: disable=unused-import -except ImportError: - pass # Only requried for type hints +from typing import Any, Dict, List, Optional # pylint: disable=C0411 class OsdHelper(object): DEFAULT_OSD_IDS = [0, 1, 2] @staticmethod - def _gen_osdmap_tree_node(node_id, node_type, children=None, update_data=None): - # type: (int, str, List[int], Dict[str, Any]) -> Dict[str, Any] + def _gen_osdmap_tree_node(node_id: int, node_type: str, children: Optional[List[int]] = None, + update_data: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: assert node_type in ['root', 'host', 'osd'] if node_type in ['root', 'host']: assert children is not None @@ -69,8 +61,7 @@ class OsdHelper(object): return update_dict(node, update_data) if update_data else node @staticmethod - def _gen_osd_stats(osd_id, update_data=None): - # type: (int, Dict[str, Any]) -> Dict[str, Any] + def _gen_osd_stats(osd_id: int, update_data: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: stats = { 'osd': osd_id, 'up_from': 11, @@ -112,8 +103,7 @@ class OsdHelper(object): return stats if not update_data else update_dict(stats, update_data) @staticmethod - def _gen_osd_map_osd(osd_id): - # type: (int) -> Dict[str, Any] + def _gen_osd_map_osd(osd_id: int) -> Dict[str, Any]: return { 'osd': osd_id, 'up': 1, @@ -180,26 +170,22 @@ class OsdHelper(object): } @classmethod - def gen_osdmap(cls, ids=None): - # type: (List[int]) -> Dict[str, Any] + def gen_osdmap(cls, ids: Optional[List[int]] = None) -> Dict[str, Any]: return {str(i): cls._gen_osd_map_osd(i) for i in ids or cls.DEFAULT_OSD_IDS} @classmethod - def gen_osd_stats(cls, ids=None): - # type: (List[int]) -> List[Dict[str, Any]] + def gen_osd_stats(cls, ids: Optional[List[int]] = None) -> List[Dict[str, Any]]: return [cls._gen_osd_stats(i) for i in ids or cls.DEFAULT_OSD_IDS] @classmethod - def gen_osdmap_tree_nodes(cls, ids=None): - # type: (List[int]) -> List[Dict[str, Any]] + def gen_osdmap_tree_nodes(cls, ids: Optional[List[int]] = None) -> List[Dict[str, Any]]: return [ cls._gen_osdmap_tree_node(-1, 'root', [-3]), cls._gen_osdmap_tree_node(-3, 'host', ids or cls.DEFAULT_OSD_IDS), ] + [cls._gen_osdmap_tree_node(node_id, 'osd') for node_id in ids or cls.DEFAULT_OSD_IDS] @classmethod - def gen_mgr_get_counter(cls): - # type: () -> List[List[int]] + def gen_mgr_get_counter(cls) -> List[List[int]]: return [[1551973855, 35], [1551973860, 35], [1551973865, 35], [1551973870, 35]] @@ -328,3 +314,13 @@ class OsdTest(ControllerTestCase): } self._task_post('/api/osd', data) self.assertStatus(400) + + @mock.patch('dashboard.controllers.osd.CephService') + def test_osd_mark_all_actions(self, instance): + fake_client = mock.Mock() + instance.return_value = fake_client + action_list = ['OUT', 'IN', 'DOWN'] + for action in action_list: + data = {'action': action} + self._task_put('/api/osd/1/mark', data) + self.assertStatus(200)