[pve-devel] [PATCH ha-manager v2 06/26] rules: add global checks between location and colocation rules

[Daniel Kral]

On 6/20/25 16:31, Daniel Kral wrote:
> +=head3 check_positive_colocation_location_consistency($positive_rules, $location_rules)
> +
> +Returns a list of positive colocation rule ids defined in C<$positive_rules>,
> +where the services in the positive colocation rule are restricted to a disjoint
> +set of nodes by their location rules, defined in C<$location_rules>. That is,
> +the positive colocation rule cannot be fullfilled as the services cannot be
> +placed on the same node.
> +
> +If there are none, the returned list is empty.
> +
> +=cut
> +
> +sub check_positive_colocation_location_consistency {
> +    my ($positive_rules, $location_rules) = @_;
> +
> +    my @errors = ();
> +
> +    while (my ($positiveid, $positive_rule) = each %$positive_rules) {
> +        my $allowed_nodes;
> +        my $services = $positive_rule->{services};
> +
> +        for my $locationid (keys %$location_rules) {
> +            my $location_rule = $location_rules->{$locationid};
> +
> +            next if !$location_rule->{strict};

The "strict" requirement will be removed in a v3.

Service affinity (colocation rules) is determined after node affinity 
(location rules). That is, service affinity selects from the nodes, 
which are in the highest priority node group determined by the node 
affinity rules. Even if a service is in a non-strict node affinity rule, 
the service affinity can only select one of the highest priority nodes, 
no matter if it is strict or non-strict...

I also realized now that it is still in question what positive service 
affinity rules should be allowed if one (or more) of their services are 
also in a node affinity rule.

Consider the case where vm:101, vm:102 and vm:103 must be kept on the 
same node and only one is in a node affinity rule restricting vm:101 to 
only node1. What does that node affinity rule state? Should that rule 
infer that vm:102 and vm:103 are also in the node affinity rule now 
(rather implicit for the user)?

I'd rather make these combinations invalid and remind the user that all 
services should be put in the node affinity rule first with the same 
node selection and then they can create the service affinity rule, but 
feedback on that would be much appreciated.

> +            next if PVE::HashTools::sets_are_disjoint($services, $location_rule->{services});
> +
> +            $allowed_nodes = { $location_rule->{nodes}->%* } if !defined($allowed_nodes);
> +            $allowed_nodes = PVE::HashTools::set_intersect($allowed_nodes, $location_rule->{nodes});
> +
> +            if (keys %$allowed_nodes < 1) {
> +                push @errors, $positiveid;
> +                last; # early return to check next positive colocation rule
> +            }
> +        }
> +    }
> +
> +    @errors = sort @errors;
> +    return \@errors;
> +}
> +
> +__PACKAGE__->register_check(
> +    sub {
> +        my ($args) = @_;
> +
> +        return check_positive_colocation_location_consistency(
> +            $args->{positive_rules},
> +            $args->{location_rules},
> +        );
> +    },
> +    sub {
> +        my ($ruleids, $errors) = @_;
> +
> +        for my $ruleid (@$ruleids) {
> +            push @{ $errors->{$ruleid}->{services} },
> +                "two or more services are restricted to different nodes";
> +        }
> +    },
> +);
> +
> +=head3 check_negative_colocation_location_consistency($negative_rules, $location_rules)
> +
> +Returns a list of negative colocation rule ids defined in C<$negative_rules>,
> +where the services in the negative colocation rule are restricted to less nodes
> +than needed to keep them separate by their location rules, defined in
> +C<$location_rules>. That is, the negative colocation rule cannot be fullfilled
> +as there are not enough nodes to spread the services on.
> +
> +If there are none, the returned list is empty.
> +
> +=cut
> +
> +sub check_negative_colocation_location_consistency {
> +    my ($negative_rules, $location_rules) = @_;
> +
> +    my @errors = ();
> +
> +    while (my ($negativeid, $negative_rule) = each %$negative_rules) {
> +        my $allowed_nodes = {};
> +        my $located_services;
> +        my $services = $negative_rule->{services};
> +
> +        for my $locationid (keys %$location_rules) {
> +            my $location_rule = $location_rules->{$locationid};
> +
> +            my $location_services = $location_rule->{services};
> +            my $common_services = PVE::HashTools::set_intersect($services, $location_services);
> +
> +            next if !$location_rule->{strict};

Same argument as above regarding the strictness.

> +            next if keys %$common_services < 1;
> +
> +            $located_services = PVE::HashTools::set_union($located_services, $common_services);
> +            $allowed_nodes = PVE::HashTools::set_union($allowed_nodes, $location_rule->{nodes});
> +
> +            if (keys %$allowed_nodes < keys %$located_services) {
> +                push @errors, $negativeid;
> +                last; # early return to check next negative colocation rule
> +            }
> +        }
> +    }
> +
> +    @errors = sort @errors;
> +    return \@errors;
> +}