From 1dbebbd86c434137829fb9fce88cdc368f9d4993 Mon Sep 17 00:00:00 2001 From: Abhishek Desai Date: Mon, 11 Aug 2025 17:23:52 +0530 Subject: [PATCH] mgr/dashboard : 72522 - Remove service instances column to imporve API perf fixes : https://tracker.ceph.com/issues/72522 Signed-off-by: Abhishek Desai --- src/pybind/mgr/dashboard/controllers/host.py | 11 ++- .../frontend/cypress/e2e/cluster/hosts.po.ts | 17 +--- .../workflow/08-hosts.e2e-spec.ts | 4 - .../cluster/hosts/hosts.component.spec.ts | 40 ---------- .../app/ceph/cluster/hosts/hosts.component.ts | 6 -- .../src/app/shared/api/host.service.spec.ts | 4 +- .../src/app/shared/api/host.service.ts | 1 + src/pybind/mgr/dashboard/openapi.yaml | 6 ++ src/pybind/mgr/dashboard/tests/test_host.py | 77 +++++++++++++++++-- 9 files changed, 90 insertions(+), 76 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/host.py b/src/pybind/mgr/dashboard/controllers/host.py index ca15df5245c01..cb0d8411a9626 100644 --- a/src/pybind/mgr/dashboard/controllers/host.py +++ b/src/pybind/mgr/dashboard/controllers/host.py @@ -266,12 +266,14 @@ class Host(RESTController): @EndpointDoc("List Host Specifications", parameters={ 'sources': (str, 'Host Sources'), - 'facts': (bool, 'Host Facts') + 'facts': (bool, 'Host Facts'), + 'include_service_instances': (bool, 'Include Service Instances') }, responses={200: LIST_HOST_SCHEMA}) @RESTController.MethodMap(version=APIVersion(1, 3)) def list(self, sources=None, facts=False, offset: int = 0, - limit: int = 5, search: str = '', sort: str = ''): + limit: int = 5, search: str = '', sort: str = '', + include_service_instances=True): hosts = get_hosts(sources) params = ['hostname'] paginator = ListPaginator(int(offset), int(limit), sort, search, hosts, @@ -284,8 +286,9 @@ class Host(RESTController): for host in hosts: if 'services' not in host: host['services'] = [] - host['service_instances'] = populate_service_instances( - host['hostname'], host['services']) + if str_to_bool(include_service_instances): + host['service_instances'] = populate_service_instances( + host['hostname'], host['services']) if str_to_bool(facts): if orch.available(): if not orch.get_missing_features(['get_facts']): diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/cluster/hosts.po.ts b/src/pybind/mgr/dashboard/frontend/cypress/e2e/cluster/hosts.po.ts index 70957e4dada2f..42da0ef2ed755 100644 --- a/src/pybind/mgr/dashboard/frontend/cypress/e2e/cluster/hosts.po.ts +++ b/src/pybind/mgr/dashboard/frontend/cypress/e2e/cluster/hosts.po.ts @@ -10,9 +10,8 @@ export class HostsPageHelper extends PageHelper { columnIndex = { hostname: 2, - services: 3, - labels: 4, - status: 5 + labels: 3, + status: 4 }; check_for_host() { @@ -171,16 +170,4 @@ export class HostsPageHelper extends PageHelper { this.expectTableCount('total', 0); }); } - - checkServiceInstancesExist(hostname: string, instances: string[]) { - this.getTableCell(this.columnIndex.hostname, hostname, true) - .parent() - .find(`[cdstabledata]:nth-child(${this.columnIndex.services}) .badge`) - .should(($ele) => { - const serviceInstances = $ele.toArray().map((v) => v.innerText); - for (const instance of instances) { - expect(serviceInstances).to.include(instance); - } - }); - } } diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/orchestrator/workflow/08-hosts.e2e-spec.ts b/src/pybind/mgr/dashboard/frontend/cypress/e2e/orchestrator/workflow/08-hosts.e2e-spec.ts index 605dc31d6267e..de366ebfdb53e 100644 --- a/src/pybind/mgr/dashboard/frontend/cypress/e2e/orchestrator/workflow/08-hosts.e2e-spec.ts +++ b/src/pybind/mgr/dashboard/frontend/cypress/e2e/orchestrator/workflow/08-hosts.e2e-spec.ts @@ -41,8 +41,4 @@ describe('Host Page', () => { hosts.add(hostnames[3]); hosts.checkExist(hostnames[3], true, true); }); - - it('should show the exact count of daemons', () => { - hosts.checkServiceInstancesExist(hostnames[0], ['mgr: 1', 'prometheus: 1']); - }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts index c7e1c31fc3b8c..34bf44c683c92 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts @@ -122,46 +122,6 @@ describe('HostsComponent', () => { expect(spans[0].textContent.trim()).toBe(hostname); }); - it('should show the exact count of the repeating daemons', () => { - const hostname = 'ceph.dev'; - const payload = [ - { - service_instances: [ - { - type: 'mgr', - count: 2 - }, - { - type: 'osd', - count: 3 - }, - { - type: 'rgw', - count: 1 - } - ], - hostname: hostname, - labels: ['foo', 'bar'], - headers: headers - } - ]; - - OrchestratorHelper.mockStatus(false); - fixture.detectChanges(); - hostListSpy.and.callFake(() => of(payload)); - fixture.detectChanges(); - - component.getHosts(new CdTableFetchDataContext(() => undefined)); - fixture.detectChanges(); - - const spans = fixture.debugElement.nativeElement.querySelectorAll( - '[cdstabledata] span span.badge.badge-background-primary' - ); - expect(spans[0].textContent).toContain('mgr: 2'); - expect(spans[1].textContent).toContain('osd: 3'); - expect(spans[2].textContent).toContain('rgw: 1'); - }); - it('should test if host facts are transformed correctly if orch available', () => { const features = [OrchestratorFeature.HOST_FACTS]; const payload = [ diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts index d652465846afe..200ddba0b76a1 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts @@ -222,12 +222,6 @@ export class HostsComponent extends ListWithDetails implements OnDestroy, OnInit flexGrow: 1, cellTemplate: this.hostNameTpl }, - { - name: $localize`Service Instances`, - prop: 'service_instances', - flexGrow: 1.5, - cellTemplate: this.servicesTpl - }, { name: $localize`Labels`, prop: 'labels', diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.spec.ts index 49b48cd6cfcfc..ce13a1011ffe7 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.spec.ts @@ -31,7 +31,9 @@ describe('HostService', () => { let result: any[] = [{}, {}]; const hostContext = new CdTableFetchDataContext(() => undefined); service.list(hostContext.toParams(), 'true').subscribe((resp) => (result = resp)); - const req = httpTesting.expectOne('api/host?offset=0&limit=10&search=&sort=%2Bname&facts=true'); + const req = httpTesting.expectOne( + 'api/host?offset=0&limit=10&search=&sort=%2Bname&facts=true&include_service_instances=false' + ); expect(req.request.method).toBe('GET'); req.flush([{ foo: 1 }, { bar: 2 }]); tick(); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts index 0fcd63c940c48..51d8235f5329e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts @@ -30,6 +30,7 @@ export class HostService extends ApiClient { list(params: any, facts: string): Observable { params = params.set('facts', facts); + params = params.set('include_service_instances', false); return this.http .get(this.baseURL, { headers: { Accept: this.getVersionHeaderValue(1, 2) }, diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index 4a2715e4bf020..9e85080ccb9a4 100755 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -5465,6 +5465,12 @@ paths: name: sort schema: type: string + - default: true + description: Include Service Instances + in: query + name: include_service_instances + schema: + type: boolean responses: '200': content: diff --git a/src/pybind/mgr/dashboard/tests/test_host.py b/src/pybind/mgr/dashboard/tests/test_host.py index 99fd6ce3d84b3..ff7971cd3556f 100644 --- a/src/pybind/mgr/dashboard/tests/test_host.py +++ b/src/pybind/mgr/dashboard/tests/test_host.py @@ -107,8 +107,7 @@ class HostControllerTest(ControllerTestCase): }, 'cpu_count': 1, 'memory_total_kb': 1024, - 'services': [], - 'service_instances': [{'type': 'mon', 'count': 1}] + 'services': [] }, { 'hostname': 'host-1', 'sources': { @@ -117,8 +116,7 @@ class HostControllerTest(ControllerTestCase): }, 'cpu_count': 2, 'memory_total_kb': 1024, - 'services': [], - 'service_instances': [{'type': 'mon', 'count': 1}] + 'services': [] }] # test with orchestrator available with patch_orch(True, hosts=hosts_without_facts) as fake_client: @@ -129,8 +127,11 @@ class HostControllerTest(ControllerTestCase): return [hosts_facts[0]] return [hosts_facts[1]] fake_client.hosts.get_facts.side_effect = get_facts_mock - # test with ?facts=true - self._get('{}?facts=true'.format(self.URL_HOST), version=APIVersion(1, 3)) + # test with ?facts=true (explicitly disable service_instances) + self._get( + '{}?facts=true&include_service_instances=false'.format(self.URL_HOST), + version=APIVersion(1, 3) + ) self.assertStatus(200) self.assertHeader('Content-Type', APIVersion(1, 3).to_mime_type()) @@ -165,6 +166,70 @@ class HostControllerTest(ControllerTestCase): APIVersion(1, 3).to_mime_type()) self.assertJsonBody(hosts_without_facts) + def test_host_list_include_service_instances_flag(self): + + orch_hosts = [ + HostSpec('host-0'), + HostSpec('host-1') + ] + + host0_daemons = [ + DaemonDescription(hostname='host-0', daemon_type='mon', daemon_id='a'), + ] + host1_daemons = [ + DaemonDescription(hostname='host-1', daemon_type='mon', daemon_id='a'), + DaemonDescription(hostname='host-1', daemon_type='mon', daemon_id='b'), + ] + + with patch_orch(True, hosts=orch_hosts) as fake_client: + def list_daemons_mock(hostname=None, **_kwargs): + if hostname == 'host-0': + return host0_daemons + if hostname == 'host-1': + return host1_daemons + return [] + + fake_client.services.list_daemons.side_effect = list_daemons_mock + # include_service_instances=true should include expected counts + self._get(f'{self.URL_HOST}?include_service_instances=true', version=APIVersion(1, 3)) + self.assertStatus(200) + body = self.json_body() + self.assertEqual(len(body), 2) + by_host = {h['hostname']: h for h in body} + self.assertIn('service_instances', by_host['host-0']) + self.assertIn('service_instances', by_host['host-1']) + self.assertEqual(by_host['host-0']['service_instances'], [{'type': 'mon', 'count': 1}]) + self.assertEqual(by_host['host-1']['service_instances'], [{'type': 'mon', 'count': 2}]) + + # include_service_instances=false should omit service_instances + self._get(f'{self.URL_HOST}?include_service_instances=false', version=APIVersion(1, 3)) + self.assertStatus(200) + body = self.json_body() + by_host = {h['hostname']: h for h in body} + self.assertNotIn('service_instances', by_host['host-0']) + self.assertNotIn('service_instances', by_host['host-1']) + + # facts=true and include_service_instances=true + def get_facts_mock(hostname: str): + return [{ + 'hostname': hostname, + 'cpu_count': 1 if hostname == 'host-0' else 2, + 'memory_total_kb': 1024 + }] + + fake_client.hosts.get_facts.side_effect = get_facts_mock + self._get( + f'{self.URL_HOST}?facts=true&include_service_instances=true', + version=APIVersion(1, 3) + ) + self.assertStatus(200) + body = self.json_body() + by_host = {h['hostname']: h for h in body} + self.assertIn('service_instances', by_host['host-0']) + self.assertIn('service_instances', by_host['host-1']) + self.assertIn('cpu_count', by_host['host-0']) + self.assertIn('cpu_count', by_host['host-1']) + def test_get_1(self): mgr.list_servers.return_value = [] -- 2.39.5