[pve-devel] [PATCH ha-manager v3 01/15] tree-wide: make arguments for select_service_node explicit

From: Daniel Kral <d.kral@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH ha-manager v3 01/15] tree-wide: make arguments for select_service_node explicit
Date: Fri,  4 Jul 2025 20:16:41 +0200	[thread overview]
Message-ID: <20250704181659.465441-3-d.kral@proxmox.com> (raw)
In-Reply-To: <20250704181659.465441-1-d.kral@proxmox.com>

Explicitly state all the parameters at all call sites for
select_service_node(...) to clarify in which states these are.

The call site in next_state_recovery(...) sets $best_scored to 1, as it
should find the next best node when recovering from the failed node
$current_node. All references to $best_scored in select_service_node()
are there to check whether $current_node can be selected, but as
$current_node is not available anyway, so this change should not change
the result of select_service_node(...).

Otherwise, $sd->{failed_nodes} and $sd->{maintenance_node} should
contain only the failed $current_node in next_state_recovery(...), and
therefore both can be passed as these should be impossible states here
anyway. A cleaner way could be to explicitly remove them beforehand or
do extra checks in select_service_node(...).

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 src/PVE/HA/Manager.pm      | 11 ++++++++++-
 src/test/test_failover1.pl | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 12292e6..85f2b1a 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -971,6 +971,7 @@ sub next_state_started {
                 $try_next,
                 $sd->{failed_nodes},
                 $sd->{maintenance_node},
+                0, # best_score
             );
 
             if ($node && ($sd->{node} ne $node)) {
@@ -1083,7 +1084,15 @@ sub next_state_recovery {
     $self->recompute_online_node_usage(); # we want the most current node state
 
     my $recovery_node = select_service_node(
-        $self->{groups}, $self->{online_node_usage}, $sid, $cd, $sd->{node},
+        $self->{groups},
+        $self->{online_node_usage},
+        $sid,
+        $cd,
+        $sd->{node},
+        0, # try_next
+        $sd->{failed_nodes},
+        $sd->{maintenance_node},
+        1, # best_score
     );
 
     if ($recovery_node) {
diff --git a/src/test/test_failover1.pl b/src/test/test_failover1.pl
index 371bdcf..2478b2b 100755
--- a/src/test/test_failover1.pl
+++ b/src/test/test_failover1.pl
@@ -24,13 +24,26 @@ my $service_conf = {
     group => 'prefer_node1',
 };
 
+my $sd = {
+    failed_nodes => undef,
+    maintenance_node => undef,
+};
+
 my $current_node = $service_conf->{node};
 
 sub test {
     my ($expected_node, $try_next) = @_;
 
     my $node = PVE::HA::Manager::select_service_node(
-        $groups, $online_node_usage, "vm:111", $service_conf, $current_node, $try_next,
+        $groups,
+        $online_node_usage,
+        "vm:111",
+        $service_conf,
+        $current_node,
+        $try_next,
+        $sd->{failed_nodes},
+        $sd->{maintenance_node},
+        0, # best_score
     );
 
     my (undef, undef, $line) = caller();
-- 
2.39.5



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

