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.k...@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
