From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with UTF8SMTPS id 0614D62EB3 for ; Tue, 24 Nov 2020 14:53:33 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id 3817ADEF4 for ; Tue, 24 Nov 2020 14:53:32 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id D1006DEE5 for ; Tue, 24 Nov 2020 14:53:30 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 64C33406E4; Tue, 24 Nov 2020 14:53:30 +0100 (CET) To: Proxmox VE development discussion , Alwin Antreich References: <20201124105811.1416723-1-a.antreich@proxmox.com> <20201124105811.1416723-6-a.antreich@proxmox.com> From: Dominik Csapak Message-ID: <4db5058a-ca82-e1d8-513a-3f98208294b1@proxmox.com> Date: Tue, 24 Nov 2020 14:53:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:84.0) Gecko/20100101 Thunderbird/84.0 MIME-Version: 1.0 In-Reply-To: <20201124105811.1416723-6-a.antreich@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.316 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH manager v2 5/8] ceph: gui: rework pool input panel X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Nov 2020 13:53:33 -0000 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 > --- > 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') + ' '; > + } > + > + 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' > }); >