[pve-devel] [PATCH ha-manager v2 18/18] factor out counting of active services into helper

[Daniel Kral <d.kral@proxmox.com>]

Make the counting its own helper to be used throughout the code base to
have a single source of truth of what constitutes a active services.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
  - change new code to use `for` instead of `foreach`
  - changes from the previous two patches

 src/PVE/API2/HA/Status.pm | 17 ++---------------
 src/PVE/HA/LRM.pm         | 20 +-------------------
 src/PVE/HA/Manager.pm     | 14 +-------------
 src/PVE/HA/Tools.pm       | 25 +++++++++++++++++++++++++
 4 files changed, 29 insertions(+), 47 deletions(-)

diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
index 3966a4b3..282577cc 100644
--- a/src/PVE/API2/HA/Status.pm
+++ b/src/PVE/API2/HA/Status.pm
@@ -194,21 +194,8 @@ __PACKAGE__->register_method({
         }
 
         foreach my $node (sort keys %{ $status->{node_status} }) {
-            my $active_count = 0;
-            for my $sid (keys $status->{service_status}->%*) {
-                my $sd = $status->{service_status}->{$sid};
-                my $target = $sd->{target};
-                next
-                    if (!$sd->{node} || $sd->{node} ne $nodename)
-                    && (!$target || $target ne $nodename);
-                my $req_state = $sd->{state};
-                next if !defined($req_state);
-                next if $req_state eq 'stopped';
-                next if $req_state eq 'freeze';
-                next if $req_state eq 'error';
-                next if $req_state eq 'request_start';
-                $active_count++;
-            }
+            my $active_count =
+                PVE::HA::Tools::count_active_services($status->{service_status}, $node);
             my $lrm_status = PVE::HA::Config::read_lrm_status($node);
             my $id = "lrm:$node";
             if (!$lrm_status->{timestamp}) {
diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index b645e5b0..1342c8eb 100644
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -297,25 +297,7 @@ sub active_service_count {
 
     my $ss = $self->{service_status};
 
-    my $count = 0;
-    foreach my $sid (keys %$ss) {
-        my $sd = $ss->{$sid};
-        my $target = $sd->{target}; # count as active if we are the target.
-        next if (!$sd->{node} || $sd->{node} ne $nodename) && (!$target || $target ne $nodename);
-        my $req_state = $sd->{state};
-        next if !defined($req_state);
-        next if $req_state eq 'stopped';
-        # NOTE: 'ignored' ones are already dropped by the manager from service_status
-        next if $req_state eq 'freeze';
-        # erroneous services are not managed by HA, don't count them as active
-        next if $req_state eq 'error';
-        # request_start is for (optional) better node selection for stop -> started transition
-        next if $req_state eq 'request_start';
-
-        $count++;
-    }
-
-    return $count;
+    return PVE::HA::Tools::count_active_services($ss, $nodename);
 }
 
 my $wrote_lrm_status_at_startup = 0;
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 4cdddb55..e6f8dc25 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -541,19 +541,7 @@ my $have_groups_been_migrated = sub {
 my $is_lrm_active_or_idle = sub {
     my ($ss, $node, $lrm_state) = @_;
 
-    my $active_count = 0;
-    for my $sid (keys %$ss) {
-        my $sd = $ss->{$sid};
-        my $target = $sd->{target}; # count as active if we are the target.
-        next if (!$sd->{node} || $sd->{node} ne $node) && (!$target || $target ne $node);
-        my $req_state = $sd->{state};
-        next if !defined($req_state);
-        next if $req_state eq 'stopped';
-        next if $req_state eq 'freeze';
-        next if $req_state eq 'error';
-        next if $req_state eq 'request_start';
-        $active_count++;
-    }
+    my $active_count = PVE::HA::Tools::count_active_services($ss, $node);
 
     return 1 if $lrm_state eq 'active';
     return 1 if $lrm_state eq 'wait_for_agent_lock' && !$active_count;
diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
index 47b4aa03..1fa53df8 100644
--- a/src/PVE/HA/Tools.pm
+++ b/src/PVE/HA/Tools.pm
@@ -188,6 +188,31 @@ sub count_fenced_services {
     return $count;
 }
 
+sub count_active_services {
+    my ($ss, $node) = @_;
+
+    my $active_count = 0;
+
+    for my $sid (keys %$ss) {
+        my $sd = $ss->{$sid};
+        my $target = $sd->{target}; # count as active if we are the target.
+        next if (!$sd->{node} || $sd->{node} ne $node) && (!$target || $target ne $node);
+        my $req_state = $sd->{state};
+        next if !defined($req_state);
+        next if $req_state eq 'stopped';
+        # NOTE: 'ignored' ones are already dropped by the manager from service_status
+        next if $req_state eq 'freeze';
+        # erroneous services are not managed by HA, don't count them as active
+        next if $req_state eq 'error';
+        # request_start is for (optional) better node selection for stop -> started transition
+        next if $req_state eq 'request_start';
+
+        $active_count++;
+    }
+
+    return $active_count;
+}
+
 sub get_verbose_service_state {
     my ($service_state, $service_conf) = @_;
 
-- 
2.47.3



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