diff --git a/qa/tasks/mgr/dashboard/test_feedback.py b/qa/tasks/mgr/dashboard/test_feedback.py index a395adf4865..39d736ea08c 100644 --- a/qa/tasks/mgr/dashboard/test_feedback.py +++ b/qa/tasks/mgr/dashboard/test_feedback.py @@ -6,7 +6,7 @@ class FeedbackTest(MgrModuleTestCase): @classmethod def setUpClass(cls): super().setUpClass() - cls._ceph_cmd(['mgr', 'module', 'enable', 'feedback'], wait=3) + cls._ceph_cmd(['mgr', 'module', 'enable', 'feedback', '--force'], wait=3) # Point the feedback module at an unreachable host so the test # does not depend on tracker.ceph.com being available. Any # create_issue call will fail fast with a ConnectionError diff --git a/src/pybind/mgr/dashboard/controllers/mgr_modules.py b/src/pybind/mgr/dashboard/controllers/mgr_modules.py index 57bb9b5ffb8..249c111e00c 100644 --- a/src/pybind/mgr/dashboard/controllers/mgr_modules.py +++ b/src/pybind/mgr/dashboard/controllers/mgr_modules.py @@ -1,11 +1,13 @@ # -*- coding: utf-8 -*- +from typing import Optional + from .. import mgr from ..security import Scope from ..services.ceph_service import CephService from ..services.exception import handle_send_command_error from ..tools import find_object_in_list, str_to_bool -from . import APIDoc, APIRouter, EndpointDoc, RESTController, allow_empty_body +from . import APIDoc, APIRouter, EndpointDoc, Param, RESTController, allow_empty_body MGR_MODULE_SCHEMA = ([{ "name": (str, "Module Name"), @@ -94,15 +96,27 @@ class MgrModules(RESTController): @RESTController.Resource('POST') @handle_send_command_error('mgr_modules') @allow_empty_body - def enable(self, module_name): + @EndpointDoc("Enable Mgr module", + parameters={ + 'force': Param( + bool, + 'Force enablement when not all mgr daemons support the module', + True, + False) + }) + def enable(self, module_name, force: Optional[bool] = False): """ Enable the specified Ceph Mgr module. :param module_name: The name of the Ceph Mgr module. :type module_name: str + :param force: Force enablement when not all mgr daemons support the module. + :type force: bool """ assert self._is_module_managed(module_name) - CephService.send_command( - 'mon', 'mgr module enable', module=module_name) + cmd_kwargs = {'module': module_name} + if force: + cmd_kwargs['force'] = True + CephService.send_command('mon', 'mgr module enable', **cmd_kwargs) @RESTController.Resource('POST') @handle_send_command_error('mgr_modules') diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts index 68fad64f689..ffe59f37e37 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.spec.ts @@ -146,7 +146,7 @@ describe('MgrModuleListComponent', () => { tick(mgrModuleService.REFRESH_INTERVAL); tick(mgrModuleService.REFRESH_INTERVAL); tick(mgrModuleService.REFRESH_INTERVAL); - expect(mgrModuleService.enable).toHaveBeenCalledWith('foo'); + expect(mgrModuleService.enable).toHaveBeenCalledWith('foo', false); expect(mgrModuleService.list).toHaveBeenCalledTimes(2); expect(component.table.refreshBtn).toHaveBeenCalled(); })); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.html index 50a1804f35d..270f88c8443 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.html @@ -17,13 +17,13 @@ class="modal-wrapper" > @if (!isFeedbackEnabled) { - - In order to report an issue, Feedback module must be enabled. + Enable the feedback service to report issues. } @@ -31,14 +31,13 @@
+ API key @if (feedbackForm.showError('api_key', formDir, 'required')) { Ceph Tracker API key is required. + i18n>API key is required. } @if (feedbackForm.showError('api_key', formDir, 'invalidApiKey')) { Ceph Tracker API key is invalid. + i18n>API key is invalid. } - You can obtain your API key from - https://tracker.ceph.com/my/account - + + Enter your Ceph Tracker API key. You can find your API key in your + Ceph Tracker account. +
} @@ -72,12 +72,10 @@ label="Project name" id="project" formControlName="project" - cdRequiredField="Project name" [invalid]="!feedbackForm.controls.project.valid && feedbackForm.controls.project.dirty" [invalidText]="projectError" i18n > - @for (project of projects; track project) { } @@ -90,25 +88,24 @@ - +
Tracker - @for (trackerName of tracker; track trackerName) { - + > + @for (issueType of issueTypes; track issueType.value) { + } @if (feedbackForm.showError('tracker', formDir, 'required')) { - Tracker name is required. + Issue type is required. }
@@ -116,25 +113,25 @@
- Subject + Issue title @if (feedbackForm.showError('subject', formDir, 'required')) { - Subject is required. + Issue title is required. }
@@ -142,7 +139,6 @@
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.spec.ts index c5e27b99054..d8faa782a76 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.spec.ts @@ -5,9 +5,10 @@ import { RouterTestingModule } from '@angular/router/testing'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; -import { throwError } from 'rxjs'; +import { throwError, of as observableOf } from 'rxjs'; import { FeedbackService } from '~/app/shared/api/feedback.service'; +import { MgrModuleService } from '~/app/shared/api/mgr-module.service'; import { ComponentsModule } from '~/app/shared/components/components.module'; import { configureTestBed, FormHelper } from '~/testing/unit-test-helper'; import { FeedbackComponent } from './feedback.component'; @@ -18,6 +19,7 @@ describe('FeedbackComponent', () => { let component: FeedbackComponent; let fixture: ComponentFixture; let feedbackService: FeedbackService; + let mgrModuleService: MgrModuleService; let formHelper: FormHelper; configureTestBed({ @@ -37,6 +39,7 @@ describe('FeedbackComponent', () => { fixture = TestBed.createComponent(FeedbackComponent); component = fixture.componentInstance; feedbackService = TestBed.inject(FeedbackService); + mgrModuleService = TestBed.inject(MgrModuleService); fixture.detectChanges(); }); @@ -73,4 +76,40 @@ describe('FeedbackComponent', () => { formHelper.expectError('api_key', 'invalidApiKey'); }); + + it('should enable feedback module with force', () => { + spyOn(mgrModuleService, 'updateModuleState'); + spyOn(mgrModuleService.updateCompleted$, 'subscribe').and.callThrough(); + + component.enableFeedbackModule(); + + expect(mgrModuleService.updateModuleState).toHaveBeenCalledWith( + 'feedback', + false, + null, + null, + 'Enabled Feedback Module', + false, + undefined, + true + ); + expect(mgrModuleService.updateCompleted$.subscribe).toHaveBeenCalled(); + }); + + it('should refresh feedback state after module enablement', () => { + spyOn(feedbackService, 'isKeyExist').and.returnValues( + throwError({ status: 404 }), + observableOf(true) + ); + spyOn(mgrModuleService, 'updateModuleState'); + + component.ngOnInit(); + expect(component.isFeedbackEnabled).toEqual(false); + + component.enableFeedbackModule(); + mgrModuleService.updateCompleted$.next(); + + expect(component.isFeedbackEnabled).toEqual(true); + expect(component.feedbackForm.enabled).toEqual(true); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.ts index 40509cbe797..655358050ec 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/shared/feedback/feedback.component.ts @@ -28,7 +28,10 @@ export class FeedbackComponent extends CdForm implements OnInit, OnDestroy { 'ceph_volume', 'core_ceph' ]; - tracker: string[] = ['bug', 'feature']; + issueTypes = [ + { value: 'bug', label: $localize`Bug` }, + { value: 'feature', label: $localize`Feature request` } + ]; api_key: string; keySub: Subscription; submit: string; @@ -48,8 +51,15 @@ export class FeedbackComponent extends CdForm implements OnInit, OnDestroy { ngOnInit() { this.createForm(); + this.loadFeedbackState(); + } + + private loadFeedbackState() { + this.keySub?.unsubscribe(); this.keySub = this.feedbackService.isKeyExist().subscribe({ next: (data: boolean) => { + this.isFeedbackEnabled = true; + this.feedbackForm.enable(); this.isAPIKeySet = data; if (this.isAPIKeySet) { this.feedbackForm.get('api_key').clearValidators(); @@ -64,8 +74,8 @@ export class FeedbackComponent extends CdForm implements OnInit, OnDestroy { private createForm() { this.feedbackForm = new CdFormGroup({ - project: new UntypedFormControl('', Validators.required), - tracker: new UntypedFormControl(this.tracker[0], Validators.required), + project: new UntypedFormControl(this.projects[0], Validators.required), + tracker: new UntypedFormControl(this.issueTypes[0].value, Validators.required), subject: new UntypedFormControl('', Validators.required), description: new UntypedFormControl('', Validators.required), api_key: new UntypedFormControl('', Validators.required) @@ -110,7 +120,13 @@ export class FeedbackComponent extends CdForm implements OnInit, OnDestroy { null, null, 'Enabled Feedback Module', + false, + undefined, true ); + const subscription = this.mgrModuleService.updateCompleted$.subscribe(() => { + subscription.unsubscribe(); + this.loadFeedbackState(); + }); } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/feedback.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/feedback.service.ts index c450bbe076f..b3ea2ffc15b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/feedback.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/feedback.service.ts @@ -2,6 +2,8 @@ import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; import * as _ from 'lodash'; +import { throwError as observableThrowError } from 'rxjs'; +import { catchError } from 'rxjs/operators'; @Injectable({ providedIn: 'root' @@ -11,7 +13,14 @@ export class FeedbackService { baseUIURL = 'api/feedback'; isKeyExist() { - return this.http.get('ui-api/feedback/api_key/exist'); + return this.http.get('ui-api/feedback/api_key/exist').pipe( + catchError((error) => { + if (_.isFunction(error.preventDefault)) { + error.preventDefault(); + } + return observableThrowError(() => error); + }) + ); } createIssue( diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.spec.ts index 0f65e6ebc0f..00ed2fe4c39 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.spec.ts @@ -61,6 +61,21 @@ describe('MgrModuleService', () => { service.enable('foo').subscribe(); const req = httpTesting.expectOne('api/mgr/module/foo/enable'); expect(req.request.method).toBe('POST'); + expect(req.request.body).toBeNull(); + }); + + it('should call enable with force for whitelisted modules', () => { + service.enable('feedback').subscribe(); + const req = httpTesting.expectOne('api/mgr/module/feedback/enable'); + expect(req.request.method).toBe('POST'); + expect(req.request.body).toEqual({ force: true }); + }); + + it('should call enable with explicit force', () => { + service.enable('foo', true).subscribe(); + const req = httpTesting.expectOne('api/mgr/module/foo/enable'); + expect(req.request.method).toBe('POST'); + expect(req.request.body).toEqual({ force: true }); }); it('should call disable', () => { @@ -106,7 +121,7 @@ describe('MgrModuleService', () => { tick(service.REFRESH_INTERVAL); tick(service.REFRESH_INTERVAL); tick(service.REFRESH_INTERVAL); - expect(service.enable).toHaveBeenCalledWith('foo'); + expect(service.enable).toHaveBeenCalledWith('foo', false); expect(service.list).toHaveBeenCalledTimes(2); expect(notificationService.suspendToasties).toHaveBeenCalledTimes(2); expect(blockUIService.start).toHaveBeenCalled(); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.ts index 7826523b5d0..53a508b01f3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/mgr-module.service.ts @@ -13,6 +13,9 @@ import { SummaryService } from '../services/summary.service'; const GLOBAL = 'global'; +/** Modules that require --force when not all mgr daemons support them. */ +const FORCE_ENABLE_MODULES = new Set(['feedback']); + @Injectable({ providedIn: 'root' }) @@ -60,9 +63,11 @@ export class MgrModuleService { /** * Enable the Ceph Mgr module. * @param {string} module The name of the mgr module. + * @param {boolean} force Force enablement when not all mgr daemons support the module. */ - enable(module: string) { - return this.http.post(`${this.url}/${module}/enable`, null); + enable(module: string, force: boolean = false) { + const useForce = force || FORCE_ENABLE_MODULES.has(module); + return this.http.post(`${this.url}/${module}/enable`, useForce ? { force: true } : null); } /** @@ -92,9 +97,10 @@ export class MgrModuleService { navigateTo: string = '', notificationText?: string, navigateByUrl?: boolean, - reconnectingMessage: string = $localize`Reconnecting, please wait ...` + reconnectingMessage: string = $localize`Reconnecting, please wait ...`, + force: boolean = false ): void { - const moduleToggle$ = enabled ? this.disable(module) : this.enable(module); + const moduleToggle$ = enabled ? this.disable(module) : this.enable(module, force); moduleToggle$.subscribe({ next: () => { diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index 28d785dfaeb..83f0ca14911 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -10435,13 +10435,25 @@ paths: post: description: "\n Enable the specified Ceph Mgr module.\n :param\ \ module_name: The name of the Ceph Mgr module.\n :type module_name:\ - \ str\n " + \ str\n :param force: Force enablement when not all mgr daemons support\ + \ the module.\n :type force: bool\n " parameters: - in: path name: module_name required: true schema: type: string + requestBody: + content: + application/json: + schema: + properties: + force: + default: false + description: Force enablement when not all mgr daemons support the + module + type: boolean + type: object responses: '201': content: @@ -10472,6 +10484,7 @@ paths: trace. security: - jwt: [] + summary: Enable Mgr module tags: - MgrModule /api/mgr/module/{module_name}/options: