From: Abhishek Desai <abhishek.desai1@ibm.com>
Date: Fri, 20 Mar 2026 06:00:55 +0000 (+0530)
Subject: mgr/dashboard : Select replicated rule by default in pools form
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=195cc2f8c23b8ae327f769ab7aca0f460d6029ab;p=ceph.git

mgr/dashboard : Select replicated rule by default in pools form
fixes : https://tracker.ceph.com/issues/75632
Signed-off-by: Abhishek Desai <abhishek.desai1@ibm.com>
---

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-data.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-data.ts
index ad5672aee44a..38250f685786 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-data.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-data.ts
@@ -1,7 +1,7 @@
 import { Validators } from '@angular/forms';
 
 import { SelectMessages } from '~/app/shared/components/select/select-messages.model';
-import { Pool } from '../pool';
+import { Pool, PoolType } from '../pool';
 
 export class PoolFormData {
   poolTypes: string[];
@@ -10,13 +10,13 @@ export class PoolFormData {
   applications: any;
 
   readonly APP_LABELS: Record<string, string> = {
-    cephfs: $localize`Filesystem (CephFS)`,
+    cephfs: $localize`File system (CephFS)`,
     rbd: $localize`Block (RBD)`,
     rgw: $localize`Object (RGW)`
   };
 
   constructor() {
-    this.poolTypes = ['erasure', 'replicated'];
+    this.poolTypes = [PoolType.ERASURE, PoolType.REPLICATED];
     this.applications = {
       selected: [],
       default: ['cephfs', 'rbd', 'rgw'],
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
index 9b44da73c00b..f4d207d0bf8f 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
@@ -268,29 +268,13 @@
             >
             </cds-number>
             <ng-template #sizeError>
-              @if (form.showError('size', formDir)) {
-              <span class="invalid-feedback">
-                <ul class="list-inline">
-                  <li
-                    i18n
-                  >
-                    Minimum: {{ getMinSize() }}
-                  </li>
-                  <li
-                    i18n
-                  >
-                    Maximum: {{ getMaxSize() }}
-                  </li>
-                </ul>
-              </span>
-              }
               @if (form.showError('size', formDir)) {
               <span
                 class="invalid-feedback"
                 i18n
               >
-                The size specified is out of range. A value from {{ getMinSize() }} to
-                {{ getMaxSize() }} is usable.
+                Invalid input. Replicated size must be between {{ getMinSize() }} and
+                {{ getMaxSize() }}.
               </span>
               }
             </ng-template>
@@ -668,9 +652,6 @@
               [invalid]="crushRuleRef.isInvalid"
               [invalidText]="crushRuleError"
             >
-              <option [value]="null"
-                      i18n>-- Select a crush rule --
-              </option>
               @for (rule of current.rules; track rule.rule_name) {
               <option [value]="rule.rule_name">
                 {{ rule.rule_name }}
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
index b660da528b0c..f2b894a4dedc 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
@@ -26,7 +26,7 @@ import { ModalCdsService } from '~/app/shared/services/modal-cds.service';
 import { TaskWrapperService } from '~/app/shared/services/task-wrapper.service';
 import { SharedModule } from '~/app/shared/shared.module';
 import { configureTestBed, FixtureHelper, FormHelper, Mocks } from '~/testing/unit-test-helper';
-import { Pool } from '../pool';
+import { Pool, PoolType } from '../pool';
 import { PoolModule } from '../pool.module';
 import { PoolFormComponent } from './pool-form.component';
 
@@ -228,8 +228,8 @@ describe('PoolFormComponent', () => {
       expect(form.valid).toBeFalsy();
       // With default poolType 'replicated', expect 'name' required.
       ['name'].forEach((name) => formHelper.expectError(name, 'required'));
-      // For replicated type with multiple crush rules, 'crushRule' is required.
-      formHelper.expectError('crushRule', 'required');
+      // crushRule is auto-selected when replicated rules exist, so it is valid.
+      formHelper.expectValid('crushRule');
       // Other fields are valid by default.
       ['size', 'erasureProfile', 'ecOverwrites'].forEach((name) => formHelper.expectValid(name));
       expect(component.form.get('compression').valid).toBeTruthy();
@@ -279,10 +279,13 @@ describe('PoolFormComponent', () => {
       expect(form.valid).toBeTruthy();
     });
 
-    it('validates crushRule with multiple crush rules', () => {
+    it('auto-selects `crushRule` with multiple crush rules', () => {
       formHelper.expectValidChange('poolType', 'replicated');
       form.get('crushRule').updateValueAndValidity();
-      formHelper.expectError('crushRule', 'required'); // As multiple rules exist
+      expect(form.getValue('crushRule')).toEqual(
+        component.info.crush_rules_replicated[0].rule_name
+      );
+      formHelper.expectValid('crushRule');
     });
 
     it('validates crushRule with no crush rules', () => {
@@ -513,10 +516,22 @@ describe('PoolFormComponent', () => {
         expect(control.disabled).toBe(true);
       });
 
-      it('does not select the first rule if more than one exist', () => {
+      it('selects the first rule if `replicated_rule` does not exist', () => {
         formHelper.setValue('poolType', 'replicated');
         const control = form.get('crushRule');
-        expect(control.value).toEqual(null);
+        expect(control.value).toEqual(component.info.crush_rules_replicated[0].rule_name);
+        expect(control.disabled).toBe(false);
+      });
+
+      it('selects `replicated_rule` by default when available', () => {
+        infoReturn.crush_rules_replicated = [
+          Mocks.getCrushRule({ id: 0, name: 'rep1', type: 'replicated' }),
+          Mocks.getCrushRule({ id: 1, name: 'replicated_rule', type: 'replicated' }),
+          Mocks.getCrushRule({ id: 2, name: 'rep2', type: 'replicated' })
+        ];
+        setUpPoolComponent();
+        const control = form.get('crushRule');
+        expect(control.value).toEqual('replicated_rule');
         expect(control.disabled).toBe(false);
       });
 
@@ -546,6 +561,7 @@ describe('PoolFormComponent', () => {
     });
 
     it('returns 1 as minimum and 3 as maximum if no crush rule is available', () => {
+      component.selectedCrushRule = undefined;
       expect(component.getMinSize()).toBe(1);
       expect(component.getMaxSize()).toBe(3);
     });
@@ -746,7 +762,7 @@ describe('PoolFormComponent', () => {
       });
 
       const getValidCase = () => ({
-        type: 'replicated',
+        type: PoolType.REPLICATED,
         osds: OSDS,
         size: 4,
         ecp: {
@@ -777,7 +793,7 @@ describe('PoolFormComponent', () => {
         form.get('pgNum').markAsPristine();
         fixture.detectChanges();
 
-        if (type === 'replicated') {
+        if (type === PoolType.REPLICATED) {
           // Set a valid crush rule for replicated pools
           if (
             component.info.crush_rules_replicated &&
@@ -791,7 +807,7 @@ describe('PoolFormComponent', () => {
           // Explicitly call pgCalc() to ensure calculation happens with new values
           component['pgCalc']();
           fixture.detectChanges();
-        } else if (type === 'erasure') {
+        } else if (type === PoolType.ERASURE) {
           // For erasure code, initialize an ECP with the given k/m values
           if (ecp) {
             component['initEcp']([
@@ -814,7 +830,7 @@ describe('PoolFormComponent', () => {
 
       // TODO: These tests have state pollution from parent beforeEach that sets invalid crushRule
       it.skip('does not change anything if type is not valid', () => {
-        const test = getValidCase();
+        const test: Record<string, any> = getValidCase();
         test.type = '';
         test.expected = PGS;
         testPgCalc(test);
@@ -823,7 +839,7 @@ describe('PoolFormComponent', () => {
       it.skip('does not change anything if ecp is not valid', () => {
         const test = getValidCase();
         test.expected = PGS;
-        test.type = 'erasure';
+        test.type = PoolType.ERASURE;
         test.ecp = null;
         testPgCalc(test);
       });
@@ -851,14 +867,14 @@ describe('PoolFormComponent', () => {
       it('calculates erasure code values even if selection is disabled', () => {
         component['initEcp']([{ k: 2, m: 2, name: 'bla', plugin: '', technique: '' }]);
         const test = getValidCase();
-        test.type = 'erasure';
+        test.type = PoolType.ERASURE;
         testPgCalc(test);
         expect(form.get('erasureProfile').disabled).toBeTruthy();
       });
 
       it('calculates some erasure code values', () => {
         const test = getValidCase();
-        test.type = 'erasure';
+        test.type = PoolType.ERASURE;
         testPgCalc(test);
         test.osds = 16;
         test.ecp.m = 5;
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
index 02515ad6d708..56c182123f75 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
@@ -34,7 +34,7 @@ import { FormatterService } from '~/app/shared/services/formatter.service';
 import { TaskWrapperService } from '~/app/shared/services/task-wrapper.service';
 import { CrushRuleFormModalComponent } from '../crush-rule-form-modal/crush-rule-form-modal.component';
 import { ErasureCodeProfileFormModalComponent } from '../erasure-code-profile-form/erasure-code-profile-form-modal.component';
-import { Pool } from '../pool';
+import { Pool, PoolType } from '../pool';
 import { PoolFormData } from './pool-form-data';
 import { PoolEditModeResponseModel } from '../../block/mirroring/pool-edit-mode-modal/pool-edit-mode-response.model';
 import { RbdMirroringService } from '~/app/shared/api/rbd-mirroring.service';
@@ -63,6 +63,8 @@ interface MonitorResponse {
   standalone: false
 })
 export class PoolFormComponent extends CdForm implements OnInit {
+  private static readonly DEFAULT_RULE_NAME = 'replicated_rule';
+
   @ViewChild('crushInfoTabs') crushInfoTabs: NgbNav;
   @ViewChild('ecpInfoTabs') ecpInfoTabs: NgbNav;
   @ViewChild(FormGroupDirective)
@@ -185,7 +187,7 @@ export class PoolFormComponent extends CdForm implements OnInit {
             })
           ]
         }),
-        poolType: new UntypedFormControl('replicated', {
+        poolType: new UntypedFormControl(PoolType.REPLICATED, {
           validators: [Validators.required]
         }),
         crushRule: new UntypedFormControl(null, {
@@ -236,7 +238,7 @@ export class PoolFormComponent extends CdForm implements OnInit {
       }
       this.listenToChanges();
       this.setComplexValidators();
-      this.poolTypeChange('replicated');
+      this.poolTypeChange(PoolType.REPLICATED);
     });
     this.loadingReady();
   }
@@ -266,23 +268,45 @@ export class PoolFormComponent extends CdForm implements OnInit {
   private setListControlStatus(controlName: string, arr: any[]) {
     const control = this.form.get(controlName);
     const value = control.value;
-    if (arr.length === 1 && (!value || !_.isEqual(value, arr[0]))) {
-      if (controlName === 'erasureProfile') {
+
+    if (controlName === 'erasureProfile') {
+      if (arr.length === 1 && (!value || !_.isEqual(value, arr[0]))) {
         control.setValue(arr[0].name);
-      } else {
-        control.setValue(arr[0].rule_name);
+      } else if (arr.length === 0 && value) {
+        control.setValue(null);
+      }
+    } else {
+      const selectedRule = this.validateCrushRule(value, arr);
+      if (arr.length > 0 && selectedRule) {
+        control.setValue(selectedRule);
         this.replicatedRuleChange();
+      } else if (arr.length === 0 && value) {
+        control.setValue(null);
       }
-    } else if (arr.length === 0 && value) {
-      control.setValue(null);
     }
-    if (arr.length <= 1) {
-      if (control.enabled) {
-        control.disable();
-      }
-    } else if (control.disabled) {
-      control.enable();
+
+    arr.length <= 1 ? control.disable() : control.enable();
+  }
+
+  private validateCrushRule(
+    value: string | { rule_name?: string; name?: string } | null,
+    arr: CrushRule[]
+  ): string | undefined {
+    type CrushRuleWithOptionalName = CrushRule & { name?: string };
+    const selectedName = typeof value === 'string' ? value : value?.rule_name || value?.name;
+    const isSelected =
+      selectedName &&
+      arr.some(
+        (rule: CrushRuleWithOptionalName) =>
+          rule.rule_name === selectedName || rule.name === selectedName
+      );
+    if (isSelected) {
+      return undefined;
     }
+    const defaultRule =
+      arr.find((rule: CrushRule) => rule.rule_name === PoolFormComponent.DEFAULT_RULE_NAME) ||
+      arr[0];
+    return defaultRule?.rule_name;
   }
 
   private initEditMode() {
@@ -364,10 +388,15 @@ export class PoolFormComponent extends CdForm implements OnInit {
   }
 
   private setAvailableApps(apps: string[] = this.data.applications.default) {
+    type SelectOptionWithContent = SelectOption & { content?: string };
     const selectedApps = this.data.applications.selected || [];
     this.data.applications.available = _.uniq(apps.sort()).map((x: string) => {
-      const option = new SelectOption(selectedApps.includes(x), x, this.data.APP_LABELS?.[x] || x);
-      (option as any).content = this.data.APP_LABELS?.[x] || x;
+      const option: SelectOptionWithContent = new SelectOption(
+        selectedApps.includes(x),
+        x,
+        this.data.APP_LABELS?.[x] || x
+      );
+      option.content = this.data.APP_LABELS?.[x] || x;
       return option;
     });
   }
@@ -417,7 +446,7 @@ export class PoolFormComponent extends CdForm implements OnInit {
       this.data.crushInfo = false;
 
       rule = (this.current.rules || []).find(
-        (r: CrushRule) => r.rule_name === rule || (r as any).name === rule
+        (r: CrushRule & { name?: string }) => r.rule_name === rule || r.name === rule
       );
       this.selectedCrushRule = rule;
       this.setCorrectMaxSize(rule);
@@ -460,9 +489,9 @@ export class PoolFormComponent extends CdForm implements OnInit {
   }
 
   private poolTypeChange(poolType: string) {
-    if (poolType === 'replicated') {
+    if (poolType === PoolType.REPLICATED) {
       this.setTypeBooleans(true, false);
-    } else if (poolType === 'erasure') {
+    } else if (poolType === PoolType.ERASURE) {
       this.setTypeBooleans(false, true);
     } else {
       this.setTypeBooleans(false, false);
@@ -538,11 +567,11 @@ export class PoolFormComponent extends CdForm implements OnInit {
   }
 
   getMaxSize(): number {
-    const rule = this.selectedCrushRule;
-    if (!this.info) {
+    if (!this.info || this.info.osd_count < 1) {
       return 0;
     }
     if (this.isStretchMode) return this.STRETCH_REPLICATED_MAX_SIZE;
+    const rule = this.selectedCrushRule;
     if (!rule) {
       return Math.min(this.info.osd_count, this.DEFAULT_REPLICATED_MAX_SIZE);
     }
@@ -818,7 +847,7 @@ export class PoolFormComponent extends CdForm implements OnInit {
       getInfo: () => this.poolService.getInfo(),
       initInfo: (info) => {
         this.initInfo(info);
-        this.poolTypeChange('replicated');
+        this.poolTypeChange(PoolType.REPLICATED);
       },
       findNewItem: () =>
         this.info.crush_rules_replicated.find((rule) => rule.rule_name === ruleName),
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts
index 690aa74784d6..7f07dd94296f 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts
@@ -308,6 +308,7 @@ describe('PoolListComponent', () => {
     const getPoolData = (o: object) => [
       _.merge(
         _.merge(Mocks.getPool('a', 0), {
+          application_metadata: ['Block'],
           cdIsBinary: true,
           pg_status: '',
           stats: {
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts
index 5dc4c6643ad6..655709b5ce67 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts
@@ -27,7 +27,7 @@ import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
 import { TaskListService } from '~/app/shared/services/task-list.service';
 import { TaskWrapperService } from '~/app/shared/services/task-wrapper.service';
 import { URLBuilderService } from '~/app/shared/services/url-builder.service';
-import { Pool } from '../pool';
+import { Pool, PoolType } from '../pool';
 import { PoolStat, PoolStats } from '../pool-stat';
 import { ModalCdsService } from '~/app/shared/services/modal-cds.service';
 import { DeletionImpact } from '~/app/shared/enum/delete-confirmation-modal-impact.enum';
@@ -267,9 +267,9 @@ export class PoolListComponent extends ListWithDetails implements OnInit {
     ];
     const emptyStat: PoolStat = { latest: 0, rate: 0, rates: [] };
     const applicationLabels: Record<string, string> = {
-      cephfs: $localize`filesystem`,
-      rbd: $localize`block`,
-      rgw: $localize`object`
+      cephfs: $localize`File system`,
+      rbd: $localize`Block`,
+      rgw: $localize`Object`
     };
 
     _.forEach(pools, (pool: Pool) => {
@@ -293,11 +293,11 @@ export class PoolListComponent extends ListWithDetails implements OnInit {
       });
       pool.cdIsBinary = true;
 
-      if (pool['type'] === 'erasure') {
+      if (pool['type'] === PoolType.ERASURE) {
         const erasureCodeProfile = pool['erasure_code_profile'];
         pool['data_protection'] = this.getErasureCodeProfile(erasureCodeProfile);
       }
-      if (pool['type'] === 'replicated') {
+      if (pool['type'] === PoolType.REPLICATED) {
         pool['data_protection'] = `replica: ×${pool['size']}`;
       }
 
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts
index 55c70c6f5597..d36f920407e6 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts
@@ -1,6 +1,11 @@
 import { ExecutingTask } from '~/app/shared/models/executing-task';
 import { PoolStats } from './pool-stat';
 
+export enum PoolType {
+  ERASURE = 'erasure',
+  REPLICATED = 'replicated'
+}
+
 export class Pool {
   cache_target_full_ratio_micro: number;
   fast_read: boolean;