[pve-devel] [PATCH ha-manager v2 02/18] manager: retranslate rules if nodes are added or removed

[Daniel Kral]

On Tue Sep 9, 2025 at 10:33 AM CEST, Daniel Kral wrote:
> 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 at proxmox.com>
> Reviewed-by: Michael Köppl <m.koeppl at proxmox.com>
> Tested-by: Michael Köppl <m.koeppl at proxmox.com>
> ---
> changes since v1:
>   - remove `sort` as it was not needed
>
>  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 ba59f642..92a2c05e 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -692,6 +692,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);
> @@ -745,6 +746,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 0d04cd53..95cb18a8 100644
> --- a/src/PVE/HA/NodeStatus.pm
> +++ b/src/PVE/HA/NodeStatus.pm
> @@ -92,6 +92,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;
> +}

As I'm working on making the HA scheduler updates more granular right
now, this might change again in that series:

When writing this patch I was inclined to return whether something
changed from PVE::HA::NodeStatus::update directly, but as I didn't want
to introduce any return codes there, I made this its own helper.

For the granular scheduler updates the other approach now makes more
sense, returning the changes from PVE::HA::NodeStatus::update, so that
the HA Manager can update the scheduler accordingly if nodes went online
or in other states..

So this patch should be fine as it is, but just wanted to give a
heads-up. If that series gets on the list before this is applied, I'll
send a v3 making that change here already.