Merge pull request #36845 from bk201/wip-44803

mgr/dashboard: allow getting fresh inventory data from the orchestrator

Reviewed-by: Stephan Müller <smueller@suse.com>
Reviewed-by: Volker Theile <vtheile@suse.com>

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'])

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);
     });
   });
 });

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<InventoryDevice> = [];
 
-  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;
   }
 }

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');
+    }
   });
 });

src/pybind/mgr/dashboard/frontend/src/app/shared/api/orchestrator.service.ts

@@ -55,13 +55,19 @@ export class OrchestratorService {
     });
   }
 
-  inventoryList(hostname?: string): Observable<InventoryHost[]> {
-    const options = hostname ? { params: new HttpParams().set('hostname', hostname) } : {};
-    return this.http.get<InventoryHost[]>(`${this.url}/inventory`, options);
+  inventoryList(hostname?: string, refresh?: boolean): Observable<InventoryHost[]> {
+    let params = new HttpParams();
+    if (hostname) {
+      params = params.append('hostname', hostname);
+    }
+    if (refresh) {
+      params = params.append('refresh', _.toString(refresh));
+    }
+    return this.http.get<InventoryHost[]>(`${this.url}/inventory`, { params: params });
   }
 
-  inventoryDeviceList(hostname?: string): Observable<InventoryDevice[]> {
-    return this.inventoryList(hostname).pipe(
+  inventoryDeviceList(hostname?: string, refresh?: boolean): Observable<InventoryDevice[]> {
+    return this.inventoryList(hostname, refresh).pipe(
       mergeMap((hosts: InventoryHost[]) => {
         const devices = _.flatMap(hosts, (host) => {
           return host.devices.map((device) => {