Re: [pve-devel] [PATCH ha-manager v3 10/15] manager: migrate ha groups to node affinity rules in-memory

["Michael Köppl" <m.koeppl@proxmox.com>]

Left 2 comments inline, just some minor nits.

On 7/4/25 20:16, Daniel Kral wrote:
> Migrate the currently configured groups to node affinity rules
> in-memory, so that they can be applied as such in the next patches and
> therefore replace HA groups internally.
> 
> HA node affinity rules in their initial implementation are designed to
> be as restrictive as HA groups, i.e. only allow a HA resource to be used
> in a single node affinity rule, to ease the migration between them.
> 
> HA groups map directly to node affinity rules, except that the
> 'restricted' property is renamed to 'strict' and that the 'failback'
> property is moved to the HA resources config.
> 
> The 'nofailback' property is moved to the HA resources config, because
> it allows users to set it more granularly for individual HA resources
> and allows the node affinity rules to be more extendible in the future,
> e.g. multiple node affinity rules for a single HA resource.
> 
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
>  src/PVE/HA/Config.pm  |  3 ++-
>  src/PVE/HA/Groups.pm  | 48 +++++++++++++++++++++++++++++++++++++++++++
>  src/PVE/HA/Manager.pm | 18 ++++++++++++++--
>  3 files changed, 66 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
> index 7d071f3..424a6e1 100644
> --- a/src/PVE/HA/Config.pm
> +++ b/src/PVE/HA/Config.pm
> @@ -131,7 +131,8 @@ sub read_and_check_resources_config {
>          }
>      }
>  
> -    return $conf;
> +    # TODO PVE 10: Remove digest when HA groups have been fully migrated to rules
> +    return wantarray ? ($conf, $res->{digest}) : $conf;
>  }
>  
>  sub update_resources_config {
> diff --git a/src/PVE/HA/Groups.pm b/src/PVE/HA/Groups.pm
> index 821d969..f065732 100644
> --- a/src/PVE/HA/Groups.pm
> +++ b/src/PVE/HA/Groups.pm
> @@ -6,6 +6,7 @@ use warnings;
>  use PVE::JSONSchema qw(get_standard_option);
>  use PVE::SectionConfig;
>  use PVE::HA::Tools;
> +use PVE::HA::Rules;
>  
>  use base qw(PVE::SectionConfig);
>  
> @@ -107,4 +108,51 @@ sub parse_section_header {
>  __PACKAGE__->register();
>  __PACKAGE__->init();
>  
> +# Migrate nofailback flag from $groups to $resources

nit: don't know if this comment is really necessary

> +sub migrate_groups_to_resources {
> +    my ($groups, $resources) = @_;
> +
> +    for my $sid (keys %$resources) {
> +        my $groupid = $resources->{$sid}->{group}
> +            or next; # skip resources without groups
> +
> +        $resources->{$sid}->{failback} = !$groups->{ids}->{$groupid}->{nofailback};
> +    }
> +}
> +
> +# Migrate groups from groups from $groups and $resources to node affinity rules in $rules

nit: same here. I also assume this should be "Migrate groups from
$groups and $resources..."?

> +sub migrate_groups_to_rules {
> +    my ($rules, $groups, $resources) = @_;
> +
> +    my $group_resources = {};
> +
> +    for my $sid (keys %$resources) {
> +        my $groupid = $resources->{$sid}->{group}
> +            or next; # skip resources without groups
> +
> +        $group_resources->{$groupid}->{$sid} = 1;
> +    }
> +
> +    while (my ($group, $resources) = each %$group_resources) {
> +        next if !$groups->{ids}->{$group}; # skip non-existant groups
> +
> +        my $new_ruleid = "ha-group-$group";
> +        my $nodes = {};
> +        for my $entry (keys $groups->{ids}->{$group}->{nodes}->%*) {
> +            my ($node, $priority) = PVE::HA::Tools::parse_node_priority($entry);
> +
> +            $nodes->{$node} = { priority => $priority };
> +        }
> +
> +        $rules->{ids}->{$new_ruleid} = {
> +            type => 'node-affinity',
> +            resources => $resources,
> +            nodes => $nodes,
> +            strict => $groups->{ids}->{$group}->{restricted},
> +            comment => "Generated from HA group '$group'.",
> +        };
> +        $rules->{order}->{$new_ruleid} = PVE::HA::Rules::get_next_ordinal($rules);
> +    }
> +}
> +
>  1;
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 88ff4a6..148447d 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -6,6 +6,7 @@ use warnings;
>  use Digest::MD5 qw(md5_base64);
>  
>  use PVE::Tools;
> +use PVE::HA::Groups;
>  use PVE::HA::Tools ':exit_codes';
>  use PVE::HA::NodeStatus;
>  use PVE::HA::Rules;
> @@ -47,6 +48,8 @@ sub new {
>          haenv => $haenv,
>          crs => {},
>          last_rules_digest => '',
> +        last_groups_digest => '',
> +        last_services_digest => '',
>      }, $class;
>  
>      my $old_ms = $haenv->read_manager_status();
> @@ -529,7 +532,7 @@ sub manage {
>  
>      $self->update_crs_scheduler_mode();
>  
> -    my $sc = $haenv->read_service_config();
> +    my ($sc, $services_digest) = $haenv->read_service_config();
>  
>      $self->{groups} = $haenv->read_group_config(); # update
>  
> @@ -564,7 +567,16 @@ sub manage {
>  
>      my $new_rules = $haenv->read_rules_config();
>  
> -    if ($new_rules->{digest} ne $self->{last_rules_digest}) {
> +    # TODO PVE 10: Remove group migration when HA groups have been fully migrated to rules
> +    PVE::HA::Groups::migrate_groups_to_resources($self->{groups}, $sc);
> +
> +    if (
> +        !$self->{rules}
> +        || $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}
> +    ) {
> +        PVE::HA::Groups::migrate_groups_to_rules($new_rules, $self->{groups}, $sc);
>  
>          my $messages = PVE::HA::Rules->canonicalize($new_rules);
>          $haenv->log('info', $_) for @$messages;
> @@ -572,6 +584,8 @@ sub manage {
>          $self->{rules} = $new_rules;
>  
>          $self->{last_rules_digest} = $self->{rules}->{digest};
> +        $self->{last_groups_digest} = $self->{groups}->{digest};
> +        $self->{last_services_digest} = $services_digest;
>      }
>  
>      $self->update_crm_commands();



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel