From 3201a0de2936f2883b09f3c27f7acc44a62bb64d Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Fri, 20 Jan 2023 15:58:12 +0530 Subject: [PATCH] mgr/dashboard: move service_instances logic to backend Fixes: https://tracker.ceph.com/issues/58504 Signed-off-by: Nizamudeen A (cherry picked from commit bf724b505ed9733329d8dea645859a0ebee22f1b) Conflicts: src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.html - Accept the incoming changes --- src/pybind/mgr/dashboard/controllers/host.py | 23 +++- .../ceph/cluster/hosts/hosts.component.html | 8 +- .../cluster/hosts/hosts.component.spec.ts | 20 +--- .../app/ceph/cluster/hosts/hosts.component.ts | 37 +------ .../src/app/shared/api/host.service.ts | 2 +- src/pybind/mgr/dashboard/openapi.yaml | 18 +++- src/pybind/mgr/dashboard/tests/__init__.py | 14 ++- src/pybind/mgr/dashboard/tests/test_host.py | 100 ++++++++++++++++-- 8 files changed, 157 insertions(+), 65 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/host.py b/src/pybind/mgr/dashboard/controllers/host.py index 6703a5462950..9faaa519202c 100644 --- a/src/pybind/mgr/dashboard/controllers/host.py +++ b/src/pybind/mgr/dashboard/controllers/host.py @@ -3,6 +3,7 @@ import copy import os import time +from collections import Counter from typing import Dict, List, Optional import cherrypy @@ -28,6 +29,10 @@ LIST_HOST_SCHEMA = { "type": (str, "type of service"), "id": (str, "Service Id"), }], "Services related to the host"), + "service_instances": ([{ + "type": (str, "type of service"), + "count": (int, "Number of instances of the service"), + }], "Service instances related to the host"), "ceph_version": (str, "Ceph version"), "addr": (str, "Host address"), "labels": ([str], "Labels related to the host"), @@ -151,9 +156,23 @@ def merge_hosts_by_hostname(ceph_hosts, orch_hosts): }, orch_hosts_map[hostname]) for hostname in orch_hosts_map ] hosts.extend(orch_hosts_only) + for host in hosts: + host['service_instances'] = populate_service_instances( + host['hostname'], host['services']) return hosts +def populate_service_instances(hostname, services): + orch = OrchClient.instance() + if orch.available(): + services = (daemon['daemon_type'] + for daemon in (d.to_dict() + for d in orch.services.list_daemons(hostname=hostname))) + else: + services = (daemon['type'] for daemon in services) + return [{'type': k, 'count': v} for k, v in Counter(services).items()] + + def get_hosts(sources=None): """ Get hosts from various sources. @@ -183,6 +202,8 @@ def get_hosts(sources=None): orch = OrchClient.instance() if orch.available(): return merge_hosts_by_hostname(ceph_hosts, orch.hosts.list()) + for host in ceph_hosts: + host['service_instances'] = populate_service_instances(host['hostname'], host['services']) return ceph_hosts @@ -282,7 +303,7 @@ class Host(RESTController): 'facts': (bool, 'Host Facts') }, responses={200: LIST_HOST_SCHEMA}) - @RESTController.MethodMap(version=APIVersion(1, 1)) + @RESTController.MethodMap(version=APIVersion(1, 2)) def list(self, sources=None, facts=False): hosts = get_hosts(sources) orch = OrchClient.instance() diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.html index 08249eb9fec5..69a942e56d28 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.html @@ -52,9 +52,11 @@
- - {{ instance }} + let-services="value"> + + 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 0d691f62acfd..03a61e0834d0 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 @@ -121,30 +121,18 @@ describe('HostsComponent', () => { const hostname = 'ceph.dev'; const payload = [ { - services: [ - { - type: 'mgr', - id: 'x' - }, + service_instances: [ { type: 'mgr', - id: 'y' + count: 2 }, { type: 'osd', - id: '0' - }, - { - type: 'osd', - id: '1' - }, - { - type: 'osd', - id: '2' + count: 3 }, { type: 'rgw', - id: 'rgw' + count: 1 } ], hostname: hostname, 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 9d5e67a897dd..6089965fc748 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 @@ -3,8 +3,8 @@ import { Router } from '@angular/router'; import { NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; import _ from 'lodash'; -import { Observable, Subscription } from 'rxjs'; -import { map, mergeMap } from 'rxjs/operators'; +import { Subscription } from 'rxjs'; +import { mergeMap } from 'rxjs/operators'; import { HostService } from '~/app/shared/api/host.service'; import { OrchestratorService } from '~/app/shared/api/orchestrator.service'; @@ -22,7 +22,6 @@ import { CdTableAction } from '~/app/shared/models/cd-table-action'; import { CdTableColumn } from '~/app/shared/models/cd-table-column'; import { CdTableFetchDataContext } from '~/app/shared/models/cd-table-fetch-data-context'; import { CdTableSelection } from '~/app/shared/models/cd-table-selection'; -import { Daemon } from '~/app/shared/models/daemon.interface'; import { FinishedTask } from '~/app/shared/models/finished-task'; import { OrchestratorFeature } from '~/app/shared/models/orchestrator.enum'; import { OrchestratorStatus } from '~/app/shared/models/orchestrator.interface'; @@ -491,37 +490,7 @@ export class HostsComponent extends ListWithDetails implements OnDestroy, OnInit this.orchStatus = orchStatus; const factsAvailable = this.checkHostsFactsAvailable(); return this.hostService.list(`${factsAvailable}`); - }), - map((hostList: object[]) => - hostList.map((host) => { - const counts = {}; - host['service_instances'] = new Set(); - if (this.orchStatus?.available) { - let daemons: Daemon[] = []; - let observable: Observable; - observable = this.hostService.getDaemons(host['hostname']); - observable.subscribe((dmns: Daemon[]) => { - daemons = dmns; - daemons.forEach((daemon: any) => { - counts[daemon.daemon_type] = (counts[daemon.daemon_type] || 0) + 1; - }); - daemons.map((daemon: any) => { - host['service_instances'].add( - `${daemon.daemon_type}: ${counts[daemon.daemon_type]}` - ); - }); - }); - } else { - host['services'].forEach((service: any) => { - counts[service.type] = (counts[service.type] || 0) + 1; - }); - host['services'].map((service: any) => { - host['service_instances'].add(`${service.type}: ${counts[service.type]}`); - }); - } - return host; - }) - ) + }) ) .subscribe( (hostList) => { 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 d13f415275aa..7adbd0b104c2 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 @@ -29,7 +29,7 @@ export class HostService extends ApiClient { list(facts: string): Observable { return this.http.get(this.baseURL, { - headers: { Accept: 'application/vnd.ceph.api.v1.1+json' }, + headers: { Accept: this.getVersionHeaderValue(1, 2) }, params: { facts: facts } }); } diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index e54c6ecdc62a..288d0beb3df7 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -3466,7 +3466,7 @@ paths: responses: '200': content: - application/vnd.ceph.api.v1.1+json: + application/vnd.ceph.api.v1.2+json: schema: properties: addr: @@ -3483,6 +3483,21 @@ paths: items: type: string type: array + service_instances: + description: Service instances related to the host + items: + properties: + count: + description: Number of instances of the service + type: integer + type: + description: type of service + type: string + required: + - type + - count + type: object + type: array service_type: description: '' type: string @@ -3520,6 +3535,7 @@ paths: required: - hostname - services + - service_instances - ceph_version - addr - labels diff --git a/src/pybind/mgr/dashboard/tests/__init__.py b/src/pybind/mgr/dashboard/tests/__init__.py index 705b603b853f..d262d27d7fc5 100644 --- a/src/pybind/mgr/dashboard/tests/__init__.py +++ b/src/pybind/mgr/dashboard/tests/__init__.py @@ -14,7 +14,7 @@ import cherrypy from cherrypy._cptools import HandlerWrapperTool from cherrypy.test import helper from mgr_module import HandleCommandResult -from orchestrator import HostSpec, InventoryHost +from orchestrator import DaemonDescription, HostSpec, InventoryHost from pyfakefs import fake_filesystem from .. import mgr @@ -347,12 +347,22 @@ class Waiter(threading.Thread): @contextlib.contextmanager def patch_orch(available: bool, missing_features: Optional[List[str]] = None, hosts: Optional[List[HostSpec]] = None, - inventory: Optional[List[dict]] = None): + inventory: Optional[List[dict]] = None, + daemons: Optional[List[DaemonDescription]] = None): with mock.patch('dashboard.controllers.orchestrator.OrchClient.instance') as instance: fake_client = mock.Mock() fake_client.available.return_value = available fake_client.get_missing_features.return_value = missing_features + if not daemons: + daemons = [ + DaemonDescription( + daemon_type='mon', + daemon_id='a', + hostname='node0' + ) + ] + fake_client.services.list_daemons.return_value = daemons if hosts is not None: fake_client.hosts.list.return_value = hosts diff --git a/src/pybind/mgr/dashboard/tests/test_host.py b/src/pybind/mgr/dashboard/tests/test_host.py index 5c816f08a6fd..a41d33e713c6 100644 --- a/src/pybind/mgr/dashboard/tests/test_host.py +++ b/src/pybind/mgr/dashboard/tests/test_host.py @@ -1,7 +1,7 @@ import unittest from unittest import mock -from orchestrator import HostSpec +from orchestrator import DaemonDescription, HostSpec from .. import mgr from ..controllers._version import APIVersion @@ -123,14 +123,14 @@ class HostControllerTest(ControllerTestCase): self._get('{}?facts=true'.format(self.URL_HOST), version=APIVersion(1, 1)) self.assertStatus(200) self.assertHeader('Content-Type', - 'application/vnd.ceph.api.v1.1+json') + APIVersion(1, 2).to_mime_type()) self.assertJsonBody(hosts_with_facts) # test with ?facts=false self._get('{}?facts=false'.format(self.URL_HOST), version=APIVersion(1, 1)) self.assertStatus(200) self.assertHeader('Content-Type', - 'application/vnd.ceph.api.v1.1+json') + APIVersion(1, 2).to_mime_type()) self.assertJsonBody(hosts_without_facts) # test with orchestrator available but orch backend!=cephadm @@ -152,7 +152,7 @@ class HostControllerTest(ControllerTestCase): self._get('{}?facts=false'.format(self.URL_HOST), version=APIVersion(1, 1)) self.assertStatus(200) self.assertHeader('Content-Type', - 'application/vnd.ceph.api.v1.1+json') + APIVersion(1, 2).to_mime_type()) self.assertJsonBody(hosts_without_facts) def test_get_1(self): @@ -163,7 +163,10 @@ class HostControllerTest(ControllerTestCase): self.assertStatus(404) def test_get_2(self): - mgr.list_servers.return_value = [{'hostname': 'node1'}] + mgr.list_servers.return_value = [{ + 'hostname': 'node1', + 'services': [] + }] with patch_orch(False): self._get('{}/node1'.format(self.URL_HOST)) @@ -182,6 +185,87 @@ class HostControllerTest(ControllerTestCase): self.assertIn('status', self.json_body()) self.assertIn('addr', self.json_body()) + def test_populate_service_instances(self): + mgr.list_servers.return_value = [] + + node1_daemons = [ + DaemonDescription( + hostname='node1', + daemon_type='mon', + daemon_id='a' + ), + DaemonDescription( + hostname='node1', + daemon_type='mon', + daemon_id='b' + ) + ] + + node2_daemons = [ + DaemonDescription( + hostname='node2', + daemon_type='mgr', + daemon_id='x' + ), + DaemonDescription( + hostname='node2', + daemon_type='mon', + daemon_id='c' + ) + ] + + node1_instances = [{ + 'type': 'mon', + 'count': 2 + }] + + node2_instances = [{ + 'type': 'mgr', + 'count': 1 + }, { + 'type': 'mon', + 'count': 1 + }] + + # test with orchestrator available + with patch_orch(True, + hosts=[HostSpec('node1'), HostSpec('node2')]) as fake_client: + fake_client.services.list_daemons.return_value = node1_daemons + self._get('{}/node1'.format(self.URL_HOST)) + self.assertStatus(200) + self.assertIn('service_instances', self.json_body()) + self.assertEqual(self.json_body()['service_instances'], node1_instances) + + fake_client.services.list_daemons.return_value = node2_daemons + self._get('{}/node2'.format(self.URL_HOST)) + self.assertStatus(200) + self.assertIn('service_instances', self.json_body()) + self.assertEqual(self.json_body()['service_instances'], node2_instances) + + # test with no orchestrator available + with patch_orch(False): + mgr.list_servers.return_value = [{ + 'hostname': 'node1', + 'services': [{ + 'type': 'mon', + 'id': 'a' + }, { + 'type': 'mgr', + 'id': 'b' + }] + }] + self._get('{}/node1'.format(self.URL_HOST)) + self.assertStatus(200) + self.assertIn('service_instances', self.json_body()) + self.assertEqual(self.json_body()['service_instances'], + [{ + 'type': 'mon', + 'count': 1 + }, { + 'type': 'mgr', + 'count': 1 + }]) + @mock.patch('dashboard.controllers.host.add_host') def test_add_host(self, mock_add_host): with patch_orch(True): @@ -375,9 +459,11 @@ class HostUiControllerTest(ControllerTestCase): class TestHosts(unittest.TestCase): def test_get_hosts(self): mgr.list_servers.return_value = [{ - 'hostname': 'node1' + 'hostname': 'node1', + 'services': [] }, { - 'hostname': 'localhost' + 'hostname': 'localhost', + 'services': [] }] orch_hosts = [ HostSpec('node1', labels=['foo', 'bar']), -- 2.47.3