mgr/dashboard: Use right size in pool form

Currently the max size is determined by the number of OSDs, which is
compared with the maximum of the current crush rule.

The problem with that is, that this is wrong for every crush rule that
doesn't have OSDs as failure domain and that don't have the root of the
cluster set as root of the crush rule.

Now the crush map will be used to determine how many failure domains are
really available in the cluster and how many can really be used in the
end. This number now defines the maximum size you can enter.

The crush detail view will now the new attribute usable_size and hide
the redundant information steps, ruleset, type and rule_name.

Fixes: https://tracker.ceph.com/issues/44620
Signed-off-by: Stephan Müller <smueller@suse.com>

diff --git a/qa/tasks/mgr/dashboard/test_pool.py b/qa/tasks/mgr/dashboard/test_pool.py
index 1fd414d103ec55de7ff6bd034040f1aceb2daeb9..c2c25bc1aafe3e86afd204fdf73abe14256d69e5 100644 (file)
--- a/qa/tasks/mgr/dashboard/test_pool.py
+++ b/qa/tasks/mgr/dashboard/test_pool.py
@@ -416,4 +416,5 @@ class PoolTest(DashboardTestCase):
             'erasure_code_profiles': JList(JObj({}, allow_unknown=True)),
             'used_rules': JObj({}, allow_unknown=True),
             'used_profiles': JObj({}, allow_unknown=True),
+            'nodes': JList(JObj({}, allow_unknown=True)),
         }))

diff --git a/src/pybind/mgr/dashboard/controllers/pool.py b/src/pybind/mgr/dashboard/controllers/pool.py
index 945a82a177a498adc8b65c549de18e5d17f7372e..3e9db8e9c952923c4ef69310db6f3ea4f6f58d8a 100644 (file)
--- a/src/pybind/mgr/dashboard/controllers/pool.py
+++ b/src/pybind/mgr/dashboard/controllers/pool.py
@@ -262,4 +262,5 @@ class PoolUi(Pool):
             "erasure_code_profiles": profiles,
             "used_rules": used_rules,
             "used_profiles": used_profiles,
+            'nodes': mgr.get('osd_map_tree')['nodes']
         }

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 c49c710e13fd62279c98e418dca536a62ac40048..2a3cef163470d366b29e6a626ca9866740c8f76d 100644 (file)
--- 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
@@ -142,7 +142,7 @@
               <span class="invalid-feedback"
                     *ngIf="form.showError('size', formDir)"
                     i18n>The size specified is out of range. A value from
-                {{ getMinSize() }} to {{ getMaxSize() }} is valid.</span>
+                {{ getMinSize() }} to {{ getMaxSize() }} is usable.</span>
             </div>
           </div>
 
@@ -343,7 +343,8 @@
                     <tab i18n-heading
                          heading="Crush rule"
                          class="crush-rule-info">
-                      <cd-table-key-value [renderObjects]="true"
+                      <cd-table-key-value [renderObjects]="false"
+                                          [hideKeys]="['steps', 'ruleset', 'type', 'rule_name']"
                                           [data]="form.getValue('crushRule')"
                                           [autoReload]="false">
                       </cd-table-key-value>

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 d172c7c45635208e87372ae1923741e77d2598b6..9d91e08dd6a0d0f53796bd02461f2e8844084be4 100644 (file)
--- 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
@@ -18,6 +18,7 @@ import {
   FixtureHelper,
   FormHelper,
   i18nProviders,
+  Mocks,
   modalServiceShow
 } from '../../../../testing/unit-test-helper';
 import { NotFoundComponent } from '../../../core/not-found/not-found.component';
@@ -27,7 +28,6 @@ import { PoolService } from '../../../shared/api/pool.service';
 import { CriticalConfirmationModalComponent } from '../../../shared/components/critical-confirmation-modal/critical-confirmation-modal.component';
 import { SelectBadgesComponent } from '../../../shared/components/select-badges/select-badges.component';
 import { CdFormGroup } from '../../../shared/forms/cd-form-group';
-import { CrushRule } from '../../../shared/models/crush-rule';
 import { ErasureCodeProfile } from '../../../shared/models/erasure-code-profile';
 import { Permission } from '../../../shared/models/permissions';
 import { PoolFormInfo } from '../../../shared/models/pool-form-info';
@@ -38,7 +38,7 @@ import { PoolModule } from '../pool.module';
 import { PoolFormComponent } from './pool-form.component';
 
 describe('PoolFormComponent', () => {
-  const OSDS = 8;
+  let OSDS = 15;
   let formHelper: FormHelper;
   let fixtureHelper: FixtureHelper;
   let component: PoolFormComponent;
@@ -65,44 +65,6 @@ describe('PoolFormComponent', () => {
     expect(form.getValue('pgNum')).toBe(returnValue);
   };
 
-  const createCrushRule = ({
-    id = 0,
-    name = 'somePoolName',
-    min = 1,
-    max = 10,
-    type = 'replicated'
-  }: {
-    max?: number;
-    min?: number;
-    id?: number;
-    name?: string;
-    type?: string;
-  }) => {
-    const typeNumber = type === 'erasure' ? 3 : 1;
-    const rule = new CrushRule();
-    rule.max_size = max;
-    rule.min_size = min;
-    rule.rule_id = id;
-    rule.ruleset = typeNumber;
-    rule.rule_name = name;
-    rule.steps = [
-      {
-        item_name: 'default',
-        item: -1,
-        op: 'take'
-      },
-      {
-        num: 0,
-        type: 'osd',
-        op: 'choose_firstn'
-      },
-      {
-        op: 'emit'
-      }
-    ];
-    return rule;
-  };
-
   const expectValidSubmit = (
     pool: any,
     taskName = 'pool/create',
@@ -136,12 +98,12 @@ describe('PoolFormComponent', () => {
       compression_algorithms: ['snappy'],
       compression_modes: ['none', 'passive'],
       crush_rules_replicated: [
-        createCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }),
-        createCrushRule({ id: 1, min: 3, max: 18, name: 'rep2', type: 'replicated' }),
-        createCrushRule({ id: 2, min: 1, max: 9, name: 'used_rule', type: 'replicated' })
+        Mocks.getCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' }),
+        Mocks.getCrushRule({ id: 1, min: 3, max: 18, name: 'rep2', type: 'replicated' }),
+        Mocks.getCrushRule({ id: 2, min: 1, max: 9, name: 'used_rule', type: 'replicated' })
       ],
       crush_rules_erasure: [
-        createCrushRule({ id: 3, min: 1, max: 1, name: 'ecp1', type: 'erasure' })
+        Mocks.getCrushRule({ id: 3, min: 1, max: 1, name: 'ecp1', type: 'erasure' })
       ],
       erasure_code_profiles: [ecp1],
       pg_autoscale_default_mode: 'off',
@@ -151,7 +113,8 @@ describe('PoolFormComponent', () => {
       },
       used_profiles: {
         ecp1: ['some.other.pool.uses.it']
-      }
+      },
+      nodes: Mocks.generateSimpleCrushMap(3, 5)
     };
   };
 
@@ -333,15 +296,20 @@ describe('PoolFormComponent', () => {
     });
 
     it('validates size', () => {
+      component.info.nodes = Mocks.getCrushMap();
       formHelper.setValue('poolType', 'replicated');
       formHelper.expectValid('size');
-      formHelper.setValue('crushRule', {
-        min_size: 2,
-        max_size: 6
-      });
+      formHelper.setValue('crushRule', Mocks.getCrushRule({ min: 2, max: 6 })); // 3 OSDs usable
       formHelper.expectErrorChange('size', 1, 'min');
-      formHelper.expectErrorChange('size', 8, 'max');
-      formHelper.expectValidChange('size', 6);
+      formHelper.expectErrorChange('size', 4, 'max'); // More than usable
+      formHelper.expectValidChange('size', 3);
+
+      formHelper.setValue(
+        'crushRule',
+        Mocks.getCrushRule({ min: 1, max: 2, failureDomain: 'osd-rack' }) // 4 OSDs usable
+      );
+      formHelper.expectErrorChange('size', 4, 'max'); // More than rule allows
+      formHelper.expectValidChange('size', 2);
     });
 
     it('validates compression mode default value', () => {
@@ -455,9 +423,6 @@ describe('PoolFormComponent', () => {
   describe('pool type changes', () => {
     beforeEach(() => {
       component.ngOnInit();
-      createCrushRule({ id: 3, min: 1, max: 1, name: 'ep1', type: 'erasure' });
-      createCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' });
-      createCrushRule({ id: 1, min: 3, max: 18, name: 'rep2', type: 'replicated' });
     });
 
     it('should have a default replicated size of 3', () => {
@@ -516,7 +481,7 @@ describe('PoolFormComponent', () => {
 
       it('disables rule field if only one rule exists which is used in the disabled field', () => {
         infoReturn.crush_rules_replicated = [
-          createCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' })
+          Mocks.getCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' })
         ];
         setUpPoolComponent();
         formHelper.setValue('poolType', 'replicated');
@@ -546,10 +511,7 @@ describe('PoolFormComponent', () => {
 
   describe('getMaxSize and getMinSize', () => {
     const setCrushRule = ({ min, max }: { min?: number; max?: number }) => {
-      formHelper.setValue('crushRule', {
-        min_size: min,
-        max_size: max
-      });
+      formHelper.setValue('crushRule', Mocks.getCrushRule({ min, max }));
     };
 
     it('returns 0 if osd count is 0', () => {
@@ -570,23 +532,31 @@ describe('PoolFormComponent', () => {
       expect(component.getMaxSize()).toBe(6);
     });
 
-    it('returns 1 as minimum and the osd count as maximum if no crush rule is available', () => {
+    it('returns 1 as minimum and 3 as maximum if no crush rule is available', () => {
       expect(component.getMinSize()).toBe(1);
-      expect(component.getMaxSize()).toBe(OSDS);
+      expect(component.getMaxSize()).toBe(3);
     });
 
     it('returns the osd count as maximum if the rule maximum exceeds it', () => {
       setCrushRule({ max: 100 });
-      expect(component.getMaxSize()).toBe(OSDS);
+      expect(component.getMaxSize()).toBe(15);
     });
 
     it('should return the osd count as minimum if its lower the the rule minimum', () => {
-      setCrushRule({ min: 10 });
-      expect(component.getMinSize()).toBe(10);
+      setCrushRule({ min: 20 });
+      expect(component.getMinSize()).toBe(20);
       const control = form.get('crushRule');
       expect(control.invalid).toBe(true);
       formHelper.expectError(control, 'tooFewOsds');
     });
+
+    it('should get the right maximum if the device type is defined', () => {
+      formHelper.setValue(
+        'crushRule',
+        Mocks.getCrushRule({ min: 1, max: 5, itemName: 'default~ssd' })
+      );
+      expect(form.getValue('crushRule').usable_size).toBe(5);
+    });
   });
 
   describe('application metadata', () => {
@@ -712,6 +682,7 @@ describe('PoolFormComponent', () => {
 
     describe('pgCalc', () => {
       const PGS = 1;
+      OSDS = 8;
 
       const getValidCase = () => ({
         type: 'replicated',
@@ -818,7 +789,7 @@ describe('PoolFormComponent', () => {
           }
         };
       });
-      infoReturn.crush_rules_replicated.push(createCrushRule({ id: 8, name }));
+      infoReturn.crush_rules_replicated.push(Mocks.getCrushRule({ id: 8, name }));
       component.addCrushRule();
       expect(form.getValue('crushRule').rule_name).toBe(name);
     });
@@ -1331,7 +1302,7 @@ describe('PoolFormComponent', () => {
       pool.quota_max_bytes = 1024 * 1024 * 1024;
       pool.quota_max_objects = 3000;
 
-      createCrushRule({ name: 'someRule' });
+      Mocks.getCrushRule({ name: 'someRule' });
       spyOn(poolService, 'get').and.callFake(() => of(pool));
     });
 

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 ab5b26a5742b39ec06aaf494bab35bf564f94baf..8bdc4f06622b7f75c79dd312ab2a13c630b23723 100644 (file)
--- 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
@@ -12,6 +12,7 @@ import { Observable, Subscription } from 'rxjs';
 import { CrushRuleService } from '../../../shared/api/crush-rule.service';
 import { ErasureCodeProfileService } from '../../../shared/api/erasure-code-profile.service';
 import { PoolService } from '../../../shared/api/pool.service';
+import { CrushNodeSelectionClass } from '../../../shared/classes/crush.node.selection.class';
 import { CriticalConfirmationModalComponent } from '../../../shared/components/critical-confirmation-modal/critical-confirmation-modal.component';
 import { SelectOption } from '../../../shared/components/select/select-option.model';
 import { ActionLabelsI18n, URLVerbs } from '../../../shared/constants/app.constants';
@@ -340,6 +341,7 @@ export class PoolFormComponent extends CdForm implements OnInit {
       if (!rule) {
         return;
       }
+      this.setCorrectMaxSize(rule);
       this.crushRuleIsUsedBy(rule.rule_name);
       this.replicatedRuleChange();
       this.pgCalc();
@@ -431,17 +433,16 @@ export class PoolFormComponent extends CdForm implements OnInit {
   }
 
   getMaxSize(): number {
-    if (!this.info || this.info.osd_count < 1) {
+    const rule = this.form.getValue('crushRule');
+    if (!this.info) {
       return 0;
     }
-    const osds: number = this.info.osd_count;
-    if (this.form.getValue('crushRule')) {
-      const max: number = this.form.get('crushRule').value.max_size;
-      if (max < osds) {
-        return max;
-      }
+    if (!rule) {
+      const osds = this.info.osd_count;
+      const defaultSize = 3;
+      return Math.min(osds, defaultSize);
     }
-    return osds;
+    return rule.usable_size;
   }
 
   private pgCalc() {
@@ -462,6 +463,19 @@ export class PoolFormComponent extends CdForm implements OnInit {
     }
   }
 
+  private setCorrectMaxSize(rule: CrushRule = this.form.getValue('crushRule')) {
+    if (!rule) {
+      return;
+    }
+    const domains = CrushNodeSelectionClass.searchFailureDomains(
+      this.info.nodes,
+      rule.steps[0].item_name
+    );
+    const currentDomain = domains[rule.steps[1].type];
+    const usable = currentDomain ? currentDomain.length : rule.max_size;
+    rule.usable_size = Math.min(usable, rule.max_size);
+  }
+
   private replicatedPgCalc(pgs: number): number {
     const sizeControl = this.form.get('size');
     const size = sizeControl.value;

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts
index c7c6d56ca0b248396f12cf81c9c34374c84a9b57..83c1db6b6df2a4ec1e5529b1ec6d846c1fa4c621 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts
@@ -2,6 +2,7 @@ import { CrushStep } from './crush-step';
 
 export class CrushRule {
   max_size: number;
+  usable_size?: number;
   min_size: number;
   rule_id: number;
   rule_name: string;

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts
index 4eb09e67c3c1297cd8e9030e4508e5486a05bc64..c5cc0bb6d835f4957630806d6feb9d6795fabf3e 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts
@@ -1,3 +1,4 @@
+import { CrushNode } from './crush-node';
 import { CrushRule } from './crush-rule';
 import { ErasureCodeProfile } from './erasure-code-profile';
 
@@ -15,4 +16,5 @@ export class PoolFormInfo {
   erasure_code_profiles: ErasureCodeProfile[];
   used_rules: { [rule_name: string]: string[] };
   used_profiles: { [profile_name: string]: string[] };
+  nodes: CrushNode[];
 }