mirror of
https://github.com/ceph/ceph
synced 2026-08-02 07:03:18 +00:00
mgr/dashboard : Fix feedback module enablement
fixes : https://tracker.ceph.com/issues/75734 Signed-off-by: Abhishek Desai <abhishek.desai1@ibm.com>
This commit is contained in:
parent
b51d3cd190
commit
bdbfa8a15a
@ -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
|
||||
|
||||
@ -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')
|
||||
|
||||
@ -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();
|
||||
}));
|
||||
|
||||
@ -17,13 +17,13 @@
|
||||
class="modal-wrapper"
|
||||
>
|
||||
@if (!isFeedbackEnabled) {
|
||||
<cd-alert-panel type="error"
|
||||
<cd-alert-panel type="info"
|
||||
spacingClass="mb-3"
|
||||
actionName="Enable"
|
||||
actionName="Enable feedback"
|
||||
class="align-items-center"
|
||||
(action)="enableFeedbackModule()"
|
||||
i18n-actionName>
|
||||
In order to report an issue, Feedback module must be enabled.
|
||||
Enable the feedback service to report issues.
|
||||
</cd-alert-panel>
|
||||
}
|
||||
<!-- api_key -->
|
||||
@ -31,14 +31,13 @@
|
||||
<div class="form-item">
|
||||
<cds-password-label
|
||||
labelInputID="api_key"
|
||||
label="Ceph Tracker API Key"
|
||||
cdRequiredField="Ceph Tracker API Key"
|
||||
[invalid]="!feedbackForm.controls.api_key.valid && feedbackForm.controls.api_key.dirty"
|
||||
[invalidText]="apiKeyError"
|
||||
[helperText]="apiKeyHelperTpl"
|
||||
[disabled]="!isFeedbackEnabled"
|
||||
i18n
|
||||
>
|
||||
API key
|
||||
<input
|
||||
cdsPassword
|
||||
id="api_key"
|
||||
@ -50,18 +49,19 @@
|
||||
<ng-template #apiKeyError>
|
||||
@if (feedbackForm.showError('api_key', formDir, 'required')) {
|
||||
<span class="invalid-feedback"
|
||||
i18n> Ceph Tracker API key is required. </span>
|
||||
i18n>API key is required.</span>
|
||||
} @if (feedbackForm.showError('api_key', formDir, 'invalidApiKey')) {
|
||||
<span class="invalid-feedback"
|
||||
i18n> Ceph Tracker API key is invalid. </span>
|
||||
i18n>API key is invalid.</span>
|
||||
}
|
||||
</ng-template>
|
||||
|
||||
<ng-template #apiKeyHelperTpl>
|
||||
You can obtain your API key from
|
||||
<a href="https://tracker.ceph.com/my/account"
|
||||
target="_blank">https://tracker.ceph.com/my/account
|
||||
</a>
|
||||
<ng-container i18n>
|
||||
Enter your Ceph Tracker API key. You can find your API key in your
|
||||
<a href="https://tracker.ceph.com/my/account"
|
||||
target="_blank">Ceph Tracker account</a>.
|
||||
</ng-container>
|
||||
</ng-template>
|
||||
</div>
|
||||
}
|
||||
@ -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
|
||||
>
|
||||
<option value="">--- Select a project ---</option>
|
||||
@for (project of projects; track project) {
|
||||
<option [value]="project">{{ project }}</option>
|
||||
}
|
||||
@ -90,25 +88,24 @@
|
||||
</ng-template>
|
||||
</div>
|
||||
|
||||
<!-- tracker -->
|
||||
<!-- issue type -->
|
||||
<div class="form-item cds-select-label">
|
||||
<cds-select
|
||||
label="Tracker"
|
||||
label="Issue type"
|
||||
id="tracker"
|
||||
formControlName="tracker"
|
||||
cdRequiredField="Tracker"
|
||||
[invalid]="!feedbackForm.controls.tracker.valid && feedbackForm.controls.tracker.dirty"
|
||||
[invalidText]="trackerError"
|
||||
i18n
|
||||
> Tracker
|
||||
@for (trackerName of tracker; track trackerName) {
|
||||
<option [value]="trackerName">{{ trackerName }}</option>
|
||||
>
|
||||
@for (issueType of issueTypes; track issueType.value) {
|
||||
<option [value]="issueType.value">{{ issueType.label }}</option>
|
||||
}
|
||||
</cds-select>
|
||||
|
||||
<ng-template #trackerError>
|
||||
@if (feedbackForm.showError('tracker', formDir, 'required')) {
|
||||
<span i18n>Tracker name is required.</span>
|
||||
<span i18n>Issue type is required.</span>
|
||||
}
|
||||
</ng-template>
|
||||
</div>
|
||||
@ -116,25 +113,25 @@
|
||||
<!-- subject -->
|
||||
<div class="form-item">
|
||||
<cds-text-label
|
||||
cdRequiredField="Subject"
|
||||
[invalid]="!feedbackForm.controls.subject.valid && feedbackForm.controls.subject.dirty"
|
||||
[invalidText]="subjectError"
|
||||
[disabled]="!isFeedbackEnabled"
|
||||
i18n
|
||||
>
|
||||
Subject
|
||||
Issue title
|
||||
<input
|
||||
cdsText
|
||||
id="subject"
|
||||
type="text"
|
||||
formControlName="subject"
|
||||
placeholder="Add issue title"
|
||||
placeholder="Enter a short summary of the issue"
|
||||
i18n-placeholder
|
||||
/>
|
||||
</cds-text-label>
|
||||
|
||||
<ng-template #subjectError>
|
||||
@if (feedbackForm.showError('subject', formDir, 'required')) {
|
||||
<span i18n>Subject is required.</span>
|
||||
<span i18n>Issue title is required.</span>
|
||||
}
|
||||
</ng-template>
|
||||
</div>
|
||||
@ -142,7 +139,6 @@
|
||||
<!-- description -->
|
||||
<div class="form-item">
|
||||
<cds-text-label
|
||||
cdRequiredField="Description"
|
||||
[invalid]="!feedbackForm.controls.description.valid && feedbackForm.controls.description.dirty"
|
||||
[invalidText]="descriptionError"
|
||||
[disabled]="!isFeedbackEnabled"
|
||||
@ -154,7 +150,8 @@
|
||||
id="description"
|
||||
type="text"
|
||||
formControlName="description"
|
||||
placeholder="Explain your issue"
|
||||
placeholder="Describe the issue and include steps to reproduce it, if applicable"
|
||||
i18n-placeholder
|
||||
/>
|
||||
</cds-text-label>
|
||||
|
||||
|
||||
@ -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<FeedbackComponent>;
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@ -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();
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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();
|
||||
|
||||
@ -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: () => {
|
||||
|
||||
@ -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:
|
||||
|
||||
Loading…
Reference in New Issue
Block a user