[pve-devel] [PATCH v2 manager 1/2] gui/cluster: add CorosyncLinkEdit component to support up to 8 links
Dominik Csapak
d.csapak at proxmox.com
Thu Mar 19 15:06:37 CET 2020
overall looks good, and most works, comments inline
the only 'real' issue i found is with ipv6 networks
(they do not get auto added to the links)
On 3/17/20 12:11 PM, Stefan Reiter wrote:
> CorosyncLinkEdit is a Panel that contains between one and 8
> CorosyncLinkSelectors. These can be added or removed with according
> buttons.
>
> Values submitted to the API are calculated by each
> ProxmoxNetworkSelector itself. This works because ExtJS searches
> recursively through all child components for ones with a value to be
> submitted, i.e. the CorosyncLinkEdit and CorosyncLinkSelector components
> are not part of data submission at all.
>
> Change ClusterEdit.js to use the new component for cluster join and
> create.
>
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> www/manager6/Makefile | 1 +
> www/manager6/dc/ClusterEdit.js | 148 +++++-----
> www/manager6/dc/CorosyncLinkEdit.js | 406 ++++++++++++++++++++++++++++
> 3 files changed, 478 insertions(+), 77 deletions(-)
> create mode 100644 www/manager6/dc/CorosyncLinkEdit.js
>
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 41615430..0f2224af 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -224,6 +224,7 @@ JSSRC= \
> dc/Cluster.js \
> dc/ClusterEdit.js \
> dc/PermissionView.js \
> + dc/CorosyncLinkEdit.js \
> Workspace.js
>
> lint: ${JSSRC}
> diff --git a/www/manager6/dc/ClusterEdit.js b/www/manager6/dc/ClusterEdit.js
> index e0fa5d9d..0820e4a0 100644
> --- a/www/manager6/dc/ClusterEdit.js
> +++ b/www/manager6/dc/ClusterEdit.js
> @@ -25,24 +25,20 @@ Ext.define('PVE.ClusterCreateWindow', {
> name: 'clustername'
> },
> {
> - xtype: 'proxmoxNetworkSelector',
> - fieldLabel: Ext.String.format(gettext('Link {0}'), 0),
> - emptyText: gettext("Optional, defaults to IP resolved by node's hostname"),
> - name: 'link0',
> - autoSelect: false,
> - valueField: 'address',
> - displayField: 'address',
> - skipEmptyText: true
> - }],
> - advancedItems: [{
> - xtype: 'proxmoxNetworkSelector',
> - fieldLabel: Ext.String.format(gettext('Link {0}'), 1),
> - emptyText: gettext("Optional second link for redundancy"),
> - name: 'link1',
> - autoSelect: false,
> - valueField: 'address',
> - displayField: 'address',
> - skipEmptyText: true
> + xtype: 'fieldcontainer',
> + fieldLabel: gettext("Cluster Links"),
> + style: 'padding-top: 5px',
i would rather write this either as:
style: {
'padding-top':'5px',
},
(makes it easier to add styles later)
or:
padding: '5 0 0 0',
(we use it more often)
> + items: [
> + {
> + xtype: 'pveCorosyncLinkEditor',
> + style: 'padding-bottom: 5px',
same here
> + name: 'links'
> + },
> + {
> + xtype: 'label',
> + text: gettext("Multiple links are used as failover, lower numbers have higher priority.")
> + }
> + ]
> }]
> }
> });
> @@ -149,20 +145,10 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
> info: {
> fp: '',
> ip: '',
> - clusterName: '',
> - ring0Needed: false,
> - ring1Possible: false,
> - ring1Needed: false
> + clusterName: ''
> }
> },
> formulas: {
> - ring0EmptyText: function(get) {
> - if (get('info.ring0Needed')) {
> - return gettext("Cannot use default address safely");
> - } else {
> - return gettext("Default: IP resolved by node's hostname");
> - }
> - },
> submittxt: function(get) {
> let cn = get('info.clusterName');
> if (cn) {
> @@ -188,9 +174,6 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
> change: 'recomputeSerializedInfo',
> enable: 'resetField'
> },
> - 'proxmoxtextfield[name=ring1_addr]': {
> - enable: 'ring1Needed'
> - },
> 'textfield': {
> disable: 'resetField'
> }
> @@ -198,47 +181,70 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
> resetField: function(field) {
> field.reset();
> },
> - ring1Needed: function(f) {
> - var vm = this.getViewModel();
> - f.allowBlank = !vm.get('info.ring1Needed');
> - },
> onInputTypeChange: function(field, assistedInput) {
> - var vm = this.getViewModel();
> + var view = this.getView();
> + var linkEditor = view.down('#linkEditor');
since we have a controller, we can make use of
references and 'lookup' here
e.g. set a reference on an item:
{
...
reference: 'foobar',
}
and use it like:
let foobar = view.lookup('foobar');
a controller is also a 'referenceholder' and
(as the name promises) holds references when given
this should be faster than doing view.down,
works also on the controller directly (e.g. this.lookup)
and looks a little bit nicer (no '#' needed)
> +
> + // this also clears all links
> + linkEditor.setAllowNumberEdit(!assistedInput);
> +
> if (!assistedInput) {
> - vm.set('info.ring1Possible', true);
> + linkEditor.setInfoText();
> + linkEditor.setDefaultLinks();
> }
> },
this whole function could probably also be solved with
some viewModel magic and binding
i.e. bind the values of the linkEditor to some
values in the viewmodel, and bind the value of the
checkbox to the value of the viewmodel
(i know it was already this way,
but i guess this would remove some unecessary code)
> recomputeSerializedInfo: function(field, value) {
> var vm = this.getViewModel();
> + var view = this.getView();
> +
> + var assistedEntryBox = view.down('#assistedEntry');
same 'lookup' comment as above
> + if (!assistedEntryBox.getValue()) {
> + // not in assisted entry mode, nothing to do
> + return;
> + }
> +
> + var linkEditor = view.down('#linkEditor');
> +
> var jsons = Ext.util.Base64.decode(value);
> var joinInfo = Ext.JSON.decode(jsons, true);
>
> var info = {
> fp: '',
> - ring1Needed: false,
> - ring1Possible: false,
> ip: '',
> clusterName: ''
> };
>
> - var totem = {};
> if (!(joinInfo && joinInfo.totem)) {
> field.valid = false;
> + linkEditor.setLinks([]);
> + linkEditor.setInfoText();
> } else {
> - var ring0Needed = false;
> - if (joinInfo.ring_addr !== undefined) {
> - ring0Needed = joinInfo.ring_addr[0] !== joinInfo.ipAddress;
> + var links = [];
> + var interfaces = joinInfo.totem['interface'];
nit: i'd rather use 'let' whenever possible for new code
> + Ext.Array.each(Object.keys(interfaces), iface => {
> + links.push({
> + number: interfaces[iface]['linknumber'],
> + value: '',
> + text: '',
> + allowBlank: false
> + });
> + });
if you do a 'for of loop' here, you can have the value directly
for (const iface of Object.values(interfaces)) {
..
}
or you can do a 'map'
links = Object.values.map(iface => { return { number: iface.linknumber
... }});
> +
> + linkEditor.setInfoText();
> + if (links.length == 1 && joinInfo.ring_addr !== undefined &&
> + joinInfo.ring_addr[0] === joinInfo.ipAddress) {
> +
> + links[0].allowBlank = true;
> + linkEditor.setInfoText(gettext("Leave empty to use IP resolved by node's hostname"));
> }
>
> + linkEditor.setLinks(links);
> +
> info = {
> ip: joinInfo.ipAddress,
> fp: joinInfo.fingerprint,
> - ring0Needed: ring0Needed,
> - ring1Possible: !!joinInfo.totem['interface']['1'],
> - ring1Needed: !!joinInfo.totem['interface']['1'],
> clusterName: joinInfo.totem['cluster_name']
> };
> - totem = joinInfo.totem;
> field.valid = true;
> }
>
> @@ -275,6 +281,7 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
> xtype: 'proxmoxcheckbox',
> reference: 'assistedEntry',
> name: 'assistedEntry',
> + itemId: 'assistedEntry',
> submitValue: false,
> value: true,
> autoEl: {
> @@ -312,7 +319,9 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
> readOnly: '{assistedEntry.checked}'
> },
> name: 'hostname'
> - },
> + }
nit: why remove the trailing comma?
> + ],
> + column2: [
> {
> xtype: 'textfield',
> inputType: 'password',
> @@ -322,38 +331,11 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
> name: 'password'
> }
> ],
> - column2: [
> - {
> - xtype: 'proxmoxNetworkSelector',
> - fieldLabel: Ext.String.format(gettext('Link {0}'), 0),
> - bind: {
> - emptyText: '{ring0EmptyText}',
> - allowBlank: '{!info.ring0Needed}'
> - },
> - skipEmptyText: true,
> - autoSelect: false,
> - valueField: 'address',
> - displayField: 'address',
> - name: 'link0'
> - },
> - {
> - xtype: 'proxmoxNetworkSelector',
> - fieldLabel: Ext.String.format(gettext('Link {0}'), 1),
> - skipEmptyText: true,
> - autoSelect: false,
> - valueField: 'address',
> - displayField: 'address',
> - bind: {
> - disabled: '{!info.ring1Possible}',
> - allowBlank: '{!info.ring1Needed}',
> - },
> - name: 'link1'
> - }
> - ],
> columnB: [
> {
> xtype: 'textfield',
> fieldLabel: gettext('Fingerprint'),
> + style: 'margin-top: -10px',
same as for 'padding-top'
also, why '-10px' ?
also, the whole layout seems kinda wonky (at least with
the language set to german), the password field
is higher up that the address field...
> allowBlank: false,
> bind: {
> value: '{info.fp}',
> @@ -362,5 +344,17 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
> name: 'fingerprint'
> }
> ]
> + },
> + {
> + xtype: 'fieldcontainer',
> + fieldLabel: gettext("Cluster Links"),
> + padding: '5px 0 0 0',
> + items: [
> + {
> + xtype: 'pveCorosyncLinkEditor',
> + itemId: 'linkEditor',
> + allowNumberEdit: false
> + }
> + ]
> }]
> });
> diff --git a/www/manager6/dc/CorosyncLinkEdit.js b/www/manager6/dc/CorosyncLinkEdit.js
> new file mode 100644
> index 00000000..6fb35a0e
> --- /dev/null
> +++ b/www/manager6/dc/CorosyncLinkEdit.js
> @@ -0,0 +1,406 @@
> +Ext.define('PVE.form.CorosyncLinkEditorController', {
> + extend: 'Ext.app.ViewController',
> + alias: 'controller.pveCorosyncLinkEditorController',
> +
> + addLinkIfEmpty: function() {
> + let view = this.getView();
> + if (view.items || view.items.length == 0) {
> + this.addLink();
> + }
> + },
> +
> + addEmptyLink: function() {
> + // discard parameters to allow being called from 'handler'
> + this.addLink();
> + },
> +
> + addLink: function(number, value, allowBlank) {
> + let me = this;
> + let view = me.getView();
> + let vm = view.getViewModel();
> +
> + let linkCount = vm.get('linkCount');
> + if (linkCount >= vm.get('maxLinkCount')) {
> + return;
> + }
> +
> + if (number === undefined) {
> + number = me.getNextFreeNumber();
> + }
> + if (value === undefined) {
> + value = me.getNextFreeNetwork();
> + }
> +
> + let linkSelector = Ext.create('PVE.form.CorosyncLinkSelector', {
> + maxLinkNumber: vm.get('maxLinkCount') - 1,
> + allowNumberEdit: vm.get('allowNumberEdit'),
> + allowBlankNetwork: allowBlank,
> + initNumber: number,
> + initNetwork: value,
> +
> + // needs to be set here, because we need to update the viewmodel
> + removeBtnHandler: function() {
> + let curLinkCount = vm.get('linkCount');
> +
> + if (curLinkCount <= 1) {
> + return;
> + }
> +
> + vm.set('linkCount', curLinkCount - 1);
> +
> + // 'this' is the linkSelector here
> + view.remove(this);
> +
> + me.updateDeleteButtonState();
> + }
> + });
> +
> + view.add(linkSelector);
> +
> + linkCount++;
> + vm.set('linkCount', linkCount);
> +
> + me.updateDeleteButtonState();
> + },
> +
> + // ExtJS trips on binding this for some reason, so do it manually
> + updateDeleteButtonState: function() {
> + let view = this.getView();
> + let vm = view.getViewModel();
> +
> + let disabled = vm.get('linkCount') <= 1;
> +
> + let deleteButtons = view.query('button[cls=removeLinkBtn]');
> + Ext.Array.each(deleteButtons, btn => {
> + btn.setDisabled(disabled);
> + })
> + },
> +
> + getNextFreeNetwork: function() {
> + let view = this.getView();
> + let vm = view.getViewModel();
> + let netsInUse = Ext.Array.map(
> + view.query('proxmoxNetworkSelector'), selector => selector.value);
> +
> + // default to empty field, user has to set up link manually
> + let retval = undefined;
> +
> + let nets = vm.get('networks');
> + Ext.Array.each(nets, net => {
> + if (!Ext.Array.contains(netsInUse, net)) {
> + retval = net;
> + return false; // break
> + }
> + });
> +
> + return retval;
> + },
> +
> + getNextFreeNumber: function() {
> + let view = this.getView();
> + let vm = view.getViewModel();
> + let numbersInUse = Ext.Array.map(
> + view.query('numberfield'), field => field.value);
> +
> + for (let i = 0; i < vm.get('maxLinkCount'); i++) {
> + if (!Ext.Array.contains(numbersInUse, i)) {
> + return i;
> + }
> + }
> +
> + // all numbers in use, this should never happen since add button is
> + // disabled automatically
> + return 0;
> + }
> +});
> +
> +Ext.define('PVE.form.CorosyncLinkSelector', {
> + extend: 'Ext.panel.Panel',
> + xtype: 'pveCorosyncLinkSelector',
> +
> + mixins: ['Proxmox.Mixin.CBind' ],
> + cbindData: [],
> +
> + // config
> + maxLinkNumber: 7,
> + allowNumberEdit: true,
> + allowBlankNetwork: false,
> + removeBtnHandler: undefined,
> +
> + // values
> + initNumber: 0,
> + initNetwork: '',
> +
> + layout: 'hbox',
> + bodyPadding: 5,
> + border: 0,
> +
> + items: [
> + {
> + xtype: 'numberfield',
> + cbind: {
> + maxValue: '{maxLinkNumber}',
> + readOnly: '{!allowNumberEdit}',
> + value: '{initNumber}'
> + },
> +
> + minValue: 0,
> + allowBlank: false,
> + width: 80,
> + labelWidth: 30,
> + fieldLabel: gettext('Link'),
> +
> + // see getSubmitValue of network selector
> + submitValue: false
> + },
> + {
> + xtype: 'proxmoxNetworkSelector',
> + cbind: {
> + allowBlank: '{allowBlankNetwork}',
> + value: '{initNetwork}'
> + },
> +
> + autoSelect: false,
> + valueField: 'address',
> + displayField: 'address',
> + margin: '0 5px 0 5px',
> + getSubmitValue: function() {
> + // link number is encoded into key, so we need to set
> + // field name before value retrieval
> + let me = this;
> + let numSelect = me.prev('numberfield');
> + let linkNumber = numSelect.getValue();
> + me.name = 'link' + linkNumber;
> + return me.getValue();
> + }
> + },
> + {
> + xtype: 'button',
> + iconCls: 'fa fa-trash-o',
> + cls: 'removeLinkBtn',
> +
> + cbind: {
> + hidden: '{!allowNumberEdit}'
> + },
> +
> + handler: function() {
> + let me = this;
> + let parent = me.up('pveCorosyncLinkSelector');
> + if (parent.removeBtnHandler !== undefined) {
> + parent.removeBtnHandler();
> + }
> + }
> + }
> + ],
> +
> + initComponent: function() {
> + let me = this;
> +
> + me.callParent();
> +
> + let numSelect = me.down('numberfield');
> + let netSelect = me.down('proxmoxNetworkSelector');
> +
> + numSelect.validator = this.createNoDuplicatesValidator(
> + 'numberfield',
> + gettext("Duplicate link number not allowed.")
> + );
> +
> + netSelect.validator = this.createNoDuplicatesValidator(
> + 'proxmoxNetworkSelector',
> + gettext("Duplicate link address not allowed.")
> + );
> + },
> +
> + createNoDuplicatesValidator: function(queryString, errorMsg) {
> + // linkSelector
> + let me = this;
> +
> + return function(val) {
> + let curField = this;
> + let form = me.up('form');
> + let linkEditor = me.up('pveCorosyncLinkEditor');
> +
> + if (!form.validating) {
> + // avoid recursion/double validation by setting temporary states
> + curField.validating = true;
> + form.validating = true;
> +
> + // validate all other fields as well, to always mark both
> + // parties involved in a 'duplicate' error
> + form.isValid();
> +
> + form.validating = false;
> + curField.validating = false;
> + } else if (curField.validating) {
> + // we'll be validated by the original call in the other
> + // if-branch, avoid double work
> + return true;
> + }
> +
> + if (val === undefined || (val instanceof String && val.length === 0)) {
> + // let this be caught by allowBlank, if at all
> + return true;
> + }
> +
> + let allFields = linkEditor.query(queryString);
> + let err = undefined;
> + Ext.Array.each(allFields, field => {
> + if (field != curField && field.getValue() == val) {
> + err = errorMsg;
> + return false; // break
> + }
> + });
> +
> + return err || true;
> + };
> + }
> +});
> +
> +Ext.define('PVE.form.CorosyncLinkEditor', {
> + extend: 'Ext.panel.Panel',
> + xtype: 'pveCorosyncLinkEditor',
> +
> + controller: 'pveCorosyncLinkEditorController',
> +
> + // only initial config, use setter otherwise
> + allowNumberEdit: true,
> +
> + viewModel: {
> + data: {
> + linkCount: 0,
> + maxLinkCount: 8,
> + networks: null,
> + allowNumberEdit: true,
> + infoText: ''
> + },
> + formulas: {
> + addDisabled: function(get) {
> + return !get('allowNumberEdit') ||
> + get('linkCount') >= get('maxLinkCount');
> + },
> + dockHidden: function(get) {
> + return !(get('allowNumberEdit') || get('infoText'));
> + }
> + }
> + },
> +
> + dockedItems: [{
> + xtype: 'toolbar',
> + dock: 'bottom',
> + defaultButtonUI : 'default',
> + bind: {
> + hidden: '{dockHidden}'
> + },
> + items: [
> + {
> + xtype: 'button',
> + text: gettext('Add'),
> + bind: {
> + disabled: '{addDisabled}',
> + hidden: '{!allowNumberEdit}'
> + },
> + handler: 'addEmptyLink'
> + },
> + {
> + xtype: 'label',
> + bind: {
> + text: '{infoText}'
> + }
> + }
> + ]
> + }],
> +
> + setInfoText: function(text) {
> + let me = this;
> + let vm = me.getViewModel();
> +
> + vm.set('infoText', text || '');
> + },
> +
> + setLinks: function(links) {
> + let me = this;
> + let controller = me.getController();
> + let vm = me.getViewModel();
> +
> + me.removeAll();
> + vm.set('linkCount', 0);
> +
> + Ext.Array.each(links, link => {
> + controller.addLink(link['number'], link['value'], link['allowBlank']);
> + });
> + },
> +
> + setDefaultLinks: function() {
> + let me = this;
> + let controller = me.getController();
> + let vm = me.getViewModel();
> +
> + me.removeAll();
> + vm.set('linkCount', 0);
> + controller.addLink();
> + },
> +
> + // clears all links
> + setAllowNumberEdit: function(allow) {
> + let me = this;
> + let vm = me.getViewModel();
> + vm.set('allowNumberEdit', allow);
> + me.removeAll();
> + vm.set('linkCount', 0);
> + },
> +
> + items: [{
> + // No links is never a valid scenario, but can occur during a slow load
> + xtype: 'hiddenfield',
> + submitValue: false,
> + isValid: function() {
> + let me = this;
> + let vm = me.up('pveCorosyncLinkEditor').getViewModel();
> + return vm.get('linkCount') > 0;
> + }
> + }],
> +
> + initComponent: function() {
> + let me = this;
> + let vm = me.getViewModel();
> + let controller = me.getController();
> +
> + vm.set('allowNumberEdit', me.allowNumberEdit);
> +
> + me.callParent();
> +
> + // Request local node networks to pre-populate first link.
> + Proxmox.Utils.API2Request({
> + url: '/nodes/localhost/network',
> + method: 'GET',
> + waitMsgTarget: me,
> + success: response => {
> + let data = response.result.data;
> + if (data.length < 1) {
> + return;
> + }
> +
> + data = data.filter(net => !!net.address);
this filters all ipv6 only links
> + data.sort((a, b) => a.iface.localeCompare(b.iface));
> + data = data.map(net => net.address);
also this discards all ipv6 addresses...
> +
> + vm.set('networks', data);
> +
> + // Always have at least one link, but account for delay in API,
> + // someone might have called 'setLinks' in the meantime -
> + // except if 'allowNumberEdit' is false, in which case we're
> + // probably waiting for the user to input the join info
> + if (vm.get('allowNumberEdit')) {
> + controller.addLinkIfEmpty();
> + }
> + },
> + failure: () => {
> + if (vm.get('allowNumberEdit')) {
> + controller.addLinkIfEmpty();
> + }
> + }
> + });
> + }
> +});
> +
>
More information about the pve-devel
mailing list