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<[email protected]>
---
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,



Reply via email to