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 <[email protected]>
Signed-off-by: Daniel Kral <[email protected]>
Reviewed-by: Michael Köppl <[email protected]>
Tested-by: Michael Köppl <[email protected]>
---
 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 332a42ab..6aa54439 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
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to