From 5454426a0a72ab9a44388c81ede8c2f20e35de6e Mon Sep 17 00:00:00 2001 From: Avan Thakkar Date: Mon, 22 Mar 2021 20:10:20 +0530 Subject: [PATCH] mgr/dashboard: avoid data processing in crush-map component Fixes: https://tracker.ceph.com/issues/49236 Signed-off-by: Avan Thakkar --- .../integration/cluster/crush-map.e2e-spec.ts | 4 +- .../crushmap/crushmap.component.spec.ts | 168 +++++++++--------- .../cluster/crushmap/crushmap.component.ts | 18 +- 3 files changed, 91 insertions(+), 99 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/cypress/integration/cluster/crush-map.e2e-spec.ts b/src/pybind/mgr/dashboard/frontend/cypress/integration/cluster/crush-map.e2e-spec.ts index 2274d72e7bc..0a454739fd4 100644 --- a/src/pybind/mgr/dashboard/frontend/cypress/integration/cluster/crush-map.e2e-spec.ts +++ b/src/pybind/mgr/dashboard/frontend/cypress/integration/cluster/crush-map.e2e-spec.ts @@ -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 diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.spec.ts index a56d0a90b58..bed73b5ff53 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.spec.ts @@ -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; 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); - }); - - 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(); + })); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.ts index 55db0b6e8ab..c44dad3f25e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/crushmap/crushmap.component.ts @@ -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; - 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) { -- 2.39.5