Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mgr/dashboard: Allow viewing and setting pool quotas #27945

Merged
merged 2 commits into from Jul 3, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -269,6 +269,40 @@ def test_pool_create(self):
self._task_delete("/api/pool/" + pool['pool'])
self.assertStatus(204)

def test_pool_create_with_quotas(self):
pools = [
{
'pool_data': {
'pool': 'dashboard_pool_quota1',
'pg_num': '10',
'pool_type': 'replicated',
},
'pool_quotas_to_check': {
'quota_max_objects': 0,
'quota_max_bytes': 0,
}
},
{
'pool_data': {
'pool': 'dashboard_pool_quota2',
'pg_num': '10',
'pool_type': 'replicated',
'quota_max_objects': 1024,
'quota_max_bytes': 1000,
},
'pool_quotas_to_check': {
'quota_max_objects': 1024,
'quota_max_bytes': 1000,
}
}
]

for pool in pools:
pool_name = pool['pool_data']['pool']
with self.__create_pool(pool_name, pool['pool_data']):
self._validate_pool_properties(pool['pool_quotas_to_check'],
self._get_pool(pool_name))

def test_pool_update_metadata(self):
pool_name = 'pool_update_metadata'
with self.__create_pool(pool_name):
@@ -339,6 +373,17 @@ def test_pool_update_unset_compression(self):
'compression_required_ratio': None,
}, self._get_pool(pool_name))

def test_pool_update_quotas(self):
pool_name = 'pool_update_quotas'
with self.__create_pool(pool_name):
properties = {
'quota_max_objects': 1024,
'quota_max_bytes': 1000,
}
self._task_put('/api/pool/' + pool_name, properties)
time.sleep(5)
self._validate_pool_properties(properties, self._get_pool(pool_name))

def test_pool_create_fail(self):
data = {'pool_type': u'replicated', 'rule_name': u'dnf', 'pg_num': u'8', 'pool': u'sadfs'}
self._task_post('/api/pool/', data)
@@ -119,6 +119,11 @@ def set_app(what, app):
def set_key(key, value):
CephService.send_command('mon', 'osd pool set', pool=pool, var=key, val=str(value))

quotas = {}
quotas['max_objects'] = kwargs.pop('quota_max_objects', None)
quotas['max_bytes'] = kwargs.pop('quota_max_bytes', None)
self._set_quotas(pool, quotas)

for key, value in kwargs.items():
if key == 'pool':
update_name = True
@@ -130,6 +135,12 @@ def set_key(key, value):
if update_name:
CephService.send_command('mon', 'osd pool rename', srcpool=pool, destpool=destpool)

def _set_quotas(self, pool, quotas):
for field, value in quotas.items():
if value is not None:
CephService.send_command('mon', 'osd pool set-quota',
pool=pool, field=field, val=str(value))

def _handle_update_compression_args(self, options, kwargs):
if kwargs.get('compression_mode') == 'unset' and options is not None:
def reset_arg(arg, value):
@@ -423,6 +423,61 @@
</div>
</div>

<!-- Quotas -->
<div>
<legend i18n>Quotas</legend>

<!-- Max Bytes -->
<div class="form-group"
[ngClass]="{'has-error': form.showError('max_bytes', formDir)}">
<label class="control-label col-sm-3"
for="max_bytes">
<ng-container i18n>Max bytes</ng-container>
This conversation was marked as resolved by bk201

This comment has been minimized.

Copy link
@tspmelo

tspmelo May 7, 2019

Contributor

I would suggest adding the info here and change the text to the following:

Suggested change
<ng-container i18n>Max bytes</ng-container>
<ng-container i18n>Max bytes</ng-container>
<cd-helper>
<span i18n>Leave it blank or specify 0 to disable this quota.</span>
<br>
<span i18n>A valid quota should be greater then 0.</span>
</cd-helper>

This comment has been minimized.

Copy link
@bk201

bk201 May 8, 2019

Author Contributor

When creating a new pool, it's natural not to set any quota if a user left the fields empty.
When editing a existed pool, if a user left the fields empty, should we keep previous quota values of pool? Or just disable quota (set to 0) if the field are empty?

This comment has been minimized.

Copy link
@tspmelo

tspmelo May 8, 2019

Contributor

I think if the user left the field empty, we should set the value to the default.
In this case default is disabled, right?

This comment has been minimized.

Copy link
@bk201

bk201 May 8, 2019

Author Contributor

It make sense, since the user clear the field. Thanks!

<cd-helper>
<span i18n>Leave it blank or specify 0 to disable this quota.</span>
<br>
<span i18n>A valid quota should be greater than 0.</span>
</cd-helper>
</label>
<div class="col-sm-9">
<input class="form-control"
id="max_bytes"
name="max_bytes"
type="text"
formControlName="max_bytes"
i18n-placeholder
placeholder="e.g., 10GiB"
defaultUnit="GiB"
cdDimlessBinary>
</div>
</div>

<!-- Max Objects -->
<div class="form-group"
[ngClass]="{'has-error': form.showError('max_objects', formDir)}">
<label class="control-label col-sm-3"
for="max_objects">
<ng-container i18n>Max objects</ng-container>
<cd-helper>
<span i18n>Leave it blank or specify 0 to disable this quota.</span>
<br>
<span i18n>A valid quota should be greater than 0.</span>
</cd-helper>
</label>
<div class="col-sm-9">
<input class="form-control"
id="max_objects"
min="0"
name="max_objects"
type="number"
formControlName="max_objects">
<span class="help-block"
*ngIf="form.showError('max_objects', formDir, 'min')"
i18n>The value should be greater or equal to 0</span>
</div>
</div>
</div>

<!-- Pool configuration -->
<div [hidden]="form.get('poolType').value !== 'replicated' || data.applications.selected.indexOf('rbd') === -1">
<cd-rbd-configuration-form [form]="form"
@@ -302,6 +302,15 @@ describe('PoolFormComponent', () => {
expect(form.getValue('mode')).toBe('none');
});

it('validate quotas', () => {
formHelper.expectValid('max_bytes');
formHelper.expectValid('max_objects');
formHelper.expectValidChange('max_bytes', '10 Gib');
formHelper.expectValidChange('max_bytes', '');
formHelper.expectValidChange('max_objects', '');
formHelper.expectErrorChange('max_objects', -1, 'min');
});

describe('compression form', () => {
beforeEach(() => {
formHelper.setValue('poolType', 'replicated');
@@ -930,6 +939,23 @@ describe('PoolFormComponent', () => {
size: 2
});
});

it('with quotas', () => {
setMultipleValues({
name: 'RepPoolWithQuotas',
poolType: 'replicated',
max_bytes: 1024 * 1024,
max_objects: 3000,
pgNum: 8
});
testCreate({
pool: 'RepPoolWithQuotas',
pool_type: 'replicated',
quota_max_bytes: 1024 * 1024,
quota_max_objects: 3000,
pg_num: 8
});
});
});

it('pool with compression', () => {
@@ -992,6 +1018,8 @@ describe('PoolFormComponent', () => {
pool.options.compression_required_ratio = 0.8;
pool.flags_names = 'someFlag1,someFlag2';
pool.application_metadata = ['rbd', 'rgw'];
pool.quota_max_bytes = 1024 * 1024 * 1024;
pool.quota_max_objects = 3000;

createCrushRule({ name: 'someRule' });
spyOn(poolService, 'get').and.callFake(() => of(pool));
@@ -1025,7 +1053,9 @@ describe('PoolFormComponent', () => {
'algorithm',
'minBlobSize',
'maxBlobSize',
'ratio'
'ratio',
'max_bytes',
'max_objects'
];
enabled.forEach((controlName) => {
return expect(form.get(controlName).enabled).toBeTruthy();
@@ -1043,6 +1073,8 @@ describe('PoolFormComponent', () => {
expect(form.getValue('minBlobSize')).toBe('512 KiB');
expect(form.getValue('maxBlobSize')).toBe('1 MiB');
expect(form.getValue('ratio')).toBe(pool.options.compression_required_ratio);
expect(form.getValue('max_bytes')).toBe('1 GiB');
expect(form.getValue('max_objects')).toBe(pool.quota_max_objects);
});

it('updates pgs on every change', () => {
@@ -136,7 +136,11 @@ export class PoolFormComponent implements OnInit {
validators: [Validators.required, Validators.min(1)]
}),
ecOverwrites: new FormControl(false),
compression: compressionForm
compression: compressionForm,
max_bytes: new FormControl(''),
max_objects: new FormControl(0, {
validators: [Validators.min(0)]
})
},
[
CdValidators.custom('form', () => null),
@@ -221,7 +225,9 @@ export class PoolFormComponent implements OnInit {
algorithm: pool.options.compression_algorithm,
minBlobSize: this.dimlessBinaryPipe.transform(pool.options.compression_min_blob_size),
maxBlobSize: this.dimlessBinaryPipe.transform(pool.options.compression_max_blob_size),
ratio: pool.options.compression_required_ratio
ratio: pool.options.compression_required_ratio,
max_bytes: this.dimlessBinaryPipe.transform(pool.quota_max_bytes),
max_objects: pool.quota_max_objects
};

Object.keys(dataMap).forEach((controlName: string) => {
@@ -529,7 +535,20 @@ export class PoolFormComponent implements OnInit {
formControlName: 'erasureProfile',
attr: 'name'
},
{ externalFieldName: 'rule_name', formControlName: 'crushRule', attr: 'rule_name' }
{ externalFieldName: 'rule_name', formControlName: 'crushRule', attr: 'rule_name' },
{
externalFieldName: 'quota_max_bytes',
formControlName: 'max_bytes',
replaceFn: this.formatter.toBytes,
editable: true,
resetValue: this.editing ? 0 : undefined

This comment has been minimized.

Copy link
@bk201

bk201 Jun 18, 2019

Author Contributor

When editing a pool, clearing quota field should disable pool quota.

This comment has been minimized.

Copy link
@bk201

bk201 Jun 18, 2019

Author Contributor

I've rebased and found previous changes missed this recommendation: #27945 (comment)

When editing a pool, clearing quota fields should disable corresponding quotas.
So I think it's better to start another round of review and test again, since pool testing code on master changed too.

},
{
externalFieldName: 'quota_max_objects',
formControlName: 'max_objects',
editable: true,
resetValue: this.editing ? 0 : undefined

This comment has been minimized.

Copy link
@bk201

bk201 Jun 18, 2019

Author Contributor

When editing a pool, clearing quota field should disable pool quota.

}
]);

if (this.info.is_all_bluestore) {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.