From: Tiago Melo Date: Thu, 5 Jul 2018 16:45:32 +0000 (+0100) Subject: mgr/dashboard: Improve SummaryService and TaskWrapperService X-Git-Tag: v14.0.1~766^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5ee689feb6d4dfcf8949cc8f79c84d6ddeb81a31;p=ceph.git mgr/dashboard: Improve SummaryService and TaskWrapperService When you create a new task, and it stays running, it will be added automatically to the summary data. This will allows for us to deal with it more quickly, by subscribing to the summaryService, and removes the need to pass a runningTasks array between services. Added 3 new methods to SummaryService. Signed-off-by: Tiago Melo --- diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts index 6c61b0908441..9313ff1a0716 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts @@ -2,6 +2,7 @@ import { Component, OnDestroy, OnInit, TemplateRef, ViewChild } from '@angular/c import * as _ from 'lodash'; import { BsModalRef, BsModalService } from 'ngx-bootstrap'; +import { Subscription } from 'rxjs'; import { RbdService } from '../../../shared/api/rbd.service'; import { ConfirmationModalComponent } from '../../../shared/components/confirmation-modal/confirmation-modal.component'; @@ -36,13 +37,12 @@ export class RbdListComponent implements OnInit, OnDestroy { permission: Permission; images: any; - executingTasks: ExecutingTask[] = []; columns: CdTableColumn[]; retries: number; viewCacheStatusList: any[]; selection = new CdTableSelection(); - summaryDataSubscription = null; + summaryDataSubscription: Subscription; modalRef: BsModalRef; @@ -53,7 +53,8 @@ export class RbdListComponent implements OnInit, OnDestroy { private dimlessPipe: DimlessPipe, private summaryService: SummaryService, private modalService: BsModalService, - private taskWrapper: TaskWrapperService) { + private taskWrapper: TaskWrapperService + ) { this.permission = this.authStorageService.getPermissions().rbdImage; } @@ -113,18 +114,13 @@ export class RbdListComponent implements OnInit, OnDestroy { } ]; - this.summaryService.get().subscribe((resp: any) => { - if (!resp) { + this.summaryDataSubscription = this.summaryService.subscribe((data: any) => { + if (!data) { + this.table.reset(); // Disable loading indicator. + this.viewCacheStatusList = [{ status: ViewCacheStatus.ValueException }]; return; } - this.loadImages(resp.executing_tasks); - this.summaryDataSubscription = this.summaryService.summaryData$.subscribe((data: any) => { - this.loadImages(data.executing_tasks); - }); - }, - () => { - this.table.reset(); // Disable loading indicator. - this.viewCacheStatusList = [{ status: ViewCacheStatus.ValueException }]; + this.loadImages(data.executing_tasks); }); } @@ -135,9 +131,6 @@ export class RbdListComponent implements OnInit, OnDestroy { } loadImages(executingTasks) { - if (executingTasks === null) { - executingTasks = this.executingTasks; - } this.rbdService.list().subscribe( (resp: any[]) => { let images = []; @@ -150,7 +143,7 @@ export class RbdListComponent implements OnInit, OnDestroy { images = images.concat(pool.value); }); const viewCacheStatusList = []; - _.forEach(viewCacheStatusMap, (value, key) => { + _.forEach(viewCacheStatusMap, (value: any, key) => { viewCacheStatusList.push({ status: parseInt(key, 10), statusFor: @@ -169,7 +162,6 @@ export class RbdListComponent implements OnInit, OnDestroy { ); }); this.images = this.merge(images, executingTasks); - this.executingTasks = executingTasks; }, () => { this.table.reset(); // Disable loading indicator. @@ -264,7 +256,6 @@ export class RbdListComponent implements OnInit, OnDestroy { pool_name: poolName, image_name: imageName }), - tasks: this.executingTasks, call: this.rbdService.delete(poolName, imageName) }), modalRef: this.modalRef @@ -278,12 +269,10 @@ export class RbdListComponent implements OnInit, OnDestroy { pool_name: poolName, image_name: imageName }), - tasks: this.executingTasks, call: this.rbdService.flatten(poolName, imageName) }) .subscribe(undefined, undefined, () => { this.modalRef.hide(); - this.loadImages(null); }); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.spec.ts index 3afd7bd38488..34d500012a50 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.spec.ts @@ -2,20 +2,24 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { BsModalRef } from 'ngx-bootstrap'; -import 'rxjs/add/observable/of'; -import { Observable } from 'rxjs/Observable'; +import { BehaviorSubject } from 'rxjs'; import { configureTestBed } from '../../../../testing/unit-test-helper'; import { SummaryService } from '../../../shared/services/summary.service'; import { SharedModule } from '../../../shared/shared.module'; import { AboutComponent } from './about.component'; -class SummaryServiceMock { - summaryData$ = Observable.of({ +export class SummaryServiceMock { + summaryDataSource = new BehaviorSubject({ version: 'ceph version 14.0.0-855-gb8193bb4cd ' + '(b8193bb4cda16ccc5b028c3e1df62bc72350a15d) nautilus (dev)' }); + summaryData$ = this.summaryDataSource.asObservable(); + + subscribe(call) { + return this.summaryData$.subscribe(call); + } } describe('AboutComponent', () => { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.ts index 471e4cef98b3..6b86bf31a910 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/about/about.component.ts @@ -19,7 +19,7 @@ export class AboutComponent implements OnInit, OnDestroy { constructor(public modalRef: BsModalRef, private summaryService: SummaryService) {} ngOnInit() { - this.subs = this.summaryService.summaryData$.subscribe((summary: any) => { + this.subs = this.summaryService.subscribe((summary: any) => { if (!summary) { return; } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/dashboard-help/dashboard-help.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/dashboard-help/dashboard-help.component.ts index 563f9a24349b..cdf6bc691bb0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/dashboard-help/dashboard-help.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/dashboard-help/dashboard-help.component.ts @@ -22,13 +22,17 @@ export class DashboardHelpComponent implements OnInit { ) {} ngOnInit() { - const subs = this.summaryService.summaryData$.subscribe((summary: any) => { + const subs = this.summaryService.subscribe((summary: any) => { if (!summary) { return; } + const releaseName = this.cephReleaseNamePipe.transform(summary.version); this.docsUrl = `http://docs.ceph.com/docs/${releaseName}/mgr/dashboard/`; - subs.unsubscribe(); + + setTimeout(() => { + subs.unsubscribe(); + }, 0); }); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts index ec45878d86c7..87ce7e1c86ee 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts @@ -22,7 +22,7 @@ export class NavigationComponent implements OnInit { } ngOnInit() { - this.summaryService.summaryData$.subscribe((data: any) => { + this.summaryService.subscribe((data: any) => { if (!data) { return; } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/task-manager/task-manager.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/task-manager/task-manager.component.ts index 102628c136ad..a78c8c0a755c 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/task-manager/task-manager.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/task-manager/task-manager.component.ts @@ -22,7 +22,7 @@ export class TaskManagerComponent implements OnInit { ) {} ngOnInit() { - this.summaryService.summaryData$.subscribe((data: any) => { + this.summaryService.subscribe((data: any) => { if (!data) { return; } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.spec.ts index 92d4ec9c4c19..90c919204f80 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.spec.ts @@ -1,9 +1,10 @@ import { HttpClient } from '@angular/common/http'; import { fakeAsync, TestBed, tick } from '@angular/core/testing'; -import { of as observableOf } from 'rxjs'; +import { of as observableOf, Subscriber } from 'rxjs'; import { configureTestBed } from '../../../testing/unit-test-helper'; +import { ExecutingTask } from '../models/executing-task'; import { AuthStorageService } from './auth-storage.service'; import { SummaryService } from './summary.service'; @@ -11,18 +12,19 @@ describe('SummaryService', () => { let summaryService: SummaryService; let authStorageService: AuthStorageService; + const summary = { + executing_tasks: [], + health_status: 'HEALTH_OK', + mgr_id: 'x', + rbd_mirroring: { errors: 0, warnings: 0 }, + rbd_pools: [], + have_mon_connection: true, + finished_tasks: [], + filesystems: [{ id: 1, name: 'cephfs_a' }] + }; + const httpClientSpy = { - get: () => - observableOf({ - executing_tasks: [], - health_status: 'HEALTH_OK', - mgr_id: 'x', - rbd_mirroring: { errors: 0, warnings: 0 }, - rbd_pools: [], - have_mon_connection: true, - finished_tasks: [], - filesystems: [{ id: 1, name: 'cephfs_a' }] - }) + get: () => observableOf(summary) }; configureTestBed({ @@ -48,7 +50,7 @@ describe('SummaryService', () => { authStorageService.set('foobar'); let result = false; summaryService.refresh(); - summaryService.summaryData$.subscribe((res) => { + summaryService.subscribe(() => { result = true; }); tick(5000); @@ -58,7 +60,38 @@ describe('SummaryService', () => { }) ); - it('should get summary', () => { - expect(summaryService.get()).toEqual(jasmine.any(Object)); + describe('Should test methods after first refresh', () => { + beforeEach(() => { + authStorageService.set('foobar'); + summaryService.refresh(); + }); + + it('should call getCurrentSummary', () => { + expect(summaryService.getCurrentSummary ()).toEqual(summary); + }); + + it('should call subscribe', () => { + let result; + const subscriber = summaryService.subscribe((data) => { + result = data; + }); + expect(subscriber).toEqual(jasmine.any(Subscriber)); + expect(result).toEqual(summary); + }); + + it('should call addRunningTask', () => { + summaryService.addRunningTask( + new ExecutingTask('rbd/delete', { + pool_name: 'somePool', + image_name: 'someImage' + }) + ); + const result = summaryService.getCurrentSummary (); + expect(result.executing_tasks.length).toBe(1); + expect(result.executing_tasks[0]).toEqual({ + metadata: { image_name: 'someImage', pool_name: 'somePool' }, + name: 'rbd/delete' + }); + }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.ts index 6a03d06efda8..4bf642e912bb 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/summary.service.ts @@ -1,8 +1,10 @@ import { HttpClient } from '@angular/common/http'; import { Injectable, NgZone } from '@angular/core'; -import { BehaviorSubject } from 'rxjs'; +import * as _ from 'lodash'; +import { BehaviorSubject, Subscription } from 'rxjs'; +import { ExecutingTask } from '../models/executing-task'; import { AuthStorageService } from './auth-storage.service'; import { ServicesModule } from './services.module'; @@ -26,7 +28,7 @@ export class SummaryService { refresh() { if (this.authStorageService.isLoggedIn()) { - this.http.get('api/summary').subscribe(data => { + this.http.get('api/summary').subscribe((data) => { this.summaryDataSource.next(data); }); } @@ -40,7 +42,48 @@ export class SummaryService { }); } - get() { - return this.http.get('api/summary'); + /** + * Returns the current value of summaryData + * + * @returns {object} + * @memberof SummaryService + */ + getCurrentSummary () { + return this.summaryDataSource.getValue(); + } + + /** + * Subscribes to the summaryData, + * which is updated once every 5 seconds or when a new task is created. + * + * @param {(summary: any) => void} call + * @returns {Subscription} + * @memberof SummaryService + */ + subscribe(call: (summary: any) => void): Subscription { + return this.summaryData$.subscribe(call); + } + + /** + * Inserts a newly created task to the local list of executing tasks. + * After that, it will automatically push that new information + * to all subscribers. + * + * @param {ExecutingTask} task + * @memberof SummaryService + */ + addRunningTask(task: ExecutingTask) { + const current = this.summaryDataSource.getValue(); + if (!current) { + return; + } + + if (_.isArray(current.executing_tasks)) { + current.executing_tasks.push(task); + } else { + current.executing_tasks = [task]; + } + + this.summaryDataSource.next(current); } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.spec.ts index ad024392f76d..7fbddbfdd65b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.spec.ts @@ -1,41 +1,50 @@ import { fakeAsync, TestBed, tick } from '@angular/core/testing'; import * as _ from 'lodash'; -import { Subject } from 'rxjs'; +import { BehaviorSubject } from 'rxjs'; import { configureTestBed } from '../../../testing/unit-test-helper'; import { SummaryService } from './summary.service'; import { TaskManagerService } from './task-manager.service'; +const summary = { + executing_tasks: [], + health_status: 'HEALTH_OK', + mgr_id: 'x', + rbd_mirroring: { errors: 0, warnings: 0 }, + rbd_pools: [], + have_mon_connection: true, + finished_tasks: [{ name: 'foo', metadata: {} }], + filesystems: [{ id: 1, name: 'cephfs_a' }] +}; + +export class SummaryServiceMock { + summaryDataSource = new BehaviorSubject(summary); + summaryData$ = this.summaryDataSource.asObservable(); + + refresh() { + this.summaryDataSource.next(summary); + } + subscribe(call) { + return this.summaryData$.subscribe(call); + } +} + describe('TaskManagerService', () => { let taskManagerService: TaskManagerService; + let summaryService: any; let called: boolean; - const summaryDataSource = new Subject(); - const fakeService = { - summaryData$: summaryDataSource.asObservable() - }; - - const summary = { - executing_tasks: [], - health_status: 'HEALTH_OK', - mgr_id: 'x', - rbd_mirroring: { errors: 0, warnings: 0 }, - rbd_pools: [], - have_mon_connection: true, - finished_tasks: [{ name: 'foo', metadata: {} }], - filesystems: [{ id: 1, name: 'cephfs_a' }] - }; - configureTestBed( { - providers: [TaskManagerService, { provide: SummaryService, useValue: fakeService }] + providers: [TaskManagerService, { provide: SummaryService, useClass: SummaryServiceMock }] }, true ); beforeEach(() => { taskManagerService = TestBed.get(TaskManagerService); + summaryService = TestBed.get(SummaryService); called = false; taskManagerService.subscribe('foo', {}, () => (called = true)); }); @@ -48,7 +57,7 @@ describe('TaskManagerService', () => { 'should subscribe and be notified when task is finished', fakeAsync(() => { expect(taskManagerService.subscriptions.length).toBe(1); - summaryDataSource.next(summary); + summaryService.refresh(); tick(); expect(called).toEqual(true); expect(taskManagerService.subscriptions).toEqual([]); @@ -63,7 +72,7 @@ describe('TaskManagerService', () => { executing_tasks: [{ name: 'foo', metadata: {} }], finished_tasks: [] }); - summaryDataSource.next(summary); + summaryService.refresh(); tick(); expect(taskManagerService.subscriptions).toEqual(original_subscriptions); }) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.ts index 078d54747ca4..5bbe98102212 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-manager.service.ts @@ -27,7 +27,7 @@ export class TaskManagerService { subscriptions: Array = []; constructor(summaryService: SummaryService) { - summaryService.summaryData$.subscribe((data: any) => { + summaryService.subscribe((data: any) => { if (!data) { return; } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.spec.ts index 98e37097bd10..45910cd1905e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.spec.ts @@ -5,10 +5,10 @@ import { ToastModule } from 'ng2-toastr'; import { Observable } from 'rxjs/Observable'; import { configureTestBed } from '../../../testing/unit-test-helper'; -import { ExecutingTask } from '../models/executing-task'; import { FinishedTask } from '../models/finished-task'; import { SharedModule } from '../shared.module'; import { NotificationService } from './notification.service'; +import { SummaryService } from './summary.service'; import { TaskManagerService } from './task-manager.service'; import { TaskWrapperService } from './task-wrapper.service'; @@ -30,8 +30,8 @@ describe('TaskWrapperService', () => { describe('wrapTaskAroundCall', () => { let notify: NotificationService; - let tasks: ExecutingTask[]; let passed: boolean; + let summaryService: SummaryService; const fakeCall = (status?) => new Observable((observer) => { @@ -45,31 +45,31 @@ describe('TaskWrapperService', () => { const callWrapTaskAroundCall = (status, name) => { return service.wrapTaskAroundCall({ task: new FinishedTask(name, { sth: 'else' }), - call: fakeCall(status), - tasks: tasks + call: fakeCall(status) }); }; beforeEach(() => { passed = false; - tasks = []; notify = TestBed.get(NotificationService); + summaryService = TestBed.get(SummaryService); spyOn(notify, 'show'); spyOn(service, '_handleExecutingTasks').and.callThrough(); + spyOn(summaryService, 'addRunningTask').and.callThrough(); }); it('should simulate a synchronous task', () => { callWrapTaskAroundCall(200, 'sync').subscribe(null, null, () => (passed = true)); expect(service._handleExecutingTasks).not.toHaveBeenCalled(); expect(passed).toBeTruthy(); - expect(tasks.length).toBe(0); + expect(summaryService.addRunningTask).not.toHaveBeenCalled(); }); it('should simulate a asynchronous task', () => { callWrapTaskAroundCall(202, 'async').subscribe(null, null, () => (passed = true)); expect(service._handleExecutingTasks).toHaveBeenCalled(); expect(passed).toBeTruthy(); - expect(tasks.length).toBe(1); + expect(summaryService.addRunningTask).toHaveBeenCalledTimes(1); }); it('should call notifyTask if asynchronous task would have been finished', () => { @@ -86,7 +86,7 @@ describe('TaskWrapperService', () => { callWrapTaskAroundCall(null, 'async').subscribe(null, () => (passed = true), null); expect(service._handleExecutingTasks).not.toHaveBeenCalled(); expect(passed).toBeTruthy(); - expect(tasks.length).toBe(0); + expect(summaryService.addRunningTask).not.toHaveBeenCalled(); }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.ts index ff346968a685..bed55d3f608d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-wrapper.service.ts @@ -8,6 +8,7 @@ import { ExecutingTask } from '../models/executing-task'; import { FinishedTask } from '../models/finished-task'; import { NotificationService } from './notification.service'; import { ServicesModule } from './services.module'; +import { SummaryService } from './summary.service'; import { TaskManagerMessageService } from './task-manager-message.service'; import { TaskManagerService } from './task-manager.service'; @@ -17,25 +18,19 @@ import { TaskManagerService } from './task-manager.service'; export class TaskWrapperService { constructor( private notificationService: NotificationService, + private summaryService: SummaryService, private taskManagerMessageService: TaskManagerMessageService, private taskManagerService: TaskManagerService ) {} - wrapTaskAroundCall({ - task, - call, - tasks - }: { - task: FinishedTask; - call: Observable; - tasks?: ExecutingTask[]; - }) { + wrapTaskAroundCall({ task, call }: { task: FinishedTask; call: Observable }) { return new Observable((observer: Subscriber) => { call.subscribe( (resp) => { if (resp.status === 202) { - this._handleExecutingTasks(task, tasks); + this._handleExecutingTasks(task); } else { + this.summaryService.refresh(); task.success = true; this.notificationService.notifyTask(task); } @@ -53,16 +48,16 @@ export class TaskWrapperService { }); } - _handleExecutingTasks(task: FinishedTask, tasks?: ExecutingTask[]) { + _handleExecutingTasks(task: FinishedTask) { this.notificationService.show( NotificationType.info, this.taskManagerMessageService.getRunningMessage(task), this.taskManagerMessageService.getDescription(task) ); + const executingTask = new ExecutingTask(task.name, task.metadata); - if (tasks) { - tasks.push(executingTask); - } + this.summaryService.addRunningTask(executingTask); + this.taskManagerService.subscribe( executingTask.name, executingTask.metadata,