From 1c65a8afa54d3f5200cc5776c4ce3ce237a19e3c Mon Sep 17 00:00:00 2001 From: pujashahu Date: Thu, 11 Jun 2026 23:35:20 +0530 Subject: [PATCH] mgr/dashboard: NVME- Remove block pool from create gateway page Adds unit test for creating gateway service with encryption when enabled. Fixes: https://tracker.ceph.com/issues/77359 Signed-off-by: pujashahu --- .../nvmeof-group-form.component.html | 34 +----- .../nvmeof-group-form.component.spec.ts | 101 ++++++------------ .../nvmeof-group-form.component.ts | 33 +----- .../service-form/service-form.component.html | 39 ------- .../service-form.component.spec.ts | 53 ++------- .../service-form/service-form.component.ts | 21 +--- 6 files changed, 43 insertions(+), 238 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.html index 27a5de86421..4be2ead948b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.html @@ -50,39 +50,7 @@ - -
-
- - - - - - - This field is required. - -
-
- +
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.spec.ts index 9af7f2ff76f..5b60872cd1b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.spec.ts @@ -1,7 +1,7 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; import { ReactiveFormsModule } from '@angular/forms'; import { RouterTestingModule } from '@angular/router/testing'; -import { ComponentFixture, TestBed, fakeAsync, tick } from '@angular/core/testing'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Router } from '@angular/router'; import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; @@ -14,7 +14,6 @@ import { SharedModule } from '~/app/shared/shared.module'; import { NvmeofGroupFormComponent } from './nvmeof-group-form.component'; import { CheckboxModule, GridModule, InputModule, SelectModule } from 'carbon-components-angular'; -import { PoolService } from '~/app/shared/api/pool.service'; import { TaskWrapperService } from '~/app/shared/services/task-wrapper.service'; import { CephServiceService } from '~/app/shared/api/ceph-service.service'; import { FormHelper } from '~/testing/unit-test-helper'; @@ -24,17 +23,10 @@ describe('NvmeofGroupFormComponent', () => { let fixture: ComponentFixture; let form: CdFormGroup; let formHelper: FormHelper; - let poolService: PoolService; let taskWrapperService: TaskWrapperService; let cephServiceService: CephServiceService; let router: Router; - const mockPools = [ - { pool_name: 'rbd', application_metadata: ['rbd'] }, - { pool_name: 'rbd', application_metadata: ['rbd'] }, - { pool_name: 'pool2', application_metadata: ['rgw'] } - ]; - beforeEach(async () => { await TestBed.configureTestingModule({ declarations: [NvmeofGroupFormComponent], @@ -55,13 +47,10 @@ describe('NvmeofGroupFormComponent', () => { fixture = TestBed.createComponent(NvmeofGroupFormComponent); component = fixture.componentInstance; - poolService = TestBed.inject(PoolService); taskWrapperService = TestBed.inject(TaskWrapperService); cephServiceService = TestBed.inject(CephServiceService); router = TestBed.inject(Router); - spyOn(poolService, 'list').and.returnValue(Promise.resolve(mockPools)); - component.ngOnInit(); form = component.groupForm; formHelper = new FormHelper(form); @@ -75,6 +64,7 @@ describe('NvmeofGroupFormComponent', () => { it('should initialize form with empty fields', () => { expect(form.controls.groupName.value).toBeNull(); expect(form.controls.unmanaged.value).toBe(false); + expect(form.controls.enableEncryption.value).toBe(false); }); it('should set action to CREATE on init', () => { @@ -91,57 +81,15 @@ describe('NvmeofGroupFormComponent', () => { formHelper.expectError('groupName', 'required'); }); - it('should require pool', () => { - formHelper.setValue('pool', null); - formHelper.expectError('pool', 'required'); - }); - - it('should be valid when groupName and pool are set', () => { + it('should be valid when groupName is set', () => { formHelper.setValue('groupName', 'test-group'); - formHelper.setValue('pool', 'rbd'); - expect(form.valid).toBe(true); + expect(form.controls.groupName.valid).toBe(true); }); - }); - describe('loadPools', () => { - it('should load pools and filter by rbd application metadata', fakeAsync(() => { - component.loadPools(); - tick(); - expect(component.pools.length).toBe(2); - expect(component.pools.map((p) => p.pool_name)).toEqual(['rbd', 'rbd']); - })); - - it('should set default pool to rbd if available', fakeAsync(() => { - component.groupForm.get('pool').setValue(null); - component.loadPools(); - tick(); - expect(component.groupForm.get('pool').value).toBe('rbd'); - })); - - it('should set first pool if rbd is not available', fakeAsync(() => { - component.groupForm.get('pool').setValue(null); - const poolsWithoutRbd = [{ pool_name: 'custom-pool', application_metadata: ['rbd'] }]; - (poolService.list as jasmine.Spy).and.returnValue(Promise.resolve(poolsWithoutRbd)); - component.loadPools(); - tick(); - expect(component.groupForm.get('pool').value).toBe('custom-pool'); - })); - - it('should handle empty pools', fakeAsync(() => { - (poolService.list as jasmine.Spy).and.returnValue(Promise.resolve([])); - component.loadPools(); - tick(); - expect(component.pools.length).toBe(0); - expect(component.poolsLoading).toBe(false); - })); - - it('should handle pool loading error', fakeAsync(() => { - (poolService.list as jasmine.Spy).and.returnValue(Promise.reject('error')); - component.loadPools(); - tick(); - expect(component.pools).toEqual([]); - expect(component.poolsLoading).toBe(false); - })); + it('should validate groupName for invalid characters', () => { + formHelper.setValue('groupName', 'test@group'); + formHelper.expectError('groupName', 'invalidChars'); + }); }); describe('onSubmit', () => { @@ -158,7 +106,6 @@ describe('NvmeofGroupFormComponent', () => { } as any; component.groupForm.get('groupName').setValue('test-group'); - component.groupForm.get('pool').setValue('rbd'); component.onSubmit(); expect(cephServiceService.create).not.toHaveBeenCalled(); @@ -170,16 +117,14 @@ describe('NvmeofGroupFormComponent', () => { getSelectedHostnames: (): string[] => ['host1', 'host2'] } as any; - component.groupForm.get('groupName').setValue('defalut'); - component.groupForm.get('pool').setValue('rbd'); + component.groupForm.get('groupName').setValue('default'); component.groupForm.get('unmanaged').setValue(false); component.onSubmit(); expect(cephServiceService.create).toHaveBeenCalledWith({ service_type: 'nvmeof', - service_id: 'rbd.defalut', - pool: 'rbd', - group: 'defalut', + service_id: 'default', + group: 'default', placement: { hosts: ['host1', 'host2'] }, @@ -194,15 +139,32 @@ describe('NvmeofGroupFormComponent', () => { } as any; component.groupForm.get('groupName').setValue('unmanaged-group'); - component.groupForm.get('pool').setValue('rbd'); component.groupForm.get('unmanaged').setValue(true); component.onSubmit(); expect(cephServiceService.create).toHaveBeenCalledWith( jasmine.objectContaining({ unmanaged: true, - group: 'unmanaged-group', - pool: 'rbd' + group: 'unmanaged-group' + }) + ); + }); + + it('should create service with encryption when enabled', () => { + component.gatewayNodeComponent = { + getSelectedHosts: (): any[] => [{ hostname: 'host1' }], + getSelectedHostnames: (): string[] => ['host1'] + } as any; + + component.groupForm.get('groupName').setValue('encrypted-group'); + component.groupForm.get('enableEncryption').setValue(true); + component.groupForm.get('encryptionConfig').setValue('encryption-key-123'); + component.onSubmit(); + + expect(cephServiceService.create).toHaveBeenCalledWith( + jasmine.objectContaining({ + group: 'encrypted-group', + encryption_key: 'encryption-key-123' }) ); }); @@ -214,7 +176,6 @@ describe('NvmeofGroupFormComponent', () => { } as any; component.groupForm.get('groupName').setValue('test-group'); - component.groupForm.get('pool').setValue('rbd'); component.onSubmit(); expect(router.navigateByUrl).toHaveBeenCalledWith('/block/nvmeof/gateways'); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.ts index 533025735bc..c1fb56b251e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/nvmeof-group-form/nvmeof-group-form.component.ts @@ -6,8 +6,6 @@ import { CdFormGroup } from '~/app/shared/forms/cd-form-group'; import { Permission } from '~/app/shared/models/permissions'; import { AuthStorageService } from '~/app/shared/services/auth-storage.service'; -import { PoolService } from '~/app/shared/api/pool.service'; -import { Pool } from '../../pool/pool'; import { NvmeofGatewayNodeComponent } from '../nvmeof-gateway-node/nvmeof-gateway-node.component'; import { TaskWrapperService } from '~/app/shared/services/task-wrapper.service'; import { CephServiceService } from '~/app/shared/api/ceph-service.service'; @@ -30,15 +28,12 @@ export class NvmeofGroupFormComponent extends CdForm implements OnInit { action: string; resource: string; group: string; - pools: Pool[] = []; - poolsLoading = false; pageURL: string; hasAvailableNodes = true; constructor( private authStorageService: AuthStorageService, public actionLabels: ActionLabelsI18n, - private poolService: PoolService, private taskWrapperService: TaskWrapperService, private cephServiceService: CephServiceService, private nvmeofService: NvmeofService, @@ -52,7 +47,6 @@ export class NvmeofGroupFormComponent extends CdForm implements OnInit { ngOnInit() { this.action = this.actionLabels.CREATE; this.createForm(); - this.loadPools(); } createForm() { @@ -68,9 +62,6 @@ export class NvmeofGroupFormComponent extends CdForm implements OnInit { ], [CdValidators.unique(this.nvmeofService.exists, this.nvmeofService)] ), - pool: new UntypedFormControl('rbd', { - validators: [Validators.required] - }), unmanaged: new UntypedFormControl(false), enableEncryption: new UntypedFormControl(false), encryptionConfig: new UntypedFormControl(null) @@ -118,27 +109,6 @@ export class NvmeofGroupFormComponent extends CdForm implements OnInit { return false; } - loadPools() { - this.poolsLoading = true; - this.poolService.list().then( - (pools: Pool[]) => { - this.pools = (pools || []).filter( - (pool: Pool) => pool.application_metadata && pool.application_metadata.includes('rbd') - ); - this.poolsLoading = false; - if (this.pools.length >= 1) { - const allPoolNames = this.pools.map((pool) => pool.pool_name); - const poolName = allPoolNames.includes('rbd') ? 'rbd' : this.pools[0].pool_name; - this.groupForm.patchValue({ pool: poolName }); - } - }, - () => { - this.pools = []; - this.poolsLoading = false; - } - ); - } - onSubmit() { if (this.groupForm.invalid) { return; @@ -156,12 +126,11 @@ export class NvmeofGroupFormComponent extends CdForm implements OnInit { return; } let taskUrl = `service/${URLVerbs.CREATE}`; - const serviceName = `${formValues.pool}.${formValues.groupName}`; + const serviceName = `${formValues.groupName}`; const serviceSpec: Record = { service_type: 'nvmeof', service_id: serviceName, - pool: formValues.pool, group: formValues.groupName, placement: { hosts: selectedHostnames diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html index c77c36abc14..0e7ecb82f24 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html @@ -111,45 +111,6 @@
} - - - @if (serviceForm.controls.service_type.value === 'nvmeof') { -
- - @if (rbdPools === null) { - - } - @if (rbdPools && rbdPools.length === 0) { - - } - @if (rbdPools && rbdPools.length > 0) { - - } - @for (pool of rbdPools; track pool) { - - } - - - - @if (serviceForm.showError('pool', frm, 'required')) { - This field is required. - } - -
- } - @if (serviceForm.controls.service_type.value === 'nvmeof') {
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.spec.ts index 915c320bacc..a1c2590e20f 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.spec.ts @@ -511,48 +511,27 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA== it('should set default values correctly onInit', () => { expect(form.get('service_type').value).toBe('nvmeof'); expect(form.get('group').value).toBe('default'); - expect(form.get('pool').value).toBe('rbd'); - expect(form.get('service_id').value).toBe('rbd.default'); + expect(form.get('service_id').value).toBe('default'); }); it('should reflect correct values on group change', () => { - // Initially the group value should be 'default' expect(component.serviceForm.get('group')?.value).toBe('default'); const groupInput = fixture.debugElement.query(By.css('#group')).nativeElement; - // Simulate input value change groupInput.value = 'foo'; - // Trigger the input event groupInput.dispatchEvent(new Event('input')); - // Trigger the change event groupInput.dispatchEvent(new Event('change')); fixture.detectChanges(); - // Verify values after change expect(form.get('group').value).toBe('foo'); - expect(form.get('service_id').value).toBe('rbd.foo'); - }); - - it('should reflect correct values on pool change', () => { - // Initially the pool value should be 'rbd' - expect(component.serviceForm.get('pool')?.value).toBe('rbd'); - const poolInput = fixture.debugElement.query(By.css('#pool')).nativeElement; - // Simulate input value change - form.get('pool').setValue('pool-2'); - // Trigger the input event - poolInput.dispatchEvent(new Event('input')); - // Trigger the change event - poolInput.dispatchEvent(new Event('change')); - fixture.detectChanges(); - // Verify values after change - expect(form.get('pool').value).toBe('pool-2'); - expect(form.get('service_id').value).toBe('pool-2.default'); + expect(form.get('service_id').value).toBe('foo'); }); - it('should throw error when there is no service id', () => { - formHelper.expectErrorChange('service_id', '', 'required'); + it('should hide pool selector in create mode', () => { + const poolEl = fixture.debugElement.query(By.css('#pool')); + expect(poolEl).toBeNull(); }); - it('should throw error when there is no pool', () => { - formHelper.expectErrorChange('pool', '', 'required'); + it('should throw error when there is no service id', () => { + formHelper.expectErrorChange('service_id', '', 'required'); }); it('should throw error when there is no group', () => { @@ -583,10 +562,9 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA== component.onSubmit(); expect(cephServiceService.create).toHaveBeenCalledWith({ service_type: 'nvmeof', - service_id: 'rbd.default', + service_id: 'default', placement: {}, unmanaged: false, - pool: 'rbd', group: 'default', enable_auth: false }); @@ -602,10 +580,9 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA== component.onSubmit(); expect(cephServiceService.create).toHaveBeenCalledWith({ service_type: 'nvmeof', - service_id: 'rbd.default', + service_id: 'default', placement: {}, unmanaged: false, - pool: 'rbd', group: 'default', enable_auth: true, root_ca_cert: 'root_ca_cert', @@ -840,15 +817,6 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA== expect(serviceId.disabled).toBeTruthy(); }); - it('should not edit pools for nvmeof service', () => { - component.serviceType = 'nvmeof'; - formHelper.setValue('service_type', 'nvmeof'); - component.ngOnInit(); - fixture.detectChanges(); - const poolId = fixture.componentInstance.serviceForm.get('pool'); - expect(poolId.disabled).toBeTruthy(); - }); - it('should not edit groups for nvmeof service', () => { component.serviceType = 'nvmeof'; formHelper.setValue('service_type', 'nvmeof'); @@ -862,16 +830,13 @@ x4Ea7kGVgx9kWh5XjWz9wjZvY49UKIT5ppIAWPMbLl3UpfckiuNhTA== spyOn(cephServiceService, 'update').and.stub(); component.serviceType = 'nvmeof'; formHelper.setValue('service_type', 'nvmeof'); - formHelper.setValue('pool', 'rbd'); formHelper.setValue('group', 'default'); - // mTLS disabled formHelper.setValue('enable_mtls', false); component.onSubmit(); expect(cephServiceService.update).toHaveBeenCalledWith({ service_type: 'nvmeof', placement: {}, unmanaged: false, - pool: 'rbd', group: 'default', enable_auth: false }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts index fb5f36af337..86a074fef8c 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts @@ -222,9 +222,6 @@ export class ServiceFormComponent extends CdForm implements OnInit { [ CdValidators.requiredIf({ service_type: 'iscsi' - }), - CdValidators.requiredIf({ - service_type: 'nvmeof' }) ] ], @@ -782,7 +779,6 @@ export class ServiceFormComponent extends CdForm implements OnInit { } break; case 'nvmeof': - this.serviceForm.get('pool').setValue(response[0].spec.pool); this.serviceForm.get('group').setValue(response[0].spec.group); this.serviceForm.get('enable_mtls').setValue(response[0].spec?.enable_auth); this.serviceForm.get('root_ca_cert').setValue(response[0].spec?.root_ca_cert); @@ -1155,26 +1151,14 @@ export class ServiceFormComponent extends CdForm implements OnInit { } setNvmeServiceId() { - const pool = this.serviceForm.get('pool').value; const group = this.serviceForm.get('group').value; - if (pool && group) { - this.serviceForm.get('service_id').setValue(`${pool}.${group}`); - } else if (pool) { - this.serviceForm.get('service_id').setValue(pool); - } else if (group) { + if (group) { this.serviceForm.get('service_id').setValue(group); } else { this.serviceForm.get('service_id').setValue(null); } } - setNvmeDefaultPool() { - const defaultPool = - this.rbdPools?.find((p: Pool) => p.pool_name === 'rbd')?.pool_name || - this.rbdPools?.[0].pool_name; - this.serviceForm.get('pool').setValue(defaultPool); - } - requiresServiceId(serviceType: string) { return ['mds', 'rgw', 'nfs', 'iscsi', 'nvmeof', 'smb', 'ingress'].includes(serviceType); } @@ -1182,7 +1166,6 @@ export class ServiceFormComponent extends CdForm implements OnInit { setServiceId(serviceId: string): void { const requiresServiceId: boolean = this.requiresServiceId(serviceId); if (requiresServiceId && serviceId === 'nvmeof') { - this.setNvmeDefaultPool(); this.setNvmeServiceId(); } else if (requiresServiceId) { this.serviceForm.get('service_id').setValue(null); @@ -1231,7 +1214,6 @@ export class ServiceFormComponent extends CdForm implements OnInit { this.serviceForm.get('backend_service').disable(); break; case 'nvmeof': - this.serviceForm.get('pool').disable(); this.serviceForm.get('group').disable(); break; } @@ -1344,7 +1326,6 @@ export class ServiceFormComponent extends CdForm implements OnInit { break; case 'nvmeof': - serviceSpec['pool'] = values['pool']; serviceSpec['group'] = values['group']; serviceSpec['enable_auth'] = values['enable_mtls']; if (values['enable_mtls']) { -- 2.47.3