[pve-devel] [PATCH ha-manager v2 04/18] rules: resource affinity: inter-consistency check with merged positive rules

[Daniel Kral <d.kral@proxmox.com>]

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 v1:
  - helpers are now removed in the previous patch instead of here
    (squashing error while rebasing v1)

 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 8a2bee5d..e9b368a4 100644
--- a/src/PVE/HA/Rules/ResourceAffinity.pm
+++ b/src/PVE/HA/Rules/ResourceAffinity.pm
@@ -206,14 +206,18 @@ sub check_inter_resource_affinity_rules_consistency {
 
     my @conflicts = ();
 
-    while (my ($positiveid, $positive) = each %$positive_rules) {
-        my $positive_resources = $positive->{resources};
+    my @disjoint_positive_rules =
+        PVE::HA::Rules::Helpers::find_disjoint_rules_resource_sets($positive_rules);
+
+    for my $entry (@disjoint_positive_rules) {
+        my $positive_resources = $entry->{resources};
 
         while (my ($negativeid, $negative) = each %$negative_rules) {
             my $common_resources = set_intersect($positive_resources, $negative->{resources});
             next if %$common_resources < 2;
 
-            push @conflicts, [$positiveid, $negativeid];
+            push @conflicts, ['negative', $negativeid];
+            push @conflicts, ['positive', $_] for $entry->{ruleids}->@*;
         }
     }
 
@@ -234,12 +238,15 @@ __PACKAGE__->register_check(
         my ($conflicts, $errors) = @_;
 
         for my $conflict (@$conflicts) {
-            my ($positiveid, $negativeid) = @$conflict;
+            my ($type, $ruleid) = @$conflict;
 
-            push $errors->{$positiveid}->{resources}->@*,
-                "rule shares two or more resources with a negative resource affinity rule";
-            push $errors->{$negativeid}->{resources}->@*,
-                "rule shares two or more resources with a positive resource affinity rule";
+            if ($type eq 'positive') {
+                push $errors->{$ruleid}->{resources}->@*,
+                    "rule shares two or more resources with a negative resource affinity rule";
+            } elsif ($type eq 'negative') {
+                push $errors->{$ruleid}->{resources}->@*,
+                    "rule shares two or more resources with a positive resource affinity rule";
+            }
         }
     },
 );
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