From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id EC47E1FF0E4 for ; Tue, 14 Jul 2026 10:39:55 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D9456213A2; Tue, 14 Jul 2026 10:39:54 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 14 Jul 2026 10:39:49 +0200 Message-Id: From: "Daniel Kral" To: "David Riley" , Subject: Re: [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules X-Mailer: aerc 0.21.0-147-g43ac7b685014-dirty References: <20260602100226.180071-1-d.kral@proxmox.com> <20260602100226.180071-8-d.kral@proxmox.com> <9d41e54e-4e67-46c7-9df5-90f3aa733eb4@proxmox.com> In-Reply-To: <9d41e54e-4e67-46c7-9df5-90f3aa733eb4@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784018373659 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.338 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: EPVNFI74LWDWK2PUBHFLMF3M4HALP4XG X-Message-ID-Hash: EPVNFI74LWDWK2PUBHFLMF3M4HALP4XG X-MailFrom: d.kral@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: On Mon Jul 13, 2026 at 2:43 PM CEST, David Riley wrote: > Thanks for the patch. > comments inline. > > On 6/2/26 12:02 PM, Daniel Kral wrote: [ snip ] >> @@ -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. > Very good catch, thanks! Yeah, I forgot the change listener to add the priority field to each record as it is changed. >> + }, >> + }, >> + bind: { >> + hidden: '{!showNodePriority}', >> + disabled: '{!showNodePriority}', >> }, >> }, >> ], >> @@ -74,6 +92,39 @@ Ext.define('PVE.forms.NodePrioritySelector', { >> }, >> }, >> =20 >> + invertCheckboxSelection: function () { >> + let me =3D this; >> + >> + let sm =3D me.getSelectionModel(); >> + >> + let allNodeModels =3D new Set(sm.getStore().getData().items ?? = []); > > nit: Would be better to use .getStore().getRange() [0] here instead of ac= cessing the private > .items field via the collection [1]. > Since getRange() always returns an array you can also remove nullish coal= escing > operator here as well. > Nice, will use it in the next revision! > [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#p= roperty-items > >> + let selectedNodeModels =3D new Set(sm.getSelection() ?? []); > > nit: getSelection() should always return an array so '?? []' should not b= e needed > here as well [2]. > > [2] https://docs.sencha.com/extjs/7.0.0/classic/Ext.selection.Model.html#= method-getSelection > ACK here as well >> + >> + if (!allNodeModels.size || !selectedNodeModels.size) { >> + return; >> + } >> + >> + sm.deselectAll(); >> + sm.select([...allNodeModels.difference(selectedNodeModels)]); >> + }, >> + >> + applyUseNodePriority: function (newValue) { >> + let me =3D this; >> + >> + let oldValue =3D me.getViewModel().get('showNodePriority'); >> + >> + if (newValue !=3D=3D oldValue) { >> + me.getViewModel().set('showNodePriority', newValue); >> + >> + // Prevent inverting the selection during component initial= ization >> + if (oldValue !=3D=3D null) { > > nit: Using if (oldValue !=3D null) here is would be safer because loose i= nequality catches > both null and undefined. This prevents potential initialization failures = if the ViewModel's > default behavior ever changes. Currently you set it to null. > Seems good to me as well, thanks! >> + me.invertCheckboxSelection(); >> + } >> + } >> + >> + return newValue; >> + }, >> + >> getSubmitData: function () { >> let me =3D this; >> let res =3D {}; >> @@ -91,7 +142,15 @@ Ext.define('PVE.forms.NodePrioritySelector', { >> let sm =3D me.getSelectionModel(); >> let selectedNodeModels =3D sm.getSelection() ?? []; >> let nodes =3D selectedNodeModels >> - .map(({ data }) =3D> data.node + (data.priority ? `:${data.= priority}` : '')) >> + .map(({ data }) =3D> { >> + let nodeEntry =3D data.node; >> + >> + if (me.useNodePriority && data.priority) { >> + nodeEntry +=3D `:${data.priority}`; >> + } >> + >> + return nodeEntry; >> + }) >> .join(','); >> =20 >> return nodes; >> diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager= 6/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', >> =20 >> + viewModel: { >> + data: { >> + affinity: 'positive', >> + }, >> + formulas: { >> + isPositiveNodeAffinity: (get) =3D> get('affinity') =3D=3D= =3D 'positive', >> + }, >> + }, >> + >> initComponent: function () { >> let me =3D this; >> =20 >> @@ -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 'A= void Nodes' > but it might be nice to have this link in the WebUI as well because in th= e CLI one has > to know what negative and what positive means. > Example: > # ha-manager rules add node-affinity ha-rule-negative \ > =C2=A0 =C2=A0 =C2=A0 =C2=A0 --affinity negative --resources ct:200,vm:30= 0 --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 affini= ty column so the > grid displays 'Prefer Nodes' / 'Avoid Nodes' to match the form. > Right, I thought about it as well before, but didn't go for it in the end. I'll do it now though and will slip in a preceding patch which does the same for the resource affinity rules to be consistent. >> + ], >> + bind: { >> + value: '{affinity}', >> + }, >> + }, >> ]; >> =20 >> me.columnB =3D [ >> @@ -25,6 +47,9 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', { >> xtype: 'pveNodePrioritySelector', >> name: 'nodes', >> allowBlank: false, >> + bind: { >> + useNodePriority: '{isPositiveNodeAffinity}', >> + }, >> }, >> ]; >> =20 >> diff --git a/www/manager6/ha/rules/NodeAffinityRules.js b/www/manager6/h= a/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', >> =20 >> columns: [ >> + { >> + header: gettext('Affinity'), >> + width: 75, >> + dataIndex: 'affinity', >> + }, >> { >> header: gettext('Strict'), >> width: 75,