[pve-devel] [PATCH ha-manager v2 5/8] manager: make recompute_online_node_usage use add_service_usage helper

[Daniel Kral]

As the previously introduced get_used_service_nodes(...) helper reflects
the same logic as in recompute_online_node_usage(...), introduce a new
helper add_service_usage(...), which adds service usage according to the
nodes returned by get_used_service_nodes(...).

This helper will also be used by a later patch, which introduces
granular changes to $online_node_usage in change_service_state(...).

Signed-off-by: Daniel Kral <d.kral at proxmox.com>
---
changes since v1:
 - introduce add_service_usage(...) instead of using
   get_used_service_nodes(...) and add_service_usage_to_node(...)
   directly
 - this makes $online_nodes an array again instead of creating a hash
 - reverted back the original foreach my ...
 - did not add R-b from Fiona as it wasn't discussed

 src/PVE/HA/Manager.pm | 40 ++--------------------------------------
 src/PVE/HA/Usage.pm   | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index a71de167..bf6895ad 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -279,44 +279,8 @@ sub recompute_online_node_usage {
 
     foreach my $sid (sort keys %{ $self->{ss} }) {
         my $sd = $self->{ss}->{$sid};
-        my $state = $sd->{state};
-        my $target = $sd->{target}; # optional
-        if ($online_node_usage->contains_node($sd->{node})) {
-            if (
-                $state eq 'started'
-                || $state eq 'request_stop'
-                || $state eq 'fence'
-                || $state eq 'freeze'
-                || $state eq 'error'
-                || $state eq 'recovery'
-            ) {
-                $online_node_usage->add_service_usage_to_node($sd->{node}, $sid, $sd->{node});
-            } elsif (
-                $state eq 'migrate'
-                || $state eq 'relocate'
-                || $state eq 'request_start_balance'
-            ) {
-                my $source = $sd->{node};
-                # count it for both, source and target as load is put on both
-                if ($state ne 'request_start_balance') {
-                    $online_node_usage->add_service_usage_to_node($source, $sid, $source, $target);
-                }
-                if ($online_node_usage->contains_node($target)) {
-                    $online_node_usage->add_service_usage_to_node($target, $sid, $source, $target);
-                }
-            } elsif ($state eq 'stopped' || $state eq 'request_start') {
-                # do nothing
-            } else {
-                die "should not be reached (sid = '$sid', state = '$state')";
-            }
-        } elsif (defined($target) && $online_node_usage->contains_node($target)) {
-            if ($state eq 'migrate' || $state eq 'relocate') {
-                # to correctly track maintenance modi and also consider the target as used for the
-                # case a node dies, as we cannot really know if the to-be-aborted incoming migration
-                # has already cleaned up all used resources
-                $online_node_usage->add_service_usage_to_node($target, $sid, $sd->{node}, $target);
-            }
-        }
+
+        $online_node_usage->add_service_usage($sid, $sd->{state}, $sd->{node}, $sd->{target});
     }
 
     $self->{online_node_usage} = $online_node_usage;
diff --git a/src/PVE/HA/Usage.pm b/src/PVE/HA/Usage.pm
index edea2545..e3725c92 100644
--- a/src/PVE/HA/Usage.pm
+++ b/src/PVE/HA/Usage.pm
@@ -40,6 +40,21 @@ sub add_service_usage_to_node {
     die "implement in subclass";
 }
 
+# Adds service $sid's usage to the online nodes according to their $state,
+# $service_node and $migration_target.
+sub add_service_usage {
+    my ($self, $sid, $service_state, $service_node, $migration_target) = @_;
+
+    my $online_nodes = { map { $_ => 1 } $self->list_nodes() };
+    my ($current_node, $target_node) =
+        get_used_service_nodes($online_nodes, $service_state, $service_node, $migration_target);
+
+    $self->add_service_usage_to_node($current_node, $sid, $service_node, $migration_target)
+        if $current_node;
+    $self->add_service_usage_to_node($target_node, $sid, $service_node, $migration_target)
+        if $target_node;
+}
+
 # Returns a hash with $nodename => $score pairs. A lower $score is better.
 sub score_nodes_to_start_service {
     my ($self, $sid, $service_node) = @_;
-- 
2.47.3