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.apps.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 6c61b09084414..9313ff1a07166 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 3afd7bd384886..34d500012a50f 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 471e4cef98b38..6b86bf31a910a 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 563f9a24349bb..cdf6bc691bb06 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 ec45878d86c73..87ce7e1c86eeb 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 102628c136ad5..a78c8c0a755cc 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 92d4ec9c4c199..90c919204f809 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 6a03d06efda87..4bf642e912bb6 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 ad024392f76d5..7fbddbfdd65ba 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 078d54747ca4f..5bbe981022124 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 98e37097bd10c..45910cd1905e8 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 ff346968a6856..bed55d3f608dd 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,