* [PATCH ha-manager v2 01/12] rules: node affinity: add affinity property to node affinity rules
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-06-02 10:01 ` [PATCH ha-manager v2 02/12] rules: rename ambiguous argument nodes to cluster nodes Daniel Kral
` (10 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
This is in preparation of the next patch, which adds negative node
affinity rules.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
changes since v1:
- none
src/PVE/HA/Groups.pm | 1 +
src/PVE/HA/Rules/NodeAffinity.pm | 9 +++++++++
src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg | 8 ++++----
.../defaults-for-node-affinity-rules.cfg.expect | 6 +++++-
.../inconsistent-node-resource-affinity-rules.cfg.expect | 3 +++
...inity-for-positive-resource-affinity-rules.cfg.expect | 4 ++++
...node-affinity-with-resource-affinity-rules.cfg.expect | 2 ++
...tiple-resource-refs-in-node-affinity-rules.cfg.expect | 3 +++
8 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/src/PVE/HA/Groups.pm b/src/PVE/HA/Groups.pm
index fef7eeb1..ecdfcbcd 100644
--- a/src/PVE/HA/Groups.pm
+++ b/src/PVE/HA/Groups.pm
@@ -147,6 +147,7 @@ sub migrate_groups_to_rules {
type => 'node-affinity',
resources => $resources,
nodes => $nodes,
+ affinity => 'positive',
strict => $groups->{ids}->{$group}->{restricted},
comment => $groups->{ids}->{$group}->{comment},
};
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index 3fa1fdb4..1f15ae2d 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -64,6 +64,14 @@ sub properties {
optional => 0,
},
),
+ affinity => {
+ description => "Describes whether the HA resources are supposed to"
+ . " be placed on the given nodes ('positive').",
+ type => 'string',
+ enum => ['positive'],
+ default => 'positive',
+ optional => 1,
+ },
strict => {
description => "Describes whether the node affinity rule is strict or non-strict.",
verbose_description => <<EODESC,
@@ -86,6 +94,7 @@ sub options {
return {
resources => { optional => 0 },
nodes => { optional => 0 },
+ affinity => { optional => 1 },
strict => { optional => 1 },
disable => { optional => 1 },
comment => { optional => 1 },
diff --git a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
index c8b2f2dd..27658562 100644
--- a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
+++ b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
@@ -1,21 +1,21 @@
-# Case 1: Node Affinity rules are enabled and loose by default, so set it so if it isn't yet.
+# Case 1: Node Affinity rules are enabled, positive and loose by default, so set it so if it isn't yet.
node-affinity: node-affinity-defaults
resources vm:101
nodes node1
-# Case 2: Node Affinity rule is disabled, it shouldn't be enabled afterwards.
+# Case 2: Positive Node Affinity rule is disabled, it shouldn't be enabled afterwards.
node-affinity: node-affinity-disabled
resources vm:102
nodes node2
disable
-# Case 3: Node Affinity rule is disabled with explicit 1 set, it shouldn't be enabled afterwards.
+# Case 3: Positive Node Affinity rule is disabled with explicit 1 set, it shouldn't be enabled afterwards.
node-affinity: node-affinity-disabled-explicit
resources vm:103
nodes node2
disable 1
-# Case 4: Node Affinity rule is set to strict, so it shouldn't be loose afterwards.
+# Case 4: Positive Node Affinity rule is set to strict, so it shouldn't be loose afterwards.
node-affinity: node-affinity-strict
resources vm:104
nodes node3
diff --git a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
index 35d061bd..0d6e5605 100644
--- a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
@@ -1,9 +1,10 @@
--- Log ---
--- Config ---
{
- "digest" : "c96c9de143221a82e44efa8bb4814b8248a8ea11",
+ "digest" : "fcad82def12abc4422061b79cfd0399967053d93",
"ids" : {
"node-affinity-defaults" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
@@ -15,6 +16,7 @@
"type" : "node-affinity"
},
"node-affinity-disabled" : {
+ "affinity" : "positive",
"disable" : 1,
"nodes" : {
"node2" : {
@@ -27,6 +29,7 @@
"type" : "node-affinity"
},
"node-affinity-disabled-explicit" : {
+ "affinity" : "positive",
"disable" : 1,
"nodes" : {
"node2" : {
@@ -39,6 +42,7 @@
"type" : "node-affinity"
},
"node-affinity-strict" : {
+ "affinity" : "positive",
"nodes" : {
"node3" : {
"priority" : 0
diff --git a/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg.expect
index 4317292b..42dbc004 100644
--- a/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg.expect
@@ -22,6 +22,7 @@ Drop rule 'vm503-must-be-on-node2', because at least one resource is in a positi
"type" : "resource-affinity"
},
"vm101-vm102-must-be-on-node1" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
@@ -35,6 +36,7 @@ Drop rule 'vm503-must-be-on-node2', because at least one resource is in a positi
"type" : "node-affinity"
},
"vm201-must-be-on-node1" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
@@ -55,6 +57,7 @@ Drop rule 'vm503-must-be-on-node2', because at least one resource is in a positi
"type" : "resource-affinity"
},
"vm202-must-be-on-node2" : {
+ "affinity" : "positive",
"nodes" : {
"node2" : {
"priority" : 0
diff --git a/src/test/rules_cfgs/infer-node-affinity-for-positive-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/infer-node-affinity-for-positive-resource-affinity-rules.cfg.expect
index ed339777..db505420 100644
--- a/src/test/rules_cfgs/infer-node-affinity-for-positive-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/infer-node-affinity-for-positive-resource-affinity-rules.cfg.expect
@@ -4,6 +4,7 @@
"digest" : "32ae135ef2f8bd84cd12c18af6910dce9d6bc9fa",
"ids" : {
"do-not-infer-negative1" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
@@ -19,6 +20,7 @@
"type" : "node-affinity"
},
"do-not-infer-negative2" : {
+ "affinity" : "positive",
"nodes" : {
"node3" : {
"priority" : 0
@@ -47,6 +49,7 @@
"type" : "resource-affinity"
},
"infer-multi-resources1" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
@@ -76,6 +79,7 @@
"type" : "resource-affinity"
},
"infer-single-resource1" : {
+ "affinity" : "positive",
"nodes" : {
"node3" : {
"priority" : 0
diff --git a/src/test/rules_cfgs/multi-priority-node-affinity-with-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/multi-priority-node-affinity-with-resource-affinity-rules.cfg.expect
index 68a2b75f..cc6595ac 100644
--- a/src/test/rules_cfgs/multi-priority-node-affinity-with-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/multi-priority-node-affinity-with-resource-affinity-rules.cfg.expect
@@ -8,6 +8,7 @@ Drop rule 'vm201-vm202-must-be-on-node1-or-node2', because resources are in a re
"digest" : "722a98914555f296af0916c980a9d6c780f5f072",
"ids" : {
"vm301-must-be-on-node1-with-prio-1" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 1
@@ -28,6 +29,7 @@ Drop rule 'vm201-vm202-must-be-on-node1-or-node2', because resources are in a re
"type" : "resource-affinity"
},
"vm302-must-be-on-node2-with-prio-2" : {
+ "affinity" : "positive",
"nodes" : {
"node2" : {
"priority" : 2
diff --git a/src/test/rules_cfgs/multiple-resource-refs-in-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/multiple-resource-refs-in-node-affinity-rules.cfg.expect
index 425de2b1..2776b56b 100644
--- a/src/test/rules_cfgs/multiple-resource-refs-in-node-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/multiple-resource-refs-in-node-affinity-rules.cfg.expect
@@ -7,6 +7,7 @@ Drop rule 'same-resource3', because resource 'vm:201' is already used in another
"digest" : "5865d23b1a342e7f8cfa68bd0e1da556ca8d28a6",
"ids" : {
"no-same-resource1" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
@@ -24,6 +25,7 @@ Drop rule 'same-resource3', because resource 'vm:201' is already used in another
"type" : "node-affinity"
},
"no-same-resource2" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
@@ -40,6 +42,7 @@ Drop rule 'same-resource3', because resource 'vm:201' is already used in another
"type" : "node-affinity"
},
"no-same-resource3" : {
+ "affinity" : "positive",
"nodes" : {
"node1" : {
"priority" : 0
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH ha-manager v2 02/12] rules: rename ambiguous argument nodes to cluster nodes
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
2026-06-02 10:01 ` [PATCH ha-manager v2 01/12] rules: node affinity: add affinity property to node affinity rules Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:41 ` David Riley
2026-06-02 10:01 ` [PATCH ha-manager v2 03/12] rules: node affinity: implement negative node affinity rules Daniel Kral
` (9 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
Rename any argument instances, which expects a list of currently
configured cluster nodes. This reduces the ambiguity which node list
should be supplied.
Furthermore, rename the global rule checker property 'nodes' to
'cluster-nodes' as well for the same reason.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new! (prep for using 'cluster-nodes' in transform as well)
src/PVE/HA/Rules.pm | 37 ++++++++++++++--------------
src/PVE/HA/Rules/NodeAffinity.pm | 4 +--
src/PVE/HA/Rules/ResourceAffinity.pm | 17 +++++++------
3 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 3a14eeb3..2b48cb60 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -385,13 +385,14 @@ sub get_check_arguments {
return $global_args;
}
-=head3 $class->check_feasibility($rules, $nodes)
+=head3 $class->check_feasibility($rules, $cluster_nodes)
Checks whether the given C<$rules> are feasible by running all checks, which
were registered with C<L<< register_check()|/$class->register_check(...) >>>,
and returns a hash map of erroneous rules.
-C<$nodes> is a list of the configured cluster nodes.
+C<$cluster_nodes> is a list of the configured cluster nodes, which is added as
+an argument for the registered rule feasibility checkers.
The checks are run in the order in which the rule plugins were registered,
while global checks, i.e. checks between different rule types, are run at the
@@ -400,14 +401,14 @@ very last.
=cut
sub check_feasibility {
- my ($class, $rules, $nodes) = @_;
+ my ($class, $rules, $cluster_nodes) = @_;
my $global_errors = {};
my $removable_ruleids = [];
my $global_args = $class->get_check_arguments($rules);
- $global_args->{nodes} = $nodes;
+ $global_args->{'cluster-nodes'} = $cluster_nodes;
for my $type (@$types, 'global') {
for my $entry (@{ $checkdef->{$type} }) {
@@ -421,11 +422,11 @@ sub check_feasibility {
return $global_errors;
}
-=head3 $class->transform($rules, $nodes)
+=head3 $class->transform($rules, $cluster_nodes)
Modifies C<$rules> to contain only feasible rules.
-C<$nodes> is a list of the configured cluster nodes.
+C<$cluster_nodes> is a list of the configured cluster nodes.
This is done by running all checks, which were registered with
C<L<< register_check()|/$class->register_check(...) >>> and removing any
@@ -438,10 +439,10 @@ Returns a list of messages with the reasons why rules were removed.
=cut
sub transform {
- my ($class, $rules, $nodes) = @_;
+ my ($class, $rules, $cluster_nodes) = @_;
my $messages = [];
- my $global_errors = $class->check_feasibility($rules, $nodes);
+ my $global_errors = $class->check_feasibility($rules, $cluster_nodes);
for my $ruleid (keys %$global_errors) {
delete $rules->{ids}->{$ruleid};
@@ -469,33 +470,33 @@ sub transform {
=head3 $class->plugin_compile(...)
-=head3 $class->plugin_compile($rules, $nodes)
+=head3 $class->plugin_compile($rules, $cluster_nodes)
B<MANDATORY:> Must be implemented in a I<rule plugin>.
-Called in C<$class->compile($rules, $nodes)> in order to get a more compact
-representation of the rule plugin's rules in C<$rules>, which holds only the
-relevant information for the scheduler and other users.
+Called in C<$class->compile($rules, $cluster_nodes)> in order to get a more
+compact representation of the rule plugin's rules in C<$rules>, which holds only
+the relevant information for the scheduler and other users.
-C<$nodes> is a list of the configured cluster nodes.
+C<$cluster_nodes> is a list of the configured cluster nodes.
=cut
sub plugin_compile {
- my ($class, $rules, $nodes) = @_;
+ my ($class, $rules, $cluster_nodes) = @_;
die "implement in subclass";
}
=head3 $class->compile(...)
-=head3 $class->compile($rules, $nodes)
+=head3 $class->compile($rules, $cluster_nodes)
Compiles and returns a hash, where each key-value pair represents a rule
plugin's more compact representation compiled from the more verbose rules
defined in C<$rules>.
-C<$nodes> is a list of the configured cluster nodes.
+C<$cluster_nodes> is a list of the configured cluster nodes.
The transformation to the compact representation for each rule plugin is
implemented in C<L<< plugin_compile()|/$class->plugin_compile(...) >>>.
@@ -503,13 +504,13 @@ implemented in C<L<< plugin_compile()|/$class->plugin_compile(...) >>>.
=cut
sub compile {
- my ($class, $rules, $nodes) = @_;
+ my ($class, $rules, $cluster_nodes) = @_;
my $compiled_rules = {};
for my $type (@$types) {
my $plugin = $class->lookup($type);
- my $compiled_plugin_rules = $plugin->plugin_compile($rules, $nodes);
+ my $compiled_plugin_rules = $plugin->plugin_compile($rules, $cluster_nodes);
die "plugin_compile(...) of type '$type' must return hash reference\n"
if ref($compiled_plugin_rules) ne 'HASH';
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index 1f15ae2d..5d4401f6 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -165,7 +165,7 @@ sub get_plugin_check_arguments {
}
sub plugin_compile {
- my ($class, $rules, $nodes) = @_;
+ my ($class, $rules, $cluster_nodes) = @_;
my $node_affinity = {};
@@ -178,7 +178,7 @@ sub plugin_compile {
# add remaining nodes with low priority for non-strict node affinity
if (!$rule->{strict}) {
- for my $node (@$nodes) {
+ for my $node (@$cluster_nodes) {
next if defined($effective_nodes->{$node});
$effective_nodes->{$node} = { priority => -1 };
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index 474d3000..6c43d5ff 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -102,7 +102,7 @@ sub get_plugin_check_arguments {
}
sub plugin_compile {
- my ($class, $rules, $nodes) = @_;
+ my ($class, $rules, $cluster_nodes) = @_;
my $positive = {};
my $negative = {};
@@ -173,23 +173,24 @@ __PACKAGE__->register_check(
},
);
-=head3 check_negative_resource_affinity_resources_count($negative_rules, $nodes)
+=head3 check_negative_resource_affinity_resources_count($negative_rules, $cluster_nodes)
Returns a list of negative resource affinity rule ids, defined in
-C<$negative_rules>, which do have more resources defined than available according
-to the node list C<$nodes>, i.e., there are not enough nodes to separate the
-resources on, even if all nodes are available.
+C<$negative_rules>, which do have more resources defined than available
+according to the currently configured cluster node list C<$cluster_nodes>, i.e.,
+there are not enough nodes to separate the resources on, even if all nodes are
+available.
If there are none, the returned list is empty.
=cut
sub check_negative_resource_affinity_resources_count {
- my ($negative_rules, $nodes) = @_;
+ my ($negative_rules, $cluster_nodes) = @_;
my @conflicts = ();
- my $total_node_count = @$nodes;
+ my $total_node_count = @$cluster_nodes;
while (my ($negativeid, $negative_rule) = each %$negative_rules) {
push @conflicts, $negativeid if keys $negative_rule->{resources}->%* > $total_node_count;
@@ -205,7 +206,7 @@ __PACKAGE__->register_check(
return check_negative_resource_affinity_resources_count(
$args->{negative_rules},
- $args->{nodes},
+ $args->{'cluster-nodes'},
);
},
sub {
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH ha-manager v2 02/12] rules: rename ambiguous argument nodes to cluster nodes
2026-06-02 10:01 ` [PATCH ha-manager v2 02/12] rules: rename ambiguous argument nodes to cluster nodes Daniel Kral
@ 2026-07-13 12:41 ` David Riley
0 siblings, 0 replies; 26+ messages in thread
From: David Riley @ 2026-07-13 12:41 UTC (permalink / raw)
To: Daniel Kral, pve-devel
Looks good to me.
Consider this:
Reviewed-by: David Riley <d.riley@proxmox.com>
On 6/2/26 12:03 PM, Daniel Kral wrote:
> Rename any argument instances, which expects a list of currently
> configured cluster nodes. This reduces the ambiguity which node list
> should be supplied.
>
> Furthermore, rename the global rule checker property 'nodes' to
> 'cluster-nodes' as well for the same reason.
>
> Signed-off-by: Daniel Kral<d.kral@proxmox.com>
> ---
> changes since v1:
> - new! (prep for using 'cluster-nodes' in transform as well)
>
> src/PVE/HA/Rules.pm | 37 ++++++++++++++--------------
> src/PVE/HA/Rules/NodeAffinity.pm | 4 +--
> src/PVE/HA/Rules/ResourceAffinity.pm | 17 +++++++------
> 3 files changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
> index 3a14eeb3..2b48cb60 100644
> --- a/src/PVE/HA/Rules.pm
> +++ b/src/PVE/HA/Rules.pm
> @@ -385,13 +385,14 @@ sub get_check_arguments {
> return $global_args;
> }
>
> -=head3 $class->check_feasibility($rules, $nodes)
> +=head3 $class->check_feasibility($rules, $cluster_nodes)
>
> Checks whether the given C<$rules> are feasible by running all checks, which
> were registered with C<L<< register_check()|/$class->register_check(...) >>>,
> and returns a hash map of erroneous rules.
>
> -C<$nodes> is a list of the configured cluster nodes.
> +C<$cluster_nodes> is a list of the configured cluster nodes, which is added as
> +an argument for the registered rule feasibility checkers.
>
> The checks are run in the order in which the rule plugins were registered,
> while global checks, i.e. checks between different rule types, are run at the
> @@ -400,14 +401,14 @@ very last.
> =cut
>
> sub check_feasibility {
> - my ($class, $rules, $nodes) = @_;
> + my ($class, $rules, $cluster_nodes) = @_;
>
> my $global_errors = {};
> my $removable_ruleids = [];
>
> my $global_args = $class->get_check_arguments($rules);
>
> - $global_args->{nodes} = $nodes;
> + $global_args->{'cluster-nodes'} = $cluster_nodes;
>
> for my $type (@$types, 'global') {
> for my $entry (@{ $checkdef->{$type} }) {
> @@ -421,11 +422,11 @@ sub check_feasibility {
> return $global_errors;
> }
>
> -=head3 $class->transform($rules, $nodes)
> +=head3 $class->transform($rules, $cluster_nodes)
>
> Modifies C<$rules> to contain only feasible rules.
>
> -C<$nodes> is a list of the configured cluster nodes.
> +C<$cluster_nodes> is a list of the configured cluster nodes.
>
> This is done by running all checks, which were registered with
> C<L<< register_check()|/$class->register_check(...) >>> and removing any
> @@ -438,10 +439,10 @@ Returns a list of messages with the reasons why rules were removed.
> =cut
>
> sub transform {
> - my ($class, $rules, $nodes) = @_;
> + my ($class, $rules, $cluster_nodes) = @_;
>
> my $messages = [];
> - my $global_errors = $class->check_feasibility($rules, $nodes);
> + my $global_errors = $class->check_feasibility($rules, $cluster_nodes);
>
> for my $ruleid (keys %$global_errors) {
> delete $rules->{ids}->{$ruleid};
> @@ -469,33 +470,33 @@ sub transform {
>
> =head3 $class->plugin_compile(...)
>
> -=head3 $class->plugin_compile($rules, $nodes)
> +=head3 $class->plugin_compile($rules, $cluster_nodes)
>
> B<MANDATORY:> Must be implemented in a I<rule plugin>.
>
> -Called in C<$class->compile($rules, $nodes)> in order to get a more compact
> -representation of the rule plugin's rules in C<$rules>, which holds only the
> -relevant information for the scheduler and other users.
> +Called in C<$class->compile($rules, $cluster_nodes)> in order to get a more
> +compact representation of the rule plugin's rules in C<$rules>, which holds only
> +the relevant information for the scheduler and other users.
>
> -C<$nodes> is a list of the configured cluster nodes.
> +C<$cluster_nodes> is a list of the configured cluster nodes.
>
> =cut
>
> sub plugin_compile {
> - my ($class, $rules, $nodes) = @_;
> + my ($class, $rules, $cluster_nodes) = @_;
>
> die "implement in subclass";
> }
>
> =head3 $class->compile(...)
>
> -=head3 $class->compile($rules, $nodes)
> +=head3 $class->compile($rules, $cluster_nodes)
>
> Compiles and returns a hash, where each key-value pair represents a rule
> plugin's more compact representation compiled from the more verbose rules
> defined in C<$rules>.
>
> -C<$nodes> is a list of the configured cluster nodes.
> +C<$cluster_nodes> is a list of the configured cluster nodes.
>
> The transformation to the compact representation for each rule plugin is
> implemented in C<L<< plugin_compile()|/$class->plugin_compile(...) >>>.
> @@ -503,13 +504,13 @@ implemented in C<L<< plugin_compile()|/$class->plugin_compile(...) >>>.
> =cut
>
> sub compile {
> - my ($class, $rules, $nodes) = @_;
> + my ($class, $rules, $cluster_nodes) = @_;
>
> my $compiled_rules = {};
>
> for my $type (@$types) {
> my $plugin = $class->lookup($type);
> - my $compiled_plugin_rules = $plugin->plugin_compile($rules, $nodes);
> + my $compiled_plugin_rules = $plugin->plugin_compile($rules, $cluster_nodes);
>
> die "plugin_compile(...) of type '$type' must return hash reference\n"
> if ref($compiled_plugin_rules) ne 'HASH';
> diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
> index 1f15ae2d..5d4401f6 100644
> --- a/src/PVE/HA/Rules/NodeAffinity.pm
> +++ b/src/PVE/HA/Rules/NodeAffinity.pm
> @@ -165,7 +165,7 @@ sub get_plugin_check_arguments {
> }
>
> sub plugin_compile {
> - my ($class, $rules, $nodes) = @_;
> + my ($class, $rules, $cluster_nodes) = @_;
>
> my $node_affinity = {};
>
> @@ -178,7 +178,7 @@ sub plugin_compile {
>
> # add remaining nodes with low priority for non-strict node affinity
> if (!$rule->{strict}) {
> - for my $node (@$nodes) {
> + for my $node (@$cluster_nodes) {
> next if defined($effective_nodes->{$node});
>
> $effective_nodes->{$node} = { priority => -1 };
> diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
> index 474d3000..6c43d5ff 100644
> --- a/src/PVE/HA/Rules/ResourceAffinity.pm
> +++ b/src/PVE/HA/Rules/ResourceAffinity.pm
> @@ -102,7 +102,7 @@ sub get_plugin_check_arguments {
> }
>
> sub plugin_compile {
> - my ($class, $rules, $nodes) = @_;
> + my ($class, $rules, $cluster_nodes) = @_;
>
> my $positive = {};
> my $negative = {};
> @@ -173,23 +173,24 @@ __PACKAGE__->register_check(
> },
> );
>
> -=head3 check_negative_resource_affinity_resources_count($negative_rules, $nodes)
> +=head3 check_negative_resource_affinity_resources_count($negative_rules, $cluster_nodes)
>
> Returns a list of negative resource affinity rule ids, defined in
> -C<$negative_rules>, which do have more resources defined than available according
> -to the node list C<$nodes>, i.e., there are not enough nodes to separate the
> -resources on, even if all nodes are available.
> +C<$negative_rules>, which do have more resources defined than available
> +according to the currently configured cluster node list C<$cluster_nodes>, i.e.,
> +there are not enough nodes to separate the resources on, even if all nodes are
> +available.
>
> If there are none, the returned list is empty.
>
> =cut
>
> sub check_negative_resource_affinity_resources_count {
> - my ($negative_rules, $nodes) = @_;
> + my ($negative_rules, $cluster_nodes) = @_;
>
> my @conflicts = ();
>
> - my $total_node_count = @$nodes;
> + my $total_node_count = @$cluster_nodes;
>
> while (my ($negativeid, $negative_rule) = each %$negative_rules) {
> push @conflicts, $negativeid if keys $negative_rule->{resources}->%* > $total_node_count;
> @@ -205,7 +206,7 @@ __PACKAGE__->register_check(
>
> return check_negative_resource_affinity_resources_count(
> $args->{negative_rules},
> - $args->{nodes},
> + $args->{'cluster-nodes'},
> );
> },
> sub {
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH ha-manager v2 03/12] rules: node affinity: implement negative node affinity rules
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
2026-06-02 10:01 ` [PATCH ha-manager v2 01/12] rules: node affinity: add affinity property to node affinity rules Daniel Kral
2026-06-02 10:01 ` [PATCH ha-manager v2 02/12] rules: rename ambiguous argument nodes to cluster nodes Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:43 ` David Riley
2026-06-02 10:01 ` [PATCH manager v2 04/12] ui: ha: node affinity: handle non-existent nodes Daniel Kral
` (8 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
Extend the existing node affinity rules plugin to allow users to specify
negative node affinity constraints, which specify the nodes where HA
resources SHOULD NOT/MUST NOT be placed.
Negative node affinity rules are internally represented as positive node
affinity rules, where the positive node affinity rules' nodes set is the
set complement of the negative node affinity rules' node set. As this is
semantically equivalent, this allows making no change in the apply logic.
As node priority groups do only hold semantic value for positive node
affinity rules, reject negative node affinity rules with non-zero
priorities and add all resulting nodes to the default priority group.
Furthermore, negative node affinity rules, which specify all cluster
nodes, are dropped as well as their strict variant is unsatisfiable,
because they effectively restrict HA resource to an empty node set, and
the non-strict variant is equivalent to no rule at all.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- add rule checker to disallow specifying node priorities for negative
node affinity rules (thanks @Fiona!)
- add rule checker to disallow specifying all cluster nodes
- rename s/nodes/cluster-nodes/ arguments (thanks @Fiona!)
- rename wrongly named 'positive_nodes' to 'negative_nodes'
- add more test cases
src/PVE/HA/HashTools.pm | 20 +++
src/PVE/HA/Rules.pm | 2 +
src/PVE/HA/Rules/NodeAffinity.pm | 162 +++++++++++++++++-
.../defaults-for-node-affinity-rules.cfg | 15 ++
...efaults-for-node-affinity-rules.cfg.expect | 58 ++++++-
...effective-negative-node-affinity-rules.cfg | 12 ++
...ve-negative-node-affinity-rules.cfg.expect | 17 ++
...iority-in-negative-node-affinity-rules.cfg | 11 ++
...in-negative-node-affinity-rules.cfg.expect | 45 +++++
9 files changed, 339 insertions(+), 3 deletions(-)
create mode 100644 src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg
create mode 100644 src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect
create mode 100644 src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg
create mode 100644 src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect
diff --git a/src/PVE/HA/HashTools.pm b/src/PVE/HA/HashTools.pm
index ebe47e38..b6e2136b 100644
--- a/src/PVE/HA/HashTools.pm
+++ b/src/PVE/HA/HashTools.pm
@@ -6,6 +6,7 @@ use warnings;
use base qw(Exporter);
our @EXPORT_OK = qw(
+ set_difference
set_intersect
set_union
sets_are_disjoint
@@ -29,6 +30,25 @@ more verbose implementation.
=cut
+=head3 set_difference($hash1, $hash2)
+
+Returns a hash set of the set difference between the hash sets C<$hash1> and
+C<$hash2>, i.e. the elements that are in C<$hash1> without the elements that
+are in C<$hash2>.
+
+The hashes C<$hash1> and C<$hash2> are expected to be hash sets, i.e.
+key-value pairs are always set to C<1> or another truthy value.
+
+=cut
+
+sub set_difference : prototype($$) {
+ my ($hash1, $hash2) = @_;
+
+ my $result = { map { $hash2->{$_} ? () : ($_ => 1) } keys %$hash1 };
+
+ return $result;
+}
+
=head3 set_intersect($hash1, $hash2)
Returns a hash set of the intersection of the hash sets C<$hash1> and
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 2b48cb60..bc67de20 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -461,6 +461,8 @@ sub transform {
for my $transform ($transformdef->{$type}->@*) {
my $global_args = $class->get_check_arguments($rules);
+ $global_args->{'cluster-nodes'} = $cluster_nodes;
+
$transform->($rules, $global_args);
}
}
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index 5d4401f6..bbe76720 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -9,6 +9,7 @@ use PVE::Cluster;
use PVE::JSONSchema qw(get_standard_option);
use PVE::Tools;
+use PVE::HA::HashTools qw(set_difference);
use PVE::HA::Rules;
use PVE::HA::Tools;
@@ -28,6 +29,22 @@ PVE::HA::Rules::NodeAffinity
This package provides the capability to specify and apply rules, which put
affinity constraints between a set of HA resources and a set of nodes.
+HA Node Affinity rules can be one of two types:
+
+=over
+
+=item C<'positive'>
+
+Positive node affinity rules specify the nodes, which SHOULD/MUST be preferred
+by the given HA resources.
+
+=item C<'negative'>
+
+Negative node affinity rules specify the nodes, which SHOULD/MUST be avoided by
+the given HA resources.
+
+=back
+
HA Node Affinity rules can be either C<'non-strict'> or C<'strict'>:
=over
@@ -66,9 +83,10 @@ sub properties {
),
affinity => {
description => "Describes whether the HA resources are supposed to"
- . " be placed on the given nodes ('positive').",
+ . " be placed on the given nodes ('positive'), or are supposed"
+ . " to be placed on any but the given nodes ('negative').",
type => 'string',
- enum => ['positive'],
+ enum => ['positive', 'negative'],
default => 'positive',
optional => 1,
},
@@ -256,6 +274,146 @@ __PACKAGE__->register_check(
},
);
+=head3 check_nonempty_negative_nodes_complement($node_affinity_rules, $cluster_nodes)
+
+Returns a list of negative node affinity rule ids in C<$node_affinity_rules>,
+where the complement of the negative node set is an empty node set according to
+the currently configured cluster node list C<$cluster_nodes>, i.e., the
+negative node set specifies all cluster nodes.
+
+Even though this is only relevant for strict negative node affinity rules, this
+check also includes non-strict negative node affinity rules as their effective
+node set would be equivalent to setting no rule at all.
+
+If there are none, the returned list is empty.
+
+=cut
+
+sub check_nonempty_negative_nodes_complement {
+ my ($node_affinity_rules, $cluster_nodes) = @_;
+
+ my @conflicts = ();
+
+ my $total_node_count = @$cluster_nodes;
+
+ while (my ($ruleid, $rule) = each %$node_affinity_rules) {
+ next if $rule->{affinity} ne 'negative';
+
+ push @conflicts, $ruleid if keys $rule->{nodes}->%* >= $total_node_count;
+ }
+
+ @conflicts = sort @conflicts;
+ return \@conflicts;
+}
+
+__PACKAGE__->register_check(
+ sub {
+ my ($args) = @_;
+
+ return check_nonempty_negative_nodes_complement(
+ $args->{node_affinity_rules},
+ $args->{'cluster-nodes'},
+ );
+ },
+ sub {
+ my ($ruleids, $errors) = @_;
+
+ for my $ruleid (@$ruleids) {
+ push $errors->{$ruleid}->{nodes}->@*,
+ "negative node affinity rule must not specify all cluster nodes";
+ }
+ },
+);
+
+=head3 check_unprioritized_negative_nodes($node_affinity_rules)
+
+Returns a list of negative node affinity rule ids in C<$node_affinity_rules>,
+where at least one node has a priority set. A node priority does not have any
+meaningful semantic value for negative node affinity rules.
+
+If there are none, the returned list is empty.
+
+=cut
+
+sub check_unprioritized_negative_nodes {
+ my ($node_affinity_rules) = @_;
+
+ my @conflicts = ();
+
+ while (my ($ruleid, $rule) = each %$node_affinity_rules) {
+ next if $rule->{affinity} ne 'negative';
+
+ for my $node (keys $rule->{nodes}->%*) {
+ if ($rule->{nodes}->{$node}->{priority}) {
+ push @conflicts, $ruleid;
+ last; # one non-zero priority is enough to invalidate rule
+ }
+ }
+ }
+
+ @conflicts = sort @conflicts;
+ return \@conflicts;
+}
+
+__PACKAGE__->register_check(
+ sub {
+ my ($args) = @_;
+
+ return check_unprioritized_negative_nodes($args->{node_affinity_rules});
+ },
+ sub {
+ my ($ruleids, $errors) = @_;
+
+ for my $ruleid (@$ruleids) {
+ push $errors->{$ruleid}->{nodes}->@*,
+ "negative node affinity rule must not specify node priorities";
+ }
+ },
+);
+
+=head1 NODE AFFINITY RULE TRANSFORMATION HELPERS
+
+=cut
+
+=head3 invert_negative_node_affinity_rules($rules, $node_affinity_rules, $cluster_nodes)
+
+Modifies C<$rules> such that all negative node affinity rules, defined in
+C<$node_affinity_rules>, are transformed to positive node affinity rules, where
+the nodes set is the complement of the negative node affinity rules' nodes set.
+
+C<$cluster_nodes> is a list of the configured cluster nodes, which is used as
+the universal set to build the complement node set.
+
+=cut
+
+sub invert_negative_node_affinity_rules {
+ my ($rules, $node_affinity_rules, $cluster_nodes) = @_;
+
+ # set_difference(...) requires a hash set instead of a list
+ my $cluster_nodes_hash = { map { $_ => 1 } @$cluster_nodes };
+
+ while (my ($node_affinity_id, $node_affinity_rule) = each %$node_affinity_rules) {
+ next if $node_affinity_rule->{affinity} ne 'negative';
+
+ my $negative_nodes = { map { $_ => 1 } keys $node_affinity_rule->{nodes}->%* };
+ my $positive_nodes = set_difference($cluster_nodes_hash, $negative_nodes);
+ $positive_nodes->{$_} = { priority => 0 } for keys %$positive_nodes;
+
+ $rules->{ids}->{$node_affinity_id}->{affinity} = 'positive';
+ $rules->{ids}->{$node_affinity_id}->{nodes} = $positive_nodes;
+ }
+}
+
+__PACKAGE__->register_transform(sub {
+ my ($rules, $args) = @_;
+
+ invert_negative_node_affinity_rules(
+ $rules,
+ $args->{node_affinity_rules},
+ $args->{'cluster-nodes'},
+ );
+});
+
=head1 NODE AFFINITY RULE HELPERS
=cut
diff --git a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
index 27658562..b89d725d 100644
--- a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
+++ b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
@@ -20,3 +20,18 @@ node-affinity: node-affinity-strict
resources vm:104
nodes node3
strict 1
+
+# Case 5: Non-strict negative node affinity rule is transformed to positive node affinity rule with complement node set
+# and specified nodes as fallback internally.
+node-affinity: negative-node-affinity-defaults
+ resources vm:105
+ nodes node2
+ affinity negative
+
+# Case 6: Strict negative node affinity rule is transformed to positive node affinity rule with complement node set
+# internally.
+node-affinity: negative-node-affinity-strict
+ resources vm:106
+ nodes node3
+ affinity negative
+ strict 1
diff --git a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
index 0d6e5605..877f1967 100644
--- a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
@@ -1,8 +1,39 @@
--- Log ---
--- Config ---
{
- "digest" : "fcad82def12abc4422061b79cfd0399967053d93",
+ "digest" : "308a8b704d8b4dc3fb83d8c97c86133caf735755",
"ids" : {
+ "negative-node-affinity-defaults" : {
+ "affinity" : "positive",
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:105" : 1
+ },
+ "type" : "node-affinity"
+ },
+ "negative-node-affinity-strict" : {
+ "affinity" : "positive",
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:106" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
"node-affinity-defaults" : {
"affinity" : "positive",
"nodes" : {
@@ -56,6 +87,8 @@
}
},
"order" : {
+ "negative-node-affinity-defaults" : 5,
+ "negative-node-affinity-strict" : 6,
"node-affinity-defaults" : 1,
"node-affinity-disabled" : 2,
"node-affinity-disabled-explicit" : 3,
@@ -84,6 +117,29 @@
"priority" : 0
}
}
+ },
+ "vm:105" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : -1
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:106" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 0
+ }
+ }
}
},
"resource-affinity" : {
diff --git a/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg
new file mode 100644
index 00000000..a238799a
--- /dev/null
+++ b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg
@@ -0,0 +1,12 @@
+# Case 1: Remove non-strict negative node affinity rule, which specifies all cluster nodes.
+node-affinity: nonstrict-negative-node-affinity-with-all-cluster-nodes
+ resources vm:101
+ nodes node1,node2,node3
+ affinity negative
+
+# Case 2: Remove strict negative node affinity rule, which specifies all cluster nodes.
+node-affinity: strict-negative-node-affinity-with-all-cluster-nodes
+ resources vm:102
+ nodes node1,node2,node3
+ affinity negative
+ strict
diff --git a/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect
new file mode 100644
index 00000000..dec03663
--- /dev/null
+++ b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect
@@ -0,0 +1,17 @@
+--- Log ---
+Drop rule 'nonstrict-negative-node-affinity-with-all-cluster-nodes', because negative node affinity rule must not specify all cluster nodes.
+Drop rule 'strict-negative-node-affinity-with-all-cluster-nodes', because negative node affinity rule must not specify all cluster nodes.
+--- Config ---
+{
+ "digest" : "7a68821f23f0b5b110bfac64a66fd7a01564dd74",
+ "ids" : {},
+ "order" : {}
+}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {},
+ "positive" : {}
+ }
+}
diff --git a/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg
new file mode 100644
index 00000000..64a2fd63
--- /dev/null
+++ b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg
@@ -0,0 +1,11 @@
+# Case 1: Do not remove negative node affinity rules with node priorities set to zero as it is equivalent to no node priorities set at all.
+node-affinity: negative-node-affinity-with-default-node-priorities
+ resources vm:101
+ nodes node1:0,node2:0
+ affinity negative
+
+# Case 2: Remove negative node affinity rules with non-zero node priorities.
+node-affinity: negative-node-affinity-with-non-zero-node-priorities
+ resources vm:102
+ nodes node1:2,node2:1
+ affinity negative
diff --git a/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect
new file mode 100644
index 00000000..ef50b25e
--- /dev/null
+++ b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect
@@ -0,0 +1,45 @@
+--- Log ---
+Drop rule 'negative-node-affinity-with-non-zero-node-priorities', because negative node affinity rule must not specify node priorities.
+--- Config ---
+{
+ "digest" : "d95cf03b7f6c8078854506cc79127fd942b868bd",
+ "ids" : {
+ "negative-node-affinity-with-default-node-priorities" : {
+ "affinity" : "positive",
+ "nodes" : {
+ "node3" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:101" : 1
+ },
+ "type" : "node-affinity"
+ }
+ },
+ "order" : {
+ "negative-node-affinity-with-default-node-priorities" : 1
+ }
+}
+--- Compiled Config ---
+{
+ "node-affinity" : {
+ "vm:101" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : -1
+ },
+ "node2" : {
+ "priority" : -1
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ }
+ },
+ "resource-affinity" : {
+ "negative" : {},
+ "positive" : {}
+ }
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH ha-manager v2 03/12] rules: node affinity: implement negative node affinity rules
2026-06-02 10:01 ` [PATCH ha-manager v2 03/12] rules: node affinity: implement negative node affinity rules Daniel Kral
@ 2026-07-13 12:43 ` David Riley
2026-07-14 7:36 ` Daniel Kral
0 siblings, 1 reply; 26+ messages in thread
From: David Riley @ 2026-07-13 12:43 UTC (permalink / raw)
To: pve-devel, Daniel Kral
Thanks for the patch series.
comments inline.
On 6/2/26 12:04 PM, Daniel Kral wrote:
> Extend the existing node affinity rules plugin to allow users to specify
> negative node affinity constraints, which specify the nodes where HA
> resources SHOULD NOT/MUST NOT be placed.
>
> Negative node affinity rules are internally represented as positive node
> affinity rules, where the positive node affinity rules' nodes set is the
> set complement of the negative node affinity rules' node set. As this is
> semantically equivalent, this allows making no change in the apply logic.
>
> As node priority groups do only hold semantic value for positive node
> affinity rules, reject negative node affinity rules with non-zero
> priorities and add all resulting nodes to the default priority group.
>
> Furthermore, negative node affinity rules, which specify all cluster
> nodes, are dropped as well as their strict variant is unsatisfiable,
> because they effectively restrict HA resource to an empty node set, and
> the non-strict variant is equivalent to no rule at all.
>
> Signed-off-by: Daniel Kral<d.kral@proxmox.com>
> ---
> changes since v1:
> - add rule checker to disallow specifying node priorities for negative
> node affinity rules (thanks @Fiona!)
> - add rule checker to disallow specifying all cluster nodes
> - rename s/nodes/cluster-nodes/ arguments (thanks @Fiona!)
> - rename wrongly named 'positive_nodes' to 'negative_nodes'
> - add more test cases
>
> src/PVE/HA/HashTools.pm | 20 +++
> src/PVE/HA/Rules.pm | 2 +
> src/PVE/HA/Rules/NodeAffinity.pm | 162 +++++++++++++++++-
> .../defaults-for-node-affinity-rules.cfg | 15 ++
> ...efaults-for-node-affinity-rules.cfg.expect | 58 ++++++-
> ...effective-negative-node-affinity-rules.cfg | 12 ++
> ...ve-negative-node-affinity-rules.cfg.expect | 17 ++
> ...iority-in-negative-node-affinity-rules.cfg | 11 ++
> ...in-negative-node-affinity-rules.cfg.expect | 45 +++++
> 9 files changed, 339 insertions(+), 3 deletions(-)
> create mode 100644 src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg
> create mode 100644 src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect
> create mode 100644 src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg
> create mode 100644 src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect
>
> diff --git a/src/PVE/HA/HashTools.pm b/src/PVE/HA/HashTools.pm
> index ebe47e38..b6e2136b 100644
> --- a/src/PVE/HA/HashTools.pm
> +++ b/src/PVE/HA/HashTools.pm
> @@ -6,6 +6,7 @@ use warnings;
> use base qw(Exporter);
>
> our @EXPORT_OK = qw(
> + set_difference
> set_intersect
> set_union
> sets_are_disjoint
> @@ -29,6 +30,25 @@ more verbose implementation.
>
> =cut
>
> +=head3 set_difference($hash1, $hash2)
> +
> +Returns a hash set of the set difference between the hash sets C<$hash1> and
> +C<$hash2>, i.e. the elements that are in C<$hash1> without the elements that
> +are in C<$hash2>.
> +
> +The hashes C<$hash1> and C<$hash2> are expected to be hash sets, i.e.
> +key-value pairs are always set to C<1> or another truthy value.
> +
> +=cut
> +
> +sub set_difference : prototype($$) {
Why did you opt for prototype($$) here?
`set_intersect` and `set_union` don't seem to make use of this.
> + my ($hash1, $hash2) = @_;
> +
> + my $result = { map { $hash2->{$_} ? () : ($_ => 1) } keys %$hash1 };
> +
> + return $result;
> +}
> +
> =head3 set_intersect($hash1, $hash2)
>
> Returns a hash set of the intersection of the hash sets C<$hash1> and
> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
> index 2b48cb60..bc67de20 100644
> --- a/src/PVE/HA/Rules.pm
> +++ b/src/PVE/HA/Rules.pm
> @@ -461,6 +461,8 @@ sub transform {
> for my $transform ($transformdef->{$type}->@*) {
> my $global_args = $class->get_check_arguments($rules);
>
> + $global_args->{'cluster-nodes'} = $cluster_nodes;
> +
> $transform->($rules, $global_args);
> }
> }
> diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
> index 5d4401f6..bbe76720 100644
> --- a/src/PVE/HA/Rules/NodeAffinity.pm
> +++ b/src/PVE/HA/Rules/NodeAffinity.pm
> @@ -9,6 +9,7 @@ use PVE::Cluster;
> use PVE::JSONSchema qw(get_standard_option);
> use PVE::Tools;
>
> +use PVE::HA::HashTools qw(set_difference);
> use PVE::HA::Rules;
> use PVE::HA::Tools;
>
> @@ -28,6 +29,22 @@ PVE::HA::Rules::NodeAffinity
> This package provides the capability to specify and apply rules, which put
> affinity constraints between a set of HA resources and a set of nodes.
>
> +HA Node Affinity rules can be one of two types:
> +
> +=over
> +
> +=item C<'positive'>
> +
> +Positive node affinity rules specify the nodes, which SHOULD/MUST be preferred
> +by the given HA resources.
> +
> +=item C<'negative'>
> +
> +Negative node affinity rules specify the nodes, which SHOULD/MUST be avoided by
> +the given HA resources.
> +
> +=back
> +
> HA Node Affinity rules can be either C<'non-strict'> or C<'strict'>:
>
> =over
> @@ -66,9 +83,10 @@ sub properties {
> ),
> affinity => {
> description => "Describes whether the HA resources are supposed to"
> - . " be placed on the given nodes ('positive').",
> + . " be placed on the given nodes ('positive'), or are supposed"
> + . " to be placed on any but the given nodes ('negative').",
> type => 'string',
> - enum => ['positive'],
> + enum => ['positive', 'negative'],
> default => 'positive',
> optional => 1,
> },
> @@ -256,6 +274,146 @@ __PACKAGE__->register_check(
> },
> );
>
> +=head3 check_nonempty_negative_nodes_complement($node_affinity_rules, $cluster_nodes)
> +
> +Returns a list of negative node affinity rule ids in C<$node_affinity_rules>,
> +where the complement of the negative node set is an empty node set according to
> +the currently configured cluster node list C<$cluster_nodes>, i.e., the
> +negative node set specifies all cluster nodes.
> +
> +Even though this is only relevant for strict negative node affinity rules, this
> +check also includes non-strict negative node affinity rules as their effective
> +node set would be equivalent to setting no rule at all.
> +
> +If there are none, the returned list is empty.
> +
> +=cut
> +
> +sub check_nonempty_negative_nodes_complement {
> + my ($node_affinity_rules, $cluster_nodes) = @_;
> +
> + my @conflicts = ();
> +
> + my $total_node_count = @$cluster_nodes;
> +
> + while (my ($ruleid, $rule) = each %$node_affinity_rules) {
I'm not a perl expert by any means, but couldn't this be considered problematic.
Consider this scenario:
You have some kind of function that runs before this is being called which,
iterates over the elements exactly like you do:
while (my ($ruleid, $rule) = each %$node_affinity_rules) {
if ($rule->{affinity} eq 'negative') {
return $ruleid;
}
}
but it has this early return statement. So if you have for example rule_a, rule_b
and rule_c and it returns early on rule_a here, the internal iterator could
still point at rule_a and never actually reset.
It might not be the case at the moment, but further down the line this could
be a wild bug to hunt down or am I misinterpreting the perldocs [0].
Might be safer to just use a regular for loop here instead, but after looking
at other parts of the code this seems to be used a lot in this file.
And I also get why it was used here, as it makes it easier to grab the key-value
pairs directly instead of looking them up manually.
Nothing to worry about right away just something to look out for.
[0] https://perldoc.perl.org/functions/each
> + next if $rule->{affinity} ne 'negative';
> +
> + push @conflicts, $ruleid if keys $rule->{nodes}->%* >= $total_node_count;
> + }
> +
> + @conflicts = sort @conflicts;
> + return \@conflicts;
> +}
> +
> +__PACKAGE__->register_check(
> + sub {
> + my ($args) = @_;
> +
> + return check_nonempty_negative_nodes_complement(
> + $args->{node_affinity_rules},
> + $args->{'cluster-nodes'},
> + );
> + },
> + sub {
> + my ($ruleids, $errors) = @_;
> +
> + for my $ruleid (@$ruleids) {
> + push $errors->{$ruleid}->{nodes}->@*,
> + "negative node affinity rule must not specify all cluster nodes";
> + }
> + },
> +);
> +
> +=head3 check_unprioritized_negative_nodes($node_affinity_rules)
> +
> +Returns a list of negative node affinity rule ids in C<$node_affinity_rules>,
> +where at least one node has a priority set. A node priority does not have any
> +meaningful semantic value for negative node affinity rules.
> +
> +If there are none, the returned list is empty.
> +
> +=cut
> +
> +sub check_unprioritized_negative_nodes {
> + my ($node_affinity_rules) = @_;
> +
> + my @conflicts = ();
> +
> + while (my ($ruleid, $rule) = each %$node_affinity_rules) {
> + next if $rule->{affinity} ne 'negative';
> +
> + for my $node (keys $rule->{nodes}->%*) {
> + if ($rule->{nodes}->{$node}->{priority}) {
> + push @conflicts, $ruleid;
> + last; # one non-zero priority is enough to invalidate rule
> + }
> + }
> + }
> +
> + @conflicts = sort @conflicts;
> + return \@conflicts;
> +}
> +
> +__PACKAGE__->register_check(
> + sub {
> + my ($args) = @_;
> +
> + return check_unprioritized_negative_nodes($args->{node_affinity_rules});
> + },
> + sub {
> + my ($ruleids, $errors) = @_;
> +
> + for my $ruleid (@$ruleids) {
> + push $errors->{$ruleid}->{nodes}->@*,
> + "negative node affinity rule must not specify node priorities";
> + }
> + },
> +);
> +
> +=head1 NODE AFFINITY RULE TRANSFORMATION HELPERS
> +
> +=cut
> +
> +=head3 invert_negative_node_affinity_rules($rules, $node_affinity_rules, $cluster_nodes)
> +
> +Modifies C<$rules> such that all negative node affinity rules, defined in
> +C<$node_affinity_rules>, are transformed to positive node affinity rules, where
> +the nodes set is the complement of the negative node affinity rules' nodes set.
> +
> +C<$cluster_nodes> is a list of the configured cluster nodes, which is used as
> +the universal set to build the complement node set.
> +
> +=cut
> +
> +sub invert_negative_node_affinity_rules {
> + my ($rules, $node_affinity_rules, $cluster_nodes) = @_;
> +
> + # set_difference(...) requires a hash set instead of a list
> + my $cluster_nodes_hash = { map { $_ => 1 } @$cluster_nodes };
> +
> + while (my ($node_affinity_id, $node_affinity_rule) = each %$node_affinity_rules) {
> + next if $node_affinity_rule->{affinity} ne 'negative';
> +
> + my $negative_nodes = { map { $_ => 1 } keys $node_affinity_rule->{nodes}->%* };
> + my $positive_nodes = set_difference($cluster_nodes_hash, $negative_nodes);
> + $positive_nodes->{$_} = { priority => 0 } for keys %$positive_nodes;
> +
> + $rules->{ids}->{$node_affinity_id}->{affinity} = 'positive';
> + $rules->{ids}->{$node_affinity_id}->{nodes} = $positive_nodes;
> + }
> +}
> +
> +__PACKAGE__->register_transform(sub {
> + my ($rules, $args) = @_;
> +
> + invert_negative_node_affinity_rules(
> + $rules,
> + $args->{node_affinity_rules},
> + $args->{'cluster-nodes'},
> + );
> +});
> +
> =head1 NODE AFFINITY RULE HELPERS
>
> =cut
nit: might be nice to split the tests into a separate commit but no hard feelings.
> diff --git a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
> index 27658562..b89d725d 100644
> --- a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
> +++ b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg
> @@ -20,3 +20,18 @@ node-affinity: node-affinity-strict
> resources vm:104
> nodes node3
> strict 1
> +
> +# Case 5: Non-strict negative node affinity rule is transformed to positive node affinity rule with complement node set
> +# and specified nodes as fallback internally.
> +node-affinity: negative-node-affinity-defaults
> + resources vm:105
> + nodes node2
> + affinity negative
> +
> +# Case 6: Strict negative node affinity rule is transformed to positive node affinity rule with complement node set
> +# internally.
> +node-affinity: negative-node-affinity-strict
> + resources vm:106
> + nodes node3
> + affinity negative
> + strict 1
> diff --git a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
> index 0d6e5605..877f1967 100644
> --- a/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
> +++ b/src/test/rules_cfgs/defaults-for-node-affinity-rules.cfg.expect
> @@ -1,8 +1,39 @@
> --- Log ---
> --- Config ---
> {
> - "digest" : "fcad82def12abc4422061b79cfd0399967053d93",
> + "digest" : "308a8b704d8b4dc3fb83d8c97c86133caf735755",
> "ids" : {
> + "negative-node-affinity-defaults" : {
> + "affinity" : "positive",
> + "nodes" : {
> + "node1" : {
> + "priority" : 0
> + },
> + "node3" : {
> + "priority" : 0
> + }
> + },
> + "resources" : {
> + "vm:105" : 1
> + },
> + "type" : "node-affinity"
> + },
> + "negative-node-affinity-strict" : {
> + "affinity" : "positive",
> + "nodes" : {
> + "node1" : {
> + "priority" : 0
> + },
> + "node2" : {
> + "priority" : 0
> + }
> + },
> + "resources" : {
> + "vm:106" : 1
> + },
> + "strict" : 1,
> + "type" : "node-affinity"
> + },
> "node-affinity-defaults" : {
> "affinity" : "positive",
> "nodes" : {
> @@ -56,6 +87,8 @@
> }
> },
> "order" : {
> + "negative-node-affinity-defaults" : 5,
> + "negative-node-affinity-strict" : 6,
> "node-affinity-defaults" : 1,
> "node-affinity-disabled" : 2,
> "node-affinity-disabled-explicit" : 3,
> @@ -84,6 +117,29 @@
> "priority" : 0
> }
> }
> + },
> + "vm:105" : {
> + "nodes" : {
> + "node1" : {
> + "priority" : 0
> + },
> + "node2" : {
> + "priority" : -1
> + },
> + "node3" : {
> + "priority" : 0
> + }
> + }
> + },
> + "vm:106" : {
> + "nodes" : {
> + "node1" : {
> + "priority" : 0
> + },
> + "node2" : {
> + "priority" : 0
> + }
> + }
> }
> },
> "resource-affinity" : {
> diff --git a/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg
> new file mode 100644
> index 00000000..a238799a
> --- /dev/null
> +++ b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg
> @@ -0,0 +1,12 @@
> +# Case 1: Remove non-strict negative node affinity rule, which specifies all cluster nodes.
> +node-affinity: nonstrict-negative-node-affinity-with-all-cluster-nodes
> + resources vm:101
> + nodes node1,node2,node3
> + affinity negative
> +
> +# Case 2: Remove strict negative node affinity rule, which specifies all cluster nodes.
> +node-affinity: strict-negative-node-affinity-with-all-cluster-nodes
> + resources vm:102
> + nodes node1,node2,node3
> + affinity negative
> + strict
> diff --git a/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect
> new file mode 100644
> index 00000000..dec03663
> --- /dev/null
> +++ b/src/test/rules_cfgs/ineffective-negative-node-affinity-rules.cfg.expect
> @@ -0,0 +1,17 @@
> +--- Log ---
> +Drop rule 'nonstrict-negative-node-affinity-with-all-cluster-nodes', because negative node affinity rule must not specify all cluster nodes.
> +Drop rule 'strict-negative-node-affinity-with-all-cluster-nodes', because negative node affinity rule must not specify all cluster nodes.
> +--- Config ---
> +{
> + "digest" : "7a68821f23f0b5b110bfac64a66fd7a01564dd74",
> + "ids" : {},
> + "order" : {}
> +}
> +--- Compiled Config ---
> +{
> + "node-affinity" : {},
> + "resource-affinity" : {
> + "negative" : {},
> + "positive" : {}
> + }
> +}
> diff --git a/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg
> new file mode 100644
> index 00000000..64a2fd63
> --- /dev/null
> +++ b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg
> @@ -0,0 +1,11 @@
> +# Case 1: Do not remove negative node affinity rules with node priorities set to zero as it is equivalent to no node priorities set at all.
> +node-affinity: negative-node-affinity-with-default-node-priorities
> + resources vm:101
> + nodes node1:0,node2:0
> + affinity negative
> +
> +# Case 2: Remove negative node affinity rules with non-zero node priorities.
> +node-affinity: negative-node-affinity-with-non-zero-node-priorities
> + resources vm:102
> + nodes node1:2,node2:1
> + affinity negative
> diff --git a/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect
> new file mode 100644
> index 00000000..ef50b25e
> --- /dev/null
> +++ b/src/test/rules_cfgs/node-priority-in-negative-node-affinity-rules.cfg.expect
> @@ -0,0 +1,45 @@
> +--- Log ---
> +Drop rule 'negative-node-affinity-with-non-zero-node-priorities', because negative node affinity rule must not specify node priorities.
> +--- Config ---
> +{
> + "digest" : "d95cf03b7f6c8078854506cc79127fd942b868bd",
> + "ids" : {
> + "negative-node-affinity-with-default-node-priorities" : {
> + "affinity" : "positive",
> + "nodes" : {
> + "node3" : {
> + "priority" : 0
> + }
> + },
> + "resources" : {
> + "vm:101" : 1
> + },
> + "type" : "node-affinity"
> + }
> + },
> + "order" : {
> + "negative-node-affinity-with-default-node-priorities" : 1
> + }
> +}
> +--- Compiled Config ---
> +{
> + "node-affinity" : {
> + "vm:101" : {
> + "nodes" : {
> + "node1" : {
> + "priority" : -1
> + },
> + "node2" : {
> + "priority" : -1
> + },
> + "node3" : {
> + "priority" : 0
> + }
> + }
> + }
> + },
> + "resource-affinity" : {
> + "negative" : {},
> + "positive" : {}
> + }
> +}
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH ha-manager v2 03/12] rules: node affinity: implement negative node affinity rules
2026-07-13 12:43 ` David Riley
@ 2026-07-14 7:36 ` Daniel Kral
0 siblings, 0 replies; 26+ messages in thread
From: Daniel Kral @ 2026-07-14 7:36 UTC (permalink / raw)
To: David Riley, pve-devel
On Mon Jul 13, 2026 at 2:43 PM CEST, David Riley wrote:
> Thanks for the patch series.
> comments inline.
>
Thanks for taking the time to review this series!
> On 6/2/26 12:04 PM, Daniel Kral wrote:
[ snip ]
>> @@ -29,6 +30,25 @@ more verbose implementation.
>>
>> =cut
>>
>> +=head3 set_difference($hash1, $hash2)
>> +
>> +Returns a hash set of the set difference between the hash sets C<$hash1> and
>> +C<$hash2>, i.e. the elements that are in C<$hash1> without the elements that
>> +are in C<$hash2>.
>> +
>> +The hashes C<$hash1> and C<$hash2> are expected to be hash sets, i.e.
>> +key-value pairs are always set to C<1> or another truthy value.
>> +
>> +=cut
>> +
>> +sub set_difference : prototype($$) {
>
>
> Why did you opt for prototype($$) here?
> `set_intersect` and `set_union` don't seem to make use of this.
>
Good catch, that seems like a remnant of when I introduced this helper
for the HA rules series back then, but the helper went unused in the
end. Will remove that prototype for a newer revision!
>> + my ($hash1, $hash2) = @_;
>> +
>> + my $result = { map { $hash2->{$_} ? () : ($_ => 1) } keys %$hash1 };
>> +
>> + return $result;
>> +}
>> +
>> =head3 set_intersect($hash1, $hash2)
>>
>> Returns a hash set of the intersection of the hash sets C<$hash1> and
[ snip ]
>> @@ -256,6 +274,146 @@ __PACKAGE__->register_check(
>> },
>> );
>>
>> +=head3 check_nonempty_negative_nodes_complement($node_affinity_rules, $cluster_nodes)
>> +
>> +Returns a list of negative node affinity rule ids in C<$node_affinity_rules>,
>> +where the complement of the negative node set is an empty node set according to
>> +the currently configured cluster node list C<$cluster_nodes>, i.e., the
>> +negative node set specifies all cluster nodes.
>> +
>> +Even though this is only relevant for strict negative node affinity rules, this
>> +check also includes non-strict negative node affinity rules as their effective
>> +node set would be equivalent to setting no rule at all.
>> +
>> +If there are none, the returned list is empty.
>> +
>> +=cut
>> +
>> +sub check_nonempty_negative_nodes_complement {
>> + my ($node_affinity_rules, $cluster_nodes) = @_;
>> +
>> + my @conflicts = ();
>> +
>> + my $total_node_count = @$cluster_nodes;
>> +
>> + while (my ($ruleid, $rule) = each %$node_affinity_rules) {
>
>
> I'm not a perl expert by any means, but couldn't this be considered problematic.
>
> Consider this scenario:
> You have some kind of function that runs before this is being called which,
> iterates over the elements exactly like you do:
>
> while (my ($ruleid, $rule) = each %$node_affinity_rules) {
> if ($rule->{affinity} eq 'negative') {
> return $ruleid;
> }
> }
>
> but it has this early return statement. So if you have for example rule_a, rule_b
> and rule_c and it returns early on rule_a here, the internal iterator could
> still point at rule_a and never actually reset.
>
> It might not be the case at the moment, but further down the line this could
> be a wild bug to hunt down or am I misinterpreting the perldocs [0].
>
> Might be safer to just use a regular for loop here instead, but after looking
> at other parts of the code this seems to be used a lot in this file.
> And I also get why it was used here, as it makes it easier to grab the key-value
> pairs directly instead of looking them up manually.
> Nothing to worry about right away just something to look out for.
>
> [0] https://perldoc.perl.org/functions/each
>
Yeah, good catch! If I remember correctly I decided on this syntax usage
here for the HA rules in a code review.
As these checkers are meant to iterate through all rules as these should
check all rules. Otherwise, one should use the mentioned for loop which
dereferences the hash entries. So this becomes only troublesome if
someone doesn't know these each-subroutine caveats and the test cases
would quite likely immediately fail if not all items are evaluated.
FWIW as we EOL PVE 8, we might be able to make this use Perl v5.40,
where for loops with multiple iterations variables [0], which AFAIK do
not have these caveats with a hash-global iterator. Then we don't have
to deal with these kind of faults anymore.
[0] https://perldoc.perl.org/perlexperiment#for-loop-with-multiple-iteration-variables
>> + next if $rule->{affinity} ne 'negative';
>> +
>> + push @conflicts, $ruleid if keys $rule->{nodes}->%* >= $total_node_count;
>> + }
>> +
>> + @conflicts = sort @conflicts;
>> + return \@conflicts;
>> +}
[ snip ]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH manager v2 04/12] ui: ha: node affinity: handle non-existent nodes
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (2 preceding siblings ...)
2026-06-02 10:01 ` [PATCH ha-manager v2 03/12] rules: node affinity: implement negative node affinity rules Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-06-02 10:01 ` [PATCH manager v2 05/12] ui: ha: node affinity: do update node selection all at once Daniel Kral
` (7 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
Though unlikely, the node in a HA node affinity rule could not exist in
the store anymore. Therefore handle non-existent nodes by adding them to
the store.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
www/manager6/ha/rules/NodeAffinityRuleEdit.js | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
index 4574d9ef..1223efc8 100644
--- a/www/manager6/ha/rules/NodeAffinityRuleEdit.js
+++ b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
@@ -99,13 +99,17 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
string.split(',').forEach(function (e, idx, array) {
let [node, priority] = e.split(':');
- store.each(function (record) {
- if (record.get('node') === node) {
- sm.select(record, true);
- record.set('priority', priority);
- record.commit();
- }
- });
+
+ let record = store.findRecord('node', node, 0, false, true, true);
+ if (record) {
+ record.set('priority', priority);
+ record.commit();
+ } else {
+ let addedRecords = store.add({ node, priority });
+ record = addedRecords[0];
+ }
+
+ sm.select(record, true);
});
nodegrid.reconfigure(store);
};
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH manager v2 05/12] ui: ha: node affinity: do update node selection all at once
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (3 preceding siblings ...)
2026-06-02 10:01 ` [PATCH manager v2 04/12] ui: ha: node affinity: handle non-existent nodes Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-06-02 10:01 ` [PATCH manager v2 06/12] ui: ha: node affinity: move node priority selector into separate component Daniel Kral
` (6 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
www/manager6/ha/rules/NodeAffinityRuleEdit.js | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
index 1223efc8..b4b6a13c 100644
--- a/www/manager6/ha/rules/NodeAffinityRuleEdit.js
+++ b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
@@ -95,10 +95,8 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
});
update_node_selection = function (string) {
- sm.deselectAll(true);
-
- string.split(',').forEach(function (e, idx, array) {
- let [node, priority] = e.split(':');
+ let nodes = string.split(',').map((item) => {
+ let [node, priority] = item.split(':');
let record = store.findRecord('node', node, 0, false, true, true);
if (record) {
@@ -109,8 +107,15 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
record = addedRecords[0];
}
- sm.select(record, true);
+ return record;
});
+
+ if (nodes.length) {
+ sm.select(nodes);
+ } else {
+ sm.deselectAll();
+ }
+
nodegrid.reconfigure(store);
};
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH manager v2 06/12] ui: ha: node affinity: move node priority selector into separate component
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (4 preceding siblings ...)
2026-06-02 10:01 ` [PATCH manager v2 05/12] ui: ha: node affinity: do update node selection all at once Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:43 ` David Riley
2026-06-02 10:01 ` [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules Daniel Kral
` (5 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
PVE.form.NodePrioritySelector is a reduced adaption of
PVE.form.VMSelector, which adds error highlights to the node priority
selector and fetches the node list from the API directly.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
www/manager6/Makefile | 1 +
www/manager6/ha/NodePrioritySelector.js | 169 ++++++++++++++++++
www/manager6/ha/rules/NodeAffinityRuleEdit.js | 136 +-------------
3 files changed, 177 insertions(+), 129 deletions(-)
create mode 100644 www/manager6/ha/NodePrioritySelector.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index d4dd3f35..eb0e9d9c 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -151,6 +151,7 @@ JSSRC= \
window/DirMapEdit.js \
window/GuestImport.js \
ha/Fencing.js \
+ ha/NodePrioritySelector.js \
ha/ResourceEdit.js \
ha/Resources.js \
ha/RuleEdit.js \
diff --git a/www/manager6/ha/NodePrioritySelector.js b/www/manager6/ha/NodePrioritySelector.js
new file mode 100644
index 00000000..ec6ac02a
--- /dev/null
+++ b/www/manager6/ha/NodePrioritySelector.js
@@ -0,0 +1,169 @@
+Ext.define('PVE.forms.NodePrioritySelector', {
+ extend: 'Ext.grid.Panel',
+ alias: 'widget.pveNodePrioritySelector',
+
+ mixins: {
+ field: 'Ext.form.field.Field',
+ },
+
+ allowBlank: true,
+ selectAll: false,
+ isFormField: true,
+
+ store: {
+ autoLoad: true,
+ fields: ['node', 'cpu', 'mem', 'priority'],
+ proxy: {
+ type: 'proxmox',
+ url: '/api2/json/nodes',
+ },
+ sorters: [
+ {
+ property: 'node',
+ direction: 'ASC',
+ },
+ ],
+ },
+
+ columns: [
+ {
+ header: gettext('Node'),
+ flex: 1,
+ dataIndex: 'node',
+ },
+ {
+ header: gettext('Memory usage') + ' %',
+ renderer: PVE.Utils.render_mem_usage_percent,
+ sortable: true,
+ width: 150,
+ dataIndex: 'mem',
+ },
+ {
+ header: gettext('CPU usage'),
+ renderer: Proxmox.Utils.render_cpu,
+ sortable: true,
+ width: 150,
+ dataIndex: 'cpu',
+ },
+ {
+ header: gettext('Priority'),
+ xtype: 'widgetcolumn',
+ dataIndex: 'priority',
+ sortable: true,
+ stopSelection: true,
+ widget: {
+ xtype: 'proxmoxintegerfield',
+ minValue: 0,
+ maxValue: 1000,
+ isFormField: false,
+ },
+ },
+ ],
+
+ selModel: {
+ selType: 'checkboxmodel',
+ mode: 'SIMPLE',
+ },
+
+ checkChangeEvents: ['selectionchange', 'change'],
+
+ listeners: {
+ selectionchange: function () {
+ // to trigger validity and error checks
+ this.checkChange();
+ },
+ },
+
+ getSubmitData: function () {
+ let me = this;
+ let res = {};
+ res[me.name] = me.getValue();
+ return res;
+ },
+
+ getValue: function () {
+ let me = this;
+
+ if (me.savedValue !== undefined) {
+ return me.savedValue;
+ }
+
+ let sm = me.getSelectionModel();
+ let selectedNodeModels = sm.getSelection() ?? [];
+ let nodes = selectedNodeModels
+ .map(({ data }) => data.node + (data.priority ? `:${data.priority}` : ''))
+ .join(',');
+
+ return nodes;
+ },
+
+ setValueSelection: function (value) {
+ let me = this;
+
+ let store = me.getStore();
+ let nodes = value.split(',').map((item) => {
+ let [node, priority] = item.split(':');
+
+ let record = store.findRecord('node', node, 0, false, true, true);
+ if (record) {
+ record.set('priority', priority);
+ record.commit();
+ } else {
+ let addedRecords = store.add({ node, priority });
+ record = addedRecords[0];
+ }
+
+ return record;
+ });
+
+ let sm = me.getSelectionModel();
+ if (nodes.length) {
+ sm.select(nodes);
+ } else {
+ sm.deselectAll();
+ }
+
+ me.getErrors();
+ },
+
+ setValue: function (value) {
+ let me = this;
+
+ let store = me.getStore();
+ if (!store.isLoaded()) {
+ me.savedValue = value;
+ store.on(
+ 'load',
+ function () {
+ me.setValueSelection(value);
+ delete me.savedValue;
+ },
+ { single: true },
+ );
+ } else {
+ me.setValueSelection(value);
+ }
+
+ return me.mixins.field.setValue.call(me, value);
+ },
+
+ getErrors: function (value) {
+ let me = this;
+
+ if (!me.isDisabled() && me.allowBlank === false && me.getValue().length === 0) {
+ me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+ return [gettext('No nodes selected')];
+ }
+
+ me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+
+ return [];
+ },
+
+ initComponent: function () {
+ let me = this;
+
+ me.callParent();
+ me.initField();
+ },
+});
diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
index b4b6a13c..77da18b1 100644
--- a/www/manager6/ha/rules/NodeAffinityRuleEdit.js
+++ b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
@@ -4,133 +4,6 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
initComponent: function () {
let me = this;
- /* TODO Node selector should be factored out in its own component */
- let update_nodefield, update_node_selection;
-
- let sm = Ext.create('Ext.selection.CheckboxModel', {
- mode: 'SIMPLE',
- listeners: {
- selectionchange: function (model, selected) {
- update_nodefield(selected);
- },
- },
- });
-
- let store = Ext.create('Ext.data.Store', {
- fields: ['node', 'mem', 'cpu', 'priority'],
- data: PVE.data.ResourceStore.getNodes(), // use already cached data to avoid an API call
- proxy: {
- type: 'memory',
- reader: { type: 'json' },
- },
- sorters: [
- {
- property: 'node',
- direction: 'ASC',
- },
- ],
- });
-
- var nodegrid = Ext.createWidget('grid', {
- store: store,
- border: true,
- height: 300,
- selModel: sm,
- columns: [
- {
- header: gettext('Node'),
- flex: 1,
- dataIndex: 'node',
- },
- {
- header: gettext('Memory usage') + ' %',
- renderer: PVE.Utils.render_mem_usage_percent,
- sortable: true,
- width: 150,
- dataIndex: 'mem',
- },
- {
- header: gettext('CPU usage'),
- renderer: Proxmox.Utils.render_cpu,
- sortable: true,
- width: 150,
- dataIndex: 'cpu',
- },
- {
- header: gettext('Priority'),
- xtype: 'widgetcolumn',
- dataIndex: 'priority',
- sortable: true,
- stopSelection: true,
- widget: {
- xtype: 'proxmoxintegerfield',
- minValue: 0,
- maxValue: 1000,
- isFormField: false,
- listeners: {
- change: function (numberfield, value, old_value) {
- let record = numberfield.getWidgetRecord();
- record.set('priority', value);
- update_nodefield(sm.getSelection());
- record.commit();
- },
- },
- },
- },
- ],
- });
-
- let nodefield = Ext.create('Ext.form.field.Hidden', {
- name: 'nodes',
- value: '',
- listeners: {
- change: function (field, value) {
- update_node_selection(value);
- },
- },
- isValid: function () {
- let value = this.getValue();
- return value && value.length !== 0;
- },
- });
-
- update_node_selection = function (string) {
- let nodes = string.split(',').map((item) => {
- let [node, priority] = item.split(':');
-
- let record = store.findRecord('node', node, 0, false, true, true);
- if (record) {
- record.set('priority', priority);
- record.commit();
- } else {
- let addedRecords = store.add({ node, priority });
- record = addedRecords[0];
- }
-
- return record;
- });
-
- if (nodes.length) {
- sm.select(nodes);
- } else {
- sm.deselectAll();
- }
-
- nodegrid.reconfigure(store);
- };
-
- update_nodefield = function (selected) {
- let nodes = selected
- .map(({ data }) => data.node + (data.priority ? `:${data.priority}` : ''))
- .join(',');
-
- // nodefield change listener calls us again, which results in a
- // endless recursion, suspend the event temporary to avoid this
- nodefield.suspendEvent('change');
- nodefield.setValue(nodes);
- nodefield.resumeEvent('change');
- };
-
me.column2 = [
{
xtype: 'proxmoxcheckbox',
@@ -145,10 +18,15 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
uncheckedValue: 0,
defaultValue: 0,
},
- nodefield,
];
- me.columnB = [nodegrid];
+ me.columnB = [
+ {
+ xtype: 'pveNodePrioritySelector',
+ name: 'nodes',
+ allowBlank: false,
+ },
+ ];
me.callParent();
},
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH manager v2 06/12] ui: ha: node affinity: move node priority selector into separate component
2026-06-02 10:01 ` [PATCH manager v2 06/12] ui: ha: node affinity: move node priority selector into separate component Daniel Kral
@ 2026-07-13 12:43 ` David Riley
2026-07-14 7:57 ` Daniel Kral
0 siblings, 1 reply; 26+ messages in thread
From: David Riley @ 2026-07-13 12:43 UTC (permalink / raw)
To: Daniel Kral, pve-devel
Thanks for sending in this patch.
comments inline.
On 6/2/26 12:02 PM, Daniel Kral wrote:
> PVE.form.NodePrioritySelector is a reduced adaption of
> PVE.form.VMSelector, which adds error highlights to the node priority
> selector and fetches the node list from the API directly.
>
> Signed-off-by: Daniel Kral<d.kral@proxmox.com>
> ---
> changes since v1:
> - new
>
> www/manager6/Makefile | 1 +
> www/manager6/ha/NodePrioritySelector.js | 169 ++++++++++++++++++
> www/manager6/ha/rules/NodeAffinityRuleEdit.js | 136 +-------------
> 3 files changed, 177 insertions(+), 129 deletions(-)
> create mode 100644 www/manager6/ha/NodePrioritySelector.js
>
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index d4dd3f35..eb0e9d9c 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -151,6 +151,7 @@ JSSRC= \
> window/DirMapEdit.js \
> window/GuestImport.js \
> ha/Fencing.js \
> + ha/NodePrioritySelector.js \
> ha/ResourceEdit.js \
> ha/Resources.js \
> ha/RuleEdit.js \
> diff --git a/www/manager6/ha/NodePrioritySelector.js b/www/manager6/ha/NodePrioritySelector.js
> new file mode 100644
> index 00000000..ec6ac02a
> --- /dev/null
> +++ b/www/manager6/ha/NodePrioritySelector.js
> @@ -0,0 +1,169 @@
> +Ext.define('PVE.forms.NodePrioritySelector', {
> + extend: 'Ext.grid.Panel',
> + alias: 'widget.pveNodePrioritySelector',
> +
> + mixins: {
> + field: 'Ext.form.field.Field',
> + },
> +
> + allowBlank: true,
> + selectAll: false,
Does this select all actually do anything here?
I can still select all nodes in the grid?
> + isFormField: true,
> +
> + store: {
> + autoLoad: true,
> + fields: ['node', 'cpu', 'mem', 'priority'],
> + proxy: {
> + type: 'proxmox',
> + url: '/api2/json/nodes',
> + },
> + sorters: [
> + {
> + property: 'node',
> + direction: 'ASC',
> + },
> + ],
> + },
> +
> + columns: [
> + {
> + header: gettext('Node'),
> + flex: 1,
> + dataIndex: 'node',
> + },
> + {
> + header: gettext('Memory usage') + ' %',
> + renderer: PVE.Utils.render_mem_usage_percent,
> + sortable: true,
> + width: 150,
> + dataIndex: 'mem',
> + },
> + {
> + header: gettext('CPU usage'),
> + renderer: Proxmox.Utils.render_cpu,
> + sortable: true,
> + width: 150,
> + dataIndex: 'cpu',
> + },
> + {
> + header: gettext('Priority'),
> + xtype: 'widgetcolumn',
> + dataIndex: 'priority',
> + sortable: true,
> + stopSelection: true,
> + widget: {
> + xtype: 'proxmoxintegerfield',
> + minValue: 0,
> + maxValue: 1000,
> + isFormField: false,
> + },
> + },
> + ],
> +
> + selModel: {
> + selType: 'checkboxmodel',
> + mode: 'SIMPLE',
> + },
> +
> + checkChangeEvents: ['selectionchange', 'change'],
Is this change event here even used?
> +
> + listeners: {
> + selectionchange: function () {
> + // to trigger validity and error checks
> + this.checkChange();
> + },
> + },
> +
> + getSubmitData: function () {
> + let me = this;
> + let res = {};
> + res[me.name] = me.getValue();
> + return res;
> + },
> +
> + getValue: function () {
> + let me = this;
> +
> + if (me.savedValue !== undefined) {
> + return me.savedValue;
> + }
> +
> + let sm = me.getSelectionModel();
> + let selectedNodeModels = sm.getSelection() ?? [];
> + let nodes = selectedNodeModels
> + .map(({ data }) => data.node + (data.priority ? `:${data.priority}` : ''))
> + .join(',');
> +
> + return nodes;
> + },
> +
> + setValueSelection: function (value) {
> + let me = this;
> +
> + let store = me.getStore();
> + let nodes = value.split(',').map((item) => {
> + let [node, priority] = item.split(':');
> +
> + let record = store.findRecord('node', node, 0, false, true, true);
> + if (record) {
> + record.set('priority', priority);
> + record.commit();
> + } else {
> + let addedRecords = store.add({ node, priority });
> + record = addedRecords[0];
> + }
> +
> + return record;
> + });
Since this maps over a potentially long list of nodes and commits each
record individually, wrapping this block in store.beginUpdate() and
store.endUpdate() would prevent it from sending events for each set/add
operation. [0]
[0] https://docs.sencha.com/extjs/7.0.0/modern/Ext.data.Store.html#method-beginUpdate
> +
> + let sm = me.getSelectionModel();
> + if (nodes.length) {
I'm wondering if there is bug here.
If you call split on an empty string ("") using .split(',') it will return
[""], which will lead to nodes.length being 1 and therefore would
evaluate to true in this if. So it will call sm.select with [""].
> + sm.select(nodes);
> + } else {
> + sm.deselectAll();
> + }
> +
> + me.getErrors();
> + },
> +
> + setValue: function (value) {
> + let me = this;
> +
> + let store = me.getStore();
> + if (!store.isLoaded()) {
> + me.savedValue = value;
> + store.on(
> + 'load',
> + function () {
> + me.setValueSelection(value);
> + delete me.savedValue;
> + },
> + { single: true },
> + );
> + } else {
> + me.setValueSelection(value);
> + }
> +
> + return me.mixins.field.setValue.call(me, value);
> + },
> +
> + getErrors: function (value) {
> + let me = this;
> +
> + if (!me.isDisabled() && me.allowBlank === false && me.getValue().length === 0) {
> + me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
> + return [gettext('No nodes selected')];
Not sure if this returned text is visible in the UI. I played around with it and the grid turned
red if no node is selected, but due to the fact that the "Add" button is disabled anyways this
message will never appear, but it does not hurt either.
> + }
> +
> + me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
> +
> + return [];
> + },
Not sure if I like this approach. I tried to look for better solutions but it seems like
there is no extjs native way to handle this.
> +
> + initComponent: function () {
> + let me = this;
> +
> + me.callParent();
> + me.initField();
> + },
> +});
> diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
> index b4b6a13c..77da18b1 100644
> --- a/www/manager6/ha/rules/NodeAffinityRuleEdit.js
> +++ b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
> @@ -4,133 +4,6 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
> initComponent: function () {
> let me = this;
>
> - /* TODO Node selector should be factored out in its own component */
> - let update_nodefield, update_node_selection;
> -
> - let sm = Ext.create('Ext.selection.CheckboxModel', {
> - mode: 'SIMPLE',
> - listeners: {
> - selectionchange: function (model, selected) {
> - update_nodefield(selected);
> - },
> - },
> - });
> -
> - let store = Ext.create('Ext.data.Store', {
> - fields: ['node', 'mem', 'cpu', 'priority'],
> - data: PVE.data.ResourceStore.getNodes(), // use already cached data to avoid an API call
> - proxy: {
> - type: 'memory',
> - reader: { type: 'json' },
> - },
> - sorters: [
> - {
> - property: 'node',
> - direction: 'ASC',
> - },
> - ],
> - });
> -
> - var nodegrid = Ext.createWidget('grid', {
> - store: store,
> - border: true,
> - height: 300,
> - selModel: sm,
> - columns: [
> - {
> - header: gettext('Node'),
> - flex: 1,
> - dataIndex: 'node',
> - },
> - {
> - header: gettext('Memory usage') + ' %',
> - renderer: PVE.Utils.render_mem_usage_percent,
> - sortable: true,
> - width: 150,
> - dataIndex: 'mem',
> - },
> - {
> - header: gettext('CPU usage'),
> - renderer: Proxmox.Utils.render_cpu,
> - sortable: true,
> - width: 150,
> - dataIndex: 'cpu',
> - },
> - {
> - header: gettext('Priority'),
> - xtype: 'widgetcolumn',
> - dataIndex: 'priority',
> - sortable: true,
> - stopSelection: true,
> - widget: {
> - xtype: 'proxmoxintegerfield',
> - minValue: 0,
> - maxValue: 1000,
> - isFormField: false,
> - listeners: {
> - change: function (numberfield, value, old_value) {
> - let record = numberfield.getWidgetRecord();
> - record.set('priority', value);
> - update_nodefield(sm.getSelection());
> - record.commit();
> - },
> - },
> - },
> - },
> - ],
> - });
> -
> - let nodefield = Ext.create('Ext.form.field.Hidden', {
> - name: 'nodes',
> - value: '',
> - listeners: {
> - change: function (field, value) {
> - update_node_selection(value);
> - },
> - },
> - isValid: function () {
> - let value = this.getValue();
> - return value && value.length !== 0;
> - },
> - });
> -
> - update_node_selection = function (string) {
> - let nodes = string.split(',').map((item) => {
> - let [node, priority] = item.split(':');
> -
> - let record = store.findRecord('node', node, 0, false, true, true);
> - if (record) {
> - record.set('priority', priority);
> - record.commit();
> - } else {
> - let addedRecords = store.add({ node, priority });
> - record = addedRecords[0];
> - }
> -
> - return record;
> - });
> -
> - if (nodes.length) {
> - sm.select(nodes);
> - } else {
> - sm.deselectAll();
> - }
> -
> - nodegrid.reconfigure(store);
> - };
> -
> - update_nodefield = function (selected) {
> - let nodes = selected
> - .map(({ data }) => data.node + (data.priority ? `:${data.priority}` : ''))
> - .join(',');
> -
> - // nodefield change listener calls us again, which results in a
> - // endless recursion, suspend the event temporary to avoid this
> - nodefield.suspendEvent('change');
> - nodefield.setValue(nodes);
> - nodefield.resumeEvent('change');
> - };
> -
> me.column2 = [
> {
> xtype: 'proxmoxcheckbox',
> @@ -145,10 +18,15 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
> uncheckedValue: 0,
> defaultValue: 0,
> },
> - nodefield,
> ];
>
> - me.columnB = [nodegrid];
> + me.columnB = [
> + {
> + xtype: 'pveNodePrioritySelector',
> + name: 'nodes',
> + allowBlank: false,
> + },
> + ];
>
> me.callParent();
> },
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH manager v2 06/12] ui: ha: node affinity: move node priority selector into separate component
2026-07-13 12:43 ` David Riley
@ 2026-07-14 7:57 ` Daniel Kral
2026-07-14 8:51 ` David Riley
0 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-07-14 7:57 UTC (permalink / raw)
To: David Riley, pve-devel
On Mon Jul 13, 2026 at 2:43 PM CEST, David Riley wrote:
> Thanks for sending in this patch.
> comments inline.
>
> On 6/2/26 12:02 PM, Daniel Kral wrote:
[ snip ]
>> diff --git a/www/manager6/ha/NodePrioritySelector.js b/www/manager6/ha/NodePrioritySelector.js
>> new file mode 100644
>> index 00000000..ec6ac02a
>> --- /dev/null
>> +++ b/www/manager6/ha/NodePrioritySelector.js
>> @@ -0,0 +1,169 @@
>> +Ext.define('PVE.forms.NodePrioritySelector', {
>> + extend: 'Ext.grid.Panel',
>> + alias: 'widget.pveNodePrioritySelector',
>> +
>> + mixins: {
>> + field: 'Ext.form.field.Field',
>> + },
>> +
>> + allowBlank: true,
>> + selectAll: false,
>
> Does this select all actually do anything here?
> I can still select all nodes in the grid?
>
Good question, to be honest, I aligned this with the existing
PVE.form.VMSelector and similar extensions + mixins from Ext.grid.Panel
and Ext.form.field.Field...
At least for ExtJS 7.0.0 I cannot find anywhere in our codebase nor the
ExtJS codebase where this property is some flag. It only is used as a
class method in some components.
Will remove that in the next revision and might send a patch which
removes the same property from the other components as well. Haven't
checked though whether this had any functionality in past versions of
ExtJS.
>> + isFormField: true,
>> +
>> + store: {
>> + autoLoad: true,
>> + fields: ['node', 'cpu', 'mem', 'priority'],
>> + proxy: {
>> + type: 'proxmox',
>> + url: '/api2/json/nodes',
>> + },
>> + sorters: [
>> + {
>> + property: 'node',
>> + direction: 'ASC',
>> + },
>> + ],
>> + },
>> +
>> + columns: [
>> + {
>> + header: gettext('Node'),
>> + flex: 1,
>> + dataIndex: 'node',
>> + },
>> + {
>> + header: gettext('Memory usage') + ' %',
>> + renderer: PVE.Utils.render_mem_usage_percent,
>> + sortable: true,
>> + width: 150,
>> + dataIndex: 'mem',
>> + },
>> + {
>> + header: gettext('CPU usage'),
>> + renderer: Proxmox.Utils.render_cpu,
>> + sortable: true,
>> + width: 150,
>> + dataIndex: 'cpu',
>> + },
>> + {
>> + header: gettext('Priority'),
>> + xtype: 'widgetcolumn',
>> + dataIndex: 'priority',
>> + sortable: true,
>> + stopSelection: true,
>> + widget: {
>> + xtype: 'proxmoxintegerfield',
>> + minValue: 0,
>> + maxValue: 1000,
>> + isFormField: false,
>> + },
>> + },
>> + ],
>> +
>> + selModel: {
>> + selType: 'checkboxmodel',
>> + mode: 'SIMPLE',
>> + },
>> +
>> + checkChangeEvents: ['selectionchange', 'change'],
>
> Is this change event here even used?
>
You're right, doesn't seem like it... I'll remove it if I don't see any
reason to leave it here.
>> +
>> + listeners: {
>> + selectionchange: function () {
>> + // to trigger validity and error checks
>> + this.checkChange();
>> + },
>> + },
>> +
>> + getSubmitData: function () {
>> + let me = this;
>> + let res = {};
>> + res[me.name] = me.getValue();
>> + return res;
>> + },
>> +
>> + getValue: function () {
>> + let me = this;
>> +
>> + if (me.savedValue !== undefined) {
>> + return me.savedValue;
>> + }
>> +
>> + let sm = me.getSelectionModel();
>> + let selectedNodeModels = sm.getSelection() ?? [];
>> + let nodes = selectedNodeModels
>> + .map(({ data }) => data.node + (data.priority ? `:${data.priority}` : ''))
>> + .join(',');
>> +
>> + return nodes;
>> + },
>> +
>> + setValueSelection: function (value) {
>> + let me = this;
>> +
>> + let store = me.getStore();
>> + let nodes = value.split(',').map((item) => {
>> + let [node, priority] = item.split(':');
>> +
>> + let record = store.findRecord('node', node, 0, false, true, true);
>> + if (record) {
>> + record.set('priority', priority);
>> + record.commit();
>> + } else {
>> + let addedRecords = store.add({ node, priority });
>> + record = addedRecords[0];
>> + }
>> +
>> + return record;
>> + });
>
> Since this maps over a potentially long list of nodes and commits each
> record individually, wrapping this block in store.beginUpdate() and
> store.endUpdate() would prevent it from sending events for each set/add
> operation. [0]
>
> [0] https://docs.sencha.com/extjs/7.0.0/modern/Ext.data.Store.html#method-beginUpdate
>
Nice, thanks for that!
Though we don't expect the nodelist to become any longer than 48 nodes
as of now, it's good to have this in place already, will use it in the
next revision!
>> +
>> + let sm = me.getSelectionModel();
>> + if (nodes.length) {
>
> I'm wondering if there is bug here.
> If you call split on an empty string ("") using .split(',') it will return
> [""], which will lead to nodes.length being 1 and therefore would
> evaluate to true in this if. So it will call sm.select with [""].
>
>
Yeah, the whole existing code section assumes that the API never outputs
an empty string... Though this would kind of break already at the
splitting of the node items then.
This should not happen. I could slip in a preceding patch, which
filter()s it away, but my gut feeling tells me that it shouldn't ignore
wrong API output as it would make it harder to spot that there's
something wrong there... I'll think about it.
>> + sm.select(nodes);
>> + } else {
>> + sm.deselectAll();
>> + }
>> +
>> + me.getErrors();
>> + },
>> +
>> + setValue: function (value) {
>> + let me = this;
>> +
>> + let store = me.getStore();
>> + if (!store.isLoaded()) {
>> + me.savedValue = value;
>> + store.on(
>> + 'load',
>> + function () {
>> + me.setValueSelection(value);
>> + delete me.savedValue;
>> + },
>> + { single: true },
>> + );
>> + } else {
>> + me.setValueSelection(value);
>> + }
>> +
>> + return me.mixins.field.setValue.call(me, value);
>> + },
>> +
>> + getErrors: function (value) {
>> + let me = this;
>> +
>> + if (!me.isDisabled() && me.allowBlank === false && me.getValue().length === 0) {
>> + me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
>> + return [gettext('No nodes selected')];
>
> Not sure if this returned text is visible in the UI. I played around with it and the grid turned
> red if no node is selected, but due to the fact that the "Add" button is disabled anyways this
> message will never appear, but it does not hurt either.
>
Yeah, this was also taken from the VMSelector component... The nodes
field is required here and probably will always be for the node priority
selector.
>> + }
>> +
>> + me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
>> +
>> + return [];
>> + },
>
>
> Not sure if I like this approach. I tried to look for better solutions but it seems like
> there is no extjs native way to handle this.
>
What approach are you referring to exactly?
>> +
>> + initComponent: function () {
>> + let me = this;
>> +
>> + me.callParent();
>> + me.initField();
>> + },
>> +});
[ snip ]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH manager v2 06/12] ui: ha: node affinity: move node priority selector into separate component
2026-07-14 7:57 ` Daniel Kral
@ 2026-07-14 8:51 ` David Riley
0 siblings, 0 replies; 26+ messages in thread
From: David Riley @ 2026-07-14 8:51 UTC (permalink / raw)
To: Daniel Kral, pve-devel
comment inline
On 7/14/26 9:56 AM, Daniel Kral wrote:
> [ snip ]
>
>>> +
>>> + getErrors: function (value) {
>>> + let me = this;
>>> +
>>> + if (!me.isDisabled() && me.allowBlank === false && me.getValue().length === 0) {
>>> + me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
>>> + return [gettext('No nodes selected')];
>> Not sure if this returned text is visible in the UI. I played around with it and the grid turned
>> red if no node is selected, but due to the fact that the "Add" button is disabled anyways this
>> message will never appear, but it does not hurt either.
>>
> Yeah, this was also taken from the VMSelector component... The nodes
> field is required here and probably will always be for the node priority
> selector.
>
>>> + }
>>> +
>>> + me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
>>> +
>>> + return [];
>>> + },
>>
>> Not sure if I like this approach. I tried to look for better solutions but it seems like
>> there is no extjs native way to handle this.
>>
> What approach are you referring to exactly?
I meant the way getErrors adds side effects by adding/removing CSS classes to highlight
the error in the grid [0]. However, as you mentioned, this seems to be an established way
of handling it here, since it's already done this way in the VMSelector component [1].
[0] https://docs.sencha.com/ext/7.0.0/api/Ext.form.CheckboxGroup.html#method-getErrors
[1] https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/form/VMSelector.js;h=a9e90b04e43b560c7c4ef8c9735f52b1035348dd;hb=b0b650c16c520eaaf62db54810dcf8d66bc4249f#l197
>
>>> +
>>> + initComponent: function () {
>>> + let me = this;
>>> +
>>> + me.callParent();
>>> + me.initField();
>>> + },
>>> +});
> [ snip ]
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (5 preceding siblings ...)
2026-06-02 10:01 ` [PATCH manager v2 06/12] ui: ha: node affinity: move node priority selector into separate component Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:43 ` David Riley
2026-06-02 10:01 ` [PATCH manager v2 08/12] ui: ha: node affinity: do not send default node affinity rule values Daniel Kral
` (4 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
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 <d.kral@proxmox.com>
---
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}',
+ },
+ },
+ 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 ?? []);
+ let selectedNodeModels = new Set(sm.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) {
+ 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')],
+ ],
+ 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,
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules
2026-06-02 10:01 ` [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules Daniel Kral
@ 2026-07-13 12:43 ` David Riley
2026-07-14 8:39 ` Daniel Kral
0 siblings, 1 reply; 26+ messages in thread
From: David Riley @ 2026-07-13 12:43 UTC (permalink / raw)
To: Daniel Kral, pve-devel
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<d.kral@proxmox.com>
> ---
> 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,
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules
2026-07-13 12:43 ` David Riley
@ 2026-07-14 8:39 ` Daniel Kral
0 siblings, 0 replies; 26+ messages in thread
From: Daniel Kral @ 2026-07-14 8:39 UTC (permalink / raw)
To: David Riley, pve-devel
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', {
>> },
>> },
>>
>> + 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.
>
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#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
>
ACK here as well
>> +
>> + 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.
>
Seems good to me as well, thanks!
>> + 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.
>
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}',
>> + },
>> + },
>> ];
>>
>> 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,
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH manager v2 08/12] ui: ha: node affinity: do not send default node affinity rule values
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (6 preceding siblings ...)
2026-06-02 10:01 ` [PATCH manager v2 07/12] ui: ha: node affinity: allow setting affinity for node affinity rules Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:43 ` David Riley
2026-06-02 10:01 ` [PATCH docs v2 09/12] ha-manager: rules: use the correct article for terms starting with HA Daniel Kral
` (3 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
Node affinity rules are non-strict by default and of 'positive' affinity
in the backend, so the redundant information can be removed from the
request.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
www/manager6/ha/rules/NodeAffinityRuleEdit.js | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
index aecaa276..a6ef7832 100644
--- a/www/manager6/ha/rules/NodeAffinityRuleEdit.js
+++ b/www/manager6/ha/rules/NodeAffinityRuleEdit.js
@@ -10,6 +10,15 @@ Ext.define('PVE.ha.rules.NodeAffinityInputPanel', {
},
},
+ onGetValues: function (values) {
+ let me = this;
+
+ PVE.Utils.delete_if_default(values, 'strict', 0, me.isCreate);
+ PVE.Utils.delete_if_default(values, 'affinity', 'positive', me.isCreate);
+
+ return me.callParent([values]);
+ },
+
initComponent: function () {
let me = this;
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH manager v2 08/12] ui: ha: node affinity: do not send default node affinity rule values
2026-06-02 10:01 ` [PATCH manager v2 08/12] ui: ha: node affinity: do not send default node affinity rule values Daniel Kral
@ 2026-07-13 12:43 ` David Riley
0 siblings, 0 replies; 26+ messages in thread
From: David Riley @ 2026-07-13 12:43 UTC (permalink / raw)
To: Daniel Kral, pve-devel
Looks good.
Consider this:
Reviewed-by: David Riley <d.riley@proxmox.com>
On 6/2/26 12:02 PM, Daniel Kral wrote:
> Node affinity rules are non-strict by default and of 'positive' affinity in the backend, so the redundant information can be removed from the request. Signed-off-by: Daniel Kral <d.kral@proxmox.com>---changes·since·v1: -·newwww/manager6/ha/rules/NodeAffinityRuleEdit.js·|·9·+++++++++ 1·file·changed,·9·insertions(+) diff --git a/www/manager6/ha/rules/NodeAffinityRuleEdit.js b/www/manager6/ha/rules/NodeAffinityRuleEdit.js index aecaa276..a6ef7832 100644 --- a/www/manager6/ha/rules/NodeAffinityRuleEdit.js +++ b/www/manager6/ha/rules/NodeAffinityRuleEdit.js @@ -10,6 +10,15 @@Ext.define('PVE.ha.rules.NodeAffinityInputPanel',·{ ········}, ····}, +····onGetValues:·function·(values)·{+········let·me·=·this;++········PVE.Utils.delete_if_default(values,·'strict',·0,·me.isCreate);+········PVE.Utils.delete_if_default(values,·'affinity',·'positive',·me.isCreate);++········return·me.callParent([values]);+····},+····initComponent:·function·()·{ ········let·me·=·this;
> -- 2.47.3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH docs v2 09/12] ha-manager: rules: use the correct article for terms starting with HA
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (7 preceding siblings ...)
2026-06-02 10:01 ` [PATCH manager v2 08/12] ui: ha: node affinity: do not send default node affinity rule values Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:43 ` David Riley
2026-06-02 10:01 ` [PATCH docs v2 10/12] ha-manager: rules: improve resource affinity rule short description Daniel Kral
` (2 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
The correct article four any term starting with "HA" is always 'an',
since the H is silent and therefore becomes a vowel sound.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
ha-manager.adoc | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/ha-manager.adoc b/ha-manager.adoc
index 7365ed0..d34916e 100644
--- a/ha-manager.adoc
+++ b/ha-manager.adoc
@@ -719,9 +719,9 @@ on the same node.
Node Affinity Rules
^^^^^^^^^^^^^^^^^^^
-By default, a HA resource is able to run on any cluster node, but a common
-requirement is that a HA resource should run on a specific node. That can be
-implemented by defining a HA node affinity rule to make the HA resource
+By default, an HA resource is able to run on any cluster node, but a common
+requirement is that an HA resource should run on a specific node. That can be
+implemented by defining an HA node affinity rule to make the HA resource
`vm:100` prefer the node `node1`:
----
@@ -730,8 +730,8 @@ implemented by defining a HA node affinity rule to make the HA resource
By default, node affinity rules are not strict, i.e., if there is none of the
specified nodes available, the HA resource can also be moved to other nodes.
-If, on the other hand, a HA resource must be restricted to the specified nodes,
-then the node affinity rule must be set to be strict.
+If, on the other hand, an HA resource must be restricted to the specified
+nodes, then the node affinity rule must be set to be strict.
In the previous example, the node affinity rule can be modified to restrict the
resource `vm:100` to be only on `node1`:
@@ -823,8 +823,8 @@ separate nodes:
Other than node affinity rules, resource affinity rules are strict by default,
i.e., if the constraints imposed by the resource affinity rules cannot be met
-for a HA resource, the HA Manager will put the HA resource in recovery state in
-case of a failover or in error state elsewhere.
+for an HA resource, the HA Manager will put the HA resource in recovery state
+in case of a failover or in error state elsewhere.
The above commands created the following rules in the rules configuration file:
@@ -905,9 +905,9 @@ Currently, HA rules are checked for the following feasibility tests:
resources as a negative HA resources affinity rule. That is, two or more HA
resources cannot be kept together and separate at the same time.
-* An HA resource can only be part of a HA node affinity rule and a HA resource
- affinity rule at the same time, if the HA node affinity rule has a single
- priority class.
+* An HA resource can only be part of an HA node affinity rule and an HA
+ resource affinity rule at the same time, if the HA node affinity rule has a
+ single priority class.
* The HA resources of a positive HA resource affinity rule can only be part of
a single HA node affinity rule at most.
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH docs v2 09/12] ha-manager: rules: use the correct article for terms starting with HA
2026-06-02 10:01 ` [PATCH docs v2 09/12] ha-manager: rules: use the correct article for terms starting with HA Daniel Kral
@ 2026-07-13 12:43 ` David Riley
0 siblings, 0 replies; 26+ messages in thread
From: David Riley @ 2026-07-13 12:43 UTC (permalink / raw)
To: Daniel Kral, pve-devel
On 6/2/26 12:02 PM, Daniel Kral wrote:
> The correct article four any term starting with "HA" is always 'an',
> since the H is silent and therefore becomes a vowel sound.
But is it really silent? Is it not because
"Its name in English is aitch (pronounced /eɪtʃ/, plural aitches)" [0]
And therefore it should be "an HA ..."? But no hard feelings just wondering.
Otherwise it looks good to me.
Consider this:
Reviewed-by: David Riley <d.riley@proxmox.com>
[0] https://en.wikipedia.org/wiki/H
> Signed-off-by: Daniel Kral<d.kral@proxmox.com>
> ---
> changes since v1:
> - new
>
> ha-manager.adoc | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/ha-manager.adoc b/ha-manager.adoc
> index 7365ed0..d34916e 100644
> --- a/ha-manager.adoc
> +++ b/ha-manager.adoc
> @@ -719,9 +719,9 @@ on the same node.
> Node Affinity Rules
> ^^^^^^^^^^^^^^^^^^^
>
> -By default, a HA resource is able to run on any cluster node, but a common
> -requirement is that a HA resource should run on a specific node. That can be
> -implemented by defining a HA node affinity rule to make the HA resource
> +By default, an HA resource is able to run on any cluster node, but a common
> +requirement is that an HA resource should run on a specific node. That can be
> +implemented by defining an HA node affinity rule to make the HA resource
> `vm:100` prefer the node `node1`:
>
> ----
> @@ -730,8 +730,8 @@ implemented by defining a HA node affinity rule to make the HA resource
>
> By default, node affinity rules are not strict, i.e., if there is none of the
> specified nodes available, the HA resource can also be moved to other nodes.
> -If, on the other hand, a HA resource must be restricted to the specified nodes,
> -then the node affinity rule must be set to be strict.
> +If, on the other hand, an HA resource must be restricted to the specified
> +nodes, then the node affinity rule must be set to be strict.
>
> In the previous example, the node affinity rule can be modified to restrict the
> resource `vm:100` to be only on `node1`:
> @@ -823,8 +823,8 @@ separate nodes:
>
> Other than node affinity rules, resource affinity rules are strict by default,
> i.e., if the constraints imposed by the resource affinity rules cannot be met
> -for a HA resource, the HA Manager will put the HA resource in recovery state in
> -case of a failover or in error state elsewhere.
> +for an HA resource, the HA Manager will put the HA resource in recovery state
> +in case of a failover or in error state elsewhere.
>
> The above commands created the following rules in the rules configuration file:
>
> @@ -905,9 +905,9 @@ Currently, HA rules are checked for the following feasibility tests:
> resources as a negative HA resources affinity rule. That is, two or more HA
> resources cannot be kept together and separate at the same time.
>
> -* An HA resource can only be part of a HA node affinity rule and a HA resource
> - affinity rule at the same time, if the HA node affinity rule has a single
> - priority class.
> +* An HA resource can only be part of an HA node affinity rule and an HA
> + resource affinity rule at the same time, if the HA node affinity rule has a
> + single priority class.
>
> * The HA resources of a positive HA resource affinity rule can only be part of
> a single HA node affinity rule at most.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH docs v2 10/12] ha-manager: rules: improve resource affinity rule short description
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (8 preceding siblings ...)
2026-06-02 10:01 ` [PATCH docs v2 09/12] ha-manager: rules: use the correct article for terms starting with HA Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:44 ` David Riley
2026-06-02 10:01 ` [PATCH docs v2 11/12] ha-manager: rules: adapt rule configuration examples Daniel Kral
2026-06-02 10:01 ` [PATCH docs v2 12/12] ha-manager: rules: add negative node affinity rule descriptions Daniel Kral
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
For conciseness, make the following improvements to the resource
affinity rule short description in a single patch:
- Add newline to separate most important information from additional
nice-to-have information.
- Use the same order of affinity types as in the separate "Resource
affinity rules" section.
- Use the word 'must' to highlight that resource affinity rules are
currently always strict.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
ha-manager.adoc | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/ha-manager.adoc b/ha-manager.adoc
index d34916e..479b539 100644
--- a/ha-manager.adoc
+++ b/ha-manager.adoc
@@ -709,10 +709,11 @@ include::generated/ha-rules-opts.adoc[]
| HA Rule Type | Description
| `node-affinity` | Places affinity from one or more HA resources to one or
more nodes.
-| `resource-affinity` | Places affinity between two or more HA resources. The
-affinity `negative` specifies that HA resources are to be kept on separate
-nodes, while the affinity `positive` specifies that HA resources are to be kept
-on the same node.
+| `resource-affinity` | Places affinity between two or more HA resources.
+
+The affinity `positive` specifies that HA resources must be kept on the same
+node, while the affinity `negative` specifies that HA resources must be kept on
+separate nodes.
|===========================================================
[[ha_manager_node_affinity_rules]]
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH docs v2 10/12] ha-manager: rules: improve resource affinity rule short description
2026-06-02 10:01 ` [PATCH docs v2 10/12] ha-manager: rules: improve resource affinity rule short description Daniel Kral
@ 2026-07-13 12:44 ` David Riley
0 siblings, 0 replies; 26+ messages in thread
From: David Riley @ 2026-07-13 12:44 UTC (permalink / raw)
To: Daniel Kral, pve-devel
Looks good to me.
consider this:
Reviewed-by: David Riley <d.riley@proxmox.com>
On 6/2/26 12:02 PM, Daniel Kral wrote:
> For conciseness, make the following improvements to the resource
> affinity rule short description in a single patch:
>
> - Add newline to separate most important information from additional
> nice-to-have information.
> - Use the same order of affinity types as in the separate "Resource
> affinity rules" section.
> - Use the word 'must' to highlight that resource affinity rules are
> currently always strict.
>
> Signed-off-by: Daniel Kral<d.kral@proxmox.com>
> ---
> changes since v1:
> - new
>
> ha-manager.adoc | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/ha-manager.adoc b/ha-manager.adoc
> index d34916e..479b539 100644
> --- a/ha-manager.adoc
> +++ b/ha-manager.adoc
> @@ -709,10 +709,11 @@ include::generated/ha-rules-opts.adoc[]
> | HA Rule Type | Description
> | `node-affinity` | Places affinity from one or more HA resources to one or
> more nodes.
> -| `resource-affinity` | Places affinity between two or more HA resources. The
> -affinity `negative` specifies that HA resources are to be kept on separate
> -nodes, while the affinity `positive` specifies that HA resources are to be kept
> -on the same node.
> +| `resource-affinity` | Places affinity between two or more HA resources.
> +
> +The affinity `positive` specifies that HA resources must be kept on the same
> +node, while the affinity `negative` specifies that HA resources must be kept on
> +separate nodes.
> |===========================================================
>
> [[ha_manager_node_affinity_rules]]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH docs v2 11/12] ha-manager: rules: adapt rule configuration examples
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (9 preceding siblings ...)
2026-06-02 10:01 ` [PATCH docs v2 10/12] ha-manager: rules: improve resource affinity rule short description Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:44 ` David Riley
2026-06-02 10:01 ` [PATCH docs v2 12/12] ha-manager: rules: add negative node affinity rule descriptions Daniel Kral
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
The section config writer does order the options slightly different, so
adapt these to make the configuration examples deterministic.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
ha-manager.adoc | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ha-manager.adoc b/ha-manager.adoc
index 479b539..802e5b9 100644
--- a/ha-manager.adoc
+++ b/ha-manager.adoc
@@ -763,13 +763,13 @@ The above commands create the following rules in the rules configuration file:
.Node Affinity Rules Configuration Example (`/etc/pve/ha/rules.cfg`)
----
node-affinity: ha-rule-vm100
- resources vm:100
nodes node1
+ resources vm:100
strict 1
node-affinity: priority-cascade
- resources vm:200,ct:300
nodes node1:2,node2:1,node3:1,node4
+ resources vm:200,ct:300
----
Node Affinity Rule Properties
@@ -832,12 +832,12 @@ The above commands created the following rules in the rules configuration file:
.Resource Affinity Rules Configuration Example (`/etc/pve/ha/rules.cfg`)
----
resource-affinity: keep-together
- resources vm:100,vm:200
affinity positive
+ resources vm:100,vm:200
resource-affinity: keep-separate
- resources vm:200,ct:300
affinity negative
+ resources vm:200,ct:300
----
Interactions between Positive and Negative Resource Affinity Rules
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH docs v2 11/12] ha-manager: rules: adapt rule configuration examples
2026-06-02 10:01 ` [PATCH docs v2 11/12] ha-manager: rules: adapt rule configuration examples Daniel Kral
@ 2026-07-13 12:44 ` David Riley
0 siblings, 0 replies; 26+ messages in thread
From: David Riley @ 2026-07-13 12:44 UTC (permalink / raw)
To: Daniel Kral, pve-devel
consider this:
Reviewed-by: David Riley <d.riley@proxmox.com>
On 6/2/26 12:03 PM, Daniel Kral wrote:
> The section config writer does order the options slightly different, so
> adapt these to make the configuration examples deterministic.
>
> Signed-off-by: Daniel Kral<d.kral@proxmox.com>
> ---
> changes since v1:
> - new
>
> ha-manager.adoc | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/ha-manager.adoc b/ha-manager.adoc
> index 479b539..802e5b9 100644
> --- a/ha-manager.adoc
> +++ b/ha-manager.adoc
> @@ -763,13 +763,13 @@ The above commands create the following rules in the rules configuration file:
> .Node Affinity Rules Configuration Example (`/etc/pve/ha/rules.cfg`)
> ----
> node-affinity: ha-rule-vm100
> - resources vm:100
> nodes node1
> + resources vm:100
> strict 1
>
> node-affinity: priority-cascade
> - resources vm:200,ct:300
> nodes node1:2,node2:1,node3:1,node4
> + resources vm:200,ct:300
> ----
>
> Node Affinity Rule Properties
> @@ -832,12 +832,12 @@ The above commands created the following rules in the rules configuration file:
> .Resource Affinity Rules Configuration Example (`/etc/pve/ha/rules.cfg`)
> ----
> resource-affinity: keep-together
> - resources vm:100,vm:200
> affinity positive
> + resources vm:100,vm:200
>
> resource-affinity: keep-separate
> - resources vm:200,ct:300
> affinity negative
> + resources vm:200,ct:300
> ----
>
> Interactions between Positive and Negative Resource Affinity Rules
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH docs v2 12/12] ha-manager: rules: add negative node affinity rule descriptions
2026-06-02 10:01 [PATCH-SERIES docs/ha-manager/manager v2 00/12] Negative Node Affinity Rules Daniel Kral
` (10 preceding siblings ...)
2026-06-02 10:01 ` [PATCH docs v2 11/12] ha-manager: rules: adapt rule configuration examples Daniel Kral
@ 2026-06-02 10:01 ` Daniel Kral
2026-07-13 12:44 ` David Riley
11 siblings, 1 reply; 26+ messages in thread
From: Daniel Kral @ 2026-06-02 10:01 UTC (permalink / raw)
To: pve-devel
Since the node affinity rules can be either 'positive' or 'negative'
now, add some information about the new negative node affinity rules and
why these are interesting to users in larger cluster setups.
The following description about node priority classes is only slightly
changed to make the reading flow a little smoother. Additionally, the HA
resource identifiers are adapted as each can only be referenced by a
single node affinity rule.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- new
ha-manager.adoc | 42 +++++++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/ha-manager.adoc b/ha-manager.adoc
index 802e5b9..369f0ec 100644
--- a/ha-manager.adoc
+++ b/ha-manager.adoc
@@ -709,6 +709,13 @@ include::generated/ha-rules-opts.adoc[]
| HA Rule Type | Description
| `node-affinity` | Places affinity from one or more HA resources to one or
more nodes.
+
+The affinity `positive` specifies that HA resources should/must be kept on one
+of the specified nodes, while the affinity `negative` specifes that the HA
+resources should/must avoid the specified nodes.
+
+The strictness attribute `strict` specifies whether this is a soft constraint
+("should") or whether tis is a hard constraint ("must").
| `resource-affinity` | Places affinity between two or more HA resources.
The affinity `positive` specifies that HA resources must be kept on the same
@@ -741,11 +748,23 @@ resource `vm:100` to be only on `node1`:
# ha-manager rules set node-affinity ha-rule-vm100 --strict 1
----
-For bigger clusters or specific use cases, it makes sense to define a more
-detailed failover behavior. For example, the resources `vm:200` and `ct:300`
-should run on `node1`. If `node1` becomes unavailable, the resources should be
-distributed on `node2` and `node3`. If `node2` and `node3` are also
-unavailable, the resources should run on `node4`.
+Furthermore, node affinity rules use positive affinity by default, meaning that
+the preferred cluster nodes must be explicitly specified. However, in larger
+clusters, it is often more practical to define node affinity using an opt-out
+approach to avoid long and verbose lists of cluster nodes. For example, the
+following command defines an HA node affinity rule that makes the HA resource
+`ct:200` and `vm:300` avoid the node `node3`:
+
+----
+# ha-manager rules add node-affinity ha-rule-negative \
+ --affinity negative --resources ct:200,vm:300 --nodes node3
+----
+
+Moreover, specific use cases might require to define a more detailed failover
+behavior. For example, the resources `vm:400` and `ct:500` should run on
+`node1`. If `node1` becomes unavailable, the resources should be distributed on
+`node2` and `node3`. If `node2` and `node3` are also unavailable, the resources
+should run on `node4`.
To implement this behavior in a node affinity rule, nodes can be paired with
priorities to order the preference for nodes. If two or more nodes have the same
@@ -755,7 +774,7 @@ last `node4` gets the lowest priority, which can be omitted to default to `0`:
----
# ha-manager rules add node-affinity priority-cascade \
- --resources vm:200,ct:300 --nodes "node1:2,node2:1,node3:1,node4"
+ --resources vm:400,ct:500 --nodes "node1:2,node2:1,node3:1,node4"
----
The above commands create the following rules in the rules configuration file:
@@ -767,9 +786,14 @@ node-affinity: ha-rule-vm100
resources vm:100
strict 1
+node-affinity: ha-rule-negative
+ nodes node3
+ resources ct:200,vm:300
+ affinity negative
+
node-affinity: priority-cascade
nodes node1:2,node2:1,node3:1,node4
- resources vm:200,ct:300
+ resources vm:400,ct:500
----
Node Affinity Rule Properties
@@ -896,6 +920,10 @@ Currently, HA rules are checked for the following feasibility tests:
* An HA resource can only be part of a single HA node affinity rule.
+* Negative HA node affinity rules cannot specify node priorities.
+
+* Negative HA node affinity rules cannot specify all cluster nodes.
+
* An HA resource affinity rule must have at least two HA resources.
* A negative HA resource affinity rule cannot specify more HA resources than
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH docs v2 12/12] ha-manager: rules: add negative node affinity rule descriptions
2026-06-02 10:01 ` [PATCH docs v2 12/12] ha-manager: rules: add negative node affinity rule descriptions Daniel Kral
@ 2026-07-13 12:44 ` David Riley
0 siblings, 0 replies; 26+ messages in thread
From: David Riley @ 2026-07-13 12:44 UTC (permalink / raw)
To: Daniel Kral, pve-devel
On 6/2/26 12:03 PM, Daniel Kral wrote:
> Since the node affinity rules can be either 'positive' or 'negative'
> now, add some information about the new negative node affinity rules and
> why these are interesting to users in larger cluster setups.
>
> The following description about node priority classes is only slightly
> changed to make the reading flow a little smoother. Additionally, the HA
> resource identifiers are adapted as each can only be referenced by a
> single node affinity rule.
might be nicer to write:
"Additionally, HA resource identifiers were updated to reflect that each can only be
referenced by a single node affinity rule."
but no hard feelings.
> Signed-off-by: Daniel Kral<d.kral@proxmox.com>
> ---
> changes since v1:
> - new
>
> ha-manager.adoc | 42 +++++++++++++++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/ha-manager.adoc b/ha-manager.adoc
> index 802e5b9..369f0ec 100644
> --- a/ha-manager.adoc
> +++ b/ha-manager.adoc
> @@ -709,6 +709,13 @@ include::generated/ha-rules-opts.adoc[]
> | HA Rule Type | Description
> | `node-affinity` | Places affinity from one or more HA resources to one or
> more nodes.
> +
> +The affinity `positive` specifies that HA resources should/must be kept on one
nit: should or must instead of /
but no hard feelings on that.
> +of the specified nodes, while the affinity `negative` specifes that the HA
typo: specifes/specifies
> +resources should/must avoid the specified nodes.
> +
> +The strictness attribute `strict` specifies whether this is a soft constraint
> +("should") or whether tis is a hard constraint ("must").
typo: tis should probably be it
> | `resource-affinity` | Places affinity between two or more HA resources.
>
> The affinity `positive` specifies that HA resources must be kept on the same
> @@ -741,11 +748,23 @@ resource `vm:100` to be only on `node1`:
> # ha-manager rules set node-affinity ha-rule-vm100 --strict 1
> ----
>
> -For bigger clusters or specific use cases, it makes sense to define a more
> -detailed failover behavior. For example, the resources `vm:200` and `ct:300`
> -should run on `node1`. If `node1` becomes unavailable, the resources should be
> -distributed on `node2` and `node3`. If `node2` and `node3` are also
> -unavailable, the resources should run on `node4`.
> +Furthermore, node affinity rules use positive affinity by default, meaning that
> +the preferred cluster nodes must be explicitly specified. However, in larger
> +clusters, it is often more practical to define node affinity using an opt-out
> +approach to avoid long and verbose lists of cluster nodes. For example, the
> +following command defines an HA node affinity rule that makes the HA resource
typo: plural resource/resources
as we have ct:200 and vm:300
> +`ct:200` and `vm:300` avoid the node `node3`:
> +
> +----
> +# ha-manager rules add node-affinity ha-rule-negative \
> + --affinity negative --resources ct:200,vm:300 --nodes node3
> +----
> +
> +Moreover, specific use cases might require to define a more detailed failover
might sound better:
specific use cases might require defining a more detailed failover behavior.
> +behavior. For example, the resources `vm:400` and `ct:500` should run on
> +`node1`. If `node1` becomes unavailable, the resources should be distributed on
> +`node2` and `node3`. If `node2` and `node3` are also unavailable, the resources
> +should run on `node4`.
>
> To implement this behavior in a node affinity rule, nodes can be paired with
> priorities to order the preference for nodes. If two or more nodes have the same
> @@ -755,7 +774,7 @@ last `node4` gets the lowest priority, which can be omitted to default to `0`:
>
> ----
> # ha-manager rules add node-affinity priority-cascade \
> - --resources vm:200,ct:300 --nodes "node1:2,node2:1,node3:1,node4"
> + --resources vm:400,ct:500 --nodes "node1:2,node2:1,node3:1,node4"
> ----
>
> The above commands create the following rules in the rules configuration file:
> @@ -767,9 +786,14 @@ node-affinity: ha-rule-vm100
> resources vm:100
> strict 1
>
> +node-affinity: ha-rule-negative
> + nodes node3
> + resources ct:200,vm:300
> + affinity negative
> +
> node-affinity: priority-cascade
> nodes node1:2,node2:1,node3:1,node4
> - resources vm:200,ct:300
> + resources vm:400,ct:500
> ----
>
> Node Affinity Rule Properties
> @@ -896,6 +920,10 @@ Currently, HA rules are checked for the following feasibility tests:
>
> * An HA resource can only be part of a single HA node affinity rule.
>
> +* Negative HA node affinity rules cannot specify node priorities.
> +
> +* Negative HA node affinity rules cannot specify all cluster nodes.
> +
> * An HA resource affinity rule must have at least two HA resources.
>
> * A negative HA resource affinity rule cannot specify more HA resources than
^ permalink raw reply [flat|nested] 26+ messages in thread