* [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements
@ 2025-08-21 14:35 Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 01/18] config: do not add ignored resources to dependent resources Daniel Kral
` (18 more replies)
0 siblings, 19 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
I put the patches in decreasing priority:
PATCH 1 fix output of get_resource_info() about dependent resources
PATCH 2 fix retranslating ha rules on nodelist changes
PATCH 3-5 fix wrong assumption about positive resource affinity checks
PATCH 6-18 ha cleanup and performance improvements
Ad PATCH 1, 2, 3-5:
The first few patches fix some wrong assumptions and bugs.
Ad PATCH 3-5:
During the initial HA rules implementation there were quite a few
changes done to how rules were checked and translated, which moved the
merging of positive resource affinity rules to the end of the pipeline.
Not taking notice of this clearly enough, this causes two checks to have
wrong assumptions about positive positive resource affinity rules: they
assume that the resource sets of positive resource affinity rules are
already disjoint from each other, even though that is only done at a
later stage.
As it would be rather cumbersome to interleave checks/pruning infeasible
rules and rule transforms [0] (i.e. move the merging transform inbetween
the previous resource affinity checks and the inter-consistency check; I
tried that but it looked rather ugly), the best method IMO was to use
the already existing helper to find these disjoint sets. This also
provides these checks with the correct positive resource affinity rule
ids to blame instead of building any extra logic for handling that if
checks/transforms would've been interleaved.
Ad PATCH 6-18:
These patches are dependent on PATCH 2 so I put these changes in one
patch series rather than referencing them from each other.
After the fixes, the patch series contains a few patches to clean up
obvious code smells or things I wanted to differently but didn't have
the time to do when it was time for the release. The major thing here is
that the HA rules are compiled into a more compact representation now,
which are used by the scheduler and other users to be more efficient.
Otherwise, larger rule sets could slow down the HA Manager
significantly, because it's harder to traverse the rules hash (e.g. find
a ha resource's rule(s)) instead of directly accessing it in a more
compact hash (i.e. $affinity->{$sid}) for example.
Some of these changes will also make implementing some feature
enhancements such as migration blockers for node affinity rules and
negative node affinity rules easier.
Daniel Kral (18):
config: do not add ignored resources to dependent resources
manager: retranslate rules if nodes are added or removed
rules: factor out disjoint rules' resource set helper
rules: resource affinity: inter-consistency check with merged positive
rules
rules: add merged positive resource affinity info in global checks
rules: make rules sorting optional in foreach_rule helper
rename rule's canonicalize stage to transform stage
rules: make plugins register transformers instead of plugin_transform
rules: node affinity: decouple get_node_affinity helper from Usage
class
compile ha rules to a more compact representation
test: rules: use to_json instead of Data::Dumper for config output
test: rules: add compiled config output to rules config test cases
rules: node affinity: define node priority outside hash access
move minimum version check helper to ha tools
manager: move group migration cooldown variable into helper
api: status: sync active service counting with lrm's helper
manager: group migration: sync active service counting with lrm's
helper
factor out counting of active services into helper
debian/pve-ha-manager.install | 1 +
src/PVE/API2/HA/Rules.pm | 1 +
src/PVE/API2/HA/Status.pm | 17 +-
src/PVE/HA/Config.pm | 18 +-
src/PVE/HA/LRM.pm | 20 +-
src/PVE/HA/Manager.pm | 86 ++--
src/PVE/HA/NodeStatus.pm | 14 +
src/PVE/HA/Rules.pm | 170 ++++++--
src/PVE/HA/Rules/Helpers.pm | 61 +++
src/PVE/HA/Rules/Makefile | 2 +-
src/PVE/HA/Rules/NodeAffinity.pm | 90 ++--
src/PVE/HA/Rules/ResourceAffinity.pm | 200 ++++-----
src/PVE/HA/Tools.pm | 52 +++
...efaults-for-node-affinity-rules.cfg.expect | 145 ++++---
...lts-for-resource-affinity-rules.cfg.expect | 87 ++--
...onsistent-node-resource-affinity-rules.cfg | 19 +
...nt-node-resource-affinity-rules.cfg.expect | 185 ++++++---
.../inconsistent-resource-affinity-rules.cfg | 15 +
...sistent-resource-affinity-rules.cfg.expect | 21 +-
...egative-resource-affinity-rules.cfg.expect | 73 ++--
...fective-resource-affinity-rules.cfg.expect | 18 +-
...egative-resource-affinity-rules.cfg.expect | 342 +++++++++------
...ositive-resource-affinity-rules.cfg.expect | 391 +++++++++++++-----
...egative-resource-affinity-rules.cfg.expect | 186 +++++----
...ositive-resource-affinity-rules.cfg.expect | 264 +++++++++---
...ty-with-resource-affinity-rules.cfg.expect | 114 +++--
...rce-refs-in-node-affinity-rules.cfg.expect | 200 ++++++---
src/test/test_failover1.pl | 10 +-
src/test/test_rules_config.pl | 14 +-
29 files changed, 1838 insertions(+), 978 deletions(-)
create mode 100644 src/PVE/HA/Rules/Helpers.pm
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 01/18] config: do not add ignored resources to dependent resources
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed Daniel Kral
` (17 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Otherwise HA resources that are in positive resource affinity rules with
ignored HA resources will be shown as dependent resources for callers of
get_resource_motion_info(...), even though the ignored HA resource(s)
won't be affected.
These are already ignored in the HA Manager for the migrate command
since 1c9c35d4 ("config, manager: do not check ignored resources with
affinity when migrating").
Fixes: 47340b13 ("api: resources: add check for resource affinity in resource migrations")
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Config.pm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 301c62fe..b2ef26aa 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -394,7 +394,12 @@ sub get_resource_motion_info {
my $rules = read_and_check_effective_rules_config();
my ($together, $separate) = get_affinitive_resources($rules, $sid);
- push @$dependent_resources, $_ for sort keys %$together;
+ for my $csid (sort keys %$together) {
+ next if !defined($ss->{$csid});
+ next if $ss->{$csid}->{state} eq 'ignored';
+
+ push @$dependent_resources, $csid;
+ }
for my $node (keys %$ns) {
next if $ns->{$node} ne 'online';
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 01/18] config: do not add ignored resources to dependent resources Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-27 13:41 ` Michael Köppl
2025-08-27 14:55 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 03/18] rules: factor out disjoint rules' resource set helper Daniel Kral
` (16 subsequent siblings)
18 siblings, 2 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Some rule checks depend on the list of cluster nodes, e.g., to check
whether a negative resource affinity rule doesn't specify more HA
resources than cluster nodes.
The HA Manager retranslate rules only in certain conditions to reduce
unnecessary computations, but lacks a check whether cluster nodes have
been added or removed, which is different from what users are reported
through the rules API endpoints and web interface.
Fixes: 6c4c0458 ("rules: add haenv node list to the rules' canonicalization stage")
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Manager.pm | 2 ++
src/PVE/HA/NodeStatus.pm | 14 ++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index ba59f642..92a2c05e 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -692,6 +692,7 @@ sub manage {
my ($haenv, $ms, $ns, $ss) = ($self->{haenv}, $self->{ms}, $self->{ns}, $self->{ss});
my ($node_info) = $haenv->get_node_info();
+ my $has_changed_nodelist = $ns->check_for_changed_nodelist($node_info);
my ($lrm_results, $lrm_modes) = $self->read_lrm_status();
$ns->update($node_info, $lrm_modes);
@@ -745,6 +746,7 @@ sub manage {
if (
!$self->{rules}
+ || $has_changed_nodelist
|| $new_rules->{digest} ne $self->{last_rules_digest}
|| $self->{groups}->{digest} ne $self->{last_groups_digest}
|| $services_digest && $services_digest ne $self->{last_services_digest}
diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
index 0d04cd53..e2abfd6e 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -92,6 +92,20 @@ sub list_online_nodes {
return $res;
}
+sub check_for_changed_nodelist {
+ my ($self, $node_info) = @_;
+
+ for my $node (sort keys %$node_info) {
+ return 1 if !$self->{status}->{$node};
+ }
+
+ for my $node (sort keys $self->{status}->%*) {
+ return 1 if !$node_info->{$node};
+ }
+
+ return 0;
+}
+
my $delete_node = sub {
my ($self, $node) = @_;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 03/18] rules: factor out disjoint rules' resource set helper
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 01/18] config: do not add ignored resources to dependent resources Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
` (15 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
This is done in preparation to the next patches, where the helper will
be used in different packages.
As the helper is not dependent on resource affinity rules at all, rename
it to find_disjoint_rules_resource_sets(...) and adapt the documentation
accordingly.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
debian/pve-ha-manager.install | 1 +
src/PVE/HA/Rules/Helpers.pm | 61 ++++++++++++++++++++++++++++
src/PVE/HA/Rules/Makefile | 2 +-
src/PVE/HA/Rules/ResourceAffinity.pm | 6 ++-
4 files changed, 67 insertions(+), 3 deletions(-)
create mode 100644 src/PVE/HA/Rules/Helpers.pm
diff --git a/debian/pve-ha-manager.install b/debian/pve-ha-manager.install
index 2e6b7d55..38d5d60b 100644
--- a/debian/pve-ha-manager.install
+++ b/debian/pve-ha-manager.install
@@ -35,6 +35,7 @@
/usr/share/perl5/PVE/HA/Resources/PVECT.pm
/usr/share/perl5/PVE/HA/Resources/PVEVM.pm
/usr/share/perl5/PVE/HA/Rules.pm
+/usr/share/perl5/PVE/HA/Rules/Helpers.pm
/usr/share/perl5/PVE/HA/Rules/NodeAffinity.pm
/usr/share/perl5/PVE/HA/Rules/ResourceAffinity.pm
/usr/share/perl5/PVE/HA/Tools.pm
diff --git a/src/PVE/HA/Rules/Helpers.pm b/src/PVE/HA/Rules/Helpers.pm
new file mode 100644
index 00000000..2e6ea3f8
--- /dev/null
+++ b/src/PVE/HA/Rules/Helpers.pm
@@ -0,0 +1,61 @@
+package PVE::HA::Rules::Helpers;
+
+use strict;
+use warnings;
+
+use PVE::HA::HashTools qw(sets_are_disjoint);
+
+my $sort_by_lowest_resource_id = sub {
+ my ($rules) = @_;
+
+ my $lowest_rule_resource_id = {};
+ for my $ruleid (keys %$rules) {
+ my @rule_resources = sort keys $rules->{$ruleid}->{resources}->%*;
+ $lowest_rule_resource_id->{$ruleid} = $rule_resources[0];
+ }
+
+ # sort rules such that rules with the lowest numbered resource come first
+ my @sorted_ruleids = sort {
+ $lowest_rule_resource_id->{$a} cmp $lowest_rule_resource_id->{$b}
+ } sort keys %$rules;
+
+ return @sorted_ruleids;
+};
+
+# returns a list of hashes, where each hash item contains a resource set
+# disjoint from the other items and the ruleids that were used to form that
+# resource set.
+sub find_disjoint_rules_resource_sets {
+ my ($rules) = @_;
+
+ my @disjoint_rules = ();
+
+ # order needed so that it is easier to check whether there is an overlap
+ my @sorted_ruleids = $sort_by_lowest_resource_id->($rules);
+
+ for my $ruleid (@sorted_ruleids) {
+ my $rule = $rules->{$ruleid};
+
+ my $found = 0;
+ for my $entry (@disjoint_rules) {
+ next if sets_are_disjoint($rule->{resources}, $entry->{resources});
+
+ $found = 1;
+ push @{ $entry->{ruleids} }, $ruleid;
+ $entry->{resources}->{$_} = 1 for keys $rule->{resources}->%*;
+
+ last;
+ }
+ if (!$found) {
+ push @disjoint_rules,
+ {
+ ruleids => [$ruleid],
+ resources => { $rule->{resources}->%* },
+ };
+ }
+ }
+
+ return @disjoint_rules;
+}
+
+1;
diff --git a/src/PVE/HA/Rules/Makefile b/src/PVE/HA/Rules/Makefile
index 64119257..ce030d1f 100644
--- a/src/PVE/HA/Rules/Makefile
+++ b/src/PVE/HA/Rules/Makefile
@@ -1,4 +1,4 @@
-SOURCES=NodeAffinity.pm ResourceAffinity.pm
+SOURCES=Helpers.pm NodeAffinity.pm ResourceAffinity.pm
.PHONY: install
install:
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index 9bc039ba..edfa4b3d 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -3,8 +3,9 @@ package PVE::HA::Rules::ResourceAffinity;
use strict;
use warnings;
-use PVE::HA::HashTools qw(set_intersect sets_are_disjoint);
+use PVE::HA::HashTools qw(set_intersect);
use PVE::HA::Rules;
+use PVE::HA::Rules::Helpers;
use base qw(Exporter);
use base qw(PVE::HA::Rules);
@@ -319,7 +320,8 @@ a resource, in C<$rules> at a later point in time.
sub merge_connected_positive_resource_affinity_rules {
my ($rules, $positive_rules) = @_;
- my @disjoint_positive_rules = $find_disjoint_resource_affinity_rules->($positive_rules);
+ my @disjoint_positive_rules =
+ PVE::HA::Rules::Helpers::find_disjoint_rules_resource_sets($positive_rules);
for my $entry (@disjoint_positive_rules) {
next if @{ $entry->{ruleids} } < 2;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (2 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 03/18] rules: factor out disjoint rules' resource set helper Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-29 12:43 ` Michael Köppl
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 05/18] rules: add merged positive resource affinity info in global checks Daniel Kral
` (14 subsequent siblings)
18 siblings, 1 reply; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
The resource affinity rule set is checked whether it contains pairs of
positive and negative resource affinity rules, which specify two or more
HA resources in them, and prunes them as those are infeasible.
This check has the assumption that each positive resource affinity
rule's resource set is disjoint from each other, but this is only done
in the later transformation stage when positive resource affinity with
overlapping HA resources in them are merged to one rule.
For example, the following inconsistent rules are not pruned:
- positive resource affinity rule between vm:101 and vm:102
- positive resource affinity rule between vm:102 and vm:103
- negative resource affinity rule between vm:101 and vm:103
Therefore build the same disjoint positive resource affinity resource
sets as the merge_connected_positive_resource_affinity_rules(...)
subroutine, so that the inconsistency check has the necessary
information in advance.
Fixes: 367cdbfa ("rules: introduce resource affinity rule plugin")
Reported-by: Hannes Dürr <h.duerr@proxmox.com>
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Rules/ResourceAffinity.pm | 75 ++++---------------
.../inconsistent-resource-affinity-rules.cfg | 15 ++++
...sistent-resource-affinity-rules.cfg.expect | 5 +-
3 files changed, 34 insertions(+), 61 deletions(-)
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index edfa4b3d..e9b368a4 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -206,14 +206,18 @@ sub check_inter_resource_affinity_rules_consistency {
my @conflicts = ();
- while (my ($positiveid, $positive) = each %$positive_rules) {
- my $positive_resources = $positive->{resources};
+ my @disjoint_positive_rules =
+ PVE::HA::Rules::Helpers::find_disjoint_rules_resource_sets($positive_rules);
+
+ for my $entry (@disjoint_positive_rules) {
+ my $positive_resources = $entry->{resources};
while (my ($negativeid, $negative) = each %$negative_rules) {
my $common_resources = set_intersect($positive_resources, $negative->{resources});
next if %$common_resources < 2;
- push @conflicts, [$positiveid, $negativeid];
+ push @conflicts, ['negative', $negativeid];
+ push @conflicts, ['positive', $_] for $entry->{ruleids}->@*;
}
}
@@ -234,12 +238,15 @@ __PACKAGE__->register_check(
my ($conflicts, $errors) = @_;
for my $conflict (@$conflicts) {
- my ($positiveid, $negativeid) = @$conflict;
+ my ($type, $ruleid) = @$conflict;
- push $errors->{$positiveid}->{resources}->@*,
- "rule shares two or more resources with a negative resource affinity rule";
- push $errors->{$negativeid}->{resources}->@*,
- "rule shares two or more resources with a positive resource affinity rule";
+ if ($type eq 'positive') {
+ push $errors->{$ruleid}->{resources}->@*,
+ "rule shares two or more resources with a negative resource affinity rule";
+ } elsif ($type eq 'negative') {
+ push $errors->{$ruleid}->{resources}->@*,
+ "rule shares two or more resources with a positive resource affinity rule";
+ }
}
},
);
@@ -248,58 +255,6 @@ __PACKAGE__->register_check(
=cut
-my $sort_by_lowest_resource_id = sub {
- my ($rules) = @_;
-
- my $lowest_rule_resource_id = {};
- for my $ruleid (keys %$rules) {
- my @rule_resources = sort keys $rules->{$ruleid}->{resources}->%*;
- $lowest_rule_resource_id->{$ruleid} = $rule_resources[0];
- }
-
- # sort rules such that rules with the lowest numbered resource come first
- my @sorted_ruleids = sort {
- $lowest_rule_resource_id->{$a} cmp $lowest_rule_resource_id->{$b}
- } sort keys %$rules;
-
- return @sorted_ruleids;
-};
-
-# returns a list of hashes, which contain disjoint resource affinity rules, i.e.,
-# put resource affinity constraints on disjoint sets of resources
-my $find_disjoint_resource_affinity_rules = sub {
- my ($rules) = @_;
-
- my @disjoint_rules = ();
-
- # order needed so that it is easier to check whether there is an overlap
- my @sorted_ruleids = $sort_by_lowest_resource_id->($rules);
-
- for my $ruleid (@sorted_ruleids) {
- my $rule = $rules->{$ruleid};
-
- my $found = 0;
- for my $entry (@disjoint_rules) {
- next if sets_are_disjoint($rule->{resources}, $entry->{resources});
-
- $found = 1;
- push @{ $entry->{ruleids} }, $ruleid;
- $entry->{resources}->{$_} = 1 for keys $rule->{resources}->%*;
-
- last;
- }
- if (!$found) {
- push @disjoint_rules,
- {
- ruleids => [$ruleid],
- resources => { $rule->{resources}->%* },
- };
- }
- }
-
- return @disjoint_rules;
-};
-
=head3 merge_connected_positive_resource_affinity_rules($rules, $positive_rules)
Modifies C<$rules> to contain only disjoint positive resource affinity rules
diff --git a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg
index a620e293..6bfc2dad 100644
--- a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg
+++ b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg
@@ -1,3 +1,4 @@
+# Case 1: Remove positive and negative resource affinity rules, which share two or more ha resources.
resource-affinity: keep-apart1
resources vm:102,vm:103
affinity negative
@@ -9,3 +10,17 @@ resource-affinity: keep-apart2
resource-affinity: stick-together1
resources vm:101,vm:102,vm:103,vm:104,vm:106
affinity positive
+
+# Case 2: Remove positive and negative resource affinity rules, which share two or more HA resources with the positive
+# resource affinity being split in two.
+resource-affinity: split-stick-together1
+ resources vm:201,vm:202
+ affinity positive
+
+resource-affinity: split-stick-together2
+ resources vm:202,vm:203
+ affinity positive
+
+resource-affinity: split-keep-apart1
+ resources vm:201,vm:203
+ affinity negative
diff --git a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
index d4a2d7b2..8f2338d9 100644
--- a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
@@ -1,11 +1,14 @@
--- Log ---
Drop rule 'keep-apart1', because rule shares two or more resources with a positive resource affinity rule.
Drop rule 'keep-apart2', because rule shares two or more resources with a positive resource affinity rule.
+Drop rule 'split-keep-apart1', because rule shares two or more resources with a positive resource affinity rule.
+Drop rule 'split-stick-together1', because rule shares two or more resources with a negative resource affinity rule.
+Drop rule 'split-stick-together2', because rule shares two or more resources with a negative resource affinity rule.
Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule.
Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule.
--- Config ---
$VAR1 = {
- 'digest' => '50875b320034d8ac7dded185e590f5f87c4e2bb6',
+ 'digest' => '80cdc11a1d5bf2d1d500665af1210cd59aabede6',
'ids' => {},
'order' => {}
};
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 05/18] rules: add merged positive resource affinity info in global checks
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (3 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 06/18] rules: make rules sorting optional in foreach_rule helper Daniel Kral
` (13 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
The node affinity and positive resource affinity rule subset is checked
whether the HA resources in a positive resource affinity rule are in
more than one node affinity rule in total.
This check has the assumption that each positive resource affinity
rule's resource set is disjoint from each other, but this is only done
in the later transformation stage when positive resource affinity with
overlapping HA resources in them are merged to one rule.
For example, the following inconsistent rules are not pruned:
- positive resource affinity rule between vm:101 and vm:102
- positive resource affinity rule between vm:102 and vm:103
- node affinity rule for vm:101 on node1
- node affinity rule for vm:103 on node3
Therefore build the same disjoint positive resource affinity resource
sets as the merge_connected_positive_resource_affinity_rules(...)
subroutine, so that the inconsistency check has the necessary
information in advance.
Fixes: c48d9e66 ("rules: restrict inter-plugin resource references to simple cases")
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Rules.pm | 10 +++++++---
...onsistent-node-resource-affinity-rules.cfg | 19 +++++++++++++++++++
...nt-node-resource-affinity-rules.cfg.expect | 6 +++++-
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 323ad038..e13769a1 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -8,6 +8,7 @@ use PVE::Tools;
use PVE::HA::HashTools qw(set_intersect set_union sets_are_disjoint);
use PVE::HA::Tools;
+use PVE::HA::Rules::Helpers;
use base qw(PVE::SectionConfig);
@@ -580,8 +581,11 @@ sub check_single_node_affinity_per_positive_resource_affinity_rule {
my @conflicts = ();
- while (my ($positiveid, $positive_rule) = each %$positive_rules) {
- my $positive_resources = $positive_rule->{resources};
+ my @disjoint_positive_rules =
+ PVE::HA::Rules::Helpers::find_disjoint_rules_resource_sets($positive_rules);
+
+ for my $entry (@disjoint_positive_rules) {
+ my $positive_resources = $entry->{resources};
my @paired_node_affinity_rules = ();
while (my ($node_affinity_id, $node_affinity_rule) = each %$node_affinity_rules) {
@@ -590,7 +594,7 @@ sub check_single_node_affinity_per_positive_resource_affinity_rule {
push @paired_node_affinity_rules, $node_affinity_id;
}
if (@paired_node_affinity_rules > 1) {
- push @conflicts, ['positive', $positiveid];
+ push @conflicts, ['positive', $_] for $entry->{ruleids}->@*;
push @conflicts, ['node-affinity', $_] for @paired_node_affinity_rules;
}
}
diff --git a/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg b/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg
index 88e6dd0e..52402cd9 100644
--- a/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg
+++ b/src/test/rules_cfgs/inconsistent-node-resource-affinity-rules.cfg
@@ -52,3 +52,22 @@ node-affinity: vm402-must-be-on-node1
resource-affinity: vm401-vm402-must-be-kept-separate
resources vm:401,vm:402
affinity negative
+
+# Case 5: Remove positive resource affinity rules, which have overlapping HA resources, and are restricted with two different node affinity rules.
+node-affinity: vm501-must-be-on-node1
+ resources vm:501
+ nodes node1
+ strict 1
+
+node-affinity: vm503-must-be-on-node2
+ resources vm:503
+ nodes node2
+ strict 1
+
+resource-affinity: vm501-vm502-must-be-kept-together
+ resources vm:501,vm:502
+ affinity positive
+
+resource-affinity: vm502-vm503-must-be-kept-together
+ resources vm:502,vm:503
+ affinity positive
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 e12242ab..80222c86 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
@@ -5,9 +5,13 @@ Drop rule 'vm301-vm302-must-be-kept-together', because resources are in multiple
Drop rule 'vm401-must-be-on-node1', because at least one resource is in a negative resource affinity rule and this rule would restrict these to less nodes than available to the resources.
Drop rule 'vm401-vm402-must-be-kept-separate', because two or more resources are restricted to less nodes than available to the resources.
Drop rule 'vm402-must-be-on-node1', because at least one resource is in a negative resource affinity rule and this rule would restrict these to less nodes than available to the resources.
+Drop rule 'vm501-must-be-on-node1', because at least one resource is in a positive resource affinity rule and there are other resources in at least one other node affinity rule already.
+Drop rule 'vm501-vm502-must-be-kept-together', because resources are in multiple node affinity rules.
+Drop rule 'vm502-vm503-must-be-kept-together', because resources are in multiple node affinity rules.
+Drop rule 'vm503-must-be-on-node2', because at least one resource is in a positive resource affinity rule and there are other resources in at least one other node affinity rule already.
--- Config ---
$VAR1 = {
- 'digest' => 'a5d782a442bbe3bf3a4d088db82a575b382a53fe',
+ 'digest' => '00a70f4c2bdd41aed16b6e5b9860cd9fb7f0f098',
'ids' => {
'vm101-vm102-must-be-kept-together' => {
'affinity' => 'positive',
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 06/18] rules: make rules sorting optional in foreach_rule helper
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (4 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 05/18] rules: add merged positive resource affinity info in global checks Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 07/18] rename rule's canonicalize stage to transform stage Daniel Kral
` (12 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Most users of the foreach_rule helper are agnostic to the iteration
order of the rules and as sorting can become a non-neglible overhead for
larger rule sets, make sorting rules optional.
The only caller dependent on a sorted rule set is the rules' index API
endpoint.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/API2/HA/Rules.pm | 1 +
src/PVE/HA/Rules.pm | 10 ++++++----
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/PVE/API2/HA/Rules.pm b/src/PVE/API2/HA/Rules.pm
index ab431019..fb8b465b 100644
--- a/src/PVE/API2/HA/Rules.pm
+++ b/src/PVE/API2/HA/Rules.pm
@@ -194,6 +194,7 @@ __PACKAGE__->register_method({
},
type => $type,
sid => $resource,
+ sorted => 1,
);
return $res;
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index e13769a1..784f2d36 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -428,7 +428,7 @@ Filters the given C<$rules> according to the C<$opts> and loops over the
resulting rules in the order as defined in the section config and executes
C<$func> with the parameters C<L<< ($rule, $ruleid) >>>.
-The following key-value pairs for C<$opts> as filter properties are:
+The following key-value pairs for C<$opts> are:
=over
@@ -438,6 +438,8 @@ The following key-value pairs for C<$opts> as filter properties are:
=item C<$exclude_disabled_rules>: Limits C<$rules> to those which are enabled.
+=item C<$sorted>: Sorts C<$rules> according to C<< $rules->{order} >>.
+
=back
=cut
@@ -447,9 +449,9 @@ sub foreach_rule : prototype($$;%) {
my $sid = $opts{sid};
- my @ruleids = sort {
- $rules->{order}->{$a} <=> $rules->{order}->{$b}
- } keys %{ $rules->{ids} };
+ my @ruleids = keys $rules->{ids}->%*;
+ @ruleids = sort { $rules->{order}->{$a} <=> $rules->{order}->{$b} } @ruleids
+ if defined($opts{sorted});
for my $ruleid (@ruleids) {
my $rule = $rules->{ids}->{$ruleid};
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 07/18] rename rule's canonicalize stage to transform stage
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (5 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 06/18] rules: make rules sorting optional in foreach_rule helper Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 08/18] rules: make plugins register transformers instead of plugin_transform Daniel Kral
` (11 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
The name 'canonicalize' was chosen when the actual goal was to transform
the rules in a canonical form for the scheduler, but as development went
on they are used to transform $rules and induce new rules from $rules,
so 'canonicalize' is not the right term anymore.
'transform' is chosen because it's broad enough for future use but still
conveys what is done with the $rules.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Config.pm | 2 +-
src/PVE/HA/Manager.pm | 2 +-
src/PVE/HA/Rules.pm | 18 +++++++++---------
src/PVE/HA/Rules/ResourceAffinity.pm | 4 ++--
src/test/test_rules_config.pl | 2 +-
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index b2ef26aa..bd87a0ab 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -237,7 +237,7 @@ sub read_and_check_effective_rules_config {
PVE::HA::Groups::migrate_groups_to_rules($rules, $groups, $resources);
- PVE::HA::Rules->canonicalize($rules, $nodes);
+ PVE::HA::Rules->transform($rules, $nodes);
return $rules;
}
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 92a2c05e..12d3c0e8 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -754,7 +754,7 @@ sub manage {
PVE::HA::Groups::migrate_groups_to_rules($new_rules, $self->{groups}, $sc);
my $nodes = $self->{ns}->list_nodes();
- my $messages = PVE::HA::Rules->canonicalize($new_rules, $nodes);
+ my $messages = PVE::HA::Rules->transform($new_rules, $nodes);
$haenv->log('info', $_) for @$messages;
$self->{rules} = $new_rules;
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 784f2d36..c81b1525 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -360,7 +360,7 @@ sub check_feasibility : prototype($$$) {
return $global_errors;
}
-=head3 $class->plugin_canonicalize($rules)
+=head3 $class->plugin_transform($rules)
B<OPTIONAL:> Can be implemented in the I<rule plugin>.
@@ -368,11 +368,11 @@ Modifies the C<$rules> to a plugin-specific canonical form.
=cut
-sub plugin_canonicalize : prototype($$) {
+sub plugin_transform : prototype($$) {
my ($class, $rules) = @_;
}
-=head3 $class->canonicalize($rules, $nodes)
+=head3 $class->transform($rules, $nodes)
Modifies C<$rules> to contain only feasible rules.
@@ -386,7 +386,7 @@ Returns a list of messages with the reasons why rules were removed.
=cut
-sub canonicalize : prototype($$$) {
+sub transform : prototype($$$) {
my ($class, $rules, $nodes) = @_;
my $messages = [];
@@ -407,11 +407,11 @@ sub canonicalize : prototype($$$) {
for my $type (@$types) {
my $plugin = $class->lookup($type);
- eval { $plugin->plugin_canonicalize($rules) };
- next if $@; # plugin doesn't implement plugin_canonicalize(...)
+ eval { $plugin->plugin_transform($rules) };
+ next if $@; # plugin doesn't implement plugin_transform(...)
}
- $class->global_canonicalize($rules);
+ $class->global_transform($rules);
return $messages;
}
@@ -709,7 +709,7 @@ __PACKAGE__->register_check(
},
);
-=head1 INTER-PLUGIN RULE CANONICALIZATION HELPERS
+=head1 INTER-PLUGIN RULE TRANSFORMATION HELPERS
=cut
@@ -750,7 +750,7 @@ sub create_implicit_positive_resource_affinity_node_affinity_rules {
}
}
-sub global_canonicalize {
+sub global_transform {
my ($class, $rules) = @_;
my $args = $class->get_check_arguments($rules);
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index e9b368a4..947e1580 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -251,7 +251,7 @@ __PACKAGE__->register_check(
},
);
-=head1 RESOURCE AFFINITY RULE CANONICALIZATION HELPERS
+=head1 RESOURCE AFFINITY RULE TRANSFORMATION HELPERS
=cut
@@ -381,7 +381,7 @@ sub create_implicit_negative_resource_affinity_rules {
}
}
-sub plugin_canonicalize {
+sub plugin_transform {
my ($class, $rules) = @_;
my $args = $class->get_plugin_check_arguments($rules);
diff --git a/src/test/test_rules_config.pl b/src/test/test_rules_config.pl
index c2a7af43..3c2dac4b 100755
--- a/src/test/test_rules_config.pl
+++ b/src/test/test_rules_config.pl
@@ -53,7 +53,7 @@ sub check_cfg {
print "--- Log ---\n";
my $cfg = PVE::HA::Rules->parse_config($cfg_fn, $raw);
PVE::HA::Rules->set_rule_defaults($_) for values %{ $cfg->{ids} };
- my $messages = PVE::HA::Rules->canonicalize($cfg, $nodes);
+ my $messages = PVE::HA::Rules->transform($cfg, $nodes);
print $_ for @$messages;
print "--- Config ---\n";
{
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 08/18] rules: make plugins register transformers instead of plugin_transform
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (6 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 07/18] rename rule's canonicalize stage to transform stage Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 09/18] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
` (10 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Replaces the rule plugins' plugin_transform(...) method by a
register_transform(...) call for each declared rule transformer to make
their interface more similar to the one for rule plugin checks.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Rules.pm | 85 ++++++++++++++++++++--------
src/PVE/HA/Rules/ResourceAffinity.pm | 22 +++----
2 files changed, 71 insertions(+), 36 deletions(-)
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index c81b1525..a075feac 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -32,6 +32,12 @@ the feasibility between rules of the same type and and between rules of
different types, and prune the rule set in such a way, that it becomes feasible
again, while minimizing the amount of rules that need to be pruned.
+More so, the rules given by the config file might not be in the best format to
+be used internally or does not contain the implicitly stated rules, which are
+induced by the relationship between different rules. Therefore, this package
+also provides the capability to C<L<register transforms|/REGISTERING TRANSFORMS>>
+to implement these internal rule transformations.
+
This packages inherits its config-related methods from C<L<PVE::SectionConfig>>
and therefore rule plugins need to implement methods from there as well.
@@ -90,6 +96,28 @@ and blames these errors on the I<comment> property:
}
);
+=head2 REGISTERING TRANSFORMS
+
+Rule transforms are used for transforming the rule set in such a way that
+the rules provided by the rules config are easier to work with (for example,
+transforming rules into equivalent forms) or make the rule set more complete
+(e.g. explicitly create semantically implicit rules).
+
+C<L<< Registering transforms|/$class->register_transform(...) >>> is the same
+as for registering checks. Following up on the example from that section, the
+following example shows a possible rule plugin's transform, which removes the
+I<comment> property from each rule:
+
+ __PACKAGE__->register_transformer(
+ sub {
+ my ($rules, $args) = @_;
+
+ for my $ruleid (keys $args->{custom_rules}->%*) {
+ delete $rules->{ids}->{$ruleid}->{comment};
+ }
+ }
+ );
+
=head1 METHODS
=cut
@@ -246,10 +274,11 @@ sub set_rule_defaults : prototype($$) {
}
}
-# Rule checks definition and methods
+# Rule checks and transforms definition and methods
my $types = [];
my $checkdef;
+my $transformdef;
sub register {
my ($class) = @_;
@@ -279,6 +308,23 @@ sub register_check : prototype($$$) {
];
}
+=head3 $class->register_transform(...)
+
+=head3 $class->register_transform($transform_func)
+
+Used to register rule transformers for a rule plugin.
+
+=cut
+
+sub register_transform : prototype($$) {
+ my ($class, $transform_func) = @_;
+
+ my $type = eval { $class->type() };
+ $type = 'global' if $@;
+
+ push $transformdef->{$type}->@*, $transform_func;
+}
+
=head3 $class->get_plugin_check_arguments(...)
=head3 $class->get_plugin_check_arguments($rules)
@@ -287,6 +333,7 @@ B<OPTIONAL:> Can be implemented in the I<rule plugin>.
Returns a hash, usually subsets of rules relevant to the plugin, which are
passed to the plugin's C<L<< registered checks|/$class->register_check(...) >>>
+and C<L<< registered transforms|/$class->register_transform(...) >>>
so that the creation of these can be shared inbetween rule check
implementations.
@@ -360,18 +407,6 @@ sub check_feasibility : prototype($$$) {
return $global_errors;
}
-=head3 $class->plugin_transform($rules)
-
-B<OPTIONAL:> Can be implemented in the I<rule plugin>.
-
-Modifies the C<$rules> to a plugin-specific canonical form.
-
-=cut
-
-sub plugin_transform : prototype($$) {
- my ($class, $rules) = @_;
-}
-
=head3 $class->transform($rules, $nodes)
Modifies C<$rules> to contain only feasible rules.
@@ -380,7 +415,9 @@ C<$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
-rule, which makes the rule set infeasible.
+rule, which makes the rule set infeasible, and afterwards running all
+transforms on the feasible rule set, which were registered with
+C<L<< register_transform()|/$class->register_transform(...) >>>.
Returns a list of messages with the reasons why rules were removed.
@@ -405,13 +442,13 @@ sub transform : prototype($$$) {
}
}
- for my $type (@$types) {
- my $plugin = $class->lookup($type);
- eval { $plugin->plugin_transform($rules) };
- next if $@; # plugin doesn't implement plugin_transform(...)
- }
+ for my $type (@$types, 'global') {
+ for my $transform ($transformdef->{$type}->@*) {
+ my $global_args = $class->get_check_arguments($rules);
- $class->global_transform($rules);
+ $transform->($rules, $global_args);
+ }
+ }
return $messages;
}
@@ -750,16 +787,14 @@ sub create_implicit_positive_resource_affinity_node_affinity_rules {
}
}
-sub global_transform {
- my ($class, $rules) = @_;
-
- my $args = $class->get_check_arguments($rules);
+__PACKAGE__->register_transform(sub {
+ my ($rules, $args) = @_;
create_implicit_positive_resource_affinity_node_affinity_rules(
$rules,
$args->{positive_rules},
$args->{node_affinity_rules},
);
-}
+});
1;
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index 947e1580..f2d57ce6 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -298,6 +298,12 @@ sub merge_connected_positive_resource_affinity_rules {
}
}
+__PACKAGE__->register_transform(sub {
+ my ($rules, $args) = @_;
+
+ merge_connected_positive_resource_affinity_rules($rules, $args->{positive_rules});
+});
+
# retrieve the existing negative resource affinity relationships for any of the
# $resources in the $negative_rules; returns a hash map, where the keys are the
# resources to be separated from and the values are subsets of the $resources
@@ -381,23 +387,17 @@ sub create_implicit_negative_resource_affinity_rules {
}
}
-sub plugin_transform {
- my ($class, $rules) = @_;
+# must come after merging connected positive rules, because of this helpers
+# assumptions about resource sets and inter-resource affinity consistency
+__PACKAGE__->register_transform(sub {
+ my ($rules, $args) = @_;
- my $args = $class->get_plugin_check_arguments($rules);
-
- merge_connected_positive_resource_affinity_rules($rules, $args->{positive_rules});
-
- $args = $class->get_plugin_check_arguments($rules);
-
- # must come after merging connected positive rules, because of this helpers
- # assumptions about resource sets and inter-resource affinity consistency
create_implicit_negative_resource_affinity_rules(
$rules,
$args->{positive_rules},
$args->{negative_rules},
);
-}
+});
=head1 RESOURCE AFFINITY RULE HELPERS
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 09/18] rules: node affinity: decouple get_node_affinity helper from Usage class
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (7 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 08/18] rules: make plugins register transformers instead of plugin_transform Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation Daniel Kral
` (9 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
This is done in preparation of the next patch, which moves some of the
logic of get_node_affinity(...) in the HA rule translaton stage.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
This will also become useful for another feature enhancement, which will
bring migration blockers to node affinity rules as well.
src/PVE/HA/Manager.pm | 3 ++-
src/PVE/HA/Rules/NodeAffinity.pm | 10 +++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 12d3c0e8..3013d369 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -151,7 +151,8 @@ sub select_service_node {
my ($current_node, $tried_nodes, $maintenance_fallback) =
$sd->@{qw(node failed_nodes maintenance_node)};
- my ($allowed_nodes, $pri_nodes) = get_node_affinity($rules, $sid, $online_node_usage);
+ my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() };
+ my ($allowed_nodes, $pri_nodes) = get_node_affinity($rules, $sid, $online_nodes);
return undef if !%$pri_nodes;
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index 5394832e..b7abf9a4 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -241,7 +241,7 @@ my $get_resource_node_affinity_rule = sub {
Returns a list of two hashes representing the node affinity of C<$sid>
according to the node affinity rules in C<$rules> and the available nodes in
-C<$online_node_usage>.
+the C<$online_nodes> hash.
The first hash is a hash set of available nodes, i.e. nodes where the
resource C<$sid> is allowed to be assigned to, and the second hash is a hash set
@@ -252,20 +252,20 @@ If there are no available nodes at all, returns C<undef>.
=cut
sub get_node_affinity : prototype($$$) {
- my ($rules, $sid, $online_node_usage) = @_;
+ my ($rules, $sid, $online_nodes) = @_;
my $node_affinity_rule = $get_resource_node_affinity_rule->($rules, $sid);
# default to a node affinity rule with all available nodes
if (!$node_affinity_rule) {
- for my $node ($online_node_usage->list_nodes()) {
+ for my $node (keys %$online_nodes) {
$node_affinity_rule->{nodes}->{$node} = { priority => 0 };
}
}
# add remaining nodes with low priority for non-strict node affinity rules
if (!$node_affinity_rule->{strict}) {
- for my $node ($online_node_usage->list_nodes()) {
+ for my $node (keys %$online_nodes) {
next if defined($node_affinity_rule->{nodes}->{$node});
$node_affinity_rule->{nodes}->{$node} = { priority => -1 };
@@ -276,7 +276,7 @@ sub get_node_affinity : prototype($$$) {
my $prioritized_nodes = {};
while (my ($node, $props) = each %{ $node_affinity_rule->{nodes} }) {
- next if !$online_node_usage->contains_node($node); # node is offline
+ next if !defined($online_nodes->{$node}); # node is offline
$allowed_nodes->{$node} = 1;
$prioritized_nodes->{ $props->{priority} }->{$node} = 1;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (8 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 09/18] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-27 15:19 ` Daniel Kral
2025-08-29 12:43 ` Michael Köppl
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 11/18] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
` (8 subsequent siblings)
18 siblings, 2 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
The initial implementation of fetching a ha resource's rules in the
scheduler and other users is rather inefficient as it needs to go
through the raw rules hash map and gathers quite a lot of information
which doesn't change unless the HA rules need to be re-translated
anyway.
Therefore add a compile stage when re-translating the HA rules, which
compiles each rule plugin's rules into a more compact data structure.
This reduces the unnecessary recomputation of invariant data done in the
hot path of the scheduler's select_service_node(...).
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
I didn't add this to the commit message as I'm already referencing the
future in quite a lot of these, so I'll put it here:
The $node_affinity contains a `nodes` property for each ha resource
$sid, because in a future patch series, we'll need more information
about the $node_affinity to properly implement the migration blockers
for node affinity rules as well. I didn't want to add a patch in that
future series just to make a big unnecessary diff there, but if that's
better I'll happily send a v2 for this.
src/PVE/HA/Config.pm | 9 +--
src/PVE/HA/Manager.pm | 29 +++++----
src/PVE/HA/Rules.pm | 59 +++++++++++++++++
src/PVE/HA/Rules/NodeAffinity.pm | 82 ++++++++++++------------
src/PVE/HA/Rules/ResourceAffinity.pm | 95 ++++++++++++++--------------
src/test/test_failover1.pl | 10 +--
6 files changed, 175 insertions(+), 109 deletions(-)
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index bd87a0ab..af2a1cde 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -224,7 +224,7 @@ sub read_and_check_rules_config {
return $rules;
}
-sub read_and_check_effective_rules_config {
+sub read_and_compile_rules_config {
my $rules = read_and_check_rules_config();
@@ -239,7 +239,7 @@ sub read_and_check_effective_rules_config {
PVE::HA::Rules->transform($rules, $nodes);
- return $rules;
+ return PVE::HA::Rules->compile($rules, $nodes);
}
sub write_rules_config {
@@ -391,8 +391,9 @@ sub get_resource_motion_info {
my $ss = $manager_status->{service_status};
my $ns = $manager_status->{node_status};
- my $rules = read_and_check_effective_rules_config();
- my ($together, $separate) = get_affinitive_resources($rules, $sid);
+ my $affinity = read_and_compile_rules_config();
+ my ($together, $separate) =
+ get_affinitive_resources($affinity->{'resource-affinity'}, $sid);
for my $csid (sort keys %$together) {
next if !defined($ss->{$csid});
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 3013d369..748667e8 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -121,11 +121,11 @@ sub flush_master_status {
=head3 select_service_node(...)
-=head3 select_service_node($rules, $online_node_usage, $sid, $service_conf, $sd, $node_preference)
+=head3 select_service_node($affinity, $online_node_usage, $sid, $service_conf, $sd, $node_preference)
Used to select the best fitting node for the service C<$sid>, with the
-configuration C<$service_conf> and state C<$sd>, according to the rules defined
-in C<$rules>, available node utilization in C<$online_node_usage>, and the
+configuration C<$service_conf> and state C<$sd>, according to the affinity
+in C<$affinity>, available node utilization in C<$online_node_usage>, and the
given C<$node_preference>.
The C<$node_preference> can be set to:
@@ -143,20 +143,21 @@ The C<$node_preference> can be set to:
=cut
sub select_service_node {
- my ($rules, $online_node_usage, $sid, $service_conf, $sd, $node_preference) = @_;
+ my ($affinity, $online_node_usage, $sid, $service_conf, $sd, $node_preference) = @_;
die "'$node_preference' is not a valid node_preference for select_service_node\n"
if $node_preference !~ m/(none|best-score|try-next)/;
my ($current_node, $tried_nodes, $maintenance_fallback) =
$sd->@{qw(node failed_nodes maintenance_node)};
+ my ($node_affinity, $resource_affinity) = $affinity->@{qw(node-affinity resource-affinity)};
my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() };
- my ($allowed_nodes, $pri_nodes) = get_node_affinity($rules, $sid, $online_nodes);
+ my ($allowed_nodes, $pri_nodes) = get_node_affinity($node_affinity, $sid, $online_nodes);
return undef if !%$pri_nodes;
- my ($together, $separate) = get_resource_affinity($rules, $sid, $online_node_usage);
+ my ($together, $separate) = get_resource_affinity($resource_affinity, $sid, $online_node_usage);
# stay on current node if possible (avoids random migrations)
if (
@@ -418,7 +419,8 @@ sub execute_migration {
my ($haenv, $ss) = $self->@{qw(haenv ss)};
- my ($together, $separate) = get_affinitive_resources($self->{rules}, $sid);
+ my $resource_affinity = $self->{affinity}->{'resource-affinity'};
+ my ($together, $separate) = get_affinitive_resources($resource_affinity, $sid);
for my $csid (sort keys %$separate) {
next if !defined($ss->{$csid});
@@ -746,7 +748,7 @@ sub manage {
PVE::HA::Groups::migrate_groups_to_resources($self->{groups}, $sc);
if (
- !$self->{rules}
+ !$self->{affinity}
|| $has_changed_nodelist
|| $new_rules->{digest} ne $self->{last_rules_digest}
|| $self->{groups}->{digest} ne $self->{last_groups_digest}
@@ -756,11 +758,12 @@ sub manage {
my $nodes = $self->{ns}->list_nodes();
my $messages = PVE::HA::Rules->transform($new_rules, $nodes);
+ my $new_affinity = PVE::HA::Rules->compile($new_rules, $nodes);
$haenv->log('info', $_) for @$messages;
- $self->{rules} = $new_rules;
+ $self->{affinity} = $new_affinity;
- $self->{last_rules_digest} = $self->{rules}->{digest};
+ $self->{last_rules_digest} = $new_rules->{digest};
$self->{last_groups_digest} = $self->{groups}->{digest};
$self->{last_services_digest} = $services_digest;
}
@@ -1020,7 +1023,7 @@ sub next_state_request_start {
if ($self->{crs}->{rebalance_on_request_start}) {
my $selected_node = select_service_node(
- $self->{rules},
+ $self->{affinity},
$self->{online_node_usage},
$sid,
$cd,
@@ -1187,7 +1190,7 @@ sub next_state_started {
}
my $node = select_service_node(
- $self->{rules},
+ $self->{affinity},
$self->{online_node_usage},
$sid,
$cd,
@@ -1306,7 +1309,7 @@ sub next_state_recovery {
$self->recompute_online_node_usage(); # we want the most current node state
my $recovery_node = select_service_node(
- $self->{rules},
+ $self->{affinity},
$self->{online_node_usage},
$sid,
$cd,
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index a075feac..d7593532 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -47,6 +47,12 @@ Each I<rule plugin> is required to implement the methods C<L<type()>>,
C<L<properties()>>, and C<L<options>> from the C<L<PVE::SectionConfig>> to
extend the properties of this I<base plugin> with plugin-specific properties.
+Each I<rule plugin> is required to implement the method
+C<L<< plugin_compile()|/$class->plugin_compile(...) >>> to distill a compiled
+representation of the more verbose C<$rules> from the config file, which is
+returned by C<L<< compile()|/$class->compile(...) >>> to be more appropriate
+and efficient for the scheduler and other users.
+
=head2 REGISTERING CHECKS
In order to C<L<< register checks|/$class->register_check(...) >>> for a rule
@@ -453,6 +459,59 @@ sub transform : prototype($$$) {
return $messages;
}
+=head3 $class->plugin_compile(...)
+
+=head3 $class->plugin_compile($rules, $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.
+
+C<$nodes> is a list of the configured cluster nodes.
+
+=cut
+
+sub plugin_compile {
+ my ($class, $rules, $nodes) = @_;
+
+ die "implement in subclass";
+}
+
+=head3 $class->compile(...)
+
+=head3 $class->compile($rules, $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.
+
+The transformation to the compact representation for each rule plugin is
+implemented in C<L<< plugin_compile()|/$class->plugin_compile(...) >>>.
+
+=cut
+
+sub compile {
+ my ($class, $rules, $nodes) = @_;
+
+ my $affinity = {};
+
+ for my $type (@$types) {
+ my $plugin = $class->lookup($type);
+ my $plugin_affinity = $plugin->plugin_compile($rules, $nodes);
+
+ die "plugin_compile(...) of type '$type' must return hash reference\n"
+ if ref($plugin_affinity) ne 'HASH';
+
+ $affinity->{$type} = $plugin_affinity;
+ }
+
+ return $affinity;
+}
+
=head1 FUNCTIONS
=cut
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index b7abf9a4..1af442ec 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -155,6 +155,40 @@ sub get_plugin_check_arguments {
return $result;
}
+sub plugin_compile {
+ my ($class, $rules, $nodes) = @_;
+
+ my $node_affinity = {};
+
+ PVE::HA::Rules::foreach_rule(
+ $rules,
+ sub {
+ my ($rule) = @_;
+
+ my $effective_nodes = dclone($rule->{nodes});
+
+ # add remaining nodes with low priority for non-strict node affinity
+ if (!$rule->{strict}) {
+ for my $node (@$nodes) {
+ next if defined($effective_nodes->{$node});
+
+ $effective_nodes->{$node} = { priority => -1 };
+ }
+ }
+
+ for my $sid (keys $rule->{resources}->%*) {
+ $node_affinity->{$sid} = {
+ nodes => $effective_nodes,
+ };
+ }
+ },
+ type => 'node-affinity',
+ exclude_disabled_rules => 1,
+ );
+
+ return $node_affinity;
+}
+
=head1 NODE AFFINITY RULE CHECKERS
=cut
@@ -217,30 +251,10 @@ __PACKAGE__->register_check(
=cut
-my $get_resource_node_affinity_rule = sub {
- my ($rules, $sid) = @_;
-
- # with the current restriction a resource can only be in one node affinity rule
- my $node_affinity_rule;
- PVE::HA::Rules::foreach_rule(
- $rules,
- sub {
- my ($rule) = @_;
-
- $node_affinity_rule = dclone($rule) if !$node_affinity_rule;
- },
- sid => $sid,
- type => 'node-affinity',
- exclude_disabled_rules => 1,
- );
-
- return $node_affinity_rule;
-};
-
-=head3 get_node_affinity($rules, $sid, $online_node_usage)
+=head3 get_node_affinity($node_affinity, $sid, $online_node_usage)
Returns a list of two hashes representing the node affinity of C<$sid>
-according to the node affinity rules in C<$rules> and the available nodes in
+according to the node affinity C<$node_affinity> and the available nodes in
the C<$online_nodes> hash.
The first hash is a hash set of available nodes, i.e. nodes where the
@@ -251,31 +265,15 @@ If there are no available nodes at all, returns C<undef>.
=cut
-sub get_node_affinity : prototype($$$) {
- my ($rules, $sid, $online_nodes) = @_;
+sub get_node_affinity {
+ my ($node_affinity, $sid, $online_nodes) = @_;
- my $node_affinity_rule = $get_resource_node_affinity_rule->($rules, $sid);
-
- # default to a node affinity rule with all available nodes
- if (!$node_affinity_rule) {
- for my $node (keys %$online_nodes) {
- $node_affinity_rule->{nodes}->{$node} = { priority => 0 };
- }
- }
-
- # add remaining nodes with low priority for non-strict node affinity rules
- if (!$node_affinity_rule->{strict}) {
- for my $node (keys %$online_nodes) {
- next if defined($node_affinity_rule->{nodes}->{$node});
-
- $node_affinity_rule->{nodes}->{$node} = { priority => -1 };
- }
- }
+ return ($online_nodes, $online_nodes) if !defined($node_affinity->{$sid}->{nodes});
my $allowed_nodes = {};
my $prioritized_nodes = {};
- while (my ($node, $props) = each %{ $node_affinity_rule->{nodes} }) {
+ while (my ($node, $props) = each $node_affinity->{$sid}->{nodes}->%*) {
next if !defined($online_nodes->{$node}); # node is offline
$allowed_nodes->{$node} = 1;
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index f2d57ce6..c3cde6e8 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -100,6 +100,35 @@ sub get_plugin_check_arguments {
return $result;
}
+sub plugin_compile {
+ my ($class, $rules, $nodes) = @_;
+
+ my $positive = {};
+ my $negative = {};
+
+ PVE::HA::Rules::foreach_rule(
+ $rules,
+ sub {
+ my ($rule, $ruleid) = @_;
+
+ my $affinity_set = $rule->{affinity} eq 'positive' ? $positive : $negative;
+
+ for my $sid (keys $rule->{resources}->%*) {
+ for my $csid (keys $rule->{resources}->%*) {
+ $affinity_set->{$sid}->{$csid} = 1 if $csid ne $sid;
+ }
+ }
+ },
+ type => 'resource-affinity',
+ exclude_disabled_rules => 1,
+ );
+
+ return {
+ positive => $positive,
+ negative => $negative,
+ };
+}
+
=head1 RESOURCE AFFINITY RULE CHECKERS
=cut
@@ -403,12 +432,12 @@ __PACKAGE__->register_transform(sub {
=cut
-=head3 get_affinitive_resources($rules, $sid)
+=head3 get_affinitive_resources($resource_affinity, $sid)
Returns a list of two hash sets, where the first hash set contains the
resources, which C<$sid> is positively affinitive to, and the second hash
contains the resources, which C<$sid> is negatively affinitive to, acording to
-the resource affinity rules in C<$rules>.
+the resource's resource affinity in C<$resource_affinity>.
Note that a resource C<$sid> becomes part of any negative affinity relation
of its positively affinitive resources.
@@ -429,36 +458,20 @@ affinitive to C<'ct:200'> and C<'ct:201'>, the returned value will be:
=cut
sub get_affinitive_resources : prototype($$) {
- my ($rules, $sid) = @_;
+ my ($resource_affinity, $sid) = @_;
- my $together = {};
- my $separate = {};
-
- PVE::HA::Rules::foreach_rule(
- $rules,
- sub {
- my ($rule, $ruleid) = @_;
-
- my $affinity_set = $rule->{affinity} eq 'positive' ? $together : $separate;
-
- for my $csid (sort keys %{ $rule->{resources} }) {
- $affinity_set->{$csid} = 1 if $csid ne $sid;
- }
- },
- sid => $sid,
- type => 'resource-affinity',
- exclude_disabled_rules => 1,
- );
+ my $together = $resource_affinity->{positive}->{$sid} // {};
+ my $separate = $resource_affinity->{negative}->{$sid} // {};
return ($together, $separate);
}
-=head3 get_resource_affinity($rules, $sid, $online_node_usage)
+=head3 get_resource_affinity($resource_affinity, $sid, $online_node_usage)
Returns a list of two hashes, where the first describes the positive resource
affinity and the second hash describes the negative resource affinity for
-resource C<$sid> according to the resource affinity rules in C<$rules> and the
-resource locations in C<$online_node_usage>.
+resource C<$sid> according to the resource's resource affinity in
+C<$resource_affinity> and the resource locations in C<$online_node_usage>.
For the positive resource affinity of a resource C<$sid>, each element in the
hash represents an online node, where other resources, which C<$sid> is in
@@ -487,36 +500,26 @@ resource C<$sid> is in a negative affinity with, the returned value will be:
=cut
sub get_resource_affinity : prototype($$$) {
- my ($rules, $sid, $online_node_usage) = @_;
+ my ($resource_affinity, $sid, $online_node_usage) = @_;
my $together = {};
my $separate = {};
- PVE::HA::Rules::foreach_rule(
- $rules,
- sub {
- my ($rule) = @_;
+ my ($positive, $negative) = get_affinitive_resources($resource_affinity, $sid);
- for my $csid (keys %{ $rule->{resources} }) {
- next if $csid eq $sid;
+ for my $csid (keys $positive->%*) {
+ my $nodes = $online_node_usage->get_service_nodes($csid);
+ next if !$nodes || !@$nodes; # skip unassigned ha resources
- my $nodes = $online_node_usage->get_service_nodes($csid);
+ $together->{$_}++ for @$nodes;
+ }
- next if !$nodes || !@$nodes; # skip unassigned nodes
+ for my $csid (keys $negative->%*) {
+ my $nodes = $online_node_usage->get_service_nodes($csid);
+ next if !$nodes || !@$nodes; # skip unassigned ha resources
- if ($rule->{affinity} eq 'positive') {
- $together->{$_}++ for @$nodes;
- } elsif ($rule->{affinity} eq 'negative') {
- $separate->{$_} = 1 for @$nodes;
- } else {
- die "unimplemented resource affinity type $rule->{affinity}\n";
- }
- }
- },
- sid => $sid,
- type => 'resource-affinity',
- exclude_disabled_rules => 1,
- );
+ $separate->{$_} = 1 for @$nodes;
+ }
return ($together, $separate);
}
diff --git a/src/test/test_failover1.pl b/src/test/test_failover1.pl
index 78a001eb..62476a93 100755
--- a/src/test/test_failover1.pl
+++ b/src/test/test_failover1.pl
@@ -20,11 +20,13 @@ node-affinity: prefer_node1
nodes node1
EOD
+my $nodes = ['node1', 'node2', 'node3'];
+
+my $affinity = PVE::HA::Rules->compile($rules, $nodes);
+
# Relies on the fact that the basic plugin doesn't use the haenv.
my $online_node_usage = PVE::HA::Usage::Basic->new();
-$online_node_usage->add_node("node1");
-$online_node_usage->add_node("node2");
-$online_node_usage->add_node("node3");
+$online_node_usage->add_node($_) for @$nodes;
my $service_conf = {
node => 'node1',
@@ -43,7 +45,7 @@ sub test {
my $select_node_preference = $try_next ? 'try-next' : 'none';
my $node = PVE::HA::Manager::select_service_node(
- $rules, $online_node_usage, "vm:111", $service_conf, $sd, $select_node_preference,
+ $affinity, $online_node_usage, "vm:111", $service_conf, $sd, $select_node_preference,
);
my (undef, undef, $line) = caller();
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 11/18] test: rules: use to_json instead of Data::Dumper for config output
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (9 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 12/18] test: rules: add compiled config output to rules config test cases Daniel Kral
` (7 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Makes the output cleaner and won't change unrelated config properties
because of Data::Dumper's space indentation if rule configs are
modified in future patches.
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
...efaults-for-node-affinity-rules.cfg.expect | 116 ++++----
...lts-for-resource-affinity-rules.cfg.expect | 72 ++---
...nt-node-resource-affinity-rules.cfg.expect | 130 ++++-----
...sistent-resource-affinity-rules.cfg.expect | 10 +-
...egative-resource-affinity-rules.cfg.expect | 52 ++--
...fective-resource-affinity-rules.cfg.expect | 10 +-
...egative-resource-affinity-rules.cfg.expect | 254 +++++++++---------
...ositive-resource-affinity-rules.cfg.expect | 218 +++++++--------
...egative-resource-affinity-rules.cfg.expect | 142 +++++-----
...ositive-resource-affinity-rules.cfg.expect | 136 +++++-----
...ty-with-resource-affinity-rules.cfg.expect | 84 +++---
...rce-refs-in-node-affinity-rules.cfg.expect | 116 ++++----
src/test/test_rules_config.pl | 9 +-
13 files changed, 673 insertions(+), 676 deletions(-)
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 59a2c364..8ea928f2 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,60 +1,60 @@
--- Log ---
--- Config ---
-$VAR1 = {
- 'digest' => 'c96c9de143221a82e44efa8bb4814b8248a8ea11',
- 'ids' => {
- 'node-affinity-defaults' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:101' => 1
- },
- 'type' => 'node-affinity'
- },
- 'node-affinity-disabled' => {
- 'disable' => 1,
- 'nodes' => {
- 'node2' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:102' => 1
- },
- 'type' => 'node-affinity'
- },
- 'node-affinity-disabled-explicit' => {
- 'disable' => 1,
- 'nodes' => {
- 'node2' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:103' => 1
- },
- 'type' => 'node-affinity'
- },
- 'node-affinity-strict' => {
- 'nodes' => {
- 'node3' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:104' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- }
- },
- 'order' => {
- 'node-affinity-defaults' => 1,
- 'node-affinity-disabled' => 2,
- 'node-affinity-disabled-explicit' => 3,
- 'node-affinity-strict' => 4
- }
- };
+{
+ "digest" : "c96c9de143221a82e44efa8bb4814b8248a8ea11",
+ "ids" : {
+ "node-affinity-defaults" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:101" : 1
+ },
+ "type" : "node-affinity"
+ },
+ "node-affinity-disabled" : {
+ "disable" : 1,
+ "nodes" : {
+ "node2" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:102" : 1
+ },
+ "type" : "node-affinity"
+ },
+ "node-affinity-disabled-explicit" : {
+ "disable" : 1,
+ "nodes" : {
+ "node2" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:103" : 1
+ },
+ "type" : "node-affinity"
+ },
+ "node-affinity-strict" : {
+ "nodes" : {
+ "node3" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:104" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ }
+ },
+ "order" : {
+ "node-affinity-defaults" : 1,
+ "node-affinity-disabled" : 2,
+ "node-affinity-disabled-explicit" : 3,
+ "node-affinity-strict" : 4
+ }
+}
diff --git a/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect
index 7384b0b8..7af19a18 100644
--- a/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect
@@ -1,38 +1,38 @@
--- Log ---
--- Config ---
-$VAR1 = {
- 'digest' => '9ac7cc517f02c41e3403085ec02f6a9259f2ac94',
- 'ids' => {
- 'resource-affinity-defaults' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'resource-affinity-disabled' => {
- 'affinity' => 'negative',
- 'disable' => 1,
- 'resources' => {
- 'vm:201' => 1,
- 'vm:202' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'resource-affinity-disabled-explicit' => {
- 'affinity' => 'negative',
- 'disable' => 1,
- 'resources' => {
- 'vm:301' => 1,
- 'vm:302' => 1
- },
- 'type' => 'resource-affinity'
- }
- },
- 'order' => {
- 'resource-affinity-defaults' => 1,
- 'resource-affinity-disabled' => 2,
- 'resource-affinity-disabled-explicit' => 3
- }
- };
+{
+ "digest" : "9ac7cc517f02c41e3403085ec02f6a9259f2ac94",
+ "ids" : {
+ "resource-affinity-defaults" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "resource-affinity-disabled" : {
+ "affinity" : "negative",
+ "disable" : 1,
+ "resources" : {
+ "vm:201" : 1,
+ "vm:202" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "resource-affinity-disabled-explicit" : {
+ "affinity" : "negative",
+ "disable" : 1,
+ "resources" : {
+ "vm:301" : 1,
+ "vm:302" : 1
+ },
+ "type" : "resource-affinity"
+ }
+ },
+ "order" : {
+ "resource-affinity-defaults" : 1,
+ "resource-affinity-disabled" : 2,
+ "resource-affinity-disabled-explicit" : 3
+ }
+}
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 80222c86..ad517077 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
@@ -10,68 +10,68 @@ Drop rule 'vm501-vm502-must-be-kept-together', because resources are in multiple
Drop rule 'vm502-vm503-must-be-kept-together', because resources are in multiple node affinity rules.
Drop rule 'vm503-must-be-on-node2', because at least one resource is in a positive resource affinity rule and there are other resources in at least one other node affinity rule already.
--- Config ---
-$VAR1 = {
- 'digest' => '00a70f4c2bdd41aed16b6e5b9860cd9fb7f0f098',
- 'ids' => {
- 'vm101-vm102-must-be-kept-together' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'vm101-vm102-must-be-on-node1' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- },
- 'vm201-must-be-on-node1' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:201' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- },
- 'vm201-vm202-must-be-kept-separate' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:201' => 1,
- 'vm:202' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'vm202-must-be-on-node2' => {
- 'nodes' => {
- 'node2' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:202' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- }
- },
- 'order' => {
- 'vm101-vm102-must-be-kept-together' => 2,
- 'vm101-vm102-must-be-on-node1' => 1,
- 'vm201-must-be-on-node1' => 3,
- 'vm201-vm202-must-be-kept-separate' => 5,
- 'vm202-must-be-on-node2' => 4
- }
- };
+{
+ "digest" : "00a70f4c2bdd41aed16b6e5b9860cd9fb7f0f098",
+ "ids" : {
+ "vm101-vm102-must-be-kept-together" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "vm101-vm102-must-be-on-node1" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
+ "vm201-must-be-on-node1" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:201" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
+ "vm201-vm202-must-be-kept-separate" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:201" : 1,
+ "vm:202" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "vm202-must-be-on-node2" : {
+ "nodes" : {
+ "node2" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:202" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ }
+ },
+ "order" : {
+ "vm101-vm102-must-be-kept-together" : 2,
+ "vm101-vm102-must-be-on-node1" : 1,
+ "vm201-must-be-on-node1" : 3,
+ "vm201-vm202-must-be-kept-separate" : 5,
+ "vm202-must-be-on-node2" : 4
+ }
+}
diff --git a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
index 8f2338d9..f47828c6 100644
--- a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
@@ -7,8 +7,8 @@ Drop rule 'split-stick-together2', because rule shares two or more resources wit
Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule.
Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule.
--- Config ---
-$VAR1 = {
- 'digest' => '80cdc11a1d5bf2d1d500665af1210cd59aabede6',
- 'ids' => {},
- 'order' => {}
- };
+{
+ "digest" : "80cdc11a1d5bf2d1d500665af1210cd59aabede6",
+ "ids" : {},
+ "order" : {}
+}
diff --git a/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect
index 8a2b8797..e2c1ad11 100644
--- a/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect
@@ -2,29 +2,29 @@
Drop rule 'remove-me1', because rule defines more resources than available nodes.
Drop rule 'remove-me2', because rule defines more resources than available nodes.
--- Config ---
-$VAR1 = {
- 'digest' => '68633cedeeb355ef78fe28221ef3f16537b3e788',
- 'ids' => {
- 'do-not-remove-me1' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-remove-me2' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1,
- 'vm:103' => 1
- },
- 'type' => 'resource-affinity'
- }
- },
- 'order' => {
- 'do-not-remove-me1' => 1,
- 'do-not-remove-me2' => 2
- }
- };
+{
+ "digest" : "68633cedeeb355ef78fe28221ef3f16537b3e788",
+ "ids" : {
+ "do-not-remove-me1" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-remove-me2" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "type" : "resource-affinity"
+ }
+ },
+ "order" : {
+ "do-not-remove-me1" : 1,
+ "do-not-remove-me2" : 2
+ }
+}
diff --git a/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect
index b2d468b2..4bbc782a 100644
--- a/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect
@@ -2,8 +2,8 @@
Drop rule 'lonely-resource1', because rule is ineffective as there are less than two resources.
Drop rule 'lonely-resource2', because rule is ineffective as there are less than two resources.
--- Config ---
-$VAR1 = {
- 'digest' => 'fe89f8c8f5acc29f807eaa0cec5974b6e957a596',
- 'ids' => {},
- 'order' => {}
- };
+{
+ "digest" : "fe89f8c8f5acc29f807eaa0cec5974b6e957a596",
+ "ids" : {},
+ "order" : {}
+}
diff --git a/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect
index 09364d41..d3f1c7c3 100644
--- a/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect
@@ -2,130 +2,130 @@
Drop rule 'do-not-infer-inconsistent-negative2', because rule shares two or more resources with a positive resource affinity rule.
Drop rule 'do-not-infer-inconsistent-positive1', because rule shares two or more resources with a negative resource affinity rule.
--- Config ---
-$VAR1 = {
- 'digest' => 'd8724dfe2130bb642b98e021da973aa0ec0695f0',
- 'ids' => {
- '_implicit-negative-infer-simple-positive1-vm:202-vm:204' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:202' => 1,
- 'vm:204' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-infer-simple-positive1-vm:203-vm:204' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:203' => 1,
- 'vm:204' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-infer-two-positive1-vm:301-vm:304' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:301' => 1,
- 'vm:304' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-infer-two-positive1-vm:301-vm:305' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:301' => 1,
- 'vm:305' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-infer-two-positive1-vm:302-vm:304' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:302' => 1,
- 'vm:304' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-infer-two-positive1-vm:303-vm:305' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:303' => 1,
- 'vm:305' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-infer-inconsistent-negative1' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:401' => 1,
- 'vm:404' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-infer-positive1' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1,
- 'vm:103' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-simple-negative1' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:201' => 1,
- 'vm:204' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-simple-positive1' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:201' => 1,
- 'vm:202' => 1,
- 'vm:203' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-two-negative1' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:303' => 1,
- 'vm:304' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-two-negative2' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:302' => 1,
- 'vm:305' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-two-positive1' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:301' => 1,
- 'vm:302' => 1,
- 'vm:303' => 1
- },
- 'type' => 'resource-affinity'
- }
- },
- 'order' => {
- '_implicit-negative-infer-simple-positive1-vm:202-vm:204' => 2,
- '_implicit-negative-infer-simple-positive1-vm:203-vm:204' => 2,
- '_implicit-negative-infer-two-positive1-vm:301-vm:304' => 2,
- '_implicit-negative-infer-two-positive1-vm:301-vm:305' => 2,
- '_implicit-negative-infer-two-positive1-vm:302-vm:304' => 2,
- '_implicit-negative-infer-two-positive1-vm:303-vm:305' => 2,
- 'do-not-infer-inconsistent-negative1' => 8,
- 'do-not-infer-positive1' => 1,
- 'infer-simple-negative1' => 3,
- 'infer-simple-positive1' => 2,
- 'infer-two-negative1' => 5,
- 'infer-two-negative2' => 6,
- 'infer-two-positive1' => 4
- }
- };
+{
+ "digest" : "d8724dfe2130bb642b98e021da973aa0ec0695f0",
+ "ids" : {
+ "_implicit-negative-infer-simple-positive1-vm:202-vm:204" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:202" : 1,
+ "vm:204" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-infer-simple-positive1-vm:203-vm:204" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:203" : 1,
+ "vm:204" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-infer-two-positive1-vm:301-vm:304" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:301" : 1,
+ "vm:304" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-infer-two-positive1-vm:301-vm:305" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:301" : 1,
+ "vm:305" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-infer-two-positive1-vm:302-vm:304" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:302" : 1,
+ "vm:304" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-infer-two-positive1-vm:303-vm:305" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:303" : 1,
+ "vm:305" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-infer-inconsistent-negative1" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:401" : 1,
+ "vm:404" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-infer-positive1" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-simple-negative1" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:201" : 1,
+ "vm:204" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-simple-positive1" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:201" : 1,
+ "vm:202" : 1,
+ "vm:203" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-two-negative1" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:303" : 1,
+ "vm:304" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-two-negative2" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:302" : 1,
+ "vm:305" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-two-positive1" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1
+ },
+ "type" : "resource-affinity"
+ }
+ },
+ "order" : {
+ "_implicit-negative-infer-simple-positive1-vm:202-vm:204" : 2,
+ "_implicit-negative-infer-simple-positive1-vm:203-vm:204" : 2,
+ "_implicit-negative-infer-two-positive1-vm:301-vm:304" : 2,
+ "_implicit-negative-infer-two-positive1-vm:301-vm:305" : 2,
+ "_implicit-negative-infer-two-positive1-vm:302-vm:304" : 2,
+ "_implicit-negative-infer-two-positive1-vm:303-vm:305" : 2,
+ "do-not-infer-inconsistent-negative1" : 8,
+ "do-not-infer-positive1" : 1,
+ "infer-simple-negative1" : 3,
+ "infer-simple-positive1" : 2,
+ "infer-two-negative1" : 5,
+ "infer-two-negative2" : 6,
+ "infer-two-positive1" : 4
+ }
+}
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 33c56c62..3f5cd6d8 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
@@ -1,111 +1,111 @@
--- Log ---
--- Config ---
-$VAR1 = {
- 'digest' => '32ae135ef2f8bd84cd12c18af6910dce9d6bc9fa',
- 'ids' => {
- 'do-not-infer-negative1' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- },
- 'node2' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:203' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- },
- 'do-not-infer-negative2' => {
- 'nodes' => {
- 'node3' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:201' => 1
- },
- 'type' => 'node-affinity'
- },
- 'do-not-infer-negative3' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:201' => 1,
- 'vm:203' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-infer-positive1' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1,
- 'vm:103' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-multi-resources1' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- },
- 'node3' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:401' => 1,
- 'vm:402' => 1,
- 'vm:403' => 1,
- 'vm:404' => 1,
- 'vm:405' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- },
- 'infer-multi-resources2' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:401' => 1,
- 'vm:402' => 1,
- 'vm:403' => 1,
- 'vm:404' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-single-resource1' => {
- 'nodes' => {
- 'node3' => {
- 'priority' => 0
- }
- },
- 'resources' => {
- 'vm:301' => 1,
- 'vm:302' => 1,
- 'vm:303' => 1
- },
- 'type' => 'node-affinity'
- },
- 'infer-single-resource2' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:301' => 1,
- 'vm:302' => 1,
- 'vm:303' => 1
- },
- 'type' => 'resource-affinity'
- }
- },
- 'order' => {
- 'do-not-infer-negative1' => 2,
- 'do-not-infer-negative2' => 3,
- 'do-not-infer-negative3' => 4,
- 'do-not-infer-positive1' => 1,
- 'infer-multi-resources1' => 7,
- 'infer-multi-resources2' => 8,
- 'infer-single-resource1' => 5,
- 'infer-single-resource2' => 6
- }
- };
+{
+ "digest" : "32ae135ef2f8bd84cd12c18af6910dce9d6bc9fa",
+ "ids" : {
+ "do-not-infer-negative1" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:203" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
+ "do-not-infer-negative2" : {
+ "nodes" : {
+ "node3" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:201" : 1
+ },
+ "type" : "node-affinity"
+ },
+ "do-not-infer-negative3" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:201" : 1,
+ "vm:203" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-infer-positive1" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-multi-resources1" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:401" : 1,
+ "vm:402" : 1,
+ "vm:403" : 1,
+ "vm:404" : 1,
+ "vm:405" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
+ "infer-multi-resources2" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:401" : 1,
+ "vm:402" : 1,
+ "vm:403" : 1,
+ "vm:404" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-single-resource1" : {
+ "nodes" : {
+ "node3" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1
+ },
+ "type" : "node-affinity"
+ },
+ "infer-single-resource2" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1
+ },
+ "type" : "resource-affinity"
+ }
+ },
+ "order" : {
+ "do-not-infer-negative1" : 2,
+ "do-not-infer-negative2" : 3,
+ "do-not-infer-negative3" : 4,
+ "do-not-infer-positive1" : 1,
+ "infer-multi-resources1" : 7,
+ "infer-multi-resources2" : 8,
+ "infer-single-resource1" : 5,
+ "infer-single-resource2" : 6
+ }
+}
diff --git a/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect
index 876c2030..0002dc2a 100644
--- a/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect
@@ -1,73 +1,73 @@
--- Log ---
--- Config ---
-$VAR1 = {
- 'digest' => '5695bd62a65966a275a62a01d2d8fbc370d91668',
- 'ids' => {
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:101-vm:105' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:105' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:102-vm:104' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:102' => 1,
- 'vm:104' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:104' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:103' => 1,
- 'vm:104' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:105' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:103' => 1,
- 'vm:105' => 1
- },
- 'type' => 'resource-affinity'
- },
- '_merged-infer-connected-positive1-infer-connected-positive2' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1,
- 'vm:103' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-connected-negative1' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:104' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'infer-connected-negative2' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:102' => 1,
- 'vm:105' => 1
- },
- 'type' => 'resource-affinity'
- }
- },
- 'order' => {
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:101-vm:105' => 2,
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:102-vm:104' => 2,
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:104' => 2,
- '_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:105' => 2,
- '_merged-infer-connected-positive1-infer-connected-positive2' => 1,
- 'infer-connected-negative1' => 3,
- 'infer-connected-negative2' => 4
- }
- };
+{
+ "digest" : "5695bd62a65966a275a62a01d2d8fbc370d91668",
+ "ids" : {
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:101-vm:105" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:105" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:102-vm:104" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:102" : 1,
+ "vm:104" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:104" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:103" : 1,
+ "vm:104" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:105" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:103" : 1,
+ "vm:105" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "_merged-infer-connected-positive1-infer-connected-positive2" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-connected-negative1" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:104" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "infer-connected-negative2" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:102" : 1,
+ "vm:105" : 1
+ },
+ "type" : "resource-affinity"
+ }
+ },
+ "order" : {
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:101-vm:105" : 2,
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:102-vm:104" : 2,
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:104" : 2,
+ "_implicit-negative-_merged-infer-connected-positive1-infer-connected-positive2-vm:103-vm:105" : 2,
+ "_merged-infer-connected-positive1-infer-connected-positive2" : 1,
+ "infer-connected-negative1" : 3,
+ "infer-connected-negative2" : 4
+ }
+}
diff --git a/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect
index e57a792b..935a4f7c 100644
--- a/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect
@@ -1,70 +1,70 @@
--- Log ---
--- Config ---
-$VAR1 = {
- 'digest' => '920d9caac206fc0dd893753bfb2cab3e6d6a9b9b',
- 'ids' => {
- '_merged-merge-positive1-merge-positive3-merge-positive4-merge-positive2-merge-positive5' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:301' => 1,
- 'vm:302' => 1,
- 'vm:303' => 1,
- 'vm:304' => 1,
- 'vm:305' => 1,
- 'vm:306' => 1,
- 'vm:307' => 1,
- 'vm:308' => 1,
- 'vm:309' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-merge-negative1' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-merge-negative2' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:102' => 1,
- 'vm:103' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-merge-negative3' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:104' => 1,
- 'vm:105' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-merge-positive1' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:201' => 1,
- 'vm:202' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'do-not-merge-positive2' => {
- 'affinity' => 'positive',
- 'resources' => {
- 'vm:203' => 1,
- 'vm:204' => 1
- },
- 'type' => 'resource-affinity'
- }
- },
- 'order' => {
- '_merged-merge-positive1-merge-positive3-merge-positive4-merge-positive2-merge-positive5' => 6,
- 'do-not-merge-negative1' => 1,
- 'do-not-merge-negative2' => 2,
- 'do-not-merge-negative3' => 3,
- 'do-not-merge-positive1' => 4,
- 'do-not-merge-positive2' => 5
- }
- };
+{
+ "digest" : "920d9caac206fc0dd893753bfb2cab3e6d6a9b9b",
+ "ids" : {
+ "_merged-merge-positive1-merge-positive3-merge-positive4-merge-positive2-merge-positive5" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-merge-negative1" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-merge-negative2" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-merge-negative3" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:104" : 1,
+ "vm:105" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-merge-positive1" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:201" : 1,
+ "vm:202" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "do-not-merge-positive2" : {
+ "affinity" : "positive",
+ "resources" : {
+ "vm:203" : 1,
+ "vm:204" : 1
+ },
+ "type" : "resource-affinity"
+ }
+ },
+ "order" : {
+ "_merged-merge-positive1-merge-positive3-merge-positive4-merge-positive2-merge-positive5" : 6,
+ "do-not-merge-negative1" : 1,
+ "do-not-merge-negative2" : 2,
+ "do-not-merge-negative3" : 3,
+ "do-not-merge-positive1" : 4,
+ "do-not-merge-positive2" : 5
+ }
+}
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 92d12929..e2d5ee00 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
@@ -4,45 +4,45 @@ Drop rule 'vm101-vm102-should-be-on-node1-or-node2', because resources are in a
Drop rule 'vm201-vm202-must-be-kept-together', because resources are in node affinity rules with multiple priorities.
Drop rule 'vm201-vm202-must-be-on-node1-or-node2', because resources are in a resource affinity rule and cannot be in a node affinity rule with multiple priorities.
--- Config ---
-$VAR1 = {
- 'digest' => '722a98914555f296af0916c980a9d6c780f5f072',
- 'ids' => {
- 'vm301-must-be-on-node1-with-prio-1' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 1
- }
- },
- 'resources' => {
- 'vm:301' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- },
- 'vm301-vm302-must-be-kept-together' => {
- 'affinity' => 'negative',
- 'resources' => {
- 'vm:301' => 1,
- 'vm:302' => 1
- },
- 'type' => 'resource-affinity'
- },
- 'vm302-must-be-on-node2-with-prio-2' => {
- 'nodes' => {
- 'node2' => {
- 'priority' => 2
- }
- },
- 'resources' => {
- 'vm:302' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- }
- },
- 'order' => {
- 'vm301-must-be-on-node1-with-prio-1' => 5,
- 'vm301-vm302-must-be-kept-together' => 7,
- 'vm302-must-be-on-node2-with-prio-2' => 6
- }
- };
+{
+ "digest" : "722a98914555f296af0916c980a9d6c780f5f072",
+ "ids" : {
+ "vm301-must-be-on-node1-with-prio-1" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 1
+ }
+ },
+ "resources" : {
+ "vm:301" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
+ "vm301-vm302-must-be-kept-together" : {
+ "affinity" : "negative",
+ "resources" : {
+ "vm:301" : 1,
+ "vm:302" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "vm302-must-be-on-node2-with-prio-2" : {
+ "nodes" : {
+ "node2" : {
+ "priority" : 2
+ }
+ },
+ "resources" : {
+ "vm:302" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ }
+ },
+ "order" : {
+ "vm301-must-be-on-node1-with-prio-1" : 5,
+ "vm301-vm302-must-be-kept-together" : 7,
+ "vm302-must-be-on-node2-with-prio-2" : 6
+ }
+}
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 3fd0c9ca..30633d8c 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
@@ -3,61 +3,61 @@ Drop rule 'same-resource1', because resource 'vm:201' is already used in another
Drop rule 'same-resource2', because resource 'vm:201' is already used in another node affinity rule.
Drop rule 'same-resource3', because resource 'vm:201' is already used in another node affinity rule.
--- Config ---
-$VAR1 = {
- 'digest' => '5865d23b1a342e7f8cfa68bd0e1da556ca8d28a6',
- 'ids' => {
- 'no-same-resource1' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- },
- 'node2' => {
- 'priority' => 2
- }
- },
- 'resources' => {
- 'vm:101' => 1,
- 'vm:102' => 1,
- 'vm:103' => 1
- },
- 'strict' => 0,
- 'type' => 'node-affinity'
- },
- 'no-same-resource2' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- },
- 'node2' => {
- 'priority' => 2
- }
- },
- 'resources' => {
- 'vm:104' => 1,
- 'vm:105' => 1
- },
- 'strict' => 0,
- 'type' => 'node-affinity'
- },
- 'no-same-resource3' => {
- 'nodes' => {
- 'node1' => {
- 'priority' => 0
- },
- 'node2' => {
- 'priority' => 2
- }
- },
- 'resources' => {
- 'vm:106' => 1
- },
- 'strict' => 1,
- 'type' => 'node-affinity'
- }
- },
- 'order' => {
- 'no-same-resource1' => 1,
- 'no-same-resource2' => 2,
- 'no-same-resource3' => 3
- }
- };
+{
+ "digest" : "5865d23b1a342e7f8cfa68bd0e1da556ca8d28a6",
+ "ids" : {
+ "no-same-resource1" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ }
+ },
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "strict" : 0,
+ "type" : "node-affinity"
+ },
+ "no-same-resource2" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ }
+ },
+ "resources" : {
+ "vm:104" : 1,
+ "vm:105" : 1
+ },
+ "strict" : 0,
+ "type" : "node-affinity"
+ },
+ "no-same-resource3" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ }
+ },
+ "resources" : {
+ "vm:106" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ }
+ },
+ "order" : {
+ "no-same-resource1" : 1,
+ "no-same-resource2" : 2,
+ "no-same-resource3" : 3
+ }
+}
diff --git a/src/test/test_rules_config.pl b/src/test/test_rules_config.pl
index 3c2dac4b..edfcb3b7 100755
--- a/src/test/test_rules_config.pl
+++ b/src/test/test_rules_config.pl
@@ -6,11 +6,11 @@ use Getopt::Long;
use lib qw(..);
+use JSON;
+
use Test::More;
use Test::MockModule;
-use Data::Dumper;
-
use PVE::HA::Rules;
use PVE::HA::Rules::NodeAffinity;
use PVE::HA::Rules::ResourceAffinity;
@@ -56,10 +56,7 @@ sub check_cfg {
my $messages = PVE::HA::Rules->transform($cfg, $nodes);
print $_ for @$messages;
print "--- Config ---\n";
- {
- local $Data::Dumper::Sortkeys = 1;
- print Dumper($cfg);
- }
+ print to_json($cfg, { canonical => 1, pretty => 1, utf8 => 1 });
select(STDOUT);
}
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 12/18] test: rules: add compiled config output to rules config test cases
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (10 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 11/18] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 13/18] rules: node affinity: define node priority outside hash access Daniel Kral
` (6 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Since there exists a compiled representation of the HA rules config now,
which is currently used by both the HA Manager and API endpoints, add
the compiled representation to the test cases to document and follow
changes through these test cases.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
...efaults-for-node-affinity-rules.cfg.expect | 29 +++
...lts-for-resource-affinity-rules.cfg.expect | 15 ++
...nt-node-resource-affinity-rules.cfg.expect | 51 ++++++
...sistent-resource-affinity-rules.cfg.expect | 8 +
...egative-resource-affinity-rules.cfg.expect | 21 +++
...fective-resource-affinity-rules.cfg.expect | 8 +
...egative-resource-affinity-rules.cfg.expect | 88 +++++++++
...ositive-resource-affinity-rules.cfg.expect | 173 ++++++++++++++++++
...egative-resource-affinity-rules.cfg.expect | 44 +++++
...ositive-resource-affinity-rules.cfg.expect | 128 +++++++++++++
...ty-with-resource-affinity-rules.cfg.expect | 30 +++
...rce-refs-in-node-affinity-rules.cfg.expect | 84 +++++++++
src/test/test_rules_config.pl | 3 +
13 files changed, 682 insertions(+)
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 8ea928f2..35d061bd 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
@@ -58,3 +58,32 @@
"node-affinity-strict" : 4
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {
+ "vm:101" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : -1
+ },
+ "node3" : {
+ "priority" : -1
+ }
+ }
+ },
+ "vm:104" : {
+ "nodes" : {
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ }
+ },
+ "resource-affinity" : {
+ "negative" : {},
+ "positive" : {}
+ }
+}
diff --git a/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect
index 7af19a18..d6a1121e 100644
--- a/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/defaults-for-resource-affinity-rules.cfg.expect
@@ -36,3 +36,18 @@
"resource-affinity-disabled-explicit" : 3
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {
+ "vm:101" : {
+ "vm:102" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1
+ }
+ },
+ "positive" : {}
+ }
+}
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 ad517077..4317292b 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
@@ -75,3 +75,54 @@ Drop rule 'vm503-must-be-on-node2', because at least one resource is in a positi
"vm202-must-be-on-node2" : 4
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {
+ "vm:101" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:102" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:201" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:202" : {
+ "nodes" : {
+ "node2" : {
+ "priority" : 0
+ }
+ }
+ }
+ },
+ "resource-affinity" : {
+ "negative" : {
+ "vm:201" : {
+ "vm:202" : 1
+ },
+ "vm:202" : {
+ "vm:201" : 1
+ }
+ },
+ "positive" : {
+ "vm:101" : {
+ "vm:102" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1
+ }
+ }
+ }
+}
diff --git a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
index f47828c6..70d51ffd 100644
--- a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect
@@ -12,3 +12,11 @@ Drop rule 'stick-together1', because rule shares two or more resources with a ne
"ids" : {},
"order" : {}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {},
+ "positive" : {}
+ }
+}
diff --git a/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect
index e2c1ad11..42fa7d24 100644
--- a/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/ineffective-negative-resource-affinity-rules.cfg.expect
@@ -28,3 +28,24 @@ Drop rule 'remove-me2', because rule defines more resources than available nodes
"do-not-remove-me2" : 2
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {
+ "vm:101" : {
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1,
+ "vm:103" : 1
+ },
+ "vm:103" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ }
+ },
+ "positive" : {}
+ }
+}
diff --git a/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect
index 4bbc782a..9c61944f 100644
--- a/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/ineffective-resource-affinity-rules.cfg.expect
@@ -7,3 +7,11 @@ Drop rule 'lonely-resource2', because rule is ineffective as there are less than
"ids" : {},
"order" : {}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {},
+ "positive" : {}
+ }
+}
diff --git a/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect
index d3f1c7c3..f863c9fa 100644
--- a/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/infer-implicit-negative-resource-affinity-rules.cfg.expect
@@ -129,3 +129,91 @@ Drop rule 'do-not-infer-inconsistent-positive1', because rule shares two or more
"infer-two-positive1" : 4
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {
+ "vm:201" : {
+ "vm:204" : 1
+ },
+ "vm:202" : {
+ "vm:204" : 1
+ },
+ "vm:203" : {
+ "vm:204" : 1
+ },
+ "vm:204" : {
+ "vm:201" : 1,
+ "vm:202" : 1,
+ "vm:203" : 1
+ },
+ "vm:301" : {
+ "vm:304" : 1,
+ "vm:305" : 1
+ },
+ "vm:302" : {
+ "vm:304" : 1,
+ "vm:305" : 1
+ },
+ "vm:303" : {
+ "vm:304" : 1,
+ "vm:305" : 1
+ },
+ "vm:304" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1
+ },
+ "vm:305" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1
+ },
+ "vm:401" : {
+ "vm:404" : 1
+ },
+ "vm:404" : {
+ "vm:401" : 1
+ }
+ },
+ "positive" : {
+ "vm:101" : {
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1,
+ "vm:103" : 1
+ },
+ "vm:103" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "vm:201" : {
+ "vm:202" : 1,
+ "vm:203" : 1
+ },
+ "vm:202" : {
+ "vm:201" : 1,
+ "vm:203" : 1
+ },
+ "vm:203" : {
+ "vm:201" : 1,
+ "vm:202" : 1
+ },
+ "vm:301" : {
+ "vm:302" : 1,
+ "vm:303" : 1
+ },
+ "vm:302" : {
+ "vm:301" : 1,
+ "vm:303" : 1
+ },
+ "vm:303" : {
+ "vm:301" : 1,
+ "vm:302" : 1
+ }
+ }
+ }
+}
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 3f5cd6d8..ed339777 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
@@ -109,3 +109,176 @@
"infer-single-resource2" : 6
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {
+ "vm:201" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : -1
+ },
+ "node2" : {
+ "priority" : -1
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:203" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:301" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : -1
+ },
+ "node2" : {
+ "priority" : -1
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:302" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : -1
+ },
+ "node2" : {
+ "priority" : -1
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:303" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : -1
+ },
+ "node2" : {
+ "priority" : -1
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:401" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:402" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:403" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:404" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:405" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node3" : {
+ "priority" : 0
+ }
+ }
+ }
+ },
+ "resource-affinity" : {
+ "negative" : {
+ "vm:201" : {
+ "vm:203" : 1
+ },
+ "vm:203" : {
+ "vm:201" : 1
+ }
+ },
+ "positive" : {
+ "vm:101" : {
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1,
+ "vm:103" : 1
+ },
+ "vm:103" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "vm:301" : {
+ "vm:302" : 1,
+ "vm:303" : 1
+ },
+ "vm:302" : {
+ "vm:301" : 1,
+ "vm:303" : 1
+ },
+ "vm:303" : {
+ "vm:301" : 1,
+ "vm:302" : 1
+ },
+ "vm:401" : {
+ "vm:402" : 1,
+ "vm:403" : 1,
+ "vm:404" : 1
+ },
+ "vm:402" : {
+ "vm:401" : 1,
+ "vm:403" : 1,
+ "vm:404" : 1
+ },
+ "vm:403" : {
+ "vm:401" : 1,
+ "vm:402" : 1,
+ "vm:404" : 1
+ },
+ "vm:404" : {
+ "vm:401" : 1,
+ "vm:402" : 1,
+ "vm:403" : 1
+ }
+ }
+ }
+}
diff --git a/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect
index 0002dc2a..98c8079a 100644
--- a/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/merge-and-infer-implicit-negative-resource-affinity-rules.cfg.expect
@@ -71,3 +71,47 @@
"infer-connected-negative2" : 4
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {
+ "vm:101" : {
+ "vm:104" : 1,
+ "vm:105" : 1
+ },
+ "vm:102" : {
+ "vm:104" : 1,
+ "vm:105" : 1
+ },
+ "vm:103" : {
+ "vm:104" : 1,
+ "vm:105" : 1
+ },
+ "vm:104" : {
+ "vm:101" : 1,
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "vm:105" : {
+ "vm:101" : 1,
+ "vm:102" : 1,
+ "vm:103" : 1
+ }
+ },
+ "positive" : {
+ "vm:101" : {
+ "vm:102" : 1,
+ "vm:103" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1,
+ "vm:103" : 1
+ },
+ "vm:103" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ }
+ }
+ }
+}
diff --git a/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect
index 935a4f7c..07461626 100644
--- a/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect
+++ b/src/test/rules_cfgs/merge-connected-positive-resource-affinity-rules.cfg.expect
@@ -68,3 +68,131 @@
"do-not-merge-positive2" : 5
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {
+ "vm:101" : {
+ "vm:102" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1,
+ "vm:103" : 1
+ },
+ "vm:103" : {
+ "vm:102" : 1
+ },
+ "vm:104" : {
+ "vm:105" : 1
+ },
+ "vm:105" : {
+ "vm:104" : 1
+ }
+ },
+ "positive" : {
+ "vm:201" : {
+ "vm:202" : 1
+ },
+ "vm:202" : {
+ "vm:201" : 1
+ },
+ "vm:203" : {
+ "vm:204" : 1
+ },
+ "vm:204" : {
+ "vm:203" : 1
+ },
+ "vm:301" : {
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "vm:302" : {
+ "vm:301" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "vm:303" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "vm:304" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "vm:305" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "vm:306" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "vm:307" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:308" : 1,
+ "vm:309" : 1
+ },
+ "vm:308" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:309" : 1
+ },
+ "vm:309" : {
+ "vm:301" : 1,
+ "vm:302" : 1,
+ "vm:303" : 1,
+ "vm:304" : 1,
+ "vm:305" : 1,
+ "vm:306" : 1,
+ "vm:307" : 1,
+ "vm:308" : 1
+ }
+ }
+ }
+}
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 e2d5ee00..68a2b75f 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
@@ -46,3 +46,33 @@ Drop rule 'vm201-vm202-must-be-on-node1-or-node2', because resources are in a re
"vm302-must-be-on-node2-with-prio-2" : 6
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {
+ "vm:301" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 1
+ }
+ }
+ },
+ "vm:302" : {
+ "nodes" : {
+ "node2" : {
+ "priority" : 2
+ }
+ }
+ }
+ },
+ "resource-affinity" : {
+ "negative" : {
+ "vm:301" : {
+ "vm:302" : 1
+ },
+ "vm:302" : {
+ "vm:301" : 1
+ }
+ },
+ "positive" : {}
+ }
+}
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 30633d8c..425de2b1 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
@@ -61,3 +61,87 @@ Drop rule 'same-resource3', because resource 'vm:201' is already used in another
"no-same-resource3" : 3
}
}
+--- Compiled Config ---
+{
+ "node-affinity" : {
+ "vm:101" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ },
+ "node3" : {
+ "priority" : -1
+ }
+ }
+ },
+ "vm:102" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ },
+ "node3" : {
+ "priority" : -1
+ }
+ }
+ },
+ "vm:103" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ },
+ "node3" : {
+ "priority" : -1
+ }
+ }
+ },
+ "vm:104" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ },
+ "node3" : {
+ "priority" : -1
+ }
+ }
+ },
+ "vm:105" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ },
+ "node3" : {
+ "priority" : -1
+ }
+ }
+ },
+ "vm:106" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ },
+ "node2" : {
+ "priority" : 2
+ }
+ }
+ }
+ },
+ "resource-affinity" : {
+ "negative" : {},
+ "positive" : {}
+ }
+}
diff --git a/src/test/test_rules_config.pl b/src/test/test_rules_config.pl
index edfcb3b7..f0792ff9 100755
--- a/src/test/test_rules_config.pl
+++ b/src/test/test_rules_config.pl
@@ -54,9 +54,12 @@ sub check_cfg {
my $cfg = PVE::HA::Rules->parse_config($cfg_fn, $raw);
PVE::HA::Rules->set_rule_defaults($_) for values %{ $cfg->{ids} };
my $messages = PVE::HA::Rules->transform($cfg, $nodes);
+ my $compiled_cfg = PVE::HA::Rules->compile($cfg, $nodes);
print $_ for @$messages;
print "--- Config ---\n";
print to_json($cfg, { canonical => 1, pretty => 1, utf8 => 1 });
+ print "--- Compiled Config ---\n";
+ print to_json($compiled_cfg, { canonical => 1, pretty => 1, utf8 => 1 });
select(STDOUT);
}
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 13/18] rules: node affinity: define node priority outside hash access
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (11 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 12/18] test: rules: add compiled config output to rules config test cases Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 14/18] move minimum version check helper to ha tools Daniel Kral
` (5 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Rules/NodeAffinity.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index 1af442ec..f8279ad2 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -276,8 +276,10 @@ sub get_node_affinity {
while (my ($node, $props) = each $node_affinity->{$sid}->{nodes}->%*) {
next if !defined($online_nodes->{$node}); # node is offline
+ my $node_priority = $props->{priority} // 0;
+
$allowed_nodes->{$node} = 1;
- $prioritized_nodes->{ $props->{priority} }->{$node} = 1;
+ $prioritized_nodes->{$node_priority}->{$node} = 1;
}
my $preferred_nodes = {};
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 14/18] move minimum version check helper to ha tools
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (12 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 13/18] rules: node affinity: define node priority outside hash access Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 15/18] manager: move group migration cooldown variable into helper Daniel Kral
` (4 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Debloats the already quite big Manager package from an independent
helper.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Manager.pm | 30 ++----------------------------
src/PVE/HA/Tools.pm | 27 +++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 748667e8..14a20c5b 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -539,33 +539,6 @@ my $have_groups_been_migrated = sub {
return keys $groups->{ids}->%* < 1;
};
-my $get_version_parts = sub {
- my ($node_version) = @_;
-
- return $node_version =~ m/^(\d+)\.(\d+)(?:\.|-)(\d+)(?:~(\d+))?/;
-};
-
-my $has_node_min_version = sub {
- my ($node_version, $min_version) = @_;
-
- my ($major, $minor, $patch, $rev) = $get_version_parts->($node_version);
- my ($min_major, $min_minor, $min_patch, $min_rev) = $get_version_parts->($min_version);
-
- return 0 if $major < $min_major;
- return 0 if $major == $min_major && $minor < $min_minor;
- return 0 if $major == $min_major && $minor == $min_minor && $patch < $min_patch;
-
- $min_rev //= 0;
- return 0
- if $major == $min_major
- && $minor == $min_minor
- && $patch == $min_patch
- && defined($rev)
- && $rev < $min_rev;
-
- return 1;
-};
-
my $is_lrm_active_or_idle = sub {
my ($ss, $node, $lrm_state) = @_;
@@ -622,7 +595,8 @@ my $assert_cluster_can_migrate_ha_groups = sub {
$haenv->log(
'notice', "ha groups migration: node '$node' has version '$node_version'",
);
- my $has_min_version = $has_node_min_version->($node_version, $HA_RULES_MINVERSION);
+ my $has_min_version =
+ PVE::HA::Tools::has_min_version($node_version, $HA_RULES_MINVERSION);
die "node '$node' needs at least pve-manager version '$HA_RULES_MINVERSION'\n"
if !$has_min_version;
}
diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
index 71eb5d0b..47b4aa03 100644
--- a/src/PVE/HA/Tools.pm
+++ b/src/PVE/HA/Tools.pm
@@ -237,6 +237,33 @@ sub upid_wait {
PVE::ProcFSTools::upid_wait($upid, $waitfunc, 5);
}
+sub has_min_version {
+ my ($version, $min_version) = @_;
+
+ my $get_version_parts = sub {
+ my ($version_str) = @_;
+
+ return $version_str =~ m/^(\d+)\.(\d+)(?:\.|-)(\d+)(?:~(\d+))?/;
+ };
+
+ my ($major, $minor, $patch, $rev) = $get_version_parts->($version);
+ my ($min_major, $min_minor, $min_patch, $min_rev) = $get_version_parts->($min_version);
+
+ return 0 if $major < $min_major;
+ return 0 if $major == $min_major && $minor < $min_minor;
+ return 0 if $major == $min_major && $minor == $min_minor && $patch < $min_patch;
+
+ $min_rev //= 0;
+ return 0
+ if $major == $min_major
+ && $minor == $min_minor
+ && $patch == $min_patch
+ && defined($rev)
+ && $rev < $min_rev;
+
+ return 1;
+}
+
# bash auto completion helper
# NOTE: we use PVE::HA::Config here without declaring an 'use' clause above as
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 15/18] manager: move group migration cooldown variable into helper
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (13 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 14/18] move minimum version check helper to ha tools Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-29 12:43 ` Michael Köppl
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 16/18] api: status: sync active service counting with lrm's helper Daniel Kral
` (3 subsequent siblings)
18 siblings, 1 reply; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
The variable is only used in the try_persistent_group_migration(...)
helper. While at it, add a comment and change the name to make its
meaning clearer.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Manager.pm | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 14a20c5b..8d600a7d 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -41,8 +41,6 @@ use PVE::HA::Usage::Static;
# patches for changing above, as that set is mostly sensible and should be easy to remember once
# spending a bit time in the HA code base.
-my $group_migration_cooldown = 6;
-
sub new {
my ($this, $haenv) = @_;
@@ -640,13 +638,16 @@ my $migrate_group_persistently = sub {
sub try_persistent_group_migration {
my ($self) = @_;
+ # rounds to wait until next ha group migration try
+ my $group_migration_cooldown_round = 6;
+
my ($haenv, $ns, $ss) = ($self->{haenv}, $self->{ns}, $self->{ss});
return if $have_groups_been_migrated->($haenv);
$self->{group_migration_round}--;
return if $self->{group_migration_round} > 0;
- $self->{group_migration_round} = $group_migration_cooldown;
+ $self->{group_migration_round} = $group_migration_cooldown_round;
$haenv->log('notice', "start ha group migration...");
@@ -656,8 +657,8 @@ sub try_persistent_group_migration {
$haenv->log('err', "ha groups migration failed");
$haenv->log(
'notice',
- "retry ha groups migration in $group_migration_cooldown rounds (~ "
- . $group_migration_cooldown * 10
+ "retry ha groups migration in $group_migration_cooldown_round rounds (~ "
+ . $group_migration_cooldown_round * 10
. " seconds)",
);
}
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 16/18] api: status: sync active service counting with lrm's helper
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (14 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 15/18] manager: move group migration cooldown variable into helper Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-09-02 8:10 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 17/18] manager: group migration: " Daniel Kral
` (2 subsequent siblings)
18 siblings, 1 reply; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
There are some inconsistencies between the status API endpoint's and the
LRM's active_service_count(...) helper's counting of active services.
Sync these by:
- Counting a migrating service as active on both the source and target
node's LRM as has been introduced in commit a94a38f9 ("LRM: count
incoming migrations towards a nodes active resources").
- Not counting services as active if these are in error or request_start
state as has been introduced in commit 38545741 ("LRM: do not count
erroneous service as active") and commit 4931b586 ("manager: add new
intermediate state for stop->start transitions") respectively.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/API2/HA/Status.pm | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
index 6e13c2c8..3d9aabaa 100644
--- a/src/PVE/API2/HA/Status.pm
+++ b/src/PVE/API2/HA/Status.pm
@@ -197,13 +197,21 @@ __PACKAGE__->register_method({
my $active_count = {};
foreach my $sid (sort keys %{ $status->{service_status} }) {
my $sd = $status->{service_status}->{$sid};
+ my $target = $sd->{target}; # count as active if we are the target.
next if !$sd->{node};
$active_count->{ $sd->{node} } = 0 if !defined($active_count->{ $sd->{node} });
my $req_state = $sd->{state};
next if !defined($req_state);
next if $req_state eq 'stopped';
next if $req_state eq 'freeze';
+ next if $req_state eq 'error';
+ next if $req_state eq 'request_start';
$active_count->{ $sd->{node} }++;
+
+ if ($target && $target ne $sd->{node}) {
+ $active_count->{$target} = 0 if !defined($active_count->{$target});
+ $active_count->{$target}++;
+ }
}
foreach my $node (sort keys %{ $status->{node_status} }) {
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 17/18] manager: group migration: sync active service counting with lrm's helper
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (15 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 16/18] api: status: sync active service counting with lrm's helper Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper Daniel Kral
2025-08-29 12:44 ` [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Michael Köppl
18 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
There are some inconsistencies between the HA group migration's
is_lrm_active_or_idle helper's and the LRM's active_service_count(...)
helper's counting of active services. Sync these by:
- Counting a migrating service as active on both the source and target
node's LRM as has been introduced in commit a94a38f9 ("LRM: count
incoming migrations towards a nodes active resources").
- Not counting services as active if these are in error or request_start
state as has been introduced in commit 38545741 ("LRM: do not count
erroneous service as active") and commit 4931b586 ("manager: add new
intermediate state for stop->start transitions") respectively.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/HA/Manager.pm | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 8d600a7d..0cbeb28d 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -543,11 +543,14 @@ my $is_lrm_active_or_idle = sub {
my $active_count = 0;
for my $sid (sort keys %$ss) {
my $sd = $ss->{$sid};
- next if $sd->{node} ne $node;
+ my $target = $sd->{target}; # count as active if we are the target.
+ next if (!$sd->{node} || $sd->{node} ne $node) && (!$target || $target ne $node);
my $req_state = $sd->{state};
next if !defined($req_state);
next if $req_state eq 'stopped';
next if $req_state eq 'freeze';
+ next if $req_state eq 'error';
+ next if $req_state eq 'request_start';
$active_count++;
}
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (16 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 17/18] manager: group migration: " Daniel Kral
@ 2025-08-21 14:35 ` Daniel Kral
2025-08-29 12:44 ` Michael Köppl
2025-08-29 12:44 ` [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Michael Köppl
18 siblings, 1 reply; 34+ messages in thread
From: Daniel Kral @ 2025-08-21 14:35 UTC (permalink / raw)
To: pve-devel
Make the counting its own helper to be used throughout the code base to
have a single source of truth of what constitutes a active services.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
src/PVE/API2/HA/Status.pm | 25 +++----------------------
src/PVE/HA/LRM.pm | 20 +-------------------
src/PVE/HA/Manager.pm | 14 +-------------
src/PVE/HA/Tools.pm | 25 +++++++++++++++++++++++++
4 files changed, 30 insertions(+), 54 deletions(-)
diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
index 3d9aabaa..282577cc 100644
--- a/src/PVE/API2/HA/Status.pm
+++ b/src/PVE/API2/HA/Status.pm
@@ -193,28 +193,9 @@ __PACKAGE__->register_method({
};
}
- # compute active services for all nodes
- my $active_count = {};
- foreach my $sid (sort keys %{ $status->{service_status} }) {
- my $sd = $status->{service_status}->{$sid};
- my $target = $sd->{target}; # count as active if we are the target.
- next if !$sd->{node};
- $active_count->{ $sd->{node} } = 0 if !defined($active_count->{ $sd->{node} });
- my $req_state = $sd->{state};
- next if !defined($req_state);
- next if $req_state eq 'stopped';
- next if $req_state eq 'freeze';
- next if $req_state eq 'error';
- next if $req_state eq 'request_start';
- $active_count->{ $sd->{node} }++;
-
- if ($target && $target ne $sd->{node}) {
- $active_count->{$target} = 0 if !defined($active_count->{$target});
- $active_count->{$target}++;
- }
- }
-
foreach my $node (sort keys %{ $status->{node_status} }) {
+ my $active_count =
+ PVE::HA::Tools::count_active_services($status->{service_status}, $node);
my $lrm_status = PVE::HA::Config::read_lrm_status($node);
my $id = "lrm:$node";
if (!$lrm_status->{timestamp}) {
@@ -235,7 +216,7 @@ __PACKAGE__->register_method({
if ($lrm_mode ne 'active') {
$status_str = "$lrm_mode mode";
} else {
- if ($lrm_state eq 'wait_for_agent_lock' && !$active_count->{$node}) {
+ if ($lrm_state eq 'wait_for_agent_lock' && !$active_count) {
$status_str = 'idle';
} else {
$status_str = $lrm_state;
diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index b645e5b0..1342c8eb 100644
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -297,25 +297,7 @@ sub active_service_count {
my $ss = $self->{service_status};
- my $count = 0;
- foreach my $sid (keys %$ss) {
- my $sd = $ss->{$sid};
- my $target = $sd->{target}; # count as active if we are the target.
- next if (!$sd->{node} || $sd->{node} ne $nodename) && (!$target || $target ne $nodename);
- my $req_state = $sd->{state};
- next if !defined($req_state);
- next if $req_state eq 'stopped';
- # NOTE: 'ignored' ones are already dropped by the manager from service_status
- next if $req_state eq 'freeze';
- # erroneous services are not managed by HA, don't count them as active
- next if $req_state eq 'error';
- # request_start is for (optional) better node selection for stop -> started transition
- next if $req_state eq 'request_start';
-
- $count++;
- }
-
- return $count;
+ return PVE::HA::Tools::count_active_services($ss, $nodename);
}
my $wrote_lrm_status_at_startup = 0;
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 0cbeb28d..374c9022 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -540,19 +540,7 @@ my $have_groups_been_migrated = sub {
my $is_lrm_active_or_idle = sub {
my ($ss, $node, $lrm_state) = @_;
- my $active_count = 0;
- for my $sid (sort keys %$ss) {
- my $sd = $ss->{$sid};
- my $target = $sd->{target}; # count as active if we are the target.
- next if (!$sd->{node} || $sd->{node} ne $node) && (!$target || $target ne $node);
- my $req_state = $sd->{state};
- next if !defined($req_state);
- next if $req_state eq 'stopped';
- next if $req_state eq 'freeze';
- next if $req_state eq 'error';
- next if $req_state eq 'request_start';
- $active_count++;
- }
+ my $active_count = PVE::HA::Tools::count_active_services($ss, $node);
return 1 if $lrm_state eq 'active';
return 1 if $lrm_state eq 'wait_for_agent_lock' && !$active_count;
diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
index 47b4aa03..32f08723 100644
--- a/src/PVE/HA/Tools.pm
+++ b/src/PVE/HA/Tools.pm
@@ -188,6 +188,31 @@ sub count_fenced_services {
return $count;
}
+sub count_active_services {
+ my ($ss, $node) = @_;
+
+ my $count = 0;
+
+ foreach my $sid (keys %$ss) {
+ my $sd = $ss->{$sid};
+ my $target = $sd->{target}; # count as active if we are the target.
+ next if (!$sd->{node} || $sd->{node} ne $node) && (!$target || $target ne $node);
+ my $req_state = $sd->{state};
+ next if !defined($req_state);
+ next if $req_state eq 'stopped';
+ # NOTE: 'ignored' ones are already dropped by the manager from service_status
+ next if $req_state eq 'freeze';
+ # erroneous services are not managed by HA, don't count them as active
+ next if $req_state eq 'error';
+ # request_start is for (optional) better node selection for stop -> started transition
+ next if $req_state eq 'request_start';
+
+ $count++;
+ }
+
+ return $count;
+}
+
sub get_verbose_service_state {
my ($service_state, $service_conf) = @_;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed Daniel Kral
@ 2025-08-27 13:41 ` Michael Köppl
2025-08-27 14:33 ` Daniel Kral
2025-08-27 14:55 ` Daniel Kral
1 sibling, 1 reply; 34+ messages in thread
From: Michael Köppl @ 2025-08-27 13:41 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
2 minor suggestions inline
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> Some rule checks depend on the list of cluster nodes, e.g., to check
> whether a negative resource affinity rule doesn't specify more HA
> resources than cluster nodes.
>
> The HA Manager retranslate rules only in certain conditions to reduce
> unnecessary computations, but lacks a check whether cluster nodes have
> been added or removed, which is different from what users are reported
> through the rules API endpoints and web interface.
>
> Fixes: 6c4c0458 ("rules: add haenv node list to the rules' canonicalization stage")
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> src/PVE/HA/Manager.pm | 2 ++
> src/PVE/HA/NodeStatus.pm | 14 ++++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index ba59f642..92a2c05e 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -692,6 +692,7 @@ sub manage {
> my ($haenv, $ms, $ns, $ss) = ($self->{haenv}, $self->{ms}, $self->{ns}, $self->{ss});
>
> my ($node_info) = $haenv->get_node_info();
> + my $has_changed_nodelist = $ns->check_for_changed_nodelist($node_info);
> my ($lrm_results, $lrm_modes) = $self->read_lrm_status();
>
> $ns->update($node_info, $lrm_modes);
> @@ -745,6 +746,7 @@ sub manage {
>
> if (
> !$self->{rules}
> + || $has_changed_nodelist
> || $new_rules->{digest} ne $self->{last_rules_digest}
> || $self->{groups}->{digest} ne $self->{last_groups_digest}
> || $services_digest && $services_digest ne $self->{last_services_digest}
> diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
> index 0d04cd53..e2abfd6e 100644
> --- a/src/PVE/HA/NodeStatus.pm
> +++ b/src/PVE/HA/NodeStatus.pm
> @@ -92,6 +92,20 @@ sub list_online_nodes {
> return $res;
> }
>
> +sub check_for_changed_nodelist {
> + my ($self, $node_info) = @_;
> +
> + for my $node (sort keys %$node_info) {
> + return 1 if !$self->{status}->{$node};
> + }
> +
> + for my $node (sort keys $self->{status}->%*) {
> + return 1 if !$node_info->{$node};
> + }
> +
> + return 0;
> +}
Is the sort necessary for the loops? It seems to me that the order of keys
doesn't really matter for this.
Also, could it make sense (not necessarily in this series) to move this to
HashTools? It's not used anywhere else now, but I might come in handy
for further extensions of the HA rules implementation, whenever you need
to check if 2 sets match exactly.
> +
> my $delete_node = sub {
> my ($self, $node) = @_;
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed
2025-08-27 13:41 ` Michael Köppl
@ 2025-08-27 14:33 ` Daniel Kral
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-27 14:33 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
Thanks for reviewing!
On Wed Aug 27, 2025 at 3:41 PM CEST, Michael Köppl wrote:
> On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
>> diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
>> index 0d04cd53..e2abfd6e 100644
>> --- a/src/PVE/HA/NodeStatus.pm
>> +++ b/src/PVE/HA/NodeStatus.pm
>> @@ -92,6 +92,20 @@ sub list_online_nodes {
>> return $res;
>> }
>>
>> +sub check_for_changed_nodelist {
>> + my ($self, $node_info) = @_;
>> +
>> + for my $node (sort keys %$node_info) {
>> + return 1 if !$self->{status}->{$node};
>> + }
>> +
>> + for my $node (sort keys $self->{status}->%*) {
>> + return 1 if !$node_info->{$node};
>> + }
>> +
>> + return 0;
>> +}
> Is the sort necessary for the loops? It seems to me that the order of keys
> doesn't really matter for this.
That's right, the sort is unnecessary here and should be removed in a
v2.
>
> Also, could it make sense (not necessarily in this series) to move this to
> HashTools? It's not used anywhere else now, but I might come in handy
> for further extensions of the HA rules implementation, whenever you need
> to check if 2 sets match exactly.
Hm, could be useful if there's another use case for that, but I'd let
that check stay here for now as else it would be a helper calling a
single-use helper.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed Daniel Kral
2025-08-27 13:41 ` Michael Köppl
@ 2025-08-27 14:55 ` Daniel Kral
1 sibling, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-27 14:55 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> Some rule checks depend on the list of cluster nodes, e.g., to check
> whether a negative resource affinity rule doesn't specify more HA resources than cluster nodes.
>
> The HA Manager retranslate rules only in certain conditions to reduce
> unnecessary computations, but lacks a check whether cluster nodes have
> been added or removed, which is different from what users are reported
> through the rules API endpoints and web interface.
>
> Fixes: 6c4c0458 ("rules: add haenv node list to the rules' canonicalization stage")
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> src/PVE/HA/Manager.pm | 2 ++
> src/PVE/HA/NodeStatus.pm | 14 ++++++++++++++
> 2 files changed, 16 insertions(+)
As @Michael and I briefly taked about this off-list, the nodelist
shouldn't cange too much in production (i.e. the PVE2 environment), but
this check makes the HA rules retranslation more correct as the checks
are dependent on $nodes.
AFAICT the main reasons the nodelist changes in production is that a
node joins or leaves, where PVE::HA::Env::PVE2::get_node_info($self)
gets the nodelist from PVE::Cluster::get_members().
Even though the pve-ha-crm systemd unit has an ordering dependency on
pve-cluster, pvedaemon, ..., which are restarted on node join, these are
only restarted on the newly added node AFAICS when calling
PVE::Cluster::Setup::join, so the HA Manager isn't updated with the new
nodelist in that case, therefore the added condition in this patch is
required.
I noticed this when implementing the plugin_compile for the node
affinity rules, which heavily depend on the $nodes. Without this
additional condition, at least 'test-crs-static2' will fail as it
doesn't power on all nodes at once, but only powers node4 some time
later. The nodelist won't be updated and therefore the HA node affinity
rule in that test cases won't get retranslated too, which will change
the behavior of the node (re)assignment.
The check helper could definitely been implemented nicer, but I didn't
want to overcomplicate things here.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation Daniel Kral
@ 2025-08-27 15:19 ` Daniel Kral
2025-08-29 12:43 ` Michael Köppl
1 sibling, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-27 15:19 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
> index b7abf9a4..1af442ec 100644
> --- a/src/PVE/HA/Rules/NodeAffinity.pm
> +++ b/src/PVE/HA/Rules/NodeAffinity.pm
> @@ -251,31 +265,15 @@ If there are no available nodes at all, returns C<undef>.
>
> =cut
>
> -sub get_node_affinity : prototype($$$) {
> - my ($rules, $sid, $online_nodes) = @_;
> +sub get_node_affinity {
> + my ($node_affinity, $sid, $online_nodes) = @_;
>
> - my $node_affinity_rule = $get_resource_node_affinity_rule->($rules, $sid);
> -
> - # default to a node affinity rule with all available nodes
> - if (!$node_affinity_rule) {
> - for my $node (keys %$online_nodes) {
> - $node_affinity_rule->{nodes}->{$node} = { priority => 0 };
> - }
> - }
> -
> - # add remaining nodes with low priority for non-strict node affinity rules
> - if (!$node_affinity_rule->{strict}) {
> - for my $node (keys %$online_nodes) {
> - next if defined($node_affinity_rule->{nodes}->{$node});
> -
> - $node_affinity_rule->{nodes}->{$node} = { priority => -1 };
> - }
> - }
> + return ($online_nodes, $online_nodes) if !defined($node_affinity->{$sid}->{nodes});
Just noticed that the `$node_affinity->{$sid}->{nodes}` could cause
auto-vivification in the $node_affinity hash, so it should be something
like
return ($online_nodes, $online_nodes) if !defined($node_affinity->{$sid});
as the key $sid should only exist if there does actually exist a
rule for that HA resource $sid and `nodes` cannot be empty as it's a
required field in the section config, but a more auto-vivification-proof
solution should be used here.
I'll change that in a v2.
>
> my $allowed_nodes = {};
> my $prioritized_nodes = {};
>
> - while (my ($node, $props) = each %{ $node_affinity_rule->{nodes} }) {
> + while (my ($node, $props) = each $node_affinity->{$sid}->{nodes}->%*) {
> next if !defined($online_nodes->{$node}); # node is offline
>
> $allowed_nodes->{$node} = 1;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
@ 2025-08-29 12:43 ` Michael Köppl
2025-08-29 13:28 ` Daniel Kral
0 siblings, 1 reply; 34+ messages in thread
From: Michael Köppl @ 2025-08-29 12:43 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
One nit inline
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> @@ -248,58 +255,6 @@ __PACKAGE__->register_check(
>
> =cut
>
> -my $sort_by_lowest_resource_id = sub {
> - my ($rules) = @_;
> -
> - my $lowest_rule_resource_id = {};
> - for my $ruleid (keys %$rules) {
> - my @rule_resources = sort keys $rules->{$ruleid}->{resources}->%*;
> - $lowest_rule_resource_id->{$ruleid} = $rule_resources[0];
> - }
> -
> - # sort rules such that rules with the lowest numbered resource come first
> - my @sorted_ruleids = sort {
> - $lowest_rule_resource_id->{$a} cmp $lowest_rule_resource_id->{$b}
> - } sort keys %$rules;
> -
> - return @sorted_ruleids;
> -};
> -
> -# returns a list of hashes, which contain disjoint resource affinity rules, i.e.,
> -# put resource affinity constraints on disjoint sets of resources
> -my $find_disjoint_resource_affinity_rules = sub {
> - my ($rules) = @_;
> -
> - my @disjoint_rules = ();
> -
> - # order needed so that it is easier to check whether there is an overlap
> - my @sorted_ruleids = $sort_by_lowest_resource_id->($rules);
> -
> - for my $ruleid (@sorted_ruleids) {
> - my $rule = $rules->{$ruleid};
> -
> - my $found = 0;
> - for my $entry (@disjoint_rules) {
> - next if sets_are_disjoint($rule->{resources}, $entry->{resources});
> -
> - $found = 1;
> - push @{ $entry->{ruleids} }, $ruleid;
> - $entry->{resources}->{$_} = 1 for keys $rule->{resources}->%*;
> -
> - last;
> - }
> - if (!$found) {
> - push @disjoint_rules,
> - {
> - ruleids => [$ruleid],
> - resources => { $rule->{resources}->%* },
> - };
> - }
> - }
> -
> - return @disjoint_rules;
> -};
> -
nit: I think the removal of this would have been better to do in the
previous patch. It's not really related to the changes made in this
patch AFAICT and the function is not used anymore at this point.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 15/18] manager: move group migration cooldown variable into helper
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 15/18] manager: move group migration cooldown variable into helper Daniel Kral
@ 2025-08-29 12:43 ` Michael Köppl
0 siblings, 0 replies; 34+ messages in thread
From: Michael Köppl @ 2025-08-29 12:43 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> The variable is only used in the try_persistent_group_migration(...)
> helper. While at it, add a comment and change the name to make its
> meaning clearer.
>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> src/PVE/HA/Manager.pm | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 14a20c5b..8d600a7d 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -41,8 +41,6 @@ use PVE::HA::Usage::Static;
> # patches for changing above, as that set is mostly sensible and should be easy to remember once
> # spending a bit time in the HA code base.
>
> -my $group_migration_cooldown = 6;
> -
> sub new {
> my ($this, $haenv) = @_;
>
> @@ -640,13 +638,16 @@ my $migrate_group_persistently = sub {
> sub try_persistent_group_migration {
> my ($self) = @_;
>
> + # rounds to wait until next ha group migration try
> + my $group_migration_cooldown_round = 6;
nit: I think something like $group_migration_cooldown_rounds or
$group_migration_cooldown_round_count would be better.
> +
> my ($haenv, $ns, $ss) = ($self->{haenv}, $self->{ns}, $self->{ss});
>
> return if $have_groups_been_migrated->($haenv);
>
> $self->{group_migration_round}--;
> return if $self->{group_migration_round} > 0;
> - $self->{group_migration_round} = $group_migration_cooldown;
> + $self->{group_migration_round} = $group_migration_cooldown_round;
>
> $haenv->log('notice', "start ha group migration...");
>
> @@ -656,8 +657,8 @@ sub try_persistent_group_migration {
> $haenv->log('err', "ha groups migration failed");
> $haenv->log(
> 'notice',
> - "retry ha groups migration in $group_migration_cooldown rounds (~ "
> - . $group_migration_cooldown * 10
> + "retry ha groups migration in $group_migration_cooldown_round rounds (~ "
> + . $group_migration_cooldown_round * 10
> . " seconds)",
> );
> }
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation Daniel Kral
2025-08-27 15:19 ` Daniel Kral
@ 2025-08-29 12:43 ` Michael Köppl
2025-08-29 13:42 ` Daniel Kral
1 sibling, 1 reply; 34+ messages in thread
From: Michael Köppl @ 2025-08-29 12:43 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
2 comments inline
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> sub write_rules_config {
> @@ -391,8 +391,9 @@ sub get_resource_motion_info {
> my $ss = $manager_status->{service_status};
> my $ns = $manager_status->{node_status};
>
> - my $rules = read_and_check_effective_rules_config();
> - my ($together, $separate) = get_affinitive_resources($rules, $sid);
> + my $affinity = read_and_compile_rules_config();
nit: maybe something like $compiled_rules? Calling a variable for this
structure $affinity didn't make it quite clear to me what this is,
especially when later on it's used as $affinity->{'resource-affinity'}.
Maybe I'm just misinterpreting this, though.
Or could you maybe explain why this compiled rule structure is then
called affinity?
> + my ($together, $separate) =
> + get_affinitive_resources($affinity->{'resource-affinity'}, $sid);
>
> for my $csid (sort keys %$together) {
> next if !defined($ss->{$csid});
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 3013d369..748667e8 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -121,11 +121,11 @@ sub flush_master_status {
>
> =head3 select_service_node(...)
>
> -=head3 select_service_node($rules, $online_node_usage, $sid, $service_conf, $sd, $node_preference)
> +=head3 select_service_node($affinity, $online_node_usage, $sid, $service_conf, $sd, $node_preference)
>
> Used to select the best fitting node for the service C<$sid>, with the
> -configuration C<$service_conf> and state C<$sd>, according to the rules defined
> -in C<$rules>, available node utilization in C<$online_node_usage>, and the
> +configuration C<$service_conf> and state C<$sd>, according to the affinity
> +in C<$affinity>, available node utilization in C<$online_node_usage>, and the
> given C<$node_preference>.
>
> The C<$node_preference> can be set to:
> @@ -143,20 +143,21 @@ The C<$node_preference> can be set to:
> =cut
>
> sub select_service_node {
> - my ($rules, $online_node_usage, $sid, $service_conf, $sd, $node_preference) = @_;
> + my ($affinity, $online_node_usage, $sid, $service_conf, $sd, $node_preference) = @_;
>
> die "'$node_preference' is not a valid node_preference for select_service_node\n"
> if $node_preference !~ m/(none|best-score|try-next)/;
>
> my ($current_node, $tried_nodes, $maintenance_fallback) =
> $sd->@{qw(node failed_nodes maintenance_node)};
> + my ($node_affinity, $resource_affinity) = $affinity->@{qw(node-affinity resource-affinity)};
>
> my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() };
> - my ($allowed_nodes, $pri_nodes) = get_node_affinity($rules, $sid, $online_nodes);
> + my ($allowed_nodes, $pri_nodes) = get_node_affinity($node_affinity, $sid, $online_nodes);
>
> return undef if !%$pri_nodes;
>
> - my ($together, $separate) = get_resource_affinity($rules, $sid, $online_node_usage);
> + my ($together, $separate) = get_resource_affinity($resource_affinity, $sid, $online_node_usage);
>
> # stay on current node if possible (avoids random migrations)
> if (
> @@ -418,7 +419,8 @@ sub execute_migration {
>
> my ($haenv, $ss) = $self->@{qw(haenv ss)};
>
> - my ($together, $separate) = get_affinitive_resources($self->{rules}, $sid);
> + my $resource_affinity = $self->{affinity}->{'resource-affinity'};
> + my ($together, $separate) = get_affinitive_resources($resource_affinity, $sid);
>
> for my $csid (sort keys %$separate) {
> next if !defined($ss->{$csid});
> @@ -746,7 +748,7 @@ sub manage {
> PVE::HA::Groups::migrate_groups_to_resources($self->{groups}, $sc);
>
> if (
> - !$self->{rules}
> + !$self->{affinity}
> || $has_changed_nodelist
> || $new_rules->{digest} ne $self->{last_rules_digest}
> || $self->{groups}->{digest} ne $self->{last_groups_digest}
> @@ -756,11 +758,12 @@ sub manage {
>
> my $nodes = $self->{ns}->list_nodes();
> my $messages = PVE::HA::Rules->transform($new_rules, $nodes);
> + my $new_affinity = PVE::HA::Rules->compile($new_rules, $nodes);
> $haenv->log('info', $_) for @$messages;
>
> - $self->{rules} = $new_rules;
> + $self->{affinity} = $new_affinity;
>
> - $self->{last_rules_digest} = $self->{rules}->{digest};
> + $self->{last_rules_digest} = $new_rules->{digest};
> $self->{last_groups_digest} = $self->{groups}->{digest};
> $self->{last_services_digest} = $services_digest;
> }
> @@ -1020,7 +1023,7 @@ sub next_state_request_start {
>
> if ($self->{crs}->{rebalance_on_request_start}) {
> my $selected_node = select_service_node(
> - $self->{rules},
> + $self->{affinity},
> $self->{online_node_usage},
> $sid,
> $cd,
> @@ -1187,7 +1190,7 @@ sub next_state_started {
> }
>
> my $node = select_service_node(
> - $self->{rules},
> + $self->{affinity},
> $self->{online_node_usage},
> $sid,
> $cd,
> @@ -1306,7 +1309,7 @@ sub next_state_recovery {
> $self->recompute_online_node_usage(); # we want the most current node state
>
> my $recovery_node = select_service_node(
> - $self->{rules},
> + $self->{affinity},
> $self->{online_node_usage},
> $sid,
> $cd,
> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
> index a075feac..d7593532 100644
> --- a/src/PVE/HA/Rules.pm
> +++ b/src/PVE/HA/Rules.pm
> @@ -47,6 +47,12 @@ Each I<rule plugin> is required to implement the methods C<L<type()>>,
> C<L<properties()>>, and C<L<options>> from the C<L<PVE::SectionConfig>> to
> extend the properties of this I<base plugin> with plugin-specific properties.
>
> +Each I<rule plugin> is required to implement the method
> +C<L<< plugin_compile()|/$class->plugin_compile(...) >>> to distill a compiled
> +representation of the more verbose C<$rules> from the config file, which is
> +returned by C<L<< compile()|/$class->compile(...) >>> to be more appropriate
> +and efficient for the scheduler and other users.
Is this supposed to say "other uses" or do you actually mean "other
users"? Do you mean that this more efficient representation can (and
probably should) be used for other uses cases when e.g. iterating over
only resource affinity rules?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper Daniel Kral
@ 2025-08-29 12:44 ` Michael Köppl
2025-08-29 13:36 ` Daniel Kral
0 siblings, 1 reply; 34+ messages in thread
From: Michael Köppl @ 2025-08-29 12:44 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
During my review I was wondering why 16/18 and 17/18 added similar
checks to both the manager's and the API's active service counting and
why they're not combined into a single helper function, only to then
notice that the final patch does exactly that. Is there a reason it's
done in this order? Other patches in this series do it the other way
around or combine the introduction of a new helper function and its use.
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> Make the counting its own helper to be used throughout the code base to
> have a single source of truth of what constitutes a active services.
>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> src/PVE/API2/HA/Status.pm | 25 +++----------------------
> src/PVE/HA/LRM.pm | 20 +-------------------
> src/PVE/HA/Manager.pm | 14 +-------------
> src/PVE/HA/Tools.pm | 25 +++++++++++++++++++++++++
> 4 files changed, 30 insertions(+), 54 deletions(-)
>
> diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
> index 3d9aabaa..282577cc 100644
> --- a/src/PVE/API2/HA/Status.pm
> +++ b/src/PVE/API2/HA/Status.pm
> @@ -193,28 +193,9 @@ __PACKAGE__->register_method({
> };
> }
>
> - # compute active services for all nodes
> - my $active_count = {};
> - foreach my $sid (sort keys %{ $status->{service_status} }) {
> - my $sd = $status->{service_status}->{$sid};
> - my $target = $sd->{target}; # count as active if we are the target.
> - next if !$sd->{node};
> - $active_count->{ $sd->{node} } = 0 if !defined($active_count->{ $sd->{node} });
> - my $req_state = $sd->{state};
> - next if !defined($req_state);
> - next if $req_state eq 'stopped';
> - next if $req_state eq 'freeze';
> - next if $req_state eq 'error';
> - next if $req_state eq 'request_start';
> - $active_count->{ $sd->{node} }++;
> -
> - if ($target && $target ne $sd->{node}) {
> - $active_count->{$target} = 0 if !defined($active_count->{$target});
> - $active_count->{$target}++;
> - }
> - }
> -
> foreach my $node (sort keys %{ $status->{node_status} }) {
> + my $active_count =
> + PVE::HA::Tools::count_active_services($status->{service_status}, $node);
> my $lrm_status = PVE::HA::Config::read_lrm_status($node);
> my $id = "lrm:$node";
> if (!$lrm_status->{timestamp}) {
> @@ -235,7 +216,7 @@ __PACKAGE__->register_method({
> if ($lrm_mode ne 'active') {
> $status_str = "$lrm_mode mode";
> } else {
> - if ($lrm_state eq 'wait_for_agent_lock' && !$active_count->{$node}) {
> + if ($lrm_state eq 'wait_for_agent_lock' && !$active_count) {
> $status_str = 'idle';
> } else {
> $status_str = $lrm_state;
> diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
> index b645e5b0..1342c8eb 100644
> --- a/src/PVE/HA/LRM.pm
> +++ b/src/PVE/HA/LRM.pm
> @@ -297,25 +297,7 @@ sub active_service_count {
>
> my $ss = $self->{service_status};
>
> - my $count = 0;
> - foreach my $sid (keys %$ss) {
> - my $sd = $ss->{$sid};
> - my $target = $sd->{target}; # count as active if we are the target.
> - next if (!$sd->{node} || $sd->{node} ne $nodename) && (!$target || $target ne $nodename);
> - my $req_state = $sd->{state};
> - next if !defined($req_state);
> - next if $req_state eq 'stopped';
> - # NOTE: 'ignored' ones are already dropped by the manager from service_status
> - next if $req_state eq 'freeze';
> - # erroneous services are not managed by HA, don't count them as active
> - next if $req_state eq 'error';
> - # request_start is for (optional) better node selection for stop -> started transition
> - next if $req_state eq 'request_start';
> -
> - $count++;
> - }
> -
> - return $count;
> + return PVE::HA::Tools::count_active_services($ss, $nodename);
> }
>
> my $wrote_lrm_status_at_startup = 0;
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 0cbeb28d..374c9022 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -540,19 +540,7 @@ my $have_groups_been_migrated = sub {
> my $is_lrm_active_or_idle = sub {
> my ($ss, $node, $lrm_state) = @_;
>
> - my $active_count = 0;
> - for my $sid (sort keys %$ss) {
> - my $sd = $ss->{$sid};
> - my $target = $sd->{target}; # count as active if we are the target.
> - next if (!$sd->{node} || $sd->{node} ne $node) && (!$target || $target ne $node);
> - my $req_state = $sd->{state};
> - next if !defined($req_state);
> - next if $req_state eq 'stopped';
> - next if $req_state eq 'freeze';
> - next if $req_state eq 'error';
> - next if $req_state eq 'request_start';
> - $active_count++;
> - }
> + my $active_count = PVE::HA::Tools::count_active_services($ss, $node);
>
> return 1 if $lrm_state eq 'active';
> return 1 if $lrm_state eq 'wait_for_agent_lock' && !$active_count;
> diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
> index 47b4aa03..32f08723 100644
> --- a/src/PVE/HA/Tools.pm
> +++ b/src/PVE/HA/Tools.pm
> @@ -188,6 +188,31 @@ sub count_fenced_services {
> return $count;
> }
>
> +sub count_active_services {
> + my ($ss, $node) = @_;
> +
> + my $count = 0;
> +
> + foreach my $sid (keys %$ss) {
> + my $sd = $ss->{$sid};
> + my $target = $sd->{target}; # count as active if we are the target.
> + next if (!$sd->{node} || $sd->{node} ne $node) && (!$target || $target ne $node);
> + my $req_state = $sd->{state};
> + next if !defined($req_state);
> + next if $req_state eq 'stopped';
> + # NOTE: 'ignored' ones are already dropped by the manager from service_status
> + next if $req_state eq 'freeze';
> + # erroneous services are not managed by HA, don't count them as active
> + next if $req_state eq 'error';
> + # request_start is for (optional) better node selection for stop -> started transition
> + next if $req_state eq 'request_start';
> +
> + $count++;
> + }
> +
> + return $count;
> +}
> +
> sub get_verbose_service_state {
> my ($service_state, $service_conf) = @_;
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
` (17 preceding siblings ...)
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper Daniel Kral
@ 2025-08-29 12:44 ` Michael Köppl
2025-08-29 13:52 ` Daniel Kral
18 siblings, 1 reply; 34+ messages in thread
From: Michael Köppl @ 2025-08-29 12:44 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
Tested this series by:
- Creating rules with resources that were set to ignored, checked if the
resources are not shown as dependent resources when migrating
- Also tested what happens when the resource state was recently changed.
Noticed that when changing from 'started' to 'ignored', it can take a
few seconds before the UI reflects that (e.g. when using the migrate
dialog). This isn't unexpected, but wanted to note it nonetheless.
- Checking that conflicts between positive resource affinity rules and
node affinity rules are now detected correctly that weren't detected
before
- Manually created various combinations of node affinity and resource
affinity rules and checked that they are applied
- Also re-ran some group-to-rules migrations, just to be sure
I did not notice any problems apart from the minor delay in the UI noted
above. The rules are more robust now and the changes solve some problems
that I had encountered before.
I also had a closer look at the code and left some comments on the
individual patches, but nothing major. I had a particularly close look
at the compiled rules to be certain that they produce the expected
structure.
With my comments on the individual patches addressed, please consider
this series
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> I put the patches in decreasing priority:
>
> PATCH 1 fix output of get_resource_info() about dependent resources
> PATCH 2 fix retranslating ha rules on nodelist changes
> PATCH 3-5 fix wrong assumption about positive resource affinity checks
> PATCH 6-18 ha cleanup and performance improvements
>
>
> Ad PATCH 1, 2, 3-5:
>
> The first few patches fix some wrong assumptions and bugs.
>
>
> Ad PATCH 3-5:
>
> During the initial HA rules implementation there were quite a few
> changes done to how rules were checked and translated, which moved the
> merging of positive resource affinity rules to the end of the pipeline.
>
> Not taking notice of this clearly enough, this causes two checks to have
> wrong assumptions about positive positive resource affinity rules: they
> assume that the resource sets of positive resource affinity rules are
> already disjoint from each other, even though that is only done at a
> later stage.
>
> As it would be rather cumbersome to interleave checks/pruning infeasible
> rules and rule transforms [0] (i.e. move the merging transform inbetween
> the previous resource affinity checks and the inter-consistency check; I
> tried that but it looked rather ugly), the best method IMO was to use
> the already existing helper to find these disjoint sets. This also
> provides these checks with the correct positive resource affinity rule
> ids to blame instead of building any extra logic for handling that if
> checks/transforms would've been interleaved.
>
>
> Ad PATCH 6-18:
>
> These patches are dependent on PATCH 2 so I put these changes in one
> patch series rather than referencing them from each other.
>
> After the fixes, the patch series contains a few patches to clean up
> obvious code smells or things I wanted to differently but didn't have
> the time to do when it was time for the release. The major thing here is
> that the HA rules are compiled into a more compact representation now,
> which are used by the scheduler and other users to be more efficient.
> Otherwise, larger rule sets could slow down the HA Manager
> significantly, because it's harder to traverse the rules hash (e.g. find
> a ha resource's rule(s)) instead of directly accessing it in a more
> compact hash (i.e. $affinity->{$sid}) for example.
>
> Some of these changes will also make implementing some feature
> enhancements such as migration blockers for node affinity rules and
> negative node affinity rules easier.
>
>
> Daniel Kral (18):
> config: do not add ignored resources to dependent resources
> manager: retranslate rules if nodes are added or removed
> rules: factor out disjoint rules' resource set helper
> rules: resource affinity: inter-consistency check with merged positive
> rules
> rules: add merged positive resource affinity info in global checks
> rules: make rules sorting optional in foreach_rule helper
> rename rule's canonicalize stage to transform stage
> rules: make plugins register transformers instead of plugin_transform
> rules: node affinity: decouple get_node_affinity helper from Usage
> class
> compile ha rules to a more compact representation
> test: rules: use to_json instead of Data::Dumper for config output
> test: rules: add compiled config output to rules config test cases
> rules: node affinity: define node priority outside hash access
> move minimum version check helper to ha tools
> manager: move group migration cooldown variable into helper
> api: status: sync active service counting with lrm's helper
> manager: group migration: sync active service counting with lrm's
> helper
> factor out counting of active services into helper
>
> debian/pve-ha-manager.install | 1 +
> src/PVE/API2/HA/Rules.pm | 1 +
> src/PVE/API2/HA/Status.pm | 17 +-
> src/PVE/HA/Config.pm | 18 +-
> src/PVE/HA/LRM.pm | 20 +-
> src/PVE/HA/Manager.pm | 86 ++--
> src/PVE/HA/NodeStatus.pm | 14 +
> src/PVE/HA/Rules.pm | 170 ++++++--
> src/PVE/HA/Rules/Helpers.pm | 61 +++
> src/PVE/HA/Rules/Makefile | 2 +-
> src/PVE/HA/Rules/NodeAffinity.pm | 90 ++--
> src/PVE/HA/Rules/ResourceAffinity.pm | 200 ++++-----
> src/PVE/HA/Tools.pm | 52 +++
> ...efaults-for-node-affinity-rules.cfg.expect | 145 ++++---
> ...lts-for-resource-affinity-rules.cfg.expect | 87 ++--
> ...onsistent-node-resource-affinity-rules.cfg | 19 +
> ...nt-node-resource-affinity-rules.cfg.expect | 185 ++++++---
> .../inconsistent-resource-affinity-rules.cfg | 15 +
> ...sistent-resource-affinity-rules.cfg.expect | 21 +-
> ...egative-resource-affinity-rules.cfg.expect | 73 ++--
> ...fective-resource-affinity-rules.cfg.expect | 18 +-
> ...egative-resource-affinity-rules.cfg.expect | 342 +++++++++------
> ...ositive-resource-affinity-rules.cfg.expect | 391 +++++++++++++-----
> ...egative-resource-affinity-rules.cfg.expect | 186 +++++----
> ...ositive-resource-affinity-rules.cfg.expect | 264 +++++++++---
> ...ty-with-resource-affinity-rules.cfg.expect | 114 +++--
> ...rce-refs-in-node-affinity-rules.cfg.expect | 200 ++++++---
> src/test/test_failover1.pl | 10 +-
> src/test/test_rules_config.pl | 14 +-
> 29 files changed, 1838 insertions(+), 978 deletions(-)
> create mode 100644 src/PVE/HA/Rules/Helpers.pm
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules
2025-08-29 12:43 ` Michael Köppl
@ 2025-08-29 13:28 ` Daniel Kral
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-29 13:28 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Fri Aug 29, 2025 at 2:43 PM CEST, Michael Köppl wrote:
> One nit inline
>
> On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
>> @@ -248,58 +255,6 @@ __PACKAGE__->register_check(
>>
>> =cut
>>
>> -my $sort_by_lowest_resource_id = sub {
>> - my ($rules) = @_;
>> -
>> - my $lowest_rule_resource_id = {};
>> - for my $ruleid (keys %$rules) {
>> - my @rule_resources = sort keys $rules->{$ruleid}->{resources}->%*;
>> - $lowest_rule_resource_id->{$ruleid} = $rule_resources[0];
>> - }
>> -
>> - # sort rules such that rules with the lowest numbered resource come first
>> - my @sorted_ruleids = sort {
>> - $lowest_rule_resource_id->{$a} cmp $lowest_rule_resource_id->{$b}
>> - } sort keys %$rules;
>> -
>> - return @sorted_ruleids;
>> -};
>> -
>> -# returns a list of hashes, which contain disjoint resource affinity rules, i.e.,
>> -# put resource affinity constraints on disjoint sets of resources
>> -my $find_disjoint_resource_affinity_rules = sub {
>> - my ($rules) = @_;
>> -
>> - my @disjoint_rules = ();
>> -
>> - # order needed so that it is easier to check whether there is an overlap
>> - my @sorted_ruleids = $sort_by_lowest_resource_id->($rules);
>> -
>> - for my $ruleid (@sorted_ruleids) {
>> - my $rule = $rules->{$ruleid};
>> -
>> - my $found = 0;
>> - for my $entry (@disjoint_rules) {
>> - next if sets_are_disjoint($rule->{resources}, $entry->{resources});
>> -
>> - $found = 1;
>> - push @{ $entry->{ruleids} }, $ruleid;
>> - $entry->{resources}->{$_} = 1 for keys $rule->{resources}->%*;
>> -
>> - last;
>> - }
>> - if (!$found) {
>> - push @disjoint_rules,
>> - {
>> - ruleids => [$ruleid],
>> - resources => { $rule->{resources}->%* },
>> - };
>> - }
>> - }
>> -
>> - return @disjoint_rules;
>> -};
>> -
>
> nit: I think the removal of this would have been better to do in the
> previous patch. It's not really related to the changes made in this
> patch AFAICT and the function is not used anymore at this point.
Ah right, ACK, I forgot to reply to this patch afterwards but I wrongly
squashed that in a rebase, it should have gone to the previous patch,
but thanks for pointing it out either way! I'll move it in a v2.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper
2025-08-29 12:44 ` Michael Köppl
@ 2025-08-29 13:36 ` Daniel Kral
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-29 13:36 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Fri Aug 29, 2025 at 2:44 PM CEST, Michael Köppl wrote:
> During my review I was wondering why 16/18 and 17/18 added similar
> checks to both the manager's and the API's active service counting and
> why they're not combined into a single helper function, only to then
> notice that the final patch does exactly that. Is there a reason it's
> done in this order? Other patches in this series do it the other way
> around or combine the introduction of a new helper function and its use.
It could probably be squashed together in a single commit, but I added
the intermediate changes as individual patches because they actually
change the behavior there and only afterwards factor it out when all
behave the same way. The other patches factor it out before and then use
that helper/etc without any change in behavior.
It was more a matter of taste so that if these behavioral changes turn
out to be a cause of a bug, it's easier to bisect than having quite a
large change in one, but no hard feelings to change that in a v2.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation
2025-08-29 12:43 ` Michael Köppl
@ 2025-08-29 13:42 ` Daniel Kral
2025-09-02 7:32 ` Michael Köppl
0 siblings, 1 reply; 34+ messages in thread
From: Daniel Kral @ 2025-08-29 13:42 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Fri Aug 29, 2025 at 2:43 PM CEST, Michael Köppl wrote:
> On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
>> sub write_rules_config {
>> @@ -391,8 +391,9 @@ sub get_resource_motion_info {
>> my $ss = $manager_status->{service_status};
>> my $ns = $manager_status->{node_status};
>>
>> - my $rules = read_and_check_effective_rules_config();
>> - my ($together, $separate) = get_affinitive_resources($rules, $sid);
>> + my $affinity = read_and_compile_rules_config();
>
> nit: maybe something like $compiled_rules? Calling a variable for this
> structure $affinity didn't make it quite clear to me what this is,
> especially when later on it's used as $affinity->{'resource-affinity'}.
> Maybe I'm just misinterpreting this, though.
>
> Or could you maybe explain why this compiled rule structure is then
> called affinity?
ACK, I'll call it that as it makes it more future proof too, because
there could be rules that have nothing to do with 'affinity'.
>> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
>> index a075feac..d7593532 100644
>> --- a/src/PVE/HA/Rules.pm
>> +++ b/src/PVE/HA/Rules.pm
>> @@ -47,6 +47,12 @@ Each I<rule plugin> is required to implement the methods C<L<type()>>,
>> C<L<properties()>>, and C<L<options>> from the C<L<PVE::SectionConfig>> to
>> extend the properties of this I<base plugin> with plugin-specific properties.
>>
>> +Each I<rule plugin> is required to implement the method
>> +C<L<< plugin_compile()|/$class->plugin_compile(...) >>> to distill a compiled
>> +representation of the more verbose C<$rules> from the config file, which is
>> +returned by C<L<< compile()|/$class->compile(...) >>> to be more appropriate
>> +and efficient for the scheduler and other users.
>
> Is this supposed to say "other uses" or do you actually mean "other
> users"? Do you mean that this more efficient representation can (and
> probably should) be used for other uses cases when e.g. iterating over
> only resource affinity rules?
With "users" I meant other call sites that use the compiled rules
instead of the rules. As the helpers were converted to using the
compiled rules now too, many call sites are 'required' to use the
compiled rules anyway to be able to use these rule helpers (e.g.
get_node_affinity, get_affinitive_resources, ...).
If there's a better term I'm happy to change it.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements
2025-08-29 12:44 ` [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Michael Köppl
@ 2025-08-29 13:52 ` Daniel Kral
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-08-29 13:52 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
Thanks a lot for the review and testing! Appreciate it!
On Fri Aug 29, 2025 at 2:44 PM CEST, Michael Köppl wrote:
> Tested this series by:
> - Creating rules with resources that were set to ignored, checked if the
> resources are not shown as dependent resources when migrating
One thing that I want to point out here is that 'ignored' HA resources
are still accounted for when checking the feasibility and transforming
HA rules as that state is not really defined yet.
The 'ignored' HA resources state makes them be ignored for the HA
Manager, but I wondered how we should handle that for the HA rules
config, checks, etc.
The reason currently is that putting HA resources from any state to
'ignored' and vice-versa could make HA rules infeasible or feasible
again. Especially the former case (making HA rules infeasible through
changing HA resource states) didn't fit right for me as it feels weird
that changing HA resource states could do that.
I'll check that again when sending patches for #6613, but I'm open for
thoughts about that behavior.
> - Also tested what happens when the resource state was recently changed.
> Noticed that when changing from 'started' to 'ignored', it can take a
> few seconds before the UI reflects that (e.g. when using the migrate
> dialog). This isn't unexpected, but wanted to note it nonetheless.
What do you mean there exactly? Do you mean the state that is shown in
the HA Resources panel or other places in the UI?
> - Checking that conflicts between positive resource affinity rules and
> node affinity rules are now detected correctly that weren't detected
> before
> - Manually created various combinations of node affinity and resource
> affinity rules and checked that they are applied
> - Also re-ran some group-to-rules migrations, just to be sure
>
> I did not notice any problems apart from the minor delay in the UI noted
> above. The rules are more robust now and the changes solve some problems
> that I had encountered before.
>
> I also had a closer look at the code and left some comments on the
> individual patches, but nothing major. I had a particularly close look
> at the compiled rules to be certain that they produce the expected
> structure.
>
> With my comments on the individual patches addressed, please consider
> this series
>
> Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
> Tested-by: Michael Köppl <m.koeppl@proxmox.com>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation
2025-08-29 13:42 ` Daniel Kral
@ 2025-09-02 7:32 ` Michael Köppl
0 siblings, 0 replies; 34+ messages in thread
From: Michael Köppl @ 2025-09-02 7:32 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Fri Aug 29, 2025 at 3:42 PM CEST, Daniel Kral wrote:
>>> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
>>> index a075feac..d7593532 100644
>>> --- a/src/PVE/HA/Rules.pm
>>> +++ b/src/PVE/HA/Rules.pm
>>> @@ -47,6 +47,12 @@ Each I<rule plugin> is required to implement the methods C<L<type()>>,
>>> C<L<properties()>>, and C<L<options>> from the C<L<PVE::SectionConfig>> to
>>> extend the properties of this I<base plugin> with plugin-specific properties.
>>>
>>> +Each I<rule plugin> is required to implement the method
>>> +C<L<< plugin_compile()|/$class->plugin_compile(...) >>> to distill a compiled
>>> +representation of the more verbose C<$rules> from the config file, which is
>>> +returned by C<L<< compile()|/$class->compile(...) >>> to be more appropriate
>>> +and efficient for the scheduler and other users.
>>
>> Is this supposed to say "other uses" or do you actually mean "other
>> users"? Do you mean that this more efficient representation can (and
>> probably should) be used for other uses cases when e.g. iterating over
>> only resource affinity rules?
>
> With "users" I meant other call sites that use the compiled rules
> instead of the rules. As the helpers were converted to using the
> compiled rules now too, many call sites are 'required' to use the
> compiled rules anyway to be able to use these rule helpers (e.g.
> get_node_affinity, get_affinitive_resources, ...).
>
> If there's a better term I'm happy to change it.
I see, thanks for the explanation. I mean maybe something like "and
other calling functions", but it might also just have been me. It does
make sense the way you explained it and I assumed that's what you meant,
but wanted to be sure.
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH ha-manager 16/18] api: status: sync active service counting with lrm's helper
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 16/18] api: status: sync active service counting with lrm's helper Daniel Kral
@ 2025-09-02 8:10 ` Daniel Kral
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Kral @ 2025-09-02 8:10 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
> diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
> index 6e13c2c8..3d9aabaa 100644
> --- a/src/PVE/API2/HA/Status.pm
> +++ b/src/PVE/API2/HA/Status.pm
> @@ -197,13 +197,21 @@ __PACKAGE__->register_method({
> my $active_count = {};
> foreach my $sid (sort keys %{ $status->{service_status} }) {
> my $sd = $status->{service_status}->{$sid};
> + my $target = $sd->{target}; # count as active if we are the target.
> next if !$sd->{node};
> $active_count->{ $sd->{node} } = 0 if !defined($active_count->{ $sd->{node} });
> my $req_state = $sd->{state};
> next if !defined($req_state);
> next if $req_state eq 'stopped';
> next if $req_state eq 'freeze';
> + next if $req_state eq 'error';
> + next if $req_state eq 'request_start';
> $active_count->{ $sd->{node} }++;
> +
> + if ($target && $target ne $sd->{node}) {
> + $active_count->{$target} = 0 if !defined($active_count->{$target});
> + $active_count->{$target}++;
> + }
@Michael and I noticed that this (intermediate) change doesn't account
for cases where $sd->{node} is falsy but $sd->{target} is set, which
doesn't make it equal to the lrm's helper.
I'll see what I'll do for a v2, but I'll either just fix it in-place or
move the whole thing into the loop below already, so that $active_count
is already scalar instead of a hash reference, so it's simpler to see in
the last patch that using the introduced helper is valid.
> }
>
> foreach my $node (sort keys %{ $status->{node_status} }) {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2025-09-02 8:10 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 01/18] config: do not add ignored resources to dependent resources Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed Daniel Kral
2025-08-27 13:41 ` Michael Köppl
2025-08-27 14:33 ` Daniel Kral
2025-08-27 14:55 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 03/18] rules: factor out disjoint rules' resource set helper Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
2025-08-29 12:43 ` Michael Köppl
2025-08-29 13:28 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 05/18] rules: add merged positive resource affinity info in global checks Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 06/18] rules: make rules sorting optional in foreach_rule helper Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 07/18] rename rule's canonicalize stage to transform stage Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 08/18] rules: make plugins register transformers instead of plugin_transform Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 09/18] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation Daniel Kral
2025-08-27 15:19 ` Daniel Kral
2025-08-29 12:43 ` Michael Köppl
2025-08-29 13:42 ` Daniel Kral
2025-09-02 7:32 ` Michael Köppl
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 11/18] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 12/18] test: rules: add compiled config output to rules config test cases Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 13/18] rules: node affinity: define node priority outside hash access Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 14/18] move minimum version check helper to ha tools Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 15/18] manager: move group migration cooldown variable into helper Daniel Kral
2025-08-29 12:43 ` Michael Köppl
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 16/18] api: status: sync active service counting with lrm's helper Daniel Kral
2025-09-02 8:10 ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 17/18] manager: group migration: " Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper Daniel Kral
2025-08-29 12:44 ` Michael Köppl
2025-08-29 13:36 ` Daniel Kral
2025-08-29 12:44 ` [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Michael Köppl
2025-08-29 13:52 ` Daniel Kral
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox