From e184aac4ee26465b8db086f864c5382bbb2abf76 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Mon, 12 Sep 2022 10:21:41 +0200 Subject: [PATCH] mgr/dashboard: use ListPaginator in rbd pagination Signed-off-by: Pere Diaz Bou --- .../mgr/dashboard/controllers/_paginate.py | 0 src/pybind/mgr/dashboard/controllers/rbd.py | 3 +- .../service-daemon-list.component.spec.ts | 5 +- .../services/services.component.spec.ts | 15 +++-- .../src/app/shared/api/paginate.model.ts | 1 - .../models/cd-table-fetch-data-context.ts | 2 +- src/pybind/mgr/dashboard/openapi.yaml | 22 ++++++- .../mgr/dashboard/services/_paginate.py | 24 +++++--- .../mgr/dashboard/services/orchestrator.py | 6 +- src/pybind/mgr/dashboard/services/rbd.py | 57 +++++++------------ 10 files changed, 75 insertions(+), 60 deletions(-) create mode 100644 src/pybind/mgr/dashboard/controllers/_paginate.py diff --git a/src/pybind/mgr/dashboard/controllers/_paginate.py b/src/pybind/mgr/dashboard/controllers/_paginate.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pybind/mgr/dashboard/controllers/rbd.py b/src/pybind/mgr/dashboard/controllers/rbd.py index d83cd3973bf..4c0ac0ccc07 100644 --- a/src/pybind/mgr/dashboard/controllers/rbd.py +++ b/src/pybind/mgr/dashboard/controllers/rbd.py @@ -113,7 +113,8 @@ class Rbd(RESTController): @RESTController.MethodMap(version=APIVersion(2, 0)) # type: ignore def list(self, pool_name=None, offset: int = 0, limit: int = DEFAULT_LIMIT, search: str = '', sort: str = ''): - return self._rbd_list(pool_name, offset=offset, limit=limit, search=search, sort=sort) + return self._rbd_list(pool_name, offset=int(offset), limit=int(limit), + search=search, sort=sort) @handle_rbd_error() @handle_rados_error('pool') diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.spec.ts index 7ece91e76e5..d3ea8c018f6 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.spec.ts @@ -1,3 +1,4 @@ +import { HttpHeaders } from '@angular/common/http'; import { HttpClientTestingModule } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; @@ -19,6 +20,7 @@ import { ServiceDaemonListComponent } from './service-daemon-list.component'; describe('ServiceDaemonListComponent', () => { let component: ServiceDaemonListComponent; let fixture: ComponentFixture; + let headers: HttpHeaders; const daemons = [ { @@ -151,7 +153,8 @@ describe('ServiceDaemonListComponent', () => { of(getDaemonsByServiceName(component.serviceName)) ); - const paginate_obs = new PaginateObservable(of(services)); + headers = new HttpHeaders().set('X-Total-Count', '2'); + const paginate_obs = new PaginateObservable(of({ body: services, headers: headers })); spyOn(cephServiceService, 'list').and.returnValue(paginate_obs); context.pageInfo.offset = 0; context.pageInfo.limit = -1; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/services.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/services.component.spec.ts index a43b36e2be4..094a3cef138 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/services.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/services.component.spec.ts @@ -1,3 +1,4 @@ +import { HttpHeaders } from '@angular/common/http'; import { HttpClientTestingModule } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; @@ -21,6 +22,7 @@ import { ServicesComponent } from './services.component'; describe('ServicesComponent', () => { let component: ServicesComponent; let fixture: ComponentFixture; + let headers: HttpHeaders; const fakeAuthStorageService = { getPermissions: () => { @@ -67,10 +69,11 @@ describe('ServicesComponent', () => { component = fixture.componentInstance; const orchService = TestBed.inject(OrchestratorService); const cephServiceService = TestBed.inject(CephServiceService); - const paginate_obs = new PaginateObservable(of({ available: true })); - spyOn(orchService, 'status').and.returnValue(paginate_obs); - spyOn(cephServiceService, 'list').and.returnValue(of(services)); - fixture.detectChanges(); + spyOn(orchService, 'status').and.returnValue(of({ available: true })); + headers = new HttpHeaders().set('X-Total-Count', '2'); + const paginate_obs = new PaginateObservable(of({ body: services, headers: headers })); + + spyOn(cephServiceService, 'list').and.returnValue(paginate_obs); }); it('should create', () => { @@ -89,9 +92,9 @@ describe('ServicesComponent', () => { }); it('should return all services', () => { - const context = new CdTableFetchDataContext(() => undefined) + const context = new CdTableFetchDataContext(() => undefined); context.pageInfo.offset = 0; - context.pageInfo.limit = -1 + context.pageInfo.limit = -1; component.getServices(context); expect(component.services.length).toBe(2); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/paginate.model.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/paginate.model.ts index 7c1c391fa30..703792a7571 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/paginate.model.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/paginate.model.ts @@ -5,7 +5,6 @@ export class PaginateObservable { observable: Observable; count: number; - subscribe: any; constructor(obs: Observable) { this.observable = obs.pipe( map((response: any) => { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-fetch-data-context.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-fetch-data-context.ts index 834a89967a9..0df2d2ebbe0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-fetch-data-context.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-fetch-data-context.ts @@ -23,7 +23,7 @@ export class CdTableFetchDataContext { } toParams(): HttpParams { - if (this.pageInfo.offset == NaN) { + if (Number.isNaN(this.pageInfo.offset)) { this.pageInfo.offset = 0; } diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index 343fae4dfcb..7a14aab233b 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -8776,10 +8776,30 @@ paths: name: service_name schema: type: string + - default: 0 + in: query + name: offset + schema: + type: integer + - default: 5 + in: query + name: limit + schema: + type: integer + - default: '' + in: query + name: search + schema: + type: string + - default: +service_name + in: query + name: sort + schema: + type: string responses: '200': content: - application/vnd.ceph.api.v1.0+json: + application/vnd.ceph.api.v2.0+json: type: object description: OK '400': diff --git a/src/pybind/mgr/dashboard/services/_paginate.py b/src/pybind/mgr/dashboard/services/_paginate.py index 3acfee09cc5..c8ba300a537 100644 --- a/src/pybind/mgr/dashboard/services/_paginate.py +++ b/src/pybind/mgr/dashboard/services/_paginate.py @@ -4,6 +4,7 @@ from ..exceptions import DashboardException class ListPaginator: + # pylint: disable=W0102 def __init__(self, offset: int, limit: int, sort: str, search: str, input_list: List[Any], default_sort: str, searchable_params: List[str] = [], sortable_params: List[str] = []): @@ -17,16 +18,18 @@ class ListPaginator: self.default_sort = default_sort self.searchable_params = searchable_params self.sortable_params = sortable_params + self.count = len(self.input_list) def get_count(self): - return len(self.input_list) + return self.count def find_value(self, item: Dict[str, Any], key: str): + # dot separated keys to lookup nested values keys = key.split('.') value = item - for key in keys: - if key in value: - value = value[key] + for nested_key in keys: + if nested_key in value: + value = value[nested_key] else: return '' return value @@ -37,9 +40,15 @@ class ListPaginator: if self.limit == -1: end = len(self.input_list) + if not self.sort: + self.sort = self.default_sort + desc = self.sort[0] == '-' sort_by = self.sort[1:] + if sort_by not in self.sortable_params: + sort_by = self.default_sort[1:] + # trim down by search trimmed_list = [] if self.search: @@ -53,11 +62,10 @@ class ListPaginator: else: trimmed_list = self.input_list - if sort_by not in self.sortable_params: - sort_by = self.default_sort - def sort(item): return self.find_value(item, sort_by) - for item in sorted(trimmed_list, key=sort, reverse=desc)[self.offset:end]: + sorted_list = sorted(trimmed_list, key=sort, reverse=desc) + self.count = len(sorted_list) + for item in sorted_list[self.offset:end]: yield item diff --git a/src/pybind/mgr/dashboard/services/orchestrator.py b/src/pybind/mgr/dashboard/services/orchestrator.py index abcbf93bee1..1818164d6c9 100644 --- a/src/pybind/mgr/dashboard/services/orchestrator.py +++ b/src/pybind/mgr/dashboard/services/orchestrator.py @@ -2,7 +2,7 @@ import logging from functools import wraps -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from ceph.deployment.service_spec import ServiceSpec from orchestrator import DaemonDescription, DeviceLightLoc, HostSpec, \ @@ -100,7 +100,7 @@ class ServiceManager(ResourceManager): service_type: Optional[str] = None, service_name: Optional[str] = None, offset: int = 0, limit: int = -1, - sort: str = '+service_name', search: str = '') -> List[ServiceDescription]: + sort: str = '+service_name', search: str = '') -> Tuple[List[Dict[Any, Any]], int]: services = self.api.describe_service(service_type, service_name) services = [service.to_dict() for service in services.result] paginator = ListPaginator(offset, limit, sort, search, @@ -109,7 +109,7 @@ class ServiceManager(ResourceManager): 'status.last_refreshed', 'status.size'], sortable_params=['service_name', 'status.running', 'status.last_refreshed', 'status.size'], - default_sort='service_name') + default_sort='+service_name') return list(paginator.list()), paginator.get_count() @wait_api_result diff --git a/src/pybind/mgr/dashboard/services/rbd.py b/src/pybind/mgr/dashboard/services/rbd.py index d2a0312029b..69baf71c9f2 100644 --- a/src/pybind/mgr/dashboard/services/rbd.py +++ b/src/pybind/mgr/dashboard/services/rbd.py @@ -11,6 +11,7 @@ import rbd from .. import mgr from ..exceptions import DashboardException from ..plugins.ttl_cache import ttl_cache +from ._paginate import ListPaginator from .ceph_service import CephService try: @@ -452,50 +453,30 @@ class RbdService(object): @classmethod def rbd_pool_list(cls, pool_names: List[str], namespace: Optional[str] = None, offset: int = 0, limit: int = 5, search: str = '', sort: str = ''): - offset = int(offset) - limit = int(limit) - # let's use -1 to denotate we want ALL images for now. Iscsi currently gathers - # all images therefore, we need this. - if limit < -1: - raise DashboardException(msg=f'Wrong limit value {limit}', code=400) - - refs = cls._rbd_pool_image_refs(pool_names, namespace) - image_refs = [] - # transform to list so that we can count - for ref in refs: - if search in ref['name']: - image_refs.append(ref) - elif search in ref['pool_name']: - image_refs.append(ref) - elif search in ref['namespace']: - image_refs.append(ref) + image_refs = cls._rbd_pool_image_refs(pool_names, namespace) + params = ['name', 'pool_name', 'namespace'] + paginator = ListPaginator(offset, limit, sort, search, image_refs, + searchable_params=params, sortable_params=params, + default_sort='+name') result = [] - end = offset + limit - if len(sort) < 2: - sort = '+name' - descending = sort[0] == '-' - sort_by = sort[1:] - if sort_by not in ['name', 'pool_name', 'namespace']: - sort_by = 'name' - if limit == -1: - end = len(image_refs) - for image_ref in sorted(image_refs, key=lambda v: v[sort_by], - reverse=descending)[offset:end]: - ioctx = cls.get_ioctx(image_ref['pool_name'], namespace=image_ref['namespace']) - try: - stat = cls._rbd_image_stat( - ioctx, image_ref['pool_name'], image_ref['namespace'], image_ref['name']) - except rbd.ImageNotFound: + for image_ref in paginator.list(): + with mgr.rados.open_ioctx(image_ref['pool_name']) as ioctx: + ioctx.set_namespace(image_ref['namespace']) # Check if the RBD has been deleted partially. This happens for example if # the deletion process of the RBD has been started and was interrupted. + try: - stat = cls._rbd_image_stat_removing( - ioctx, image_ref['pool_name'], image_ref['namespace'], image_ref['id']) + stat = cls._rbd_image_stat( + ioctx, image_ref['pool_name'], image_ref['namespace'], image_ref['name']) except rbd.ImageNotFound: - continue - result.append(stat) - return result, len(image_refs) + try: + stat = cls._rbd_image_stat_removing( + ioctx, image_ref['pool_name'], image_ref['namespace'], image_ref['id']) + except rbd.ImageNotFound: + continue + result.append(stat) + return result, paginator.get_count() @classmethod def get_image(cls, image_spec): -- 2.39.5