From: Tiago Melo <tmelo@suse.com>
Date: Fri, 20 Sep 2019 16:08:03 +0000 (+0000)
Subject: mgr/dashboard: Fix OSD's safe_to_destroy method
X-Git-Tag: v15.1.0~1042^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=83b0d0d50f7f28a3002cf1e5a58a327056371820;p=ceph.git

mgr/dashboard: Fix OSD's safe_to_destroy method

We were passing the wrong value in "send_command".

Changed the ids path param to a query param.

Signed-off-by: Tiago Melo <tmelo@suse.com>
---

diff --git a/qa/tasks/mgr/dashboard/test_osd.py b/qa/tasks/mgr/dashboard/test_osd.py
index fdd84ac7ea72..610d612150d8 100644
--- a/qa/tasks/mgr/dashboard/test_osd.py
+++ b/qa/tasks/mgr/dashboard/test_osd.py
@@ -99,8 +99,11 @@ class OsdTest(DashboardTestCase):
 
     def test_safe_to_destroy(self):
         osd_dump = json.loads(self._ceph_cmd(['osd', 'dump', '-f', 'json']))
-        unused_osd_id = max(map(lambda e: e['osd'], osd_dump['osds'])) + 10
-        self._get('/api/osd/{}/safe_to_destroy'.format(unused_osd_id))
+        max_id = max(map(lambda e: e['osd'], osd_dump['osds']))
+
+        # 1 OSD safe to destroy
+        unused_osd_id = max_id + 10
+        self._get('/api/osd/safe_to_destroy?ids={}'.format(unused_osd_id))
         self.assertStatus(200)
         self.assertJsonBody({
             'is_safe_to_destroy': True,
@@ -110,8 +113,21 @@ class OsdTest(DashboardTestCase):
             'stored_pgs': [],
         })
 
+        # multiple OSDs safe to destroy
+        unused_osd_ids = [max_id + 11, max_id + 12]
+        self._get('/api/osd/safe_to_destroy?ids={}'.format(str(unused_osd_ids)))
+        self.assertStatus(200)
+        self.assertJsonBody({
+            'is_safe_to_destroy': True,
+            'active': [],
+            'missing_stats': [],
+            'safe_to_destroy': unused_osd_ids,
+            'stored_pgs': [],
+        })
+
+        # 1 OSD unsafe to destroy
         def get_destroy_status():
-            self._get('/api/osd/0/safe_to_destroy')
+            self._get('/api/osd/safe_to_destroy?ids=0')
             if 'is_safe_to_destroy' in self.jsonBody():
                 return self.jsonBody()['is_safe_to_destroy']
             return None
diff --git a/src/pybind/mgr/dashboard/controllers/osd.py b/src/pybind/mgr/dashboard/controllers/osd.py
index e9eb229aff23..5e742863a9cf 100644
--- a/src/pybind/mgr/dashboard/controllers/osd.py
+++ b/src/pybind/mgr/dashboard/controllers/osd.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import
-from . import ApiController, RESTController, UpdatePermission
+import json
+from . import ApiController, RESTController, Endpoint, ReadPermission, UpdatePermission
 from .. import mgr, logger
 from ..security import Scope
 from ..services.ceph_service import CephService, SendCommandError
@@ -196,18 +197,23 @@ class Osd(RESTController):
         CephService.send_command(
             'mon', 'osd destroy-actual', id=int(svc_id), yes_i_really_mean_it=True)
 
-    @RESTController.Resource('GET')
-    def safe_to_destroy(self, svc_id):
+    @Endpoint('GET', query_params=['ids'])
+    @ReadPermission
+    def safe_to_destroy(self, ids):
         """
-        :type svc_id: int|[int]
+        :type ids: int|[int]
         """
-        if not isinstance(svc_id, list):
-            svc_id = [svc_id]
-        svc_id = list(map(str, svc_id))
+
+        ids = json.loads(ids)
+        if isinstance(ids, list):
+            ids = list(map(str, ids))
+        else:
+            ids = [str(ids)]
+
         try:
             result = CephService.send_command(
-                'mon', 'osd safe-to-destroy', ids=svc_id, target=('mgr', ''))
-            result['is_safe_to_destroy'] = set(result['safe_to_destroy']) == set(map(int, svc_id))
+                'mon', 'osd safe-to-destroy', ids=ids, target=('mgr', ''))
+            result['is_safe_to_destroy'] = set(result['safe_to_destroy']) == set(map(int, ids))
             return result
 
         except SendCommandError as e:
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 fc1e52fff372..f30e099c52b3 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
@@ -108,7 +108,7 @@ describe('OsdService', () => {
 
   it('should return if it is safe to destroy an OSD', () => {
     service.safeToDestroy('[0,1]').subscribe();
-    const req = httpTesting.expectOne('api/osd/[0,1]/safe_to_destroy');
+    const req = httpTesting.expectOne('api/osd/safe_to_destroy?ids=[0,1]');
     expect(req.request.method).toBe('GET');
   });
 });
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 008b877b1492..f99132a7672e 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
@@ -248,6 +248,6 @@ export class OsdService {
       'safe-to-destroy': boolean;
       message?: string;
     }
-    return this.http.get<SafeToDestroyResponse>(`${this.path}/${ids}/safe_to_destroy`);
+    return this.http.get<SafeToDestroyResponse>(`${this.path}/safe_to_destroy?ids=${ids}`);
   }
 }