From: Fabrizio D'Angelo Date: Fri, 26 Jun 2020 13:50:15 +0000 (-0400) Subject: mgr/dashboard: Address API inconsistencies X-Git-Tag: wip-pdonnell-testing-20200918.022351~178^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=871df3671c0df4943e047800ff56ec076b931687;p=ceph-ci.git mgr/dashboard: Address API inconsistencies 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 (cherry picked from commit 222286b08bde3600d50a6deb25e7425c17e9e482) --- diff --git a/qa/tasks/mgr/dashboard/test_cephfs.py b/qa/tasks/mgr/dashboard/test_cephfs.py index 1be800b92c5..2dbd9add72f 100644 --- a/qa/tasks/mgr/dashboard/test_cephfs.py +++ b/qa/tasks/mgr/dashboard/test_cephfs.py @@ -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 diff --git a/src/pybind/mgr/dashboard/controllers/cephfs.py b/src/pybind/mgr/dashboard/controllers/cephfs.py index e6eb9c83efe..65969f8d8fd 100644 --- a/src/pybind/mgr/dashboard/controllers/cephfs.py +++ b/src/pybind/mgr/dashboard/controllers/cephfs.py @@ -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): diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.spec.ts index 124fad18c2b..2e102779795 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.spec.ts @@ -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); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.ts index 78f5d1a84f6..3480223dd95 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-directories/cephfs-directories.component.ts @@ -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(); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.spec.ts index 1a2ea9fef7d..f685035ea53 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.spec.ts @@ -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 }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.ts index e5c53b74195..02f31ca7b56 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/cephfs.service.ts @@ -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 });