From 111ca24675f62d9b8f40466811b22a4c46c1cecc Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Fri, 20 Sep 2024 20:35:38 +0530 Subject: [PATCH] mgr/dashboard: introduce server side pagination for osds Fixes: https://tracker.ceph.com/issues/56511 Signed-off-by: Nizamudeen A (cherry picked from commit 86378344ab0a381569b116c2112a981404f93671) Conflicts: src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts - remove the carbon cds modal import --- qa/tasks/mgr/dashboard/test_osd.py | 5 +- src/pybind/mgr/dashboard/controllers/osd.py | 28 ++++++++- .../osd/osd-list/osd-list.component.html | 6 +- .../osd/osd-list/osd-list.component.spec.ts | 59 +++++++++++-------- .../osd/osd-list/osd-list.component.ts | 14 +++-- .../src/app/shared/api/osd.service.spec.ts | 6 +- .../src/app/shared/api/osd.service.ts | 11 +++- .../src/app/shared/api/paginate.model.ts | 2 +- .../shared/classes/paginate-params.class.ts | 15 +++++ .../models/cd-table-fetch-data-context.ts | 2 +- .../src/app/shared/models/osd.model.ts | 49 +++++++++++++++ src/pybind/mgr/dashboard/openapi.yaml | 24 +++++++- src/pybind/mgr/dashboard/tests/test_osd.py | 3 +- 13 files changed, 178 insertions(+), 46 deletions(-) create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/classes/paginate-params.class.ts create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/models/osd.model.ts diff --git a/qa/tasks/mgr/dashboard/test_osd.py b/qa/tasks/mgr/dashboard/test_osd.py index 71cf3d87194ed..1b80ca966ba53 100644 --- a/qa/tasks/mgr/dashboard/test_osd.py +++ b/qa/tasks/mgr/dashboard/test_osd.py @@ -11,6 +11,7 @@ from .helper import (DashboardTestCase, JAny, JLeaf, JList, JObj, JTuple, class OsdTest(DashboardTestCase): AUTH_ROLES = ['cluster-manager'] + _VERSION = '1.1' @classmethod def setUpClass(cls): @@ -24,7 +25,7 @@ class OsdTest(DashboardTestCase): @DashboardTestCase.RunAs('test', 'test', ['block-manager']) def test_access_permissions(self): - self._get('/api/osd') + self._get('/api/osd', version=self._VERSION) self.assertStatus(403) self._get('/api/osd/0') self.assertStatus(403) @@ -33,7 +34,7 @@ class OsdTest(DashboardTestCase): self.assertSchema(data, JObj({p: JAny(none=False) for p in properties}, allow_unknown=True)) def test_list(self): - data = self._get('/api/osd') + data = self._get('/api/osd', version=self._VERSION) self.assertStatus(200) self.assertGreaterEqual(len(data), 1) diff --git a/src/pybind/mgr/dashboard/controllers/osd.py b/src/pybind/mgr/dashboard/controllers/osd.py index c9d1417720005..07d8db7755b8a 100644 --- a/src/pybind/mgr/dashboard/controllers/osd.py +++ b/src/pybind/mgr/dashboard/controllers/osd.py @@ -5,12 +5,14 @@ import logging import time from typing import Any, Dict, List, Optional, Union +import cherrypy from ceph.deployment.drive_group import DriveGroupSpec, DriveGroupValidationError # type: ignore from mgr_util import get_most_recent_rate from .. import mgr from ..exceptions import DashboardException from ..security import Scope +from ..services._paginate import ListPaginator from ..services.ceph_service import CephService, SendCommandError from ..services.exception import handle_orchestrator_error, handle_send_command_error from ..services.orchestrator import OrchClient, OrchFeature @@ -121,8 +123,30 @@ def osd_task(name, metadata, wait_for=2.0): @APIRouter('/osd', Scope.OSD) @APIDoc('OSD management API', 'OSD') class Osd(RESTController): - def list(self): - osds = self.get_osd_map() + @RESTController.MethodMap(version=APIVersion(1, 1)) + def list(self, offset: int = 0, limit: int = 10, + search: str = '', sort: str = ''): + all_osds = self.get_osd_map() + + paginator = ListPaginator(int(offset), int(limit), sort, search, + input_list=all_osds.values(), + searchable_params=['id'], + sortable_params=['id'], + default_sort='+id') + + cherrypy.response.headers['X-Total-Count'] = paginator.get_count() + + paginated_osds_list = list(paginator.list()) + # creating a dictionary to have faster lookups + paginated_osds_by_id = {osd['id']: osd for osd in paginated_osds_list} + try: + osds = { + key: paginated_osds_by_id[int(key)] + for key in all_osds.keys() + if int(key) in paginated_osds_by_id + } + except ValueError as e: + raise DashboardException(e, component='osd', http_status_code=400) # Extending by osd stats information for stat in mgr.get('osd_stats')['osd_stats']: diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html index ede9dbb19f271..b32e78c56a8af 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html @@ -6,13 +6,15 @@ i18n>OSDs List + [updateSelectionOnRefresh]="'never'" + [serverSide]="true" + [count]="count">
{ let component: OsdListComponent; @@ -142,38 +144,42 @@ describe('OsdListComponent', () => { }); describe('getOsdList', () => { - let osds: any[]; + let osds: Osd[]; let flagsSpy: jasmine.Spy; - const createOsd = (n: number) => - >{ - in: 'in', - up: 'up', - tree: { - device_class: 'ssd' - }, - stats_history: { - op_out_bytes: [ - [n, n], - [n * 2, n * 2] - ], - op_in_bytes: [ - [n * 3, n * 3], - [n * 4, n * 4] - ] - }, - stats: { - stat_bytes_used: n * n, - stat_bytes: n * n * n - }, - state: [] - }; + const createOsd = (n: number): Osd => ({ + id: n, + host: { + id: 0, + name: 'test_host' + }, + in: 1, + up: 1, + tree: { + device_class: 'ssd' + }, + stats_history: { + op_out_bytes: [ + [n, n], + [n * 2, n * 2] + ], + op_in_bytes: [ + [n * 3, n * 3], + [n * 4, n * 4] + ] + }, + stats: { + stat_bytes_used: n * n, + stat_bytes: n * n * n + }, + state: [] + }); const expectAttributeOnEveryOsd = (attr: string) => expect(component.osds.every((osd) => Boolean(_.get(osd, attr)))).toBeTruthy(); beforeEach(() => { - spyOn(osdService, 'getList').and.callFake(() => of(osds)); + spyOn(osdService, 'getList').and.callFake(() => new PaginateObservable(of(osds))); flagsSpy = spyOn(osdService, 'getFlags').and.callFake(() => of([])); osds = [createOsd(1), createOsd(2), createOsd(3)]; component.getOsdList(); @@ -531,8 +537,9 @@ describe('OsdListComponent', () => { beforeEach(() => { component.permissions = fakeAuthStorageService.getPermissions(); - spyOn(osdService, 'getList').and.callFake(() => of(fakeOsds)); + spyOn(osdService, 'getList').and.callFake(() => new PaginateObservable(of(fakeOsds))); spyOn(osdService, 'getFlags').and.callFake(() => of([])); + component.getOsdList(); }); const testTableActions = async ( diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts index 0c580fcb8a4f8..434c30588ad39 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts @@ -38,6 +38,8 @@ import { OsdPgScrubModalComponent } from '../osd-pg-scrub-modal/osd-pg-scrub-mod import { OsdRecvSpeedModalComponent } from '../osd-recv-speed-modal/osd-recv-speed-modal.component'; import { OsdReweightModalComponent } from '../osd-reweight-modal/osd-reweight-modal.component'; import { OsdScrubModalComponent } from '../osd-scrub-modal/osd-scrub-modal.component'; +import { CdTableFetchDataContext } from '~/app/shared/models/cd-table-fetch-data-context'; +import { Osd } from '~/app/shared/models/osd.model'; const BASE_URL = 'osd'; @@ -70,6 +72,7 @@ export class OsdListComponent extends ListWithDetails implements OnInit { clusterWideActions: CdTableAction[]; icons = Icons; osdSettings = new OsdSettings(); + count = 0; selection = new CdTableSelection(); osds: any[] = []; @@ -424,10 +427,13 @@ export class OsdListComponent extends ListWithDetails implements OnInit { } } - getOsdList() { - const observables = [this.osdService.getList(), this.osdService.getFlags()]; - observableForkJoin(observables).subscribe((resp: [any[], string[]]) => { - this.osds = resp[0].map((osd) => { + getOsdList(context?: CdTableFetchDataContext) { + if (!context) context = new CdTableFetchDataContext(); + const pagination_obs = this.osdService.getList(context.toParams()); + const observables = [pagination_obs.observable, this.osdService.getFlags()]; + observableForkJoin(observables).subscribe((resp: any) => { + this.osds = resp[0].map((osd: Osd) => { + this.count = pagination_obs.count; osd.collectedStates = OsdListComponent.collectStates(osd); osd.stats_history.out_bytes = osd.stats_history.op_out_bytes.map((i: string) => i[1]); osd.stats_history.in_bytes = osd.stats_history.op_in_bytes.map((i: string) => i[1]); 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 d1f9997791ae0..c81c9193a2e3c 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 @@ -3,6 +3,7 @@ import { TestBed } from '@angular/core/testing'; import { configureTestBed } from '~/testing/unit-test-helper'; import { OsdService } from './osd.service'; +import { CdTableFetchDataContext } from '../models/cd-table-fetch-data-context'; describe('OsdService', () => { let service: OsdService; @@ -64,8 +65,9 @@ describe('OsdService', () => { }); it('should call getList', () => { - service.getList().subscribe(); - const req = httpTesting.expectOne('api/osd'); + const context = new CdTableFetchDataContext(() => {}); + service.getList(context.toParams()).observable.subscribe(); + const req = httpTesting.expectOne('api/osd?offset=0&limit=10&search=&sort=%2Bname'); 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 f2ed4d7cc9e76..85a75073deafc 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 @@ -1,4 +1,4 @@ -import { HttpClient } from '@angular/common/http'; +import { HttpClient, HttpParams } from '@angular/common/http'; import { Injectable } from '@angular/core'; import _ from 'lodash'; @@ -12,6 +12,9 @@ import { OsdSettings } from '../models/osd-settings'; import { SmartDataResponseV1 } from '../models/smart'; import { DeviceService } from '../services/device.service'; import { CdFormGroup } from '../forms/cd-form-group'; +import { PaginateObservable } from './paginate.model'; +import { PaginateParams } from '../classes/paginate-params.class'; +import { Osd } from '../models/osd.model'; @Injectable({ providedIn: 'root' @@ -80,8 +83,10 @@ export class OsdService { return this.http.post(this.path, request, { observe: 'response' }); } - getList() { - return this.http.get(`${this.path}`); + getList(params: HttpParams): PaginateObservable { + return new PaginateObservable( + this.http.get(this.path, new PaginateParams(params, 1, 1)) + ); } getOsdSettings(): Observable { 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 703792a757181..77ec4e43f7cfe 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 @@ -9,7 +9,7 @@ export class PaginateObservable { this.observable = obs.pipe( map((response: any) => { this.count = Number(response.headers?.get('X-Total-Count')); - return response['body']; + return response['body'] || response; }) ); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/classes/paginate-params.class.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/classes/paginate-params.class.ts new file mode 100644 index 0000000000000..a1b079b426b9d --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/classes/paginate-params.class.ts @@ -0,0 +1,15 @@ +import { HttpParams } from '@angular/common/http'; + +export class PaginateParams { + constructor(params: HttpParams, majorVersion = 1, minorVersion = 0) { + const options = { + params: params, + headers: { + Accept: `application/vnd.ceph.api.v${majorVersion}.${minorVersion}+json` + } + }; + + options['observe'] = 'response'; + return options; + } +} 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 0df2d2ebbe071..6ea415bfee983 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 @@ -18,7 +18,7 @@ export class CdTableFetchDataContext { search = ''; sort = '+name'; - constructor(error: () => void) { + constructor(error?: () => void) { this.error = error; } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/osd.model.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/osd.model.ts new file mode 100644 index 0000000000000..f22987e439ea5 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/osd.model.ts @@ -0,0 +1,49 @@ +/* We will need to check what are all the value that the + UI need and only make them the mandatory parameters here. + For now based on what I saw in the unit test file; + osd-list.component.spec.ts, I've made the decision to make + things optional and non-optional. This should be re-evaluated. */ + +export interface Osd { + id: number; + host: Host; + stats_history: StatsHistory; + state: string[]; + stats: Stats; + collectedStates?: string[]; + in?: number; + out?: number; + up?: number; + down?: number; + destroyed?: number; + cdIsBinary?: boolean; + cdIndivFlags?: string[]; + cdClusterFlags?: string[]; + cdExecuting?: any; + tree?: Tree; + operational_status?: string; +} + +interface Tree { + device_class: string; +} + +interface Host { + id: number; + name: string; +} + +interface StatsHistory { + op_out_bytes: any[]; + op_in_bytes: any[]; + out_bytes?: any[]; + in_bytes?: any[]; +} + +interface Stats { + stat_bytes_used: number; + stat_bytes: number; + op_w?: number; + op_r?: number; + usage?: number; +} diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index 38a9cf426630c..5b2c68bae3d87 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -8483,11 +8483,31 @@ paths: - NVMe-oF Subsystem Namespace /api/osd: get: - parameters: [] + parameters: + - default: 0 + in: query + name: offset + schema: + type: integer + - default: 10 + in: query + name: limit + schema: + type: integer + - default: '' + in: query + name: search + schema: + type: string + - default: '' + in: query + name: sort + schema: + type: string responses: '200': content: - application/vnd.ceph.api.v1.0+json: + application/vnd.ceph.api.v1.1+json: type: object description: OK '400': diff --git a/src/pybind/mgr/dashboard/tests/test_osd.py b/src/pybind/mgr/dashboard/tests/test_osd.py index c3cd0dca88dca..9b6dbd10de18d 100644 --- a/src/pybind/mgr/dashboard/tests/test_osd.py +++ b/src/pybind/mgr/dashboard/tests/test_osd.py @@ -8,6 +8,7 @@ from ceph.deployment.drive_group import DeviceSelection, DriveGroupSpec # type: from ceph.deployment.service_spec import PlacementSpec from .. import mgr +from ..controllers._version import APIVersion from ..controllers.osd import Osd, OsdUi from ..services.osd import OsdDeploymentOptions from ..tests import ControllerTestCase @@ -274,7 +275,7 @@ class OsdTest(ControllerTestCase): osds_leftover = [0, 1, 2] with self._mock_osd_list(osd_stat_ids=osds_actual, osdmap_tree_node_ids=osds_leftover, osdmap_ids=osds_actual): - self._get('/api/osd') + self._get('/api/osd', version=APIVersion(1, 1)) self.assertEqual(len(self.json_body()), 2, 'It should display two OSDs without failure') self.assertStatus(200) -- 2.39.5