From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 25F391FF0E1 for ; Mon, 13 Jul 2026 14:44:36 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id EA61B214DA; Mon, 13 Jul 2026 14:44:20 +0200 (CEST) Message-ID: <9d41e54e-4e67-46c7-9df5-90f3aa733eb4@proxmox.com> Date: Mon, 13 Jul 2026 14:43:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: David Riley Subject: Re: [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules To: Daniel Kral , pve-devel@lists.proxmox.com References: <20260602100226.180071-1-d.kral@proxmox.com> <20260602100226.180071-8-d.kral@proxmox.com> Content-Language: en-US In-Reply-To: <20260602100226.180071-8-d.kral@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1783946611005 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.306 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: F2OJM43W6YSBGKD5DCR3PF2Z63BBICXG X-Message-ID-Hash: F2OJM43W6YSBGKD5DCR3PF2Z63BBICXG X-MailFrom: d.riley@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Thanks for the patch. comments inline. On 6/2/26 12:02 PM, Daniel Kral wrote: > Since the node affinity rules can be either 'positive' or 'negative' > now, allow users to define either affinity type for the node affinity > rules in the web interface as well. > > Since the priority field does not have any semantic value for negative > node affinity rules, do not use the field for the negative affinity > type. > > As the affinity type is switched from positive to negative and vice > versa, the node selection is inverted to make the (lossy) conversion > easier. The column widths are changed to minimize movement while > switching between the affinity types. > > The inversion is not done if the selection is empty, as preferring all > nodes is rather uselessly verbose and avoiding all nodes is disallowed > as this rule would not satisfiable. > > Signed-off-by: Daniel Kral > --- > changes since v1: > - new > > www/manager6/ha/NodePrioritySelector.js | 65 ++++++++++++++++++- > www/manager6/ha/rules/NodeAffinityRuleEdit.js | 25 +++++++ > www/manager6/ha/rules/NodeAffinityRules.js | 5 ++ > 3 files changed, 92 insertions(+), 3 deletions(-) > > diff --git a/www/manager6/ha/NodePrioritySelector.js b/www/manager6/ha/NodePrioritySelector.js > index ec6ac02a..b58b0ada 100644 > --- a/www/manager6/ha/NodePrioritySelector.js > +++ b/www/manager6/ha/NodePrioritySelector.js > @@ -10,6 +10,16 @@ Ext.define('PVE.forms.NodePrioritySelector', { > selectAll: false, > isFormField: true, > > + config: { > + useNodePriority: null, > + }, > + > + publishes: ['useNodePriority'], > + > + viewModel: { > + showNodePriority: null, > + }, > + > store: { > autoLoad: true, > fields: ['node', 'cpu', 'mem', 'priority'], > @@ -28,7 +38,7 @@ Ext.define('PVE.forms.NodePrioritySelector', { > columns: [ > { > header: gettext('Node'), > - flex: 1, > + width: 150, > dataIndex: 'node', > }, > { > @@ -42,7 +52,7 @@ Ext.define('PVE.forms.NodePrioritySelector', { > header: gettext('CPU usage'), > renderer: Proxmox.Utils.render_cpu, > sortable: true, > - width: 150, > + flex: 1, > dataIndex: 'cpu', > }, > { > @@ -56,6 +66,14 @@ Ext.define('PVE.forms.NodePrioritySelector', { > minValue: 0, > maxValue: 1000, > isFormField: false, > + bind: { > + hidden: '{!showNodePriority}', > + disabled: '{!showNodePriority}', Currently the priority value is not saved/sent to the backend when submitting the form. It's always undefined. Could it be that you are missing a binding here? value: '{record.priority}', I think that is because you removed the change listener from the 'xtype: 'proxmoxintegerfield' (Priority) in Patch 6. > + }, > + }, > + bind: { > + hidden: '{!showNodePriority}', > + disabled: '{!showNodePriority}', > }, > }, > ], > @@ -74,6 +92,39 @@ Ext.define('PVE.forms.NodePrioritySelector', { > }, > }, > > + invertCheckboxSelection: function () { > + let me = this; > + > + let sm = me.getSelectionModel(); > + > + let allNodeModels = new Set(sm.getStore().getData().items ?? []); nit: Would be better to use .getStore().getRange() [0] here instead of accessing the private .items field via the collection [1]. Since getRange() always returns an array you can also remove nullish coalescing operator here as well. [0] https://docs.sencha.com/extjs/7.0.0/modern/Ext.data.Store.html#method-getRange [1] https://docs.sencha.com/extjs/7.0.0/modern/Ext.util.Collection.html#property-items > + let selectedNodeModels = new Set(sm.getSelection() ?? []); nit: getSelection() should always return an array so '?? []' should not be needed here as well [2]. [2] https://docs.sencha.com/extjs/7.0.0/classic/Ext.selection.Model.html#method-getSelection > + > + if (!allNodeModels.size || !selectedNodeModels.size) { > + return; > + } > + > + sm.deselectAll(); > + sm.select([...allNodeModels.difference(selectedNodeModels)]); > + }, > + > + applyUseNodePriority: function (newValue) { > + let me = this; > + > + let oldValue = me.getViewModel().get('showNodePriority'); > + > + if (newValue !== oldValue) { > + me.getViewModel().set('showNodePriority', newValue); > + > + // Prevent inverting the selection during component initialization > + if (oldValue !== null) { nit: Using if (oldValue != null) here is would be safer because loose inequality catches both null and undefined. This prevents potential initialization failures if the ViewModel's default behavior ever changes. Currently you set it to null. > + me.invertCheckboxSelection(); > + } > + } > + > + return newValue; > + }, > + > getSubmitData: function () { > let me = this; > let res = {}; > @@ -91,7 +142,15 @@ Ext.define('PVE.forms.NodePrioritySelector', { > let sm = me.getSelectionModel(); > let selectedNodeModels = sm.getSelection() ?? []; > let nodes = selectedNodeModels > - .map(({ data }) => data.node + (data.priority ? `:${data.priority}` : '')) > + .map(({ data }) => { > + let nodeEntry = data.node; > + > + if (me.useNodePriority && data.priority) { > + nodeEntry += `:${data.priority}`; > + } > + > + return nodeEntry; > + }) > .join(','); > > return nodes; > diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager6/ha/rules/NodeAffinityRuleEdit.js > index 77da18b1..aecaa276 100644 > --- a/www/manager6/ha/rules/NodeAffinityRuleEdit.js > +++ b/www/manager6/ha/rules/NodeAffinityRuleEdit.js > @@ -1,6 +1,15 @@ > Ext.define('PVE.ha.rules.NodeAffinityInputPanel', { > extend: 'PVE.ha.RuleInputPanel', > > + viewModel: { > + data: { > + affinity: 'positive', > + }, > + formulas: { > + isPositiveNodeAffinity: (get) => get('affinity') === 'positive', > + }, > + }, > + > initComponent: function () { > let me = this; > > @@ -18,6 +27,19 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', { > uncheckedValue: 0, > defaultValue: 0, > }, > + { > + xtype: 'proxmoxKVComboBox', > + name: 'affinity', > + fieldLabel: gettext('Affinity'), > + allowBlank: false, > + comboItems: [ > + ['positive', gettext('Prefer Nodes')], > + ['negative', gettext('Avoid Nodes')], The documentation creates a link between the meaning of 'negative' and 'Avoid Nodes' but it might be nice to have this link in the WebUI as well because in the CLI one has to know what negative and what positive means. Example: # ha-manager rules add node-affinity ha-rule-negative \         --affinity negative --resources ct:200,vm:300 --nodes node3 but no hard feelings as this is well documented in Patch 12/12. It might be sufficient to introduce a tooltip for this. Especially because in the grid it shows up as 'positive' but opening the edit form switches over to 'Prefer Nodes'. It might be worth adding a renderer to the affinity column so the grid displays 'Prefer Nodes' / 'Avoid Nodes' to match the form. > + ], > + bind: { > + value: '{affinity}', > + }, > + }, > ]; > > me.columnB = [ > @@ -25,6 +47,9 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', { > xtype: 'pveNodePrioritySelector', > name: 'nodes', > allowBlank: false, > + bind: { > + useNodePriority: '{isPositiveNodeAffinity}', > + }, > }, > ]; > > diff --git a/www/manager6/ha/rules/NodeAffinityRules.js b/www/manager6/ha/rules/NodeAffinityRules.js > index 6fc42799..089dece8 100644 > --- a/www/manager6/ha/rules/NodeAffinityRules.js > +++ b/www/manager6/ha/rules/NodeAffinityRules.js > @@ -11,6 +11,11 @@ Ext.define('PVE.ha.NodeAffinityRulesView', { > stateId: 'grid-ha-node-affinity-rules', > > columns: [ > + { > + header: gettext('Affinity'), > + width: 75, > + dataIndex: 'affinity', > + }, > { > header: gettext('Strict'), > width: 75,