[pve-devel] [PATCH manager v2 5/8] ceph: gui: rework pool input panel
Dominik Csapak
d.csapak at proxmox.com
Tue Nov 24 14:53:29 CET 2020
seems not to work to set the target_size(_bytes) via gui (cli works)
comments inline
On 11/24/20 11:58 AM, Alwin Antreich wrote:
> * add the ability to edit an existing pool
> * allow adjustment of autoscale settings
> * warn if user specifies min_size 1
> * disallow min_size 1 on pool create
> * calculate min_size replica by size
>
> Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
> ---
> www/manager6/ceph/Pool.js | 276 ++++++++++++++++++++++++++++++--------
> 1 file changed, 221 insertions(+), 55 deletions(-)
>
> diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
> index 6febe1fc..35ecbf09 100644
> --- a/www/manager6/ceph/Pool.js
> +++ b/www/manager6/ceph/Pool.js
> @@ -1,94 +1,237 @@
> -Ext.define('PVE.CephCreatePool', {
> - extend: 'Proxmox.window.Edit',
> - alias: 'widget.pveCephCreatePool',
> +Ext.define('PVE.CephPoolInputPanel', {
> + extend: 'Proxmox.panel.InputPanel',
> + xtype: 'pveCephPoolInputPanel',
> + mixins: ['Proxmox.Mixin.CBind'],
>
> showProgress: true,
> onlineHelp: 'pve_ceph_pools',
>
> subject: 'Ceph Pool',
> - isCreate: true,
> - method: 'POST',
> - items: [
> + column1: [
> {
> - xtype: 'textfield',
> + xtype: 'pmxDisplayEditField',
> fieldLabel: gettext('Name'),
> + cbind: {
> + editable: '{isCreate}',
> + value: '{pool_name}',
> + disabled: '{!isCreate}'
> + },
> name: 'name',
> + labelWidth: 80,
> allowBlank: false
> },
> {
> xtype: 'proxmoxintegerfield',
> fieldLabel: gettext('Size'),
> name: 'size',
> + labelWidth: 80,
> value: 3,
> - minValue: 1,
> + minValue: 2,
> maxValue: 7,
> - allowBlank: false
> + allowBlank: false,
> + listeners: {
> + change: function(field, val) {
> + let size = Math.round(val / 2);
> + if (size > 1) {
> + field.up('inputpanel').down('field[name=min_size]').setValue(size);
> + }
> + },
> + },
> + },
> + ],
> + column2: [
> + {
> + xtype: 'proxmoxKVComboBox',
> + fieldLabel: 'PG Autoscale ' + gettext('Mode'),
like the previous patch, please combine the gettext (and optimally use
the same text as in the columns)
> + name: 'pg_autoscale_mode',
> + comboItems: [
> + ['warn', 'warn'],
> + ['on', 'on'],
> + ['off', 'off'],
> + ],
> + value: 'warn',
> + allowBlank: false,
> + autoSelect: false,
> + labelWidth: 140,
> + },
> + {
> + xtype: 'proxmoxcheckbox',
> + fieldLabel: gettext('Add as Storage'),
> + cbind: {
> + value: '{isCreate}',
> + hidden: '{!isCreate}',
i guess you'd need to disable it also on !isCreate so that the value
does not get submitted.. (for safety)
> + },
> + name: 'add_storages',
> + labelWidth: 140,
> + autoEl: {
> + tag: 'div',
> + 'data-qtip': gettext('Add the new pool to the cluster storage configuration.'),
> + },
> },
> + ],
> + advancedColumn1: [
> {
> xtype: 'proxmoxintegerfield',
> fieldLabel: gettext('Min. Size'),
> name: 'min_size',
> + labelWidth: 80,
> value: 2,
> - minValue: 1,
> + cbind: {
> + minValue: (get) => get('isCreate') ? 2 : 1,
> + },
> maxValue: 7,
> - allowBlank: false
> + allowBlank: false,
> + listeners: {
> + change: function(field, val) {
> + let warn = true;
> + let warn_text = gettext('Min. Size');
> +
> + if (val < 2) {
> + warn = false;
> + warn_text = gettext('Min. Size') + ' <i class="fa fa-exclamation-triangle warning"></i>';
> + }
> +
> + field.up().down('field[name=min_size-warning]').setHidden(warn);
please reverse the logic of 'warn' and use setHidden(!warn) or
use setVisible(warn), it's confusing otherwise
> + field.setFieldLabel(warn_text);
> + }
> + },
> + },
> + {
> + xtype: 'displayfield',
> + name: 'min_size-warning',
> + padding: 5,
> + userCls: 'pmx-hint',
> + value: 'A pool with min_size=1 could lead to data loss, incomplete PGs or unfound objects.',
> + hidden: true,
> },
> {
> xtype: 'pveCephRuleSelector',
> fieldLabel: 'Crush Rule', // do not localize
> + cbind: { nodename: '{nodename}' },
> + labelWidth: 80,
> name: 'crush_rule',
> allowBlank: false
> },
> - {
> - xtype: 'proxmoxKVComboBox',
> - fieldLabel: 'PG Autoscale Mode', // do not localize
> - name: 'pg_autoscale_mode',
> - comboItems: [
> - ['warn', 'warn'],
> - ['on', 'on'],
> - ['off', 'off'],
> - ],
> - value: 'warn',
> - allowBlank: false,
> - autoSelect: false,
> - },
> + ],
> + advancedColumn2: [
> {
> xtype: 'proxmoxintegerfield',
> - fieldLabel: 'pg_num',
> + fieldLabel: '# of PGs',
> name: 'pg_num',
> + labelWidth: 120,
> value: 128,
> - minValue: 8,
> + minValue: 1,
> maxValue: 32768,
> + allowBlank: false,
> + emptyText: 128,
> + },
> + {
> + xtype: 'numberfield',
> + fieldLabel: gettext('Target Size') + ' (GiB)',
> + name: 'target_size',
> + labelWidth: 120,
> + value: 0,
> + minValue: 0,
> allowBlank: true,
> - emptyText: gettext('Autoscale'),
> + emptyText: '0.0',
> + disabled: false,
> + listeners: {
> + change: function(field, val) {
> + let ratio = field.up().down('field[name=target_size_ratio]').getValue() > + if ( ratio === null) {
spacing
> + field.up().down('field[name=target_size_ratio]').setDisabled(val);
> + }
> + }
> + },
> },
> {
> - xtype: 'proxmoxcheckbox',
> - fieldLabel: gettext('Add as Storage'),
> - value: true,
> - name: 'add_storages',
> + xtype: 'numberfield',
> + fieldLabel: gettext('Target Size Ratio'),
> + name: 'target_size_ratio',
> + labelWidth: 120,
> + value: 0,
> + minValue: 0,
> + allowBlank: true,
> + emptyText: '0.0',
> + disabled: false,
> + listeners: {
> + change: function(field, val) {
> + field.up().down('field[name=target_size]').setDisabled(val);
> + }
i find that disabling the other field on change very weird ux
i'd rather use a selector or radio button for the mode...
also if you already have the emptytext you do not need to set
the default value, that makes the input even weirder
> + },
> autoEl: {
> tag: 'div',
> - 'data-qtip': gettext('Add the new pool to the cluster storage configuration.'),
> + 'data-qtip': gettext('If Target Size Ratio is set it takes precedence over Target Size.'),
> },
> - }
> + },
> ],
> - initComponent : function() {
> - var me = this;
>
> - if (!me.nodename) {
> - throw "no node name specified";
> + onGetValues: function(values) {
> + Object.keys(values || {}).forEach(function(name) {
> + if (values[name] === '') {
> + delete values[name];
> + }
> + });
> +
> + if (values['target_size'] && values['target_size'] !== 0) {
> + values['target_size'] = values['target_size']*1024*1024*1024;
> + } else {
> + // needs to be 0 to be cleared
> + values['target_size'] = 0;
> }
>
> - Ext.apply(me, {
> - url: "/nodes/" + me.nodename + "/ceph/pools",
> - defaults: {
> - nodename: me.nodename
> - }
> - });
> + // needs to be 0 to be cleared
> + if (!values['target_size_ratio']) {
> + values['target_size_ratio'] = 0;
> + }
i think this is the reason it does not work (set to 0 instead of delete)
>
> - me.callParent();
> - }
> + return values;
> + },
> +
> + setValues: function(values) {
> + if (values['target_size'] && values['target_size'] !== 0) {
> + values['target_size'] = values['target_size']/1024/1024/1024;
> + }
> +
> + this.callParent([values]);
> + },
> +
> + initComponent: function() {
> + let me = this;
> + me.callParent();
> + },
you can drop the fucntion if it only calls callParent
> +
> +});
> +
> +Ext.define('PVE.CephPoolEdit', {
> + extend: 'Proxmox.window.Edit',
> + alias: 'widget.pveCephPoolEdit',
> + xtype: 'pveCephPoolEdit',
> + mixins: ['Proxmox.Mixin.CBind'],
> +
> + cbindData: {
> + pool_name: '',
> + isCreate: (cfg) => !cfg.pool_name,
> + },
> +
> + cbind: {
> + autoLoad: get => !get('isCreate'),
> + url: get => get('isCreate') ?
> + `/nodes/${get('nodename')}/ceph/pools` :
> + `/nodes/${get('nodename')}/ceph/pools/${get('pool_name')}`,
> + method: get => get('isCreate') ? 'POST' : 'PUT',
> + },
> +
> + subject: gettext('Ceph Pool'),
> +
> + items: [{
> + xtype: 'pveCephPoolInputPanel',
> + cbind: {
> + nodename: '{nodename}',
> + pool_name: '{pool_name}',
> + isCreate: '{isCreate}',
> + },
> + }],
> });
>
> Ext.define('PVE.node.CephPoolList', {
> @@ -214,6 +357,9 @@ Ext.define('PVE.node.CephPoolList', {
> });
>
> var store = Ext.create('Proxmox.data.DiffStore', { rstore: rstore });
> + var reload = function() {
> + rstore.load();
> + };
>
> var regex = new RegExp("not (installed|initialized)", "i");
> PVE.Utils.handleStoreErrorOrMask(me, rstore, regex, function(me, error){
> @@ -230,16 +376,37 @@ Ext.define('PVE.node.CephPoolList', {
> var create_btn = new Ext.Button({
> text: gettext('Create'),
> handler: function() {
> - var win = Ext.create('PVE.CephCreatePool', {
> - nodename: nodename
> + var win = Ext.create('PVE.CephPoolEdit', {
> + title: gettext('Create') + ': Ceph Pool',
> + nodename: nodename,
> });
> win.show();
> - win.on('destroy', function() {
> - rstore.load();
> - });
> + win.on('destroy', reload);
> }
> });
>
> + var run_editor = function() {
> + var rec = sm.getSelection()[0];
> + if (!rec) {
> + return;
> + }
> +
> + var win = Ext.create('PVE.CephPoolEdit', {
> + title: gettext('Edit') + ': Ceph Pool',
> + nodename: nodename,
> + pool_name: rec.data.pool_name,
> + });
> + win.on('destroy', reload);
> + win.show();
> + };
> +
> + var edit_btn = new Proxmox.button.Button({
> + text: gettext('Edit'),
> + disabled: true,
> + selModel: sm,
> + handler: run_editor,
> + });
> +
> var destroy_btn = Ext.create('Proxmox.button.Button', {
> text: gettext('Destroy'),
> selModel: sm,
> @@ -261,19 +428,18 @@ Ext.define('PVE.node.CephPoolList', {
> },
> item: { type: 'CephPool', id: rec.data.pool_name }
> }).show();
> - win.on('destroy', function() {
> - rstore.load();
> - });
> + win.on('destroy', reload);
> }
> });
>
> Ext.apply(me, {
> store: store,
> selModel: sm,
> - tbar: [ create_btn, destroy_btn ],
> + tbar: [ create_btn, edit_btn, destroy_btn ],
> listeners: {
> activate: () => rstore.startUpdate(),
> destroy: () => rstore.stopUpdate(),
> + itemdblclick: run_editor,
> }
> });
>
> @@ -295,7 +461,7 @@ Ext.define('PVE.node.CephPoolList', {
> { name: 'pg_autoscale_mode', type: 'string'},
> { name: 'pg_num_final', type: 'integer'},
> { name: 'target_size_ratio', type: 'number'},
> - { name: 'target_size_bytes', type: 'integer'},
> + { name: 'target_size', type: 'integer'},
> ],
> idProperty: 'pool_name'
> });
>
More information about the pve-devel
mailing list