]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: remove max/min_size and ruleset
authorSage Weil <sage@newdream.net>
Tue, 29 Jun 2021 21:01:23 +0000 (17:01 -0400)
committerAvan Thakkar <athakkar@redhat.com>
Thu, 1 Jul 2021 18:40:14 +0000 (00:10 +0530)
Signed-off-by: Sage Weil <sage@newdream.net>
Signed-off-by: Avan Thakkar <athakkar@redhat.com>
qa/tasks/mgr/dashboard/test_crush_rule.py
src/pybind/mgr/dashboard/controllers/crush_rule.py
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/models/crush-rule.ts
src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts
src/pybind/mgr/dashboard/openapi.yaml

index 1e37553b24a006991caf3a78d3f95f0a34b4e1e1..aa2250b1d2a2eb3768924c575cc9d2288724c1fb 100644 (file)
@@ -10,11 +10,8 @@ class CrushRuleTest(DashboardTestCase):
     AUTH_ROLES = ['pool-manager']
 
     rule_schema = JObj(sub_elems={
-        'max_size': int,
-        'min_size': int,
         'rule_id': int,
         'rule_name': str,
-        'ruleset': int,
         'steps': JList(JObj({}, allow_unknown=True))
     }, allow_unknown=True)
 
@@ -24,7 +21,7 @@ class CrushRuleTest(DashboardTestCase):
         self._post('/api/crush_rule', data)
         self.assertStatus(201)
         # Makes sure rule exists
-        rule = self._get('/api/crush_rule/{}'.format(name))
+        rule = self._get('/api/crush_rule/{}'.format(name), version='2.0')
         self.assertStatus(200)
         self.assertSchemaBody(self.rule_schema)
         self.assertEqual(rule['rule_name'], name)
@@ -34,12 +31,12 @@ class CrushRuleTest(DashboardTestCase):
 
     @DashboardTestCase.RunAs('test', 'test', ['rgw-manager'])
     def test_read_access_permissions(self):
-        self._get('/api/crush_rule')
+        self._get('/api/crush_rule', version='2.0')
         self.assertStatus(403)
 
     @DashboardTestCase.RunAs('test', 'test', ['read-only'])
     def test_write_access_permissions(self):
-        self._get('/api/crush_rule')
+        self._get('/api/crush_rule', version='2.0')
         self.assertStatus(200)
         data = {'name': 'some_rule', 'root': 'default', 'failure_domain': 'osd'}
         self._post('/api/crush_rule', data)
@@ -54,7 +51,7 @@ class CrushRuleTest(DashboardTestCase):
         cls._ceph_cmd(['osd', 'crush', 'rule', 'rm', 'another_rule'])
 
     def test_list(self):
-        self._get('/api/crush_rule')
+        self._get('/api/crush_rule', version='2.0')
         self.assertStatus(200)
         self.assertSchemaBody(JList(self.rule_schema))
 
index 13986f384132122399b4df64f7d97aa7c18536a2..57f05fd39e0a0b7bcdd3363382d65388fcfa49f9 100644 (file)
@@ -11,10 +11,7 @@ from . import ApiController, ControllerDoc, Endpoint, EndpointDoc, \
 LIST_SCHEMA = {
     "rule_id": (int, 'Rule ID'),
     "rule_name": (str, 'Rule Name'),
-    "ruleset": (int, 'RuleSet related to the rule'),
     "type": (int, 'Type of Rule'),
-    "min_size": (int, 'Minimum size of Rule'),
-    "max_size": (int, 'Maximum size of Rule'),
     'steps': ([{str}], 'Steps included in the rule')
 }
 
@@ -24,9 +21,11 @@ LIST_SCHEMA = {
 class CrushRule(RESTController):
     @EndpointDoc("List Crush Rule Configuration",
                  responses={200: LIST_SCHEMA})
+    @RESTController.MethodMap(version='2.0')
     def list(self):
         return mgr.get('osd_map_crush')['rules']
 
+    @RESTController.MethodMap(version='2.0')
     def get(self, name):
         rules = mgr.get('osd_map_crush')['rules']
         for r in rules:
index dd45a4616459a6f911492e1685a16f4e4af08a1c..5b8b1459ea69e176d438e53c4cdab6c984866d50 100644 (file)
                          i18n>Crush rule</a>
                       <ng-template ngbNavContent>
                         <cd-table-key-value [renderObjects]="false"
-                                            [hideKeys]="['steps', 'ruleset', 'type', 'rule_name']"
+                                            [hideKeys]="['steps', 'type', 'rule_name']"
                                             [data]="form.getValue('crushRule')"
                                             [autoReload]="false">
                         </cd-table-key-value>
index 1d58a1778cec7b86331fb92a8ab4b3641db748f4..87b791b690307b537ac5df3a1df28e8503f380a1 100644 (file)
@@ -105,13 +105,11 @@ describe('PoolFormComponent', () => {
       compression_algorithms: ['snappy'],
       compression_modes: ['none', 'passive'],
       crush_rules_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: [
-        Mocks.getCrushRule({ id: 3, min: 1, max: 1, name: 'ecp1', type: 'erasure' })
+        Mocks.getCrushRule({ id: 0, name: 'rep1', type: 'replicated' }),
+        Mocks.getCrushRule({ id: 1, name: 'rep2', type: 'replicated' }),
+        Mocks.getCrushRule({ id: 2, name: 'used_rule', type: 'replicated' })
       ],
+      crush_rules_erasure: [Mocks.getCrushRule({ id: 3, name: 'ecp1', type: 'erasure' })],
       erasure_code_profiles: [ecp1],
       pg_autoscale_default_mode: 'off',
       pg_autoscale_modes: ['off', 'warn', 'on'],
@@ -289,7 +287,6 @@ describe('PoolFormComponent', () => {
       formHelper.expectValidChange('poolType', 'replicated');
       form.get('crushRule').updateValueAndValidity();
       formHelper.expectError('crushRule', 'required'); // As multiple rules exist
-      formHelper.expectErrorChange('crushRule', { min_size: 20 }, 'tooFewOsds');
     });
 
     it('validates crushRule with no crush rules', () => {
@@ -303,17 +300,6 @@ describe('PoolFormComponent', () => {
       component.info.nodes = Mocks.getCrushMap();
       formHelper.setValue('poolType', 'replicated');
       formHelper.expectValid('size');
-      formHelper.setValue('crushRule', Mocks.getCrushRule({ min: 2, max: 6 })); // 3 OSDs usable
-      formHelper.expectErrorChange('size', 1, 'min');
-      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 if warning is displayed when size is 1', () => {
@@ -460,13 +446,13 @@ describe('PoolFormComponent', () => {
 
       it('should set size to maximum if size exceeds maximum', () => {
         formHelper.setValue('crushRule', component.info.crush_rules_replicated[0]);
-        expect(form.getValue('size')).toBe(4);
+        expect(form.getValue('size')).toBe(10);
       });
 
       it('should set size to minimum if size is lower than minimum', () => {
         formHelper.setValue('size', -1);
         formHelper.setValue('crushRule', component.info.crush_rules_replicated[0]);
-        expect(form.getValue('size')).toBe(2);
+        expect(form.getValue('size')).toBe(1);
       });
     });
 
@@ -496,7 +482,7 @@ describe('PoolFormComponent', () => {
 
       it('disables rule field if only one rule exists which is used in the disabled field', () => {
         infoReturn.crush_rules_replicated = [
-          Mocks.getCrushRule({ id: 0, min: 2, max: 4, name: 'rep1', type: 'replicated' })
+          Mocks.getCrushRule({ id: 0, name: 'rep1', type: 'replicated' })
         ];
         setUpPoolComponent();
         formHelper.setValue('poolType', 'replicated');
@@ -525,10 +511,6 @@ describe('PoolFormComponent', () => {
   });
 
   describe('getMaxSize and getMinSize', () => {
-    const setCrushRule = ({ min, max }: { min?: number; max?: number }) => {
-      formHelper.setValue('crushRule', Mocks.getCrushRule({ min, max }));
-    };
-
     it('returns 0 if osd count is 0', () => {
       component.info.osd_count = 0;
       expect(component.getMinSize()).toBe(0);
@@ -541,36 +523,22 @@ describe('PoolFormComponent', () => {
       expect(component.getMaxSize()).toBe(0);
     });
 
-    it('returns minimum and maximum of rule', () => {
-      setCrushRule({ min: 2, max: 6 });
-      expect(component.getMinSize()).toBe(2);
-      expect(component.getMaxSize()).toBe(6);
-    });
-
     it('returns 1 as minimum and 3 as maximum if no crush rule is available', () => {
       expect(component.getMinSize()).toBe(1);
       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(15);
-    });
-
     it('should return the osd count as minimum if its lower the the rule minimum', () => {
-      setCrushRule({ min: 20 });
-      expect(component.getMinSize()).toBe(20);
+      component.info.osd_count = 0;
+      formHelper.setValue('crushRule', component.info.crush_rules_replicated[0]);
       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);
+      formHelper.setValue('crushRule', Mocks.getCrushRule({ itemName: 'default~ssd' }));
+      expect(form.getValue('crushRule').usable_size).toBe(10);
     });
   });
 
index a3cec3854857101d630c58cbde99302b1c86f6df..ae380cf74005ea6932a6366f52520b7a02928d30 100644 (file)
@@ -82,6 +82,7 @@ export class PoolFormComponent extends CdForm implements OnInit {
   pgAutoscaleModes: string[];
   crushUsage: string[] = undefined; // Will only be set if a rule is used by some pool
   ecpUsage: string[] = undefined; // Will only be set if a rule is used by some pool
+  crushRuleMaxSize = 10;
 
   private modalSubscription: Subscription;
 
@@ -155,7 +156,7 @@ export class PoolFormComponent extends CdForm implements OnInit {
           validators: [
             CdValidators.custom(
               'tooFewOsds',
-              (rule: any) => this.info && rule && this.info.osd_count < rule.min_size
+              (rule: any) => this.info && rule && this.info.osd_count < 1
             ),
             CdValidators.custom(
               'required',
@@ -422,10 +423,6 @@ export class PoolFormComponent extends CdForm implements OnInit {
     if (!this.info || this.info.osd_count < 1) {
       return 0;
     }
-    const rule = this.form.getValue('crushRule');
-    if (rule) {
-      return rule.min_size;
-    }
     return 1;
   }
 
@@ -469,8 +466,8 @@ export class PoolFormComponent extends CdForm implements OnInit {
       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);
+    const usable = currentDomain ? currentDomain.length : this.crushRuleMaxSize;
+    rule.usable_size = Math.min(usable, this.crushRuleMaxSize);
   }
 
   private replicatedPgCalc(pgs: number): number {
index 83c1db6b6df2a4ec1e5529b1ec6d846c1fa4c621..e1e31ed20fa800771140d7a00a6d5a932d8f27b8 100644 (file)
@@ -1,12 +1,10 @@
 import { CrushStep } from './crush-step';
 
 export class CrushRule {
-  max_size: number;
   usable_size?: number;
-  min_size: number;
   rule_id: number;
+  type: number;
   rule_name: string;
-  ruleset: number;
   steps: CrushStep[];
 }
 
index 66033f0330f07ccacbe6a1a8999ec8cdd4cd75a8..574919d08ec268ebe51df706d7a60f705809799f 100644 (file)
@@ -546,26 +546,19 @@ export class Mocks {
   static getCrushRule({
     id = 0,
     name = 'somePoolName',
-    min = 1,
-    max = 10,
     type = 'replicated',
     failureDomain = 'osd',
     itemName = 'default' // This string also sets the device type - "default~ssd" <- ssd usage only
   }: {
-    max?: number;
-    min?: number;
     id?: number;
     name?: string;
     type?: string;
     failureDomain?: string;
     itemName?: string;
   }): CrushRule {
-    const typeNumber = type === 'erasure' ? 3 : 1;
     const rule = new CrushRule();
-    rule.max_size = max;
-    rule.min_size = min;
+    rule.type = type === 'erasure' ? 3 : 1;
     rule.rule_id = id;
-    rule.ruleset = typeNumber;
     rule.rule_name = name;
     rule.steps = [
       {
index 921dadb0471d01011c3647591d0eb1d12aa587d3..ff2dea0e99a38b5ee86156305898a27c3f35fc5f 100644 (file)
@@ -2308,24 +2308,15 @@ paths:
       responses:
         '200':
           content:
-            application/vnd.ceph.api.v1.0+json:
+            application/vnd.ceph.api.v2.0+json:
               schema:
                 properties:
-                  max_size:
-                    description: Maximum size of Rule
-                    type: integer
-                  min_size:
-                    description: Minimum size of Rule
-                    type: integer
                   rule_id:
                     description: Rule ID
                     type: integer
                   rule_name:
                     description: Rule Name
                     type: string
-                  ruleset:
-                    description: RuleSet related to the rule
-                    type: integer
                   steps:
                     description: Steps included in the rule
                     items:
@@ -2337,10 +2328,7 @@ paths:
                 required:
                 - rule_id
                 - rule_name
-                - ruleset
                 - type
-                - min_size
-                - max_size
                 - steps
                 type: object
           description: OK
@@ -2444,7 +2432,7 @@ paths:
       responses:
         '200':
           content:
-            application/vnd.ceph.api.v1.0+json:
+            application/vnd.ceph.api.v2.0+json:
               type: object
           description: OK
         '400':