Skip to content

Commit e0e467a

Browse files
committed
mgr/dashboard: Adding group and pool name to service name
- Pre-populating the service name field with the format `nvmeof.<pool_name>.<group_name>`. - This can be changed by user but by default this value will be there. - This will help user to quickly fill form and proceed hence improving usability. - cephadm also uses this format as of now this convention so it will make UI aligned with CLI experience - updates unit tests to improve coverage - hides`count` values as that is not needed for 'nvmeof' only hosts and labels required. Fixes https://tracker.ceph.com/issues/68065 Signed-off-by: Afreen Misbah <[email protected]>
1 parent a2c6b20 commit e0e467a

File tree

4 files changed

+116
-29
lines changed

4 files changed

+116
-29
lines changed

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@
305305
</div>
306306

307307
<!-- Count -->
308-
<div *ngIf="!serviceForm.controls.unmanaged.value"
308+
<div *ngIf="!serviceForm.controls.unmanaged.value && serviceForm.controls.service_type.value !== 'nvmeof'"
309309
class="form-group row">
310310
<label class="cd-col-form-label"
311311
for="count">

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.spec.ts

Lines changed: 86 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,21 @@ import { CephServiceService } from '~/app/shared/api/ceph-service.service';
1313
import { PaginateObservable } from '~/app/shared/api/paginate.model';
1414
import { CdFormGroup } from '~/app/shared/forms/cd-form-group';
1515
import { SharedModule } from '~/app/shared/shared.module';
16-
import { configureTestBed, FormHelper } from '~/testing/unit-test-helper';
16+
import { configureTestBed, FormHelper, Mocks } from '~/testing/unit-test-helper';
1717
import { ServiceFormComponent } from './service-form.component';
18+
import { PoolService } from '~/app/shared/api/pool.service';
19+
20+
// for 'nvmeof' service
21+
const mockPools = [
22+
Mocks.getPool('pool-1', 1, ['cephfs']),
23+
Mocks.getPool('rbd', 2),
24+
Mocks.getPool('pool-2', 3)
25+
];
26+
class MockPoolService {
27+
getList() {
28+
return of(mockPools);
29+
}
30+
}
1831

1932
describe('ServiceFormComponent', () => {
2033
let component: ServiceFormComponent;
@@ -25,7 +38,7 @@ describe('ServiceFormComponent', () => {
2538

2639
configureTestBed({
2740
declarations: [ServiceFormComponent],
28-
providers: [NgbActiveModal],
41+
providers: [NgbActiveModal, { provide: PoolService, useClass: MockPoolService }],
2942
imports: [
3043
HttpClientTestingModule,
3144
NgbTypeaheadModule,
@@ -389,22 +402,54 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA==
389402

390403
describe('should test service nvmeof', () => {
391404
beforeEach(() => {
405+
component.serviceType = 'nvmeof';
392406
formHelper.setValue('service_type', 'nvmeof');
393-
formHelper.setValue('service_id', 'svc');
394-
formHelper.setValue('pool', 'xyz');
395-
formHelper.setValue('group', 'abc');
407+
component.ngOnInit();
408+
fixture.detectChanges();
396409
});
397410

398-
it('should submit nvmeof', () => {
399-
component.onSubmit();
400-
expect(cephServiceService.create).toHaveBeenCalledWith({
401-
service_type: 'nvmeof',
402-
service_id: 'svc',
403-
placement: {},
404-
unmanaged: false,
405-
pool: 'xyz',
406-
group: 'abc'
407-
});
411+
it('should set rbd pools correctly onInit', () => {
412+
expect(component.pools.length).toBe(3);
413+
expect(component.rbdPools.length).toBe(2);
414+
});
415+
416+
it('should set default values correctly onInit', () => {
417+
expect(form.get('service_type').value).toBe('nvmeof');
418+
expect(form.get('group').value).toBe('default');
419+
expect(form.get('pool').value).toBe('rbd');
420+
expect(form.get('service_id').value).toBe('rbd.default');
421+
});
422+
423+
it('should reflect correct values on group change', () => {
424+
// Initially the group value should be 'default'
425+
expect(component.serviceForm.get('group')?.value).toBe('default');
426+
const groupInput = fixture.debugElement.query(By.css('#group')).nativeElement;
427+
// Simulate input value change
428+
groupInput.value = 'foo';
429+
// Trigger the input event
430+
groupInput.dispatchEvent(new Event('input'));
431+
// Trigger the change event
432+
groupInput.dispatchEvent(new Event('change'));
433+
fixture.detectChanges();
434+
// Verify values after change
435+
expect(form.get('group').value).toBe('foo');
436+
expect(form.get('service_id').value).toBe('rbd.foo');
437+
});
438+
439+
it('should reflect correct values on pool change', () => {
440+
// Initially the pool value should be 'rbd'
441+
expect(component.serviceForm.get('pool')?.value).toBe('rbd');
442+
const poolInput = fixture.debugElement.query(By.css('#pool')).nativeElement;
443+
// Simulate input value change
444+
poolInput.value = 'pool-2';
445+
// Trigger the input event
446+
poolInput.dispatchEvent(new Event('input'));
447+
// Trigger the change event
448+
poolInput.dispatchEvent(new Event('change'));
449+
fixture.detectChanges();
450+
// Verify values after change
451+
expect(form.get('pool').value).toBe('pool-2');
452+
expect(form.get('service_id').value).toBe('pool-2.default');
408453
});
409454

410455
it('should throw error when there is no service id', () => {
@@ -418,6 +463,23 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA==
418463
it('should throw error when there is no group', () => {
419464
formHelper.expectErrorChange('group', '', 'required');
420465
});
466+
467+
it('should hide the count element when service_type is "nvmeof"', () => {
468+
const countEl = fixture.debugElement.query(By.css('#count'));
469+
expect(countEl).toBeNull();
470+
});
471+
472+
it('should submit nvmeof', () => {
473+
component.onSubmit();
474+
expect(cephServiceService.create).toHaveBeenCalledWith({
475+
service_type: 'nvmeof',
476+
service_id: 'rbd.default',
477+
placement: {},
478+
unmanaged: false,
479+
pool: 'rbd',
480+
group: 'default'
481+
});
482+
});
421483
});
422484

423485
describe('should test service smb', () => {
@@ -650,6 +712,15 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA==
650712
const poolId = fixture.debugElement.query(By.css('#pool')).nativeElement;
651713
expect(poolId.disabled).toBeTruthy();
652714
});
715+
716+
it('should not edit groups for nvmeof service', () => {
717+
component.serviceType = 'nvmeof';
718+
formHelper.setValue('service_type', 'nvmeof');
719+
component.ngOnInit();
720+
fixture.detectChanges();
721+
const groupId = fixture.debugElement.query(By.css('#group')).nativeElement;
722+
expect(groupId.disabled).toBeTruthy();
723+
});
653724
});
654725
});
655726
});

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ export class ServiceFormComponent extends CdForm implements OnInit {
212212
]
213213
],
214214
group: [
215-
null,
215+
'default',
216216
CdValidators.requiredIf({
217217
service_type: 'nvmeof'
218218
})
@@ -855,7 +855,6 @@ export class ServiceFormComponent extends CdForm implements OnInit {
855855
this.serviceForm.get('count').setValue(2);
856856
break;
857857
case 'iscsi':
858-
case 'nvmeof':
859858
case 'cephfs-mirror':
860859
case 'nfs':
861860
case 'grafana':
@@ -962,16 +961,29 @@ export class ServiceFormComponent extends CdForm implements OnInit {
962961
);
963962
}
964963

965-
setNvmeofServiceId(): void {
966-
const defaultRbdPool: string = this.rbdPools?.find((p: Pool) => p.pool_name === 'rbd')
967-
?.pool_name;
968-
if (defaultRbdPool) {
969-
this.serviceForm.get('pool').setValue(defaultRbdPool);
970-
}
964+
onNvmeofGroupChange(groupName: string) {
965+
const pool = this.serviceForm.get('pool').value;
966+
if (pool) this.serviceForm.get('service_id').setValue(`${pool}.${groupName}`);
967+
else this.serviceForm.get('service_id').setValue(groupName);
971968
}
972969

973-
onNvmeofGroupChange(groupName: string) {
974-
this.serviceForm.get('service_id').setValue(groupName);
970+
getDefaultBlockPool(): string {
971+
// returns 'rbd' pool otherwise the first block pool
972+
return (
973+
this.rbdPools?.find((p: Pool) => p.pool_name === 'rbd')?.pool_name ||
974+
this.rbdPools?.[0].pool_name
975+
);
976+
}
977+
978+
setNvmeofServiceIdAndPool(): void {
979+
const defaultBlockPool: string = this.getDefaultBlockPool();
980+
const group: string = this.serviceForm.get('group').value;
981+
if (defaultBlockPool && group) {
982+
this.serviceForm.get('pool').setValue(defaultBlockPool);
983+
this.serviceForm.get('service_id').setValue(`${defaultBlockPool}.${group}`);
984+
} else {
985+
this.serviceForm.get('service_id').setValue(null);
986+
}
975987
}
976988

977989
requiresServiceId(serviceType: string) {
@@ -981,7 +993,7 @@ export class ServiceFormComponent extends CdForm implements OnInit {
981993
setServiceId(serviceId: string): void {
982994
const requiresServiceId: boolean = this.requiresServiceId(serviceId);
983995
if (requiresServiceId && serviceId === 'nvmeof') {
984-
this.setNvmeofServiceId();
996+
this.setNvmeofServiceIdAndPool();
985997
} else if (requiresServiceId) {
986998
this.serviceForm.get('service_id').setValue(null);
987999
} else {
@@ -1019,7 +1031,10 @@ export class ServiceFormComponent extends CdForm implements OnInit {
10191031

10201032
onBlockPoolChange() {
10211033
const selectedBlockPool = this.serviceForm.get('pool').value;
1022-
if (selectedBlockPool) {
1034+
const group = this.serviceForm.get('group').value;
1035+
if (selectedBlockPool && group) {
1036+
this.serviceForm.get('service_id').setValue(`${selectedBlockPool}.${group}`);
1037+
} else if (selectedBlockPool) {
10231038
this.serviceForm.get('service_id').setValue(selectedBlockPool);
10241039
} else {
10251040
this.serviceForm.get('service_id').setValue(null);

src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,10 @@ export class Mocks {
423423
return { name, type, type_id, id, children, device_class };
424424
}
425425

426-
static getPool = (name: string, id: number): Pool => {
426+
static getPool = (name: string, id: number, application_metadata: string[] = ['rbd']): Pool => {
427427
return _.merge(new Pool(name), {
428428
pool: id,
429+
application_metadata,
429430
type: 'replicated',
430431
pg_num: 256,
431432
pg_placement_num: 256,

0 commit comments

Comments
 (0)