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.du...@proxmox.com> Signed-off-by: Daniel Kral <d.k...@proxmox.com> --- src/PVE/HA/Rules/ResourceAffinity.pm | 75 ++++--------------- .../inconsistent-resource-affinity-rules.cfg | 15 ++++ ...sistent-resource-affinity-rules.cfg.expect | 5 +- 3 files changed, 34 insertions(+), 61 deletions(-) diff --git a/src/PVE/HA/Rules/ResourceAffinity.pm b/src/PVE/HA/Rules/ResourceAffinity.pm index edfa4b3d..e9b368a4 100644 --- a/src/PVE/HA/Rules/ResourceAffinity.pm +++ b/src/PVE/HA/Rules/ResourceAffinity.pm @@ -206,14 +206,18 @@ sub check_inter_resource_affinity_rules_consistency { my @conflicts = (); - while (my ($positiveid, $positive) = each %$positive_rules) { - my $positive_resources = $positive->{resources}; + my @disjoint_positive_rules = + PVE::HA::Rules::Helpers::find_disjoint_rules_resource_sets($positive_rules); + + for my $entry (@disjoint_positive_rules) { + my $positive_resources = $entry->{resources}; while (my ($negativeid, $negative) = each %$negative_rules) { my $common_resources = set_intersect($positive_resources, $negative->{resources}); next if %$common_resources < 2; - push @conflicts, [$positiveid, $negativeid]; + push @conflicts, ['negative', $negativeid]; + push @conflicts, ['positive', $_] for $entry->{ruleids}->@*; } } @@ -234,12 +238,15 @@ __PACKAGE__->register_check( my ($conflicts, $errors) = @_; for my $conflict (@$conflicts) { - my ($positiveid, $negativeid) = @$conflict; + my ($type, $ruleid) = @$conflict; - push $errors->{$positiveid}->{resources}->@*, - "rule shares two or more resources with a negative resource affinity rule"; - push $errors->{$negativeid}->{resources}->@*, - "rule shares two or more resources with a positive resource affinity rule"; + if ($type eq 'positive') { + push $errors->{$ruleid}->{resources}->@*, + "rule shares two or more resources with a negative resource affinity rule"; + } elsif ($type eq 'negative') { + push $errors->{$ruleid}->{resources}->@*, + "rule shares two or more resources with a positive resource affinity rule"; + } } }, ); @@ -248,58 +255,6 @@ __PACKAGE__->register_check( =cut -my $sort_by_lowest_resource_id = sub { - my ($rules) = @_; - - my $lowest_rule_resource_id = {}; - for my $ruleid (keys %$rules) { - my @rule_resources = sort keys $rules->{$ruleid}->{resources}->%*; - $lowest_rule_resource_id->{$ruleid} = $rule_resources[0]; - } - - # sort rules such that rules with the lowest numbered resource come first - my @sorted_ruleids = sort { - $lowest_rule_resource_id->{$a} cmp $lowest_rule_resource_id->{$b} - } sort keys %$rules; - - return @sorted_ruleids; -}; - -# returns a list of hashes, which contain disjoint resource affinity rules, i.e., -# put resource affinity constraints on disjoint sets of resources -my $find_disjoint_resource_affinity_rules = sub { - my ($rules) = @_; - - my @disjoint_rules = (); - - # order needed so that it is easier to check whether there is an overlap - my @sorted_ruleids = $sort_by_lowest_resource_id->($rules); - - for my $ruleid (@sorted_ruleids) { - my $rule = $rules->{$ruleid}; - - my $found = 0; - for my $entry (@disjoint_rules) { - next if sets_are_disjoint($rule->{resources}, $entry->{resources}); - - $found = 1; - push @{ $entry->{ruleids} }, $ruleid; - $entry->{resources}->{$_} = 1 for keys $rule->{resources}->%*; - - last; - } - if (!$found) { - push @disjoint_rules, - { - ruleids => [$ruleid], - resources => { $rule->{resources}->%* }, - }; - } - } - - return @disjoint_rules; -}; - =head3 merge_connected_positive_resource_affinity_rules($rules, $positive_rules) Modifies C<$rules> to contain only disjoint positive resource affinity rules diff --git a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg index a620e293..6bfc2dad 100644 --- a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg +++ b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg @@ -1,3 +1,4 @@ +# Case 1: Remove positive and negative resource affinity rules, which share two or more ha resources. resource-affinity: keep-apart1 resources vm:102,vm:103 affinity negative @@ -9,3 +10,17 @@ resource-affinity: keep-apart2 resource-affinity: stick-together1 resources vm:101,vm:102,vm:103,vm:104,vm:106 affinity positive + +# Case 2: Remove positive and negative resource affinity rules, which share two or more HA resources with the positive +# resource affinity being split in two. +resource-affinity: split-stick-together1 + resources vm:201,vm:202 + affinity positive + +resource-affinity: split-stick-together2 + resources vm:202,vm:203 + affinity positive + +resource-affinity: split-keep-apart1 + resources vm:201,vm:203 + affinity negative diff --git a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect index d4a2d7b2..8f2338d9 100644 --- a/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect +++ b/src/test/rules_cfgs/inconsistent-resource-affinity-rules.cfg.expect @@ -1,11 +1,14 @@ --- Log --- Drop rule 'keep-apart1', because rule shares two or more resources with a positive resource affinity rule. Drop rule 'keep-apart2', because rule shares two or more resources with a positive resource affinity rule. +Drop rule 'split-keep-apart1', because rule shares two or more resources with a positive resource affinity rule. +Drop rule 'split-stick-together1', because rule shares two or more resources with a negative resource affinity rule. +Drop rule 'split-stick-together2', because rule shares two or more resources with a negative resource affinity rule. Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule. Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule. --- Config --- $VAR1 = { - 'digest' => '50875b320034d8ac7dded185e590f5f87c4e2bb6', + 'digest' => '80cdc11a1d5bf2d1d500665af1210cd59aabede6', 'ids' => {}, 'order' => {} }; -- 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel