* [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup
@ 2025-11-03 10:19 Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 01/21] config: do not add ignored resources to dependent resources Daniel Kral
` (21 more replies)
0 siblings, 22 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 UTC (permalink / raw)
To: pve-devel
v2: https://lore.proxmox.com/pve-devel/20250909083539.39675-1-d.kral@proxmox.com/
v1: https://lore.proxmox.com/pve-devel/20250821143705.256562-1-d.kral@proxmox.com/
This series is based on top of the granular accounting series [0]
[0] https://lore.proxmox.com/pve-devel/20251027164513.542678-1-d.kral@proxmox.com/
Changelog from v2 -> v3:
- rebased on top of master + granular accounting series [0], because
[0] is already closer to being merged and as both change some rules'
interfaces I had to choose one above the other
- fix and add clarifying notes to inter-rules feasibility checks
- remove usage of prototypes introduced by the original HA rules
series, which I had wrong assumptions about
- some minor code changes (see per-patch notes):
- use 5.36 for newly introduced module PVE::HA::Rules::Helpers
- use my sub name {} over my $name = sub {} for new private
subroutines / helpers
- change POD signature of get_node_affinity(...) too
Ran a `git rebase master --exec 'make clean && make deb'` on the series
and tested the changes to the Status API manually.
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-10 ha rules performance improvements + preparations
PATCH 11-12 make test cases use to_json(...) and add compiled configs
PATCH 13-21 various smaller cleanups related to HA rules
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-10:
These patches prepare and implement the compilation of HA rules when
these are used in the HA Manager, which improves the performance of
checking for HA rules (significant for overall performance) and when
applying these (significant only for resources with rules).
Ad PATCH 11-21:
These patches are various smaller improvements related to the core HA
rules feature, which are dependent on the changes above and/or are
easier to apply in a single patch series (e.g. due to larger changes in
a single source file):
- make rules tests use to_json(...) instead of Data::Dumper
- make rules tests also output compiled configs to document changes
- synchronize how active LRMs are determined, which included how it is
done for checking health for the HA groups migration
- add notes about the why's and how's for feasibility checks
Daniel Kral (21):
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
tree-wide: remove misused function prototype declaractions
rules: fix documentation for inter-rules checker subroutines
rules: add documentation about current feasibility check
implementations
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/HashTools.pm | 6 +-
src/PVE/HA/LRM.pm | 20 +-
src/PVE/HA/Manager.pm | 89 ++--
src/PVE/HA/NodeStatus.pm | 14 +
src/PVE/HA/Rules.pm | 215 +++++++---
src/PVE/HA/Rules/Helpers.pm | 77 ++++
src/PVE/HA/Rules/Makefile | 2 +-
src/PVE/HA/Rules/NodeAffinity.pm | 90 ++--
src/PVE/HA/Rules/ResourceAffinity.pm | 220 +++++-----
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 | 15 +-
src/test/test_rules_config.pl | 14 +-
30 files changed, 1904 insertions(+), 1007 deletions(-)
create mode 100644 src/PVE/HA/Rules/Helpers.pm
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 01/21] config: do not add ignored resources to dependent resources
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 02/21] manager: retranslate rules if nodes are added or removed Daniel Kral
` (20 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 02/21] manager: retranslate rules if nodes are added or removed
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 01/21] config: do not add ignored resources to dependent resources Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 03/21] rules: factor out disjoint rules' resource set helper Daniel Kral
` (19 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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 retranslates 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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 83167075..9c0a8e09 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -664,6 +664,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);
@@ -719,6 +720,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 1512ae2b..e5dddf3b 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -90,6 +90,20 @@ sub list_online_nodes {
return $res;
}
+sub check_for_changed_nodelist {
+ my ($self, $node_info) = @_;
+
+ for my $node (keys %$node_info) {
+ return 1 if !$self->{status}->{$node};
+ }
+
+ for my $node (keys $self->{status}->%*) {
+ return 1 if !$node_info->{$node};
+ }
+
+ return 0;
+}
+
my $delete_node = sub {
my ($self, $node) = @_;
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 03/21] rules: factor out disjoint rules' resource set helper
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 01/21] config: do not add ignored resources to dependent resources Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 02/21] manager: retranslate rules if nodes are added or removed Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 04/21] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
` (18 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
---
changes since v2:
- fix minor merge conflict for Makefile changes while rebasing
- make new HA rules helper use perl 5.36 + signatures
- change `my $name = sub { ... }` to `my sub name { ... }` as the
latter has a name in perl debugging utilities
debian/pve-ha-manager.install | 1 +
src/PVE/HA/Rules/Helpers.pm | 77 ++++++++++++++++++++++++++++
src/PVE/HA/Rules/Makefile | 2 +-
src/PVE/HA/Rules/ResourceAffinity.pm | 58 ++-------------------
4 files changed, 83 insertions(+), 55 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..7861332b
--- /dev/null
+++ b/src/PVE/HA/Rules/Helpers.pm
@@ -0,0 +1,77 @@
+package PVE::HA::Rules::Helpers;
+
+use v5.36;
+
+use PVE::HA::HashTools qw(sets_are_disjoint);
+
+my sub sort_by_lowest_resource_id($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;
+}
+
+=head3 find_disjoint_rules_resource_sets($rules)
+
+Finds the rule subsets in C<$rules>, which reference a disjoint set of resources
+with respect to the other rules. The disjoint rule resource sets are returned as
+a list of hashes, where each item contains the disjoint resource set and the
+ruleids from C<$rules> that were used to create the resource subset.
+
+For example, if one rule references the resources C<'vm:101'> and C<'vm:102'>,
+and another rule references the resources C<'vm:102'> and C<'vm:103'>, these
+will result in one disjoint rule resource set, thus the return value:
+
+ (
+ {
+ ruleids = [ 'rule1', 'rule2' ],
+ resources => {
+ 'vm:101' => 1,
+ 'vm:102' => 1,
+ 'vm:103' => 1
+ }
+ }
+ )
+
+=cut
+
+sub find_disjoint_rules_resource_sets($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 1cbde5ba..977c882a 100644
--- a/src/PVE/HA/Rules/Makefile
+++ b/src/PVE/HA/Rules/Makefile
@@ -1,4 +1,4 @@
-SIM_SOURCES=NodeAffinity.pm ResourceAffinity.pm
+SIM_SOURCES=Helpers.pm NodeAffinity.pm ResourceAffinity.pm
SOURCES=${SIM_SOURCES}
.PHONY: install
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index 9a928196..86b94d60 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 PVE::HA::Usage;
use base qw(Exporter);
@@ -248,58 +249,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
@@ -320,7 +269,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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 04/21] rules: resource affinity: inter-consistency check with merged positive rules
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (2 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 03/21] rules: factor out disjoint rules' resource set helper Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 05/21] rules: add merged positive resource affinity info in global checks Daniel Kral
` (17 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
src/PVE/HA/Rules/ResourceAffinity.pm | 23 ++++++++++++-------
.../inconsistent-resource-affinity-rules.cfg | 15 ++++++++++++
...sistent-resource-affinity-rules.cfg.expect | 5 +++-
3 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index 86b94d60..22b7b5fe 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -207,14 +207,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}->@*;
}
}
@@ -235,12 +239,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";
+ }
}
},
);
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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 05/21] rules: add merged positive resource affinity info in global checks
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (3 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 04/21] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 17:24 ` Michael Köppl
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 06/21] rules: make rules sorting optional in foreach_rule helper Daniel Kral
` (16 subsequent siblings)
21 siblings, 1 reply; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 UTC (permalink / raw)
To: pve-devel
The node affinity and positive resource affinity rule subsets are
checked whether HA resources in a positive resource affinity rule are in
more than one node affinity rule in total, and if so prunes them as
those are not feasible [0].
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>
---
changes since v2:
- none
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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 06/21] rules: make rules sorting optional in foreach_rule helper
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (4 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 05/21] rules: add merged positive resource affinity info in global checks Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 07/21] rename rule's canonicalize stage to transform stage Daniel Kral
` (15 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 07/21] rename rule's canonicalize stage to transform stage
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (5 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 06/21] rules: make rules sorting optional in foreach_rule helper Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 08/21] rules: make plugins register transformers instead of plugin_transform Daniel Kral
` (14 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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 9c0a8e09..f42373dd 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -728,7 +728,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 22b7b5fe..52e3c47b 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -252,7 +252,7 @@ __PACKAGE__->register_check(
},
);
-=head1 RESOURCE AFFINITY RULE CANONICALIZATION HELPERS
+=head1 RESOURCE AFFINITY RULE TRANSFORMATION HELPERS
=cut
@@ -382,7 +382,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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 08/21] rules: make plugins register transformers instead of plugin_transform
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (6 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 07/21] rename rule's canonicalize stage to transform stage Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 09/21] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
` (13 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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 52e3c47b..4a6e8627 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -299,6 +299,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
@@ -382,23 +388,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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 09/21] rules: node affinity: decouple get_node_affinity helper from Usage class
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (7 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 08/21] rules: make plugins register transformers instead of plugin_transform Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 10/21] compile ha rules to a more compact representation Daniel Kral
` (12 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
---
changes since v2:
- minor change due to rebase on accounting series where the
$online_nodes in select_service_node(...) is now shared by both
affinity rule helpers
- change the get_node_affinity(...) POD signature too
src/PVE/HA/Manager.pm | 4 ++--
src/PVE/HA/Rules/NodeAffinity.pm | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index f42373dd..a26bde76 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -157,11 +157,11 @@ 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;
- my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() };
my ($together, $separate) = get_resource_affinity($rules, $sid, $ss, $online_nodes);
# stay on current node if possible (avoids random migrations)
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index 5394832e..111ff6fe 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -237,11 +237,11 @@ my $get_resource_node_affinity_rule = sub {
return $node_affinity_rule;
};
-=head3 get_node_affinity($rules, $sid, $online_node_usage)
+=head3 get_node_affinity($rules, $sid, $online_nodes)
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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 10/21] compile ha rules to a more compact representation
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (8 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 09/21] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 11/21] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
` (11 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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(...).
A benchmark done with NYTProf [0] shows the following runtime
improvement, where each duration is the average time needed per call.
The benchmark was done by starting up a 3-node cluster with 1200
configured HA resources, where each HA resource is in a node affinity
rule and a negative resource affinity to two other HA resources.
+----------------------------------+--------+-------+
| Function | Before | After |
+----------------------------------+--------+-------+
| select_service_node | 10.5ms | 44µs |
| get_node_affinity | 5.26ms | 10µs |
| get_resource_affinity | 5.17ms | 12µs |
+----------------------------------+--------+-------+
[0] https://metacpan.org/pod/Devel::NYTProf
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v2:
- minor changes due to rebase
- factor out $resource_affinity in get_resource_motion_info(...) to
align it with other usages of these helpers and in preparation of
another patch series which will use $node_affinity too
src/PVE/HA/Config.pm | 9 ++-
src/PVE/HA/Manager.pm | 31 ++++----
src/PVE/HA/Rules.pm | 59 +++++++++++++++
src/PVE/HA/Rules/NodeAffinity.pm | 80 ++++++++++----------
src/PVE/HA/Rules/ResourceAffinity.pm | 105 ++++++++++++++-------------
src/test/test_failover1.pl | 15 +++-
6 files changed, 187 insertions(+), 112 deletions(-)
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index bd87a0ab..548a89e6 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 $compiled_rules = read_and_compile_rules_config();
+ my $resource_affinity = $compiled_rules->{'resource-affinity'};
+ my ($together, $separate) = get_affinitive_resources($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 a26bde76..e17e3209 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -126,12 +126,12 @@ sub flush_master_status {
=head3 select_service_node(...)
-=head3 select_service_node($rules, $online_node_usage, $sid, $service_conf, $ss, $node_preference)
+=head3 select_service_node($compiled_rules, $online_node_usage, $sid, $service_conf, $ss, $node_preference)
Used to select the best fitting node for the service C<$sid>, with the
-configuration C<$service_conf>, according to the rules defined in C<$rules>,
-available node utilization in C<$online_node_usage>, the service states in
-C<$ss> and the given C<$node_preference>.
+configuration C<$service_conf>, according to the rules defined in
+C<$compiled_rules>, available node utilization in C<$online_node_usage>, the
+service states in C<$ss> and the given C<$node_preference>.
The C<$node_preference> can be set to:
@@ -148,7 +148,7 @@ The C<$node_preference> can be set to:
=cut
sub select_service_node {
- my ($rules, $online_node_usage, $sid, $service_conf, $ss, $node_preference) = @_;
+ my ($compiled_rules, $online_node_usage, $sid, $service_conf, $ss, $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)/;
@@ -156,13 +156,15 @@ sub select_service_node {
my $sd = $ss->{$sid};
my ($current_node, $tried_nodes, $maintenance_fallback) =
$sd->@{qw(node failed_nodes maintenance_node)};
+ my ($node_affinity, $resource_affinity) =
+ $compiled_rules->@{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, $ss, $online_nodes);
+ my ($together, $separate) = get_resource_affinity($resource_affinity, $sid, $ss, $online_nodes);
# stay on current node if possible (avoids random migrations)
if (
@@ -389,7 +391,8 @@ sub execute_migration {
my ($haenv, $ss) = $self->@{qw(haenv ss)};
- my ($together, $separate) = get_affinitive_resources($self->{rules}, $sid);
+ my $resource_affinity = $self->{compiled_rules}->{'resource-affinity'};
+ my ($together, $separate) = get_affinitive_resources($resource_affinity, $sid);
for my $csid (sort keys %$separate) {
next if !defined($ss->{$csid});
@@ -719,7 +722,7 @@ sub manage {
PVE::HA::Groups::migrate_groups_to_resources($self->{groups}, $sc);
if (
- !$self->{rules}
+ !$self->{compiled_rules}
|| $has_changed_nodelist
|| $new_rules->{digest} ne $self->{last_rules_digest}
|| $self->{groups}->{digest} ne $self->{last_groups_digest}
@@ -731,9 +734,9 @@ sub manage {
my $messages = PVE::HA::Rules->transform($new_rules, $nodes);
$haenv->log('info', $_) for @$messages;
- $self->{rules} = $new_rules;
+ $self->{compiled_rules} = PVE::HA::Rules->compile($new_rules, $nodes);
- $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;
}
@@ -991,7 +994,7 @@ sub next_state_request_start {
if ($self->{crs}->{rebalance_on_request_start}) {
my $selected_node = select_service_node(
- $self->{rules},
+ $self->{compiled_rules},
$self->{online_node_usage},
$sid,
$cd,
@@ -1158,7 +1161,7 @@ sub next_state_started {
}
my $node = select_service_node(
- $self->{rules},
+ $self->{compiled_rules},
$self->{online_node_usage},
$sid,
$cd,
@@ -1272,7 +1275,7 @@ sub next_state_recovery {
my $fenced_node = $sd->{node}; # for logging purpose
my $recovery_node = select_service_node(
- $self->{rules},
+ $self->{compiled_rules},
$self->{online_node_usage},
$sid,
$cd,
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index a075feac..69c53356 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 $compiled_rules = {};
+
+ for my $type (@$types) {
+ my $plugin = $class->lookup($type);
+ my $compiled_plugin_rules = $plugin->plugin_compile($rules, $nodes);
+
+ die "plugin_compile(...) of type '$type' must return hash reference\n"
+ if ref($compiled_plugin_rules) ne 'HASH';
+
+ $compiled_rules->{$type} = $compiled_plugin_rules;
+ }
+
+ return $compiled_rules;
+}
+
=head1 FUNCTIONS
=cut
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index 111ff6fe..adef97d0 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_nodes)
+=head3 get_node_affinity($node_affinity, $sid, $online_nodes)
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
@@ -252,30 +266,14 @@ If there are no available nodes at all, returns C<undef>.
=cut
sub get_node_affinity : prototype($$$) {
- my ($rules, $sid, $online_nodes) = @_;
+ 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});
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 4a6e8627..c44d4f0b 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -101,6 +101,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
@@ -404,12 +433,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.
@@ -430,36 +459,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, $ss, $online_nodes)
+=head3 get_resource_affinity($resource_affinity, $sid, $ss, $online_nodes)
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>, the
-service status C<$ss> and the C<$online_nodes> hash.
+resource C<$sid> according to the resource's resource affinity rules in
+C<$resource_affinity>, the service status C<$ss> and the C<$online_nodes> hash.
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
@@ -488,38 +501,32 @@ resource C<$sid> is in a negative affinity with, the returned value will be:
=cut
sub get_resource_affinity : prototype($$$$) {
- my ($rules, $sid, $ss, $online_nodes) = @_;
+ my ($resource_affinity, $sid, $ss, $online_nodes) = @_;
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;
+ my $get_used_service_nodes = sub {
+ my ($sid) = @_;
+ my ($state, $node, $target) = $ss->{$sid}->@{qw(state node target)};
+ return PVE::HA::Usage::get_used_service_nodes($online_nodes, $state, $node, $target);
+ };
- my ($state, $node, $target) = $ss->{$csid}->@{qw(state node target)};
- my ($current_node, $target_node) =
- PVE::HA::Usage::get_used_service_nodes($online_nodes, $state, $node, $target);
+ for my $csid (keys $positive->%*) {
+ my ($current_node, $target_node) = $get_used_service_nodes->($csid);
- if ($rule->{affinity} eq 'positive') {
- $together->{$current_node}++ if defined($current_node);
- $together->{$target_node}++ if defined($target_node);
- } elsif ($rule->{affinity} eq 'negative') {
- $separate->{$current_node} = 1 if defined($current_node);
- $separate->{$target_node} = 1 if defined($target_node);
- } else {
- die "unimplemented resource affinity type $rule->{affinity}\n";
- }
- }
- },
- sid => $sid,
- type => 'resource-affinity',
- exclude_disabled_rules => 1,
- );
+ $together->{$current_node}++ if defined($current_node);
+ $together->{$target_node}++ if defined($target_node);
+ }
+
+ for my $csid (keys $negative->%*) {
+ my ($current_node, $target_node) = $get_used_service_nodes->($csid);
+
+ $separate->{$current_node} = 1 if defined($current_node);
+ $separate->{$target_node} = 1 if defined($target_node);
+ }
return ($together, $separate);
}
diff --git a/src/test/test_failover1.pl b/src/test/test_failover1.pl
index 495d4b4b..1d336be0 100755
--- a/src/test/test_failover1.pl
+++ b/src/test/test_failover1.pl
@@ -21,11 +21,13 @@ node-affinity: prefer_node1
nodes node1
EOD
+my $nodes = ['node1', 'node2', 'node3'];
+
+my $compiled_rules = 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',
@@ -46,7 +48,12 @@ sub test {
my $select_node_preference = $try_next ? 'try-next' : 'none';
my $node = PVE::HA::Manager::select_service_node(
- $rules, $online_node_usage, "$sid", $service_conf, $ss, $select_node_preference,
+ $compiled_rules,
+ $online_node_usage,
+ $sid,
+ $service_conf,
+ $ss,
+ $select_node_preference,
);
my (undef, undef, $line) = caller();
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 11/21] test: rules: use to_json instead of Data::Dumper for config output
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (9 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 10/21] compile ha rules to a more compact representation Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 12/21] test: rules: add compiled config output to rules config test cases Daniel Kral
` (10 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
...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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 12/21] test: rules: add compiled config output to rules config test cases
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (10 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 11/21] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 13/21] rules: node affinity: define node priority outside hash access Daniel Kral
` (9 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
...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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 13/21] rules: node affinity: define node priority outside hash access
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (11 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 12/21] test: rules: add compiled config output to rules config test cases Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 14/21] move minimum version check helper to ha tools Daniel Kral
` (8 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 UTC (permalink / raw)
To: pve-devel
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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 adef97d0..d98f896d 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -276,8 +276,10 @@ sub get_node_affinity : prototype($$$) {
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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 14/21] move minimum version check helper to ha tools
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (12 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 13/21] rules: node affinity: define node priority outside hash access Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 15/21] manager: move group migration cooldown variable into helper Daniel Kral
` (7 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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 e17e3209..360ea8cb 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -511,33 +511,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) = @_;
@@ -594,7 +567,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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 15/21] manager: move group migration cooldown variable into helper
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (13 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 14/21] move minimum version check helper to ha tools Daniel Kral
@ 2025-11-03 10:19 ` Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 16/21] api: status: sync active service counting with lrm's helper Daniel Kral
` (6 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:19 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>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---
changes since v2:
- none
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 360ea8cb..7c87a2c3 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -46,8 +46,6 @@ eval {
# 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) = @_;
@@ -612,13 +610,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_rounds = 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_rounds;
$haenv->log('notice', "start ha group migration...");
@@ -628,8 +629,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_rounds rounds (~ "
+ . $group_migration_cooldown_rounds * 10
. " seconds)",
);
}
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 16/21] api: status: sync active service counting with lrm's helper
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (14 preceding siblings ...)
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 15/21] manager: move group migration cooldown variable into helper Daniel Kral
@ 2025-11-03 10:20 ` Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 17/21] manager: group migration: " Daniel Kral
` (5 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:20 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.
- Removing the `sort` as sorting the services is not required for
counting active services.
While at it, move the counting code in the node loop, where it will be
moved in an upcoming patch, to make it easier to see that the logic is
the same as the LRM's helper.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v2:
- none
src/PVE/API2/HA/Status.pm | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
index 6e13c2c8..3966a4b3 100644
--- a/src/PVE/API2/HA/Status.pm
+++ b/src/PVE/API2/HA/Status.pm
@@ -193,20 +193,22 @@ __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};
- 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';
- $active_count->{ $sd->{node} }++;
- }
-
foreach my $node (sort keys %{ $status->{node_status} }) {
+ my $active_count = 0;
+ for my $sid (keys $status->{service_status}->%*) {
+ my $sd = $status->{service_status}->{$sid};
+ my $target = $sd->{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';
+ next if $req_state eq 'freeze';
+ next if $req_state eq 'error';
+ next if $req_state eq 'request_start';
+ $active_count++;
+ }
my $lrm_status = PVE::HA::Config::read_lrm_status($node);
my $id = "lrm:$node";
if (!$lrm_status->{timestamp}) {
@@ -227,7 +229,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;
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 17/21] manager: group migration: sync active service counting with lrm's helper
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (15 preceding siblings ...)
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 16/21] api: status: sync active service counting with lrm's helper Daniel Kral
@ 2025-11-03 10:20 ` Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 18/21] factor out counting of active services into helper Daniel Kral
` (4 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:20 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.
- Removing the `sort` as sorting the services is not required for
counting active services.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v2:
- none
src/PVE/HA/Manager.pm | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 7c87a2c3..1682e2a4 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -513,13 +513,16 @@ my $is_lrm_active_or_idle = sub {
my ($ss, $node, $lrm_state) = @_;
my $active_count = 0;
- for my $sid (sort keys %$ss) {
+ for my $sid (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.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 18/21] factor out counting of active services into helper
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (16 preceding siblings ...)
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 17/21] manager: group migration: " Daniel Kral
@ 2025-11-03 10:20 ` Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 19/21] tree-wide: remove misused function prototype declaractions Daniel Kral
` (3 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:20 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>
---
changes since v2:
- none
src/PVE/API2/HA/Status.pm | 17 ++---------------
src/PVE/HA/LRM.pm | 20 +-------------------
src/PVE/HA/Manager.pm | 14 +-------------
src/PVE/HA/Tools.pm | 25 +++++++++++++++++++++++++
4 files changed, 29 insertions(+), 47 deletions(-)
diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
index 3966a4b3..282577cc 100644
--- a/src/PVE/API2/HA/Status.pm
+++ b/src/PVE/API2/HA/Status.pm
@@ -194,21 +194,8 @@ __PACKAGE__->register_method({
}
foreach my $node (sort keys %{ $status->{node_status} }) {
- my $active_count = 0;
- for my $sid (keys $status->{service_status}->%*) {
- my $sd = $status->{service_status}->{$sid};
- my $target = $sd->{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';
- 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($status->{service_status}, $node);
my $lrm_status = PVE::HA::Config::read_lrm_status($node);
my $id = "lrm:$node";
if (!$lrm_status->{timestamp}) {
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 1682e2a4..f5843dd4 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -512,19 +512,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 (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..1fa53df8 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 $active_count = 0;
+
+ for 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';
+
+ $active_count++;
+ }
+
+ return $active_count;
+}
+
sub get_verbose_service_state {
my ($service_state, $service_conf) = @_;
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 19/21] tree-wide: remove misused function prototype declaractions
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (17 preceding siblings ...)
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 18/21] factor out counting of active services into helper Daniel Kral
@ 2025-11-03 10:20 ` Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 20/21] rules: fix documentation for inter-rules checker subroutines Daniel Kral
` (2 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:20 UTC (permalink / raw)
To: pve-devel
These do not fulfill any purpose and were introduced with false
assumptions about what these guarantee and how these work. Also we
recommend against their usage in our Perl Style Guide [0].
[0] https://pve.proxmox.com/wiki/Perl_Style_Guide#Prototypes
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v2:
- NEW!
src/PVE/HA/HashTools.pm | 6 +++---
src/PVE/HA/Rules.pm | 18 +++++++++---------
src/PVE/HA/Rules/NodeAffinity.pm | 2 +-
src/PVE/HA/Rules/ResourceAffinity.pm | 10 +++++-----
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/PVE/HA/HashTools.pm b/src/PVE/HA/HashTools.pm
index cf5c7a20..ebe47e38 100644
--- a/src/PVE/HA/HashTools.pm
+++ b/src/PVE/HA/HashTools.pm
@@ -39,7 +39,7 @@ key-value pairs are always set to C<1> or another truthy value.
=cut
-sub set_intersect : prototype($$) {
+sub set_intersect {
my ($hash1, $hash2) = @_;
my $result = { map { $hash1->{$_} && $hash2->{$_} ? ($_ => 1) : () } keys %$hash1 };
@@ -57,7 +57,7 @@ key-value pairs are always set to C<1> or another truthy value.
=cut
-sub set_union : prototype($$) {
+sub set_union {
my ($hash1, $hash2) = @_;
my $result = { map { $_ => 1 } keys %$hash1, keys %$hash2 };
@@ -77,7 +77,7 @@ Returns C<1> if they are disjoint, C<0> otherwise.
=cut
-sub sets_are_disjoint : prototype($$) {
+sub sets_are_disjoint {
my ($hash1, $hash2) = @_;
for my $key (keys %$hash1) {
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 69c53356..1c7706ce 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -264,7 +264,7 @@ haven't been explicitly set yet.
=cut
-sub set_rule_defaults : prototype($$) {
+sub set_rule_defaults {
my ($class, $rule) = @_;
if (my $plugin = $class->lookup($rule->{type})) {
@@ -303,7 +303,7 @@ Used to register rule checks for a rule plugin.
=cut
-sub register_check : prototype($$$) {
+sub register_check {
my ($class, $check_func, $collect_errors_func) = @_;
my $type = eval { $class->type() };
@@ -322,7 +322,7 @@ Used to register rule transformers for a rule plugin.
=cut
-sub register_transform : prototype($$) {
+sub register_transform {
my ($class, $transform_func) = @_;
my $type = eval { $class->type() };
@@ -345,7 +345,7 @@ implementations.
=cut
-sub get_plugin_check_arguments : prototype($$) {
+sub get_plugin_check_arguments {
my ($class, $rules) = @_;
return {};
@@ -361,7 +361,7 @@ creation of these can be shared inbetween rule check implementations.
=cut
-sub get_check_arguments : prototype($$) {
+sub get_check_arguments {
my ($class, $rules) = @_;
my $global_args = {};
@@ -391,7 +391,7 @@ very last.
=cut
-sub check_feasibility : prototype($$$) {
+sub check_feasibility {
my ($class, $rules, $nodes) = @_;
my $global_errors = {};
@@ -429,7 +429,7 @@ Returns a list of messages with the reasons why rules were removed.
=cut
-sub transform : prototype($$$) {
+sub transform {
my ($class, $rules, $nodes) = @_;
my $messages = [];
@@ -540,7 +540,7 @@ The following key-value pairs for C<$opts> are:
=cut
-sub foreach_rule : prototype($$;%) {
+sub foreach_rule {
my ($rules, $func, %opts) = @_;
my $sid = $opts{sid};
@@ -568,7 +568,7 @@ be used a newly introduced rule afterwards.
=cut
-sub get_next_ordinal : prototype($) {
+sub get_next_ordinal {
my ($rules) = @_;
my $current_order = (sort { $a <=> $b } values %{ $rules->{order} })[0] || 0;
diff --git a/src/PVE/HA/Rules/NodeAffinity.pm b/src/PVE/HA/Rules/NodeAffinity.pm
index d98f896d..3fa1fdb4 100644
--- a/src/PVE/HA/Rules/NodeAffinity.pm
+++ b/src/PVE/HA/Rules/NodeAffinity.pm
@@ -265,7 +265,7 @@ If there are no available nodes at all, returns C<undef>.
=cut
-sub get_node_affinity : prototype($$$) {
+sub get_node_affinity {
my ($node_affinity, $sid, $online_nodes) = @_;
return ($online_nodes, $online_nodes) if !defined($node_affinity->{$sid});
diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm
index c44d4f0b..4f5ffca5 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -458,7 +458,7 @@ affinitive to C<'ct:200'> and C<'ct:201'>, the returned value will be:
=cut
-sub get_affinitive_resources : prototype($$) {
+sub get_affinitive_resources {
my ($resource_affinity, $sid) = @_;
my $together = $resource_affinity->{positive}->{$sid} // {};
@@ -500,7 +500,7 @@ resource C<$sid> is in a negative affinity with, the returned value will be:
=cut
-sub get_resource_affinity : prototype($$$$) {
+sub get_resource_affinity {
my ($resource_affinity, $sid, $ss, $online_nodes) = @_;
my $together = {};
@@ -539,7 +539,7 @@ node C<$node> must be avoided.
=cut
-sub is_allowed_on_node : prototype($$$) {
+sub is_allowed_on_node {
my ($together, $separate, $node) = @_;
return $together->{$node} || !$separate->{$node};
@@ -560,7 +560,7 @@ resource has not failed running there yet.
=cut
-sub apply_positive_resource_affinity : prototype($$) {
+sub apply_positive_resource_affinity {
my ($together, $allowed_nodes) = @_;
for my $node (keys %$together) {
@@ -594,7 +594,7 @@ resource has not failed running there yet.
=cut
-sub apply_negative_resource_affinity : prototype($$) {
+sub apply_negative_resource_affinity {
my ($separate, $allowed_nodes) = @_;
my $forbidden_nodes = { $separate->%* };
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 20/21] rules: fix documentation for inter-rules checker subroutines
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (18 preceding siblings ...)
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 19/21] tree-wide: remove misused function prototype declaractions Daniel Kral
@ 2025-11-03 10:20 ` Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 21/21] rules: add documentation about current feasibility check implementations Daniel Kral
2025-11-03 17:43 ` [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Michael Köppl
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:20 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v2:
- NEW!
src/PVE/HA/Rules.pm | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 1c7706ce..9d7f12a1 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -594,9 +594,9 @@ my $has_multiple_priorities = sub {
=head3 check_single_priority_node_affinity_in_resource_affinity_rules(...)
Returns all rules in C<$resource_affinity_rules> and C<$node_affinity_rules> as
-a list of lists, each consisting of the rule type and resource id, where at
-least one resource in a resource affinity rule are in node affinity rules,
-which have multiple priority groups defined.
+a list of lists, each consisting of the rule type and rule id, where at
+least one resource in a resource affinity rule is in a node affinity rule,
+which has multiple priority groups defined.
That is, the resource affinity rule cannot be statically checked to be feasible
as the selection of the priority group is dependent on the currently online
@@ -666,9 +666,9 @@ __PACKAGE__->register_check(
=head3 check_single_node_affinity_per_positive_resource_affinity_rule(...)
Returns all rules in C<$positive_rules> and C<$node_affinity_rules> as a list of
-lists, each consisting of the rule type and resource id, where one of the
-resources is used in a positive resource affinity rule and more than one node
-affinity rule.
+lists, each item consisting of the rule type and rule id, where at least one of
+the resources is used in a positive resource affinity rule and more than one
+node affinity rule.
If there are none, the returned list is empty.
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pve-devel] [PATCH ha-manager v3 21/21] rules: add documentation about current feasibility check implementations
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (19 preceding siblings ...)
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 20/21] rules: fix documentation for inter-rules checker subroutines Daniel Kral
@ 2025-11-03 10:20 ` Daniel Kral
2025-11-03 17:43 ` [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Michael Köppl
21 siblings, 0 replies; 24+ messages in thread
From: Daniel Kral @ 2025-11-03 10:20 UTC (permalink / raw)
To: pve-devel
These notes should clarify the goals and reasons for the feasibility
checks while giving direction how these can be improved in the future.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v2:
- NEW!
src/PVE/HA/Rules.pm | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 9d7f12a1..c4a2ccea 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -32,6 +32,13 @@ 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.
+The feasibility checks must verify that all rules can be applied error-free in
+any runtime state of the HA resources, cluster nodes, and the HA Manager in a
+reasonable amount of time. This might require more rules to be rejected than
+necessary as some checks can become computationally expensive for little benefit
+to the core feature, but this can certainly be reevaluated if there are user
+requests for these cases.
+
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
@@ -598,9 +605,11 @@ a list of lists, each consisting of the rule type and rule id, where at
least one resource in a resource affinity rule is in a node affinity rule,
which has multiple priority groups defined.
-That is, the resource affinity rule cannot be statically checked to be feasible
-as the selection of the priority group is dependent on the currently online
-nodes.
+These cases are currently rejected, because the selection of the highest
+priority class is dependent on the currently online nodes and on the current
+HA resource states and node placements in the case of negative resource affinity
+rules. This makes the verification of these rules' feasibility in all possible
+states computationally hard.
If there are none, the returned list is empty.
@@ -670,6 +679,10 @@ lists, each item consisting of the rule type and rule id, where at least one of
the resources is used in a positive resource affinity rule and more than one
node affinity rule.
+These cases are currently rejected, because there is no definitive method to
+merge multiple node affinity rules, e.g. with different strictness values and
+different priority classes, yet.
+
If there are none, the returned list is empty.
=cut
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pve-devel] [PATCH ha-manager v3 05/21] rules: add merged positive resource affinity info in global checks
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 05/21] rules: add merged positive resource affinity info in global checks Daniel Kral
@ 2025-11-03 17:24 ` Michael Köppl
0 siblings, 0 replies; 24+ messages in thread
From: Michael Köppl @ 2025-11-03 17:24 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
On Mon Nov 3, 2025 at 11:19 AM CET, Daniel Kral wrote:
> The node affinity and positive resource affinity rule subsets are
> checked whether HA resources in a positive resource affinity rule are in
> more than one node affinity rule in total, and if so prunes them as
> those are not feasible [0].
nit: The link is missing
>
> 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>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
` (20 preceding siblings ...)
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 21/21] rules: add documentation about current feasibility check implementations Daniel Kral
@ 2025-11-03 17:43 ` Michael Köppl
21 siblings, 0 replies; 24+ messages in thread
From: Michael Köppl @ 2025-11-03 17:43 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
Gave v3 another spin after having reviewed v1. I again repeated the
following scenarios:
- Checked behavior with ignored resources (i.e. that ignored resources
are not shown as dependent resources when migrating).
- Checked that conflicts between positive resource affinity rules and
node affinity rules are detected correctly in the cases that weren't
detected before
- Checked various combinations of node affinity and resource affinity
rules, also checking the failback flag, max. restart, and max.
relocate params. Could not create any configurations that are
problematic.
- Ran group migrations again (also making them fail on purpose and
checking that it keeps trying to migrate) to check that the changes
to the counting of active services did not alter behavior.
Since this is now based on the granular accounting series [0], there is
a problem with 'ignored' resources, which I noted separately on that
patch series [1].
Other than that I did not notice anything off. Everything seems to work
as expected. Also had a look at the parts of the code that did not have
my R-b anymore. My comments on v1 have been addressed in v2 already and
the remaining changes in v3 lgtm. I think the addition of the benchmark
on 10/21 is nice!
Please consider this:
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
[0] https://lore.proxmox.com/pve-devel/20251027164513.542678-1-d.kral@proxmox.com/
[1] https://lore.proxmox.com/pve-devel/20251027164513.542678-1-d.kral@proxmox.com/t/#m8db5069cad93a9f81c3e3ec50af0c78681601527
On Mon Nov 3, 2025 at 11:19 AM CET, Daniel Kral wrote:
> v2: https://lore.proxmox.com/pve-devel/20250909083539.39675-1-d.kral@proxmox.com/
> v1: https://lore.proxmox.com/pve-devel/20250821143705.256562-1-d.kral@proxmox.com/
>
>
> This series is based on top of the granular accounting series [0]
>
> [0] https://lore.proxmox.com/pve-devel/20251027164513.542678-1-d.kral@proxmox.com/
>
>
> Changelog from v2 -> v3:
>
> - rebased on top of master + granular accounting series [0], because
> [0] is already closer to being merged and as both change some rules'
> interfaces I had to choose one above the other
>
> - fix and add clarifying notes to inter-rules feasibility checks
>
> - remove usage of prototypes introduced by the original HA rules
> series, which I had wrong assumptions about
>
> - some minor code changes (see per-patch notes):
> - use 5.36 for newly introduced module PVE::HA::Rules::Helpers
> - use my sub name {} over my $name = sub {} for new private
> subroutines / helpers
> - change POD signature of get_node_affinity(...) too
>
>
> Ran a `git rebase master --exec 'make clean && make deb'` on the series
> and tested the changes to the Status API manually.
>
>
> 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-10 ha rules performance improvements + preparations
> PATCH 11-12 make test cases use to_json(...) and add compiled configs
> PATCH 13-21 various smaller cleanups related to HA rules
>
>
> 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-10:
>
> These patches prepare and implement the compilation of HA rules when
> these are used in the HA Manager, which improves the performance of
> checking for HA rules (significant for overall performance) and when
> applying these (significant only for resources with rules).
>
>
> Ad PATCH 11-21:
>
> These patches are various smaller improvements related to the core HA
> rules feature, which are dependent on the changes above and/or are
> easier to apply in a single patch series (e.g. due to larger changes in
> a single source file):
>
> - make rules tests use to_json(...) instead of Data::Dumper
> - make rules tests also output compiled configs to document changes
> - synchronize how active LRMs are determined, which included how it is
> done for checking health for the HA groups migration
> - add notes about the why's and how's for feasibility checks
>
>
> Daniel Kral (21):
> 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
> tree-wide: remove misused function prototype declaractions
> rules: fix documentation for inter-rules checker subroutines
> rules: add documentation about current feasibility check
> implementations
>
> 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/HashTools.pm | 6 +-
> src/PVE/HA/LRM.pm | 20 +-
> src/PVE/HA/Manager.pm | 89 ++--
> src/PVE/HA/NodeStatus.pm | 14 +
> src/PVE/HA/Rules.pm | 215 +++++++---
> src/PVE/HA/Rules/Helpers.pm | 77 ++++
> src/PVE/HA/Rules/Makefile | 2 +-
> src/PVE/HA/Rules/NodeAffinity.pm | 90 ++--
> src/PVE/HA/Rules/ResourceAffinity.pm | 220 +++++-----
> 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 | 15 +-
> src/test/test_rules_config.pl | 14 +-
> 30 files changed, 1904 insertions(+), 1007 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] 24+ messages in thread
end of thread, other threads:[~2025-11-03 17:43 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-03 10:19 [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 01/21] config: do not add ignored resources to dependent resources Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 02/21] manager: retranslate rules if nodes are added or removed Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 03/21] rules: factor out disjoint rules' resource set helper Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 04/21] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 05/21] rules: add merged positive resource affinity info in global checks Daniel Kral
2025-11-03 17:24 ` Michael Köppl
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 06/21] rules: make rules sorting optional in foreach_rule helper Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 07/21] rename rule's canonicalize stage to transform stage Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 08/21] rules: make plugins register transformers instead of plugin_transform Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 09/21] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 10/21] compile ha rules to a more compact representation Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 11/21] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 12/21] test: rules: add compiled config output to rules config test cases Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 13/21] rules: node affinity: define node priority outside hash access Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 14/21] move minimum version check helper to ha tools Daniel Kral
2025-11-03 10:19 ` [pve-devel] [PATCH ha-manager v3 15/21] manager: move group migration cooldown variable into helper Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 16/21] api: status: sync active service counting with lrm's helper Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 17/21] manager: group migration: " Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 18/21] factor out counting of active services into helper Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 19/21] tree-wide: remove misused function prototype declaractions Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 20/21] rules: fix documentation for inter-rules checker subroutines Daniel Kral
2025-11-03 10:20 ` [pve-devel] [PATCH ha-manager v3 21/21] rules: add documentation about current feasibility check implementations Daniel Kral
2025-11-03 17:43 ` [pve-devel] [PATCH ha-manager v3 00/21] HA rules fixes + performance improvements + cleanup Michael Köppl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox