Re: [pve-devel] [RFC ha-manager v3 15/15] manager: persistently migrate ha groups to ha rules

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

Left 2 comments inline.

On 7/4/25 20:16, Daniel Kral wrote:
> 
> diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
> index 424a6e1..59bafd7 100644
> --- a/src/PVE/HA/Config.pm
> +++ b/src/PVE/HA/Config.pm
> @@ -234,6 +234,11 @@ sub read_group_config {
>      return cfs_read_file($ha_groups_config);
>  }
>  
> +sub delete_group_config {
> +
> +    unlink $ha_groups_config or die "failed to remove group config: $!\n";

This results in the following error:
	Abort HA group migration: failed to remove group config: No such file
or directory

The value in $ha_groups_config is only part of the path to the
groups.cfg file. It works for the remaining functions here because the
cfs_write_file() and cfs_read_file() functions append prepend "/etc/pve/".

Could be fixed by:
    my $group_config_file = "/etc/pve/" . $ha_groups_config;

    unlink $group_config_file or die "failed to remove group config: $!\n";

or something similar.

> +}
> +
>  sub write_group_config {
>      my ($cfg) = @_;
>  
> diff --git a/src/PVE/HA/Env.pm b/src/PVE/HA/Env.pm
> index 5cee7b3..1325676 100644
> --- a/src/PVE/HA/Env.pm
> +++ b/src/PVE/HA/Env.pm
> @@ -100,6 +100,12 @@ sub update_service_config {
>      return $self->{plug}->update_service_config($sid, $param);
>  }
>  
> +sub write_service_config {
> +    my ($self, $conf) = @_;
> +
> +    $self->{plug}->write_service_config($conf);
> +}
> +
>  sub parse_sid {
>      my ($self, $sid) = @_;
>  
> @@ -137,12 +143,24 @@ sub read_rules_config {
>      return $self->{plug}->read_rules_config();
>  }
>  
> +sub write_rules_config {
> +    my ($self, $rules) = @_;
> +
> +    $self->{plug}->write_rules_config($rules);
> +}
> +
>  sub read_group_config {
>      my ($self) = @_;
>  
>      return $self->{plug}->read_group_config();
>  }
>  
> +sub delete_group_config {
> +    my ($self) = @_;
> +
> +    $self->{plug}->delete_group_config();
> +}
> +
>  # this should return a hash containing info
>  # what nodes are members and online.
>  sub get_node_info {
> @@ -288,4 +306,10 @@ sub get_static_node_stats {
>      return $self->{plug}->get_static_node_stats();
>  }
>  
> +sub get_node_version {
> +    my ($self, $node) = @_;
> +
> +    return $self->{plug}->get_node_version($node);
> +}
> +
>  1;
> diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
> index 58fd36e..aecffc0 100644
> --- a/src/PVE/HA/Env/PVE2.pm
> +++ b/src/PVE/HA/Env/PVE2.pm
> @@ -141,6 +141,12 @@ sub update_service_config {
>      return PVE::HA::Config::update_resources_config($sid, $param);
>  }
>  
> +sub write_service_config {
> +    my ($self, $conf) = @_;
> +
> +    return PVE::HA::Config::write_resources_config($conf);
> +}
> +
>  sub parse_sid {
>      my ($self, $sid) = @_;
>  
> @@ -201,12 +207,24 @@ sub read_rules_config {
>      return PVE::HA::Config::read_and_check_rules_config();
>  }
>  
> +sub write_rules_config {
> +    my ($self, $rules) = @_;
> +
> +    PVE::HA::Config::write_rules_config($rules);
> +}
> +
>  sub read_group_config {
>      my ($self) = @_;
>  
>      return PVE::HA::Config::read_group_config();
>  }
>  
> +sub delete_group_config {
> +    my ($self) = @_;
> +
> +    PVE::HA::Config::delete_group_config();
> +}
> +
>  # this should return a hash containing info
>  # what nodes are members and online.
>  sub get_node_info {
> @@ -489,4 +507,14 @@ sub get_static_node_stats {
>      return $stats;
>  }
>  
> +sub get_node_version {
> +    my ($self, $node) = @_;
> +
> +    my $version_info = PVE::Cluster::get_node_kv('version-info', $node);
> +
> +    return undef if !$version_info->{$node};
> +
> +    return $version_info->{$node}->{version};

This throws an error, stopping the migration since it depends on all
nodes having the correct version, because the value in
$version_info->{$node} is a string. Could be fixed by:

    my $node_version_info = eval { decode_json($version_info->{$node}) };
    return $node_version_info->{version};

> +}
> +
>  1;
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 4357253..b2fd896 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -464,6 +464,97 @@ sub update_crm_commands {
>  
>  }
>


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