diff --git a/src/pybind/mgr/dashboard/controllers/orchestrator.py b/src/pybind/mgr/dashboard/controllers/orchestrator.py index d7871ebeb0b..8a6cf0f9a92 100644 --- a/src/pybind/mgr/dashboard/controllers/orchestrator.py +++ b/src/pybind/mgr/dashboard/controllers/orchestrator.py @@ -12,7 +12,7 @@ from ..exceptions import DashboardException from ..security import Scope from ..services.exception import handle_orchestrator_error from ..services.orchestrator import OrchClient, OrchFeature -from ..tools import TaskManager +from ..tools import TaskManager, str_to_bool STATUS_SCHEMA = { "available": (bool, "Orchestrator status"), @@ -122,10 +122,13 @@ class Orchestrator(RESTController): class OrchestratorInventory(RESTController): @raise_if_no_orchestrator([OrchFeature.DEVICE_LIST]) - def list(self, hostname=None): + def list(self, hostname=None, refresh=None): orch = OrchClient.instance() hosts = [hostname] if hostname else None - inventory_hosts = [host.to_json() for host in orch.inventory.list(hosts)] + do_refresh = False + if refresh is not None: + do_refresh = str_to_bool(refresh) + inventory_hosts = [host.to_json() for host in orch.inventory.list(hosts, do_refresh)] device_osd_map = get_device_osd_map() for inventory_host in inventory_hosts: host_osds = device_osd_map.get(inventory_host['name']) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.spec.ts index 3c97845e8ef..8b983e5ab27 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.spec.ts @@ -45,13 +45,17 @@ describe('InventoryComponent', () => { describe('after ngOnInit', () => { it('should load devices', () => { fixture.detectChanges(); - expect(orchService.inventoryDeviceList).toHaveBeenCalledWith(undefined); - }); + expect(orchService.inventoryDeviceList).toHaveBeenNthCalledWith(1, undefined, false); + component.refresh(); // click refresh button + expect(orchService.inventoryDeviceList).toHaveBeenNthCalledWith(2, undefined, true); - it('should load devices for a host', () => { - component.hostname = 'host0'; + const newHost = 'host0'; + component.hostname = newHost; fixture.detectChanges(); - expect(orchService.inventoryDeviceList).toHaveBeenCalledWith('host0'); + component.ngOnChanges(); + expect(orchService.inventoryDeviceList).toHaveBeenNthCalledWith(3, newHost, false); + component.refresh(); // click refresh button + expect(orchService.inventoryDeviceList).toHaveBeenNthCalledWith(4, newHost, true); }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.ts index 2c2ba53c00e..9f6cb2c9c87 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/inventory/inventory.component.ts @@ -1,4 +1,6 @@ -import { Component, Input, OnChanges, OnInit } from '@angular/core'; +import { Component, Input, NgZone, OnChanges, OnDestroy, OnInit } from '@angular/core'; + +import { Subscription, timer as observableTimer } from 'rxjs'; import { OrchestratorService } from '../../../shared/api/orchestrator.service'; import { Icons } from '../../../shared/enum/icons.enum'; @@ -10,39 +12,59 @@ import { InventoryDevice } from './inventory-devices/inventory-device.model'; templateUrl: './inventory.component.html', styleUrls: ['./inventory.component.scss'] }) -export class InventoryComponent implements OnChanges, OnInit { +export class InventoryComponent implements OnChanges, OnInit, OnDestroy { // Display inventory page only for this hostname, ignore to display all. @Input() hostname?: string; + private reloadSubscriber: Subscription; + private reloadInterval = 5000; + private firstRefresh = true; + icons = Icons; orchStatus: OrchestratorStatus; devices: Array = []; - constructor(private orchService: OrchestratorService) {} + constructor(private orchService: OrchestratorService, private ngZone: NgZone) {} ngOnInit() { this.orchService.status().subscribe((status) => { this.orchStatus = status; if (status.available) { - this.getInventory(); + // Create a timer to get cached inventory from the orchestrator. + // Do not ask the orchestrator frequently to refresh its cache data because it's expensive. + this.ngZone.runOutsideAngular(() => { + // start after first pass because the embedded table calls refresh at init. + this.reloadSubscriber = observableTimer( + this.reloadInterval, + this.reloadInterval + ).subscribe(() => { + this.ngZone.run(() => { + this.getInventory(false); + }); + }); + }); } }); } + ngOnDestroy() { + this.reloadSubscriber?.unsubscribe(); + } + ngOnChanges() { - if (this.orchStatus) { + if (this.orchStatus?.available) { this.devices = []; - this.getInventory(); + this.getInventory(false); } } - getInventory() { + getInventory(refresh: boolean) { if (this.hostname === '') { return; } - this.orchService.inventoryDeviceList(this.hostname).subscribe( + this.orchService.inventoryDeviceList(this.hostname, refresh).subscribe( (devices: InventoryDevice[]) => { this.devices = devices; }, @@ -53,6 +75,9 @@ export class InventoryComponent implements OnChanges, OnInit { } refresh() { - this.getInventory(); + // Make the first reload (triggered by table) use cached data, and + // the remaining reloads (triggered by users) ask orchestrator to refresh inventory. + this.getInventory(!this.firstRefresh); + this.firstRefresh = false; } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.spec.ts index 94c957e49c6..bf1c433a136 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.spec.ts @@ -33,16 +33,31 @@ describe('OrchestratorService', () => { expect(req.request.method).toBe('GET'); }); - it('should call inventoryList', () => { - service.inventoryList().subscribe(); - const req = httpTesting.expectOne(`${apiPath}/inventory`); - expect(req.request.method).toBe('GET'); - }); + it('should call inventoryList with arguments', () => { + const inventoryPath = `${apiPath}/inventory`; + const tests: { args: any[]; expectedUrl: any }[] = [ + { + args: [], + expectedUrl: inventoryPath + }, + { + args: ['host0'], + expectedUrl: `${inventoryPath}?hostname=host0` + }, + { + args: [undefined, true], + expectedUrl: `${inventoryPath}?refresh=true` + }, + { + args: ['host0', true], + expectedUrl: `${inventoryPath}?hostname=host0&refresh=true` + } + ]; - it('should call inventoryList with a host', () => { - const host = 'host0'; - service.inventoryList(host).subscribe(); - const req = httpTesting.expectOne(`${apiPath}/inventory?hostname=${host}`); - expect(req.request.method).toBe('GET'); + for (const test of tests) { + service.inventoryList(...test.args).subscribe(); + const req = httpTesting.expectOne(test.expectedUrl); + expect(req.request.method).toBe('GET'); + } }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.ts index 89a23393b90..6be70861e69 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.ts @@ -55,13 +55,19 @@ export class OrchestratorService { }); } - inventoryList(hostname?: string): Observable { - const options = hostname ? { params: new HttpParams().set('hostname', hostname) } : {}; - return this.http.get(`${this.url}/inventory`, options); + inventoryList(hostname?: string, refresh?: boolean): Observable { + let params = new HttpParams(); + if (hostname) { + params = params.append('hostname', hostname); + } + if (refresh) { + params = params.append('refresh', _.toString(refresh)); + } + return this.http.get(`${this.url}/inventory`, { params: params }); } - inventoryDeviceList(hostname?: string): Observable { - return this.inventoryList(hostname).pipe( + inventoryDeviceList(hostname?: string, refresh?: boolean): Observable { + return this.inventoryList(hostname, refresh).pipe( mergeMap((hosts: InventoryHost[]) => { const devices = _.flatMap(hosts, (host) => { return host.devices.map((device) => {