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>
(cherry picked from commit c76926bcd2fc9ed737b7848239b814f7b75fddc9)

diff --git a/qa/tasks/mgr/dashboard/test_pool.py b/qa/tasks/mgr/dashboard/test_pool.py
index bf40ac2062638d36bd695483064a1a585bde698d..feacf61262177937f89b774d421852b63c0afa57 100644 (file)
--- a/qa/tasks/mgr/dashboard/test_pool.py
+++ b/qa/tasks/mgr/dashboard/test_pool.py
@@ -415,4 +415,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 67fba23f164565849d13aa1d53cc6384f06e09a1..10c0a114aa64cca0331f78fe5174ae41ccc35e59 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
@@ -145,7 +145,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>
 
@@ -346,7 +346,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 c4ee052ae1f555d9c9fd7cc2cb0083b253e5da81..a44571b78281033d625a04d94c7c55a25545bb2f 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)
     };
   };
 
@@ -331,15 +294,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', () => {
@@ -453,9 +421,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', () => {
@@ -514,7 +479,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');
@@ -544,10 +509,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', () => {
@@ -568,23 +530,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', () => {
@@ -710,6 +680,7 @@ describe('PoolFormComponent', () => {
 
     describe('pgCalc', () => {
       const PGS = 1;
+      OSDS = 8;
 
       const getValidCase = () => ({
         type: 'replicated',
@@ -816,7 +787,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);
     });
@@ -1329,7 +1300,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 47519c0a0a34007aab3c4d81e6f6844676337631..797a58c477866f442b9ecbf7c1b2b00dfd0dbca8 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';
@@ -336,6 +337,7 @@ export class PoolFormComponent implements OnInit {
       if (!rule) {
         return;
       }
+      this.setCorrectMaxSize(rule);
       this.crushRuleIsUsedBy(rule.rule_name);
       this.replicatedRuleChange();
       this.pgCalc();
@@ -427,17 +429,16 @@ export class PoolFormComponent 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() {
@@ -458,6 +459,19 @@ export class PoolFormComponent 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[];
 }