]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: avoid data processing in crush-map component 40311/head
authorAvan Thakkar <athakkar@localhost.localdomain>
Mon, 22 Mar 2021 14:40:20 +0000 (20:10 +0530)
committerAvan Thakkar <athakkar@localhost.localdomain>
Thu, 6 May 2021 08:43:28 +0000 (14:13 +0530)
Fixes: https://tracker.ceph.com/issues/49236
Signed-off-by: Avan Thakkar <athakkar@redhat.com>
src/pybind/mgr/dashboard/frontend/cypress/integration/cluster/crush-map.e2e-spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.ts

index 2274d72e7bcb8138fc3b9910eeba71c60c53d4b9..0a454739fd40632fe48ec02fab8899a02fd31ab4 100644 (file)
@@ -21,13 +21,13 @@ describe('CRUSH map page', () => {
       crushmap.getPageTitle().should('equal', 'CRUSH map viewer');
 
       // Check that title appears once OSD is clicked
-      crushmap.getCrushNode(1).click();
+      crushmap.getCrushNode(0).click();
 
       crushmap
         .getLegends()
         .invoke('text')
         .then((legend) => {
-          crushmap.getCrushNode(1).should('have.text', legend);
+          crushmap.getCrushNode(0).should('have.text', legend);
         });
 
       // Check that table appears once OSD is clicked
index a56d0a90b587a5e89f2b7b43f7f38d83f51ba61c..bed73b5ff53cab90002fc164052e632fb127d2ec 100644 (file)
@@ -1,11 +1,11 @@
 import { HttpClientTestingModule } from '@angular/common/http/testing';
-import { DebugElement, Type } from '@angular/core';
-import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { DebugElement } from '@angular/core';
+import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
 
 import { TreeModule } from '@circlon/angular-tree-component';
 import { of } from 'rxjs';
 
-import { HealthService } from '~/app/shared/api/health.service';
+import { CrushRuleService } from '~/app/shared/api/crush-rule.service';
 import { SharedModule } from '~/app/shared/shared.module';
 import { configureTestBed } from '~/testing/unit-test-helper';
 import { CrushmapComponent } from './crushmap.component';
@@ -14,16 +14,19 @@ describe('CrushmapComponent', () => {
   let component: CrushmapComponent;
   let fixture: ComponentFixture<CrushmapComponent>;
   let debugElement: DebugElement;
+  let crushRuleService: CrushRuleService;
+  let crushRuleServiceInfoSpy: jasmine.Spy;
   configureTestBed({
     imports: [HttpClientTestingModule, TreeModule, SharedModule],
-    declarations: [CrushmapComponent],
-    providers: [HealthService]
+    declarations: [CrushmapComponent]
   });
 
   beforeEach(() => {
     fixture = TestBed.createComponent(CrushmapComponent);
     component = fixture.componentInstance;
     debugElement = fixture.debugElement;
+    crushRuleService = TestBed.inject(CrushRuleService);
+    crushRuleServiceInfoSpy = spyOn(crushRuleService, 'getInfo');
   });
 
   it('should create', () => {
@@ -31,33 +34,23 @@ describe('CrushmapComponent', () => {
   });
 
   it('should display right title', () => {
-    fixture.detectChanges();
     const span = debugElement.nativeElement.querySelector('.card-header');
     expect(span.textContent).toBe('CRUSH map viewer');
   });
 
-  describe('test tree', () => {
-    let healthService: HealthService;
-    const prepareGetHealth = (nodes: object[]) => {
-      spyOn(healthService, 'getFullHealth').and.returnValue(
-        of({ osd_map: { tree: { nodes: nodes } } })
-      );
-      fixture.detectChanges();
-    };
-
-    beforeEach(() => {
-      healthService = debugElement.injector.get(HealthService as Type<HealthService>);
-    });
-
-    it('should display "No nodes!" if ceph tree nodes is empty array', () => {
-      prepareGetHealth([]);
-      expect(healthService.getFullHealth).toHaveBeenCalled();
-      expect(component.nodes[0].name).toEqual('No nodes!');
-    });
+  it('should display "No nodes!" if ceph tree nodes is empty array', fakeAsync(() => {
+    crushRuleServiceInfoSpy.and.returnValue(of({ nodes: [] }));
+    fixture.detectChanges();
+    tick(5000);
+    expect(crushRuleService.getInfo).toHaveBeenCalled();
+    expect(component.nodes[0].name).toEqual('No nodes!');
+    component.ngOnDestroy();
+  }));
 
-    describe('nodes not empty', () => {
-      beforeEach(() => {
-        prepareGetHealth([
+  it('should have two root nodes', fakeAsync(() => {
+    crushRuleServiceInfoSpy.and.returnValue(
+      of({
+        nodes: [
           { children: [-2], type: 'root', name: 'default', id: -1 },
           { children: [1, 0, 2], type: 'host', name: 'my-host', id: -2 },
           { status: 'up', type: 'osd', name: 'osd.0', id: 0 },
@@ -66,77 +59,78 @@ describe('CrushmapComponent', () => {
           { children: [-4], type: 'root', name: 'default-2', id: -3 },
           { children: [4], type: 'host', name: 'my-host-2', id: -4 },
           { status: 'up', type: 'osd', name: 'osd.0-2', id: 4 }
-        ]);
-      });
-
-      it('should have two root nodes', () => {
-        expect(component.nodes).toEqual([
+        ]
+      })
+    );
+    fixture.detectChanges();
+    tick(10000);
+    expect(crushRuleService.getInfo).toHaveBeenCalled();
+    expect(component.nodes).toEqual([
+      {
+        cdId: -3,
+        children: [
           {
-            cdId: -3,
             children: [
               {
-                children: [
-                  {
-                    id: component.nodes[0].children[0].children[0].id,
-                    cdId: 4,
-                    status: 'up',
-                    type: 'osd',
-                    name: 'osd.0-2 (osd)'
-                  }
-                ],
-                id: component.nodes[0].children[0].id,
-                cdId: -4,
-                status: undefined,
-                type: 'host',
-                name: 'my-host-2 (host)'
+                id: component.nodes[0].children[0].children[0].id,
+                cdId: 4,
+                status: 'up',
+                type: 'osd',
+                name: 'osd.0-2 (osd)'
               }
             ],
-            id: component.nodes[0].id,
+            id: component.nodes[0].children[0].id,
+            cdId: -4,
             status: undefined,
-            type: 'root',
-            name: 'default-2 (root)'
-          },
+            type: 'host',
+            name: 'my-host-2 (host)'
+          }
+        ],
+        id: component.nodes[0].id,
+        status: undefined,
+        type: 'root',
+        name: 'default-2 (root)'
+      },
+      {
+        children: [
           {
             children: [
               {
-                children: [
-                  {
-                    id: component.nodes[1].children[0].children[0].id,
-                    cdId: 0,
-                    status: 'up',
-                    type: 'osd',
-                    name: 'osd.0 (osd)'
-                  },
-                  {
-                    id: component.nodes[1].children[0].children[1].id,
-                    cdId: 1,
-                    status: 'down',
-                    type: 'osd',
-                    name: 'osd.1 (osd)'
-                  },
-                  {
-                    id: component.nodes[1].children[0].children[2].id,
-                    cdId: 2,
-                    status: 'up',
-                    type: 'osd',
-                    name: 'osd.2 (osd)'
-                  }
-                ],
-                id: component.nodes[1].children[0].id,
-                cdId: -2,
-                status: undefined,
-                type: 'host',
-                name: 'my-host (host)'
+                id: component.nodes[1].children[0].children[0].id,
+                cdId: 0,
+                status: 'up',
+                type: 'osd',
+                name: 'osd.0 (osd)'
+              },
+              {
+                id: component.nodes[1].children[0].children[1].id,
+                cdId: 1,
+                status: 'down',
+                type: 'osd',
+                name: 'osd.1 (osd)'
+              },
+              {
+                id: component.nodes[1].children[0].children[2].id,
+                cdId: 2,
+                status: 'up',
+                type: 'osd',
+                name: 'osd.2 (osd)'
               }
             ],
-            id: component.nodes[1].id,
-            cdId: -1,
+            id: component.nodes[1].children[0].id,
+            cdId: -2,
             status: undefined,
-            type: 'root',
-            name: 'default (root)'
+            type: 'host',
+            name: 'my-host (host)'
           }
-        ]);
-      });
-    });
-  });
+        ],
+        id: component.nodes[1].id,
+        cdId: -1,
+        status: undefined,
+        type: 'root',
+        name: 'default (root)'
+      }
+    ]);
+    component.ngOnDestroy();
+  }));
 });
index 55db0b6e8abb98ede408440fc638520c52d3756d..c44dad3f25e07f5a80198eb193f61e496bf9ae04 100644 (file)
@@ -7,9 +7,9 @@ import {
   TreeNode,
   TREE_ACTIONS
 } from '@circlon/angular-tree-component';
-import { Subscription } from 'rxjs';
+import { Observable, Subscription } from 'rxjs';
 
-import { HealthService } from '~/app/shared/api/health.service';
+import { CrushRuleService } from '~/app/shared/api/crush-rule.service';
 import { Icons } from '~/app/shared/enum/icons.enum';
 import { TimerService } from '~/app/shared/services/timer.service';
 
@@ -18,7 +18,7 @@ import { TimerService } from '~/app/shared/services/timer.service';
   templateUrl: './crushmap.component.html',
   styleUrls: ['./crushmap.component.scss']
 })
-export class CrushmapComponent implements OnInit, OnDestroy {
+export class CrushmapComponent implements OnDestroy, OnInit {
   private sub = new Subscription();
 
   @ViewChild('tree') tree: TreeComponent;
@@ -39,17 +39,15 @@ export class CrushmapComponent implements OnInit, OnDestroy {
   metadata: any;
   metadataTitle: string;
   metadataKeyMap: { [key: number]: any } = {};
+  data$: Observable<object>;
 
-  constructor(private healthService: HealthService, private timerService: TimerService) {}
+  constructor(private crushRuleService: CrushRuleService, private timerService: TimerService) {}
 
   ngOnInit() {
-    this.healthService.getFullHealth().subscribe((data: any) => {
-      this.loadingIndicator = false;
-      this.nodes = this.abstractTreeData(data);
-    });
     this.sub = this.timerService
-      .get(() => this.healthService.getFullHealth(), 5000)
+      .get(() => this.crushRuleService.getInfo(), 5000)
       .subscribe((data: any) => {
+        this.loadingIndicator = false;
         this.nodes = this.abstractTreeData(data);
       });
   }
@@ -59,7 +57,7 @@ export class CrushmapComponent implements OnInit, OnDestroy {
   }
 
   private abstractTreeData(data: any): any[] {
-    const nodes = data.osd_map.tree.nodes || [];
+    const nodes = data.nodes || [];
     const treeNodeMap: { [key: number]: any } = {};
 
     if (0 === nodes.length) {