]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: Address API inconsistencies 35802/head
authorFabrizio D'Angelo <fdangelo@redhat.com>
Fri, 26 Jun 2020 13:50:15 +0000 (09:50 -0400)
committerAvan Thakkar <athakkar@redhat.com>
Mon, 31 Aug 2020 18:15:49 +0000 (23:45 +0530)
Cephfs endpoints are inconsistencies with other endpoints.

* Change rm endpoints from GET to DELETE requests
* Change set_quotas from POST to PUT
* Renames endpoints accordingly
* Changes related angular files
* update a few lines related to mypy
* remove future import as the python2 compatibility is not needed

Fixes: https://tracker.ceph.com/issues/46160
Signed-off-by: Fabrizio D'Angelo <fdangelo@redhat.com>
(cherry picked from commit 222286b08bde3600d50a6deb25e7425c17e9e482)

qa/tasks/mgr/dashboard/test_cephfs.py
src/pybind/mgr/dashboard/controllers/cephfs.py
src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.ts

index 1be800b92c567934e3562406c2e64103696c935f..2dbd9add72fefa386640f8c3b8b726a5db0d6439 100644 (file)
@@ -1,6 +1,4 @@
 # -*- coding: utf-8 -*-
-from __future__ import absolute_import
-
 from contextlib import contextmanager
 
 from .helper import DashboardTestCase, JObj, JList, JLeaf
@@ -21,12 +19,12 @@ class CephfsTest(DashboardTestCase):
         return self.fs.get_namespace_id()
 
     def mk_dirs(self, path, expectedStatus=200):
-        self._post("/api/cephfs/{}/mk_dirs".format(self.get_fs_id()),
+        self._post("/api/cephfs/{}/tree".format(self.get_fs_id()),
                    params={'path': path})
         self.assertStatus(expectedStatus)
 
     def rm_dir(self, path, expectedStatus=200):
-        self._post("/api/cephfs/{}/rm_dir".format(self.get_fs_id()),
+        self._delete("/api/cephfs/{}/tree".format(self.get_fs_id()),
                    params={'path': path})
         self.assertStatus(expectedStatus)
 
@@ -36,10 +34,10 @@ class CephfsTest(DashboardTestCase):
         self.assertIsInstance(data, dict)
         return data
 
-    def ls_dir(self, path, expectedLength, depth = None):
+    def ls_dir(self, path, expectedLength, depth=None):
         return self._ls_dir(path, expectedLength, depth, "api")
 
-    def ui_ls_dir(self, path, expectedLength, depth = None):
+    def ui_ls_dir(self, path, expectedLength, depth=None):
         return self._ls_dir(path, expectedLength, depth, "ui-api")
 
     def _ls_dir(self, path, expectedLength, depth, baseApiPath):
@@ -53,24 +51,24 @@ class CephfsTest(DashboardTestCase):
         self.assertEqual(len(data), expectedLength)
         return data
 
-    def setQuotas(self, bytes=None, files=None):
+    def set_quotas(self, max_bytes=None, max_files=None):
         quotas = {
-            'max_bytes': bytes,
-            'max_files': files
+            'max_bytes': max_bytes,
+            'max_files': max_files
         }
-        self._post("/api/cephfs/{}/set_quotas".format(self.get_fs_id()), data=quotas,
-                   params={'path': self.QUOTA_PATH})
+        self._put("/api/cephfs/{}/quota".format(self.get_fs_id()), data=quotas,
+                  params={'path': self.QUOTA_PATH})
         self.assertStatus(200)
 
-    def assertQuotas(self, bytes, files):
+    def assert_quotas(self, max_bytes, files):
         data = self.ls_dir('/', 1)[0]
-        self.assertEqual(data['quotas']['max_bytes'], bytes)
+        self.assertEqual(data['quotas']['max_bytes'], max_bytes)
         self.assertEqual(data['quotas']['max_files'], files)
 
     @contextmanager
     def new_quota_dir(self):
         self.mk_dirs(self.QUOTA_PATH)
-        self.setQuotas(1024**3, 1024)
+        self.set_quotas(1024 ** 3, 1024)
         yield 1
         self.rm_dir(self.QUOTA_PATH)
 
@@ -146,7 +144,7 @@ class CephfsTest(DashboardTestCase):
 
     def test_cephfs_get_quotas(self):
         fs_id = self.get_fs_id()
-        data = self._get("/api/cephfs/{}/get_quotas?path=/".format(fs_id))
+        data = self._get("/api/cephfs/{}/quota?path=/".format(fs_id))
         self.assertStatus(200)
         self.assertSchema(data, JObj({
             'max_bytes': int,
@@ -210,7 +208,7 @@ class CephfsTest(DashboardTestCase):
         fs_id = self.get_fs_id()
         self.mk_dirs('/movies/dune/extended_version')
 
-        self._post("/api/cephfs/{}/mk_snapshot".format(fs_id),
+        self._post("/api/cephfs/{}/snapshot".format(fs_id),
                    params={'path': '/movies/dune', 'name': 'test'})
         self.assertStatus(200)
 
@@ -240,7 +238,7 @@ class CephfsTest(DashboardTestCase):
         snapshots = data[0]['snapshots']
         self.assertEqual(len(snapshots), 0)
 
-        self._post("/api/cephfs/{}/rm_snapshot".format(fs_id),
+        self._delete("/api/cephfs/{}/snapshot".format(fs_id),
                    params={'path': '/movies/dune', 'name': 'test'})
         self.assertStatus(200)
 
@@ -255,27 +253,27 @@ class CephfsTest(DashboardTestCase):
 
     def test_quotas_default(self):
         self.mk_dirs(self.QUOTA_PATH)
-        self.assertQuotas(0, 0)
+        self.assert_quotas(0, 0)
         self.rm_dir(self.QUOTA_PATH)
 
     def test_quotas_set_both(self):
         with self.new_quota_dir():
-            self.assertQuotas(1024**3, 1024)
+            self.assert_quotas(1024 ** 3, 1024)
 
     def test_quotas_set_only_bytes(self):
         with self.new_quota_dir():
-            self.setQuotas(2048**3)
-            self.assertQuotas(2048**3, 1024)
+            self.set_quotas(2048 ** 3)
+            self.assert_quotas(2048 ** 3, 1024)
 
     def test_quotas_set_only_files(self):
         with self.new_quota_dir():
-            self.setQuotas(None, 2048)
-            self.assertQuotas(1024**3, 2048)
+            self.set_quotas(None, 2048)
+            self.assert_quotas(1024 ** 3, 2048)
 
     def test_quotas_unset_both(self):
         with self.new_quota_dir():
-            self.setQuotas(0, 0)
-            self.assertQuotas(0, 0)
+            self.set_quotas(0, 0)
+            self.assert_quotas(0, 0)
 
     def test_listing_of_root_dir(self):
         self.ls_dir('/', 0)  # Should not list root
index e6eb9c83efe38207630c376ca3156b0bcbfb1fe2..65969f8d8fdcd76de35b17d11d42088c0a5b48b1 100644 (file)
@@ -1,6 +1,4 @@
 # -*- coding: utf-8 -*-
-from __future__ import absolute_import
-
 from collections import defaultdict
 
 import os
@@ -81,7 +79,7 @@ class CephFS(RESTController):
                 "mds_mem.ino"
             ]
 
-        result = {}  # type: dict
+        result: dict = {}
         mds_names = self._get_mds_names(fs_id)
 
         for mds_name in mds_names:
@@ -136,7 +134,7 @@ class CephFS(RESTController):
 
     # pylint: disable=too-many-statements,too-many-branches
     def fs_status(self, fs_id):
-        mds_versions = defaultdict(list)  # type: dict
+        mds_versions: dict = defaultdict(list)
 
         fsmap = mgr.get("fs_map")
         filesystem = None
@@ -391,8 +389,8 @@ class CephFS(RESTController):
             path = os.path.normpath(path)
         return path
 
-    @RESTController.Resource('POST')
-    def mk_dirs(self, fs_id, path):
+    @RESTController.Resource('POST', path='/tree')
+    def mk_tree(self, fs_id, path):
         """
         Create a directory.
         :param fs_id: The filesystem identifier.
@@ -401,8 +399,8 @@ class CephFS(RESTController):
         cfs = self._cephfs_instance(fs_id)
         cfs.mk_dirs(path)
 
-    @RESTController.Resource('POST')
-    def rm_dir(self, fs_id, path):
+    @RESTController.Resource('DELETE', path='/tree')
+    def rm_tree(self, fs_id, path):
         """
         Remove a directory.
         :param fs_id: The filesystem identifier.
@@ -411,40 +409,26 @@ class CephFS(RESTController):
         cfs = self._cephfs_instance(fs_id)
         cfs.rm_dir(path)
 
-    @RESTController.Resource('POST')
-    def mk_snapshot(self, fs_id, path, name=None):
-        """
-        Create a snapshot.
-        :param fs_id: The filesystem identifier.
-        :param path: The path of the directory.
-        :param name: The name of the snapshot. If not specified,
-            a name using the current time in RFC3339 UTC format
-            will be generated.
-        :return: The name of the snapshot.
-        :rtype: str
-        """
-        cfs = self._cephfs_instance(fs_id)
-        return cfs.mk_snapshot(path, name)
-
-    @RESTController.Resource('POST')
-    def rm_snapshot(self, fs_id, path, name):
+    @RESTController.Resource('PUT', path='/quota')
+    def quota(self, fs_id, path, max_bytes=None, max_files=None):
         """
-        Remove a snapshot.
+        Set the quotas of the specified path.
         :param fs_id: The filesystem identifier.
-        :param path: The path of the directory.
-        :param name: The name of the snapshot.
+        :param path: The path of the directory/file.
+        :param max_bytes: The byte limit.
+        :param max_files: The file limit.
         """
         cfs = self._cephfs_instance(fs_id)
-        cfs.rm_snapshot(path, name)
+        return cfs.set_quotas(path, max_bytes, max_files)
 
-    @RESTController.Resource('GET')
+    @RESTController.Resource('GET', path='/quota')
     @EndpointDoc("Get Cephfs Quotas of the specified path",
                  parameters={
                      'fs_id': (str, 'File System Identifier'),
                      'path': (str, 'File System Path'),
                  },
                  responses={200: GET_QUOTAS_SCHEMA})
-    def get_quotas(self, fs_id, path):
+    def get_quota(self, fs_id, path):
         """
         Get the quotas of the specified path.
         :param fs_id: The filesystem identifier.
@@ -456,17 +440,31 @@ class CephFS(RESTController):
         cfs = self._cephfs_instance(fs_id)
         return cfs.get_quotas(path)
 
-    @RESTController.Resource('POST')
-    def set_quotas(self, fs_id, path, max_bytes=None, max_files=None):
+    @RESTController.Resource('POST', path='/snapshot')
+    def snapshot(self, fs_id, path, name=None):
         """
-        Set the quotas of the specified path.
+        Create a snapshot.
         :param fs_id: The filesystem identifier.
-        :param path: The path of the directory/file.
-        :param max_bytes: The byte limit.
-        :param max_files: The file limit.
+        :param path: The path of the directory.
+        :param name: The name of the snapshot. If not specified,
+            a name using the current time in RFC3339 UTC format
+            will be generated.
+        :return: The name of the snapshot.
+        :rtype: str
         """
         cfs = self._cephfs_instance(fs_id)
-        return cfs.set_quotas(path, max_bytes, max_files)
+        return cfs.mk_snapshot(path, name)
+
+    @RESTController.Resource('DELETE', path='/snapshot')
+    def rm_snapshot(self, fs_id, path, name):
+        """
+        Remove a snapshot.
+        :param fs_id: The filesystem identifier.
+        :param path: The path of the directory.
+        :param name: The name of the snapshot.
+        """
+        cfs = self._cephfs_instance(fs_id)
+        cfs.rm_snapshot(path, name)
 
 
 class CephFSClients(object):
index 124fad18c2bb41e4d3007d2f509c8b99179b65ac..2e10277979560eadab728eeee542cc8e53157c1e 100644 (file)
@@ -394,7 +394,7 @@ describe('CephfsDirectoriesComponent', () => {
     lsDirSpy = spyOn(cephfsService, 'lsDir').and.callFake(mockLib.lsDir);
     spyOn(cephfsService, 'mkSnapshot').and.callFake(mockLib.mkSnapshot);
     spyOn(cephfsService, 'rmSnapshot').and.callFake(mockLib.rmSnapshot);
-    spyOn(cephfsService, 'updateQuota').and.callFake(mockLib.updateQuota);
+    spyOn(cephfsService, 'quota').and.callFake(mockLib.updateQuota);
 
     modalShowSpy = spyOn(TestBed.inject(ModalService), 'show').and.callFake(mockLib.modalShow);
     notificationShowSpy = spyOn(TestBed.inject(NotificationService), 'show').and.stub();
@@ -777,7 +777,7 @@ describe('CephfsDirectoriesComponent', () => {
         });
 
         it('should update max_files correctly', () => {
-          expect(cephfsService.updateQuota).toHaveBeenCalledWith(1, '/a/c/b', { max_files: 5 });
+          expect(cephfsService.quota).toHaveBeenCalledWith(1, '/a/c/b', { max_files: 5 });
           assert.quotaIsNotInherited('files', 5, 10);
         });
 
@@ -803,7 +803,7 @@ describe('CephfsDirectoriesComponent', () => {
         });
 
         it('should update max_files correctly', () => {
-          expect(cephfsService.updateQuota).toHaveBeenCalledWith(1, '/a/c/b', { max_bytes: 512 });
+          expect(cephfsService.quota).toHaveBeenCalledWith(1, '/a/c/b', { max_bytes: 512 });
           assert.quotaIsNotInherited('bytes', '512 B', 1024);
         });
 
@@ -848,7 +848,7 @@ describe('CephfsDirectoriesComponent', () => {
         });
 
         it('should unset max_files correctly', () => {
-          expect(cephfsService.updateQuota).toHaveBeenCalledWith(1, '/a/c/b', { max_files: 0 });
+          expect(cephfsService.quota).toHaveBeenCalledWith(1, '/a/c/b', { max_files: 0 });
           assert.dirQuotas(2048, 0);
         });
 
@@ -868,7 +868,7 @@ describe('CephfsDirectoriesComponent', () => {
         });
 
         it('should unset max_files correctly', () => {
-          expect(cephfsService.updateQuota).toHaveBeenCalledWith(1, '/a/c/b', { max_bytes: 0 });
+          expect(cephfsService.quota).toHaveBeenCalledWith(1, '/a/c/b', { max_bytes: 0 });
           assert.dirQuotas(0, 20);
         });
 
index 78f5d1a84f6942e11107b625f7ea75d02a4433cd..3480223dd954620bbf38788c8095f511cee57b57 100644 (file)
@@ -486,7 +486,7 @@ export class CephfsDirectoriesComponent implements OnInit, OnChanges {
         : values[key] === 0
         ? this.actionLabels.UNSET
         : $localize`Updated`;
-    this.cephfsService.updateQuota(this.id, path, values).subscribe(() => {
+    this.cephfsService.quota(this.id, path, values).subscribe(() => {
       if (onSuccess) {
         onSuccess();
       }
index 1a2ea9fef7d44aecd1bdbf940072767363be25a1..f685035ea53f82e35b71402cff022717b5efdc8d 100644 (file)
@@ -66,33 +66,33 @@ describe('CephfsService', () => {
 
   it('should call mkSnapshot', () => {
     service.mkSnapshot(3, '/some/path').subscribe();
-    const req = httpTesting.expectOne('api/cephfs/3/mk_snapshot?path=%252Fsome%252Fpath');
+    const req = httpTesting.expectOne('api/cephfs/3/snapshot?path=%252Fsome%252Fpath');
     expect(req.request.method).toBe('POST');
 
     service.mkSnapshot(4, '/some/other/path', 'snap').subscribe();
-    httpTesting.expectOne('api/cephfs/4/mk_snapshot?path=%252Fsome%252Fother%252Fpath&name=snap');
+    httpTesting.expectOne('api/cephfs/4/snapshot?path=%252Fsome%252Fother%252Fpath&name=snap');
   });
 
   it('should call rmSnapshot', () => {
     service.rmSnapshot(1, '/some/path', 'snap').subscribe();
-    const req = httpTesting.expectOne('api/cephfs/1/rm_snapshot?path=%252Fsome%252Fpath&name=snap');
-    expect(req.request.method).toBe('POST');
+    const req = httpTesting.expectOne('api/cephfs/1/snapshot?path=%252Fsome%252Fpath&name=snap');
+    expect(req.request.method).toBe('DELETE');
   });
 
   it('should call updateQuota', () => {
-    service.updateQuota(1, '/some/path', { max_bytes: 1024 }).subscribe();
-    let req = httpTesting.expectOne('api/cephfs/1/set_quotas?path=%252Fsome%252Fpath');
-    expect(req.request.method).toBe('POST');
+    service.quota(1, '/some/path', { max_bytes: 1024 }).subscribe();
+    let req = httpTesting.expectOne('api/cephfs/1/quota?path=%252Fsome%252Fpath');
+    expect(req.request.method).toBe('PUT');
     expect(req.request.body).toEqual({ max_bytes: 1024 });
 
-    service.updateQuota(1, '/some/path', { max_files: 10 }).subscribe();
-    req = httpTesting.expectOne('api/cephfs/1/set_quotas?path=%252Fsome%252Fpath');
-    expect(req.request.method).toBe('POST');
+    service.quota(1, '/some/path', { max_files: 10 }).subscribe();
+    req = httpTesting.expectOne('api/cephfs/1/quota?path=%252Fsome%252Fpath');
+    expect(req.request.method).toBe('PUT');
     expect(req.request.body).toEqual({ max_files: 10 });
 
-    service.updateQuota(1, '/some/path', { max_bytes: 1024, max_files: 10 }).subscribe();
-    req = httpTesting.expectOne('api/cephfs/1/set_quotas?path=%252Fsome%252Fpath');
-    expect(req.request.method).toBe('POST');
+    service.quota(1, '/some/path', { max_bytes: 1024, max_files: 10 }).subscribe();
+    req = httpTesting.expectOne('api/cephfs/1/quota?path=%252Fsome%252Fpath');
+    expect(req.request.method).toBe('PUT');
     expect(req.request.body).toEqual({ max_bytes: 1024, max_files: 10 });
   });
 });
index e5c53b74195acf9071166895b1f1003480557d48..02f31ca7b56b82ac45f899efe289aa7c9d28889e 100644 (file)
@@ -55,20 +55,20 @@ export class CephfsService {
     if (!_.isUndefined(name)) {
       params = params.append('name', name);
     }
-    return this.http.post(`${this.baseURL}/${id}/mk_snapshot`, null, { params });
+    return this.http.post(`${this.baseURL}/${id}/snapshot`, null, { params });
   }
 
   rmSnapshot(id: number, path: string, name: string) {
     let params = new HttpParams();
     params = params.append('path', path);
     params = params.append('name', name);
-    return this.http.post(`${this.baseURL}/${id}/rm_snapshot`, null, { params });
+    return this.http.delete(`${this.baseURL}/${id}/snapshot`, { params });
   }
 
-  updateQuota(id: number, path: string, quotas: CephfsQuotas) {
+  quota(id: number, path: string, quotas: CephfsQuotas) {
     let params = new HttpParams();
     params = params.append('path', path);
-    return this.http.post(`${this.baseURL}/${id}/set_quotas`, quotas, {
+    return this.http.put(`${this.baseURL}/${id}/quota`, quotas, {
       observe: 'response',
       params
     });