[pve-devel] [PATCH ha-manager v2 06/12] config, manager: factor out resource motion info logic

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

The logic to gather dependent and blocking HA resources in
execute_migration(...) and get_resource_motion_info(...) is equivalent,
so factor them out as a separate helper.

Introduce PVE::HA::Helpers as a module to share code between modules,
which cannot depend on each other but use the same underlying data
structures (e.g. Manager and Config, LRM and CRM) and where
PVE::HA::Tools is not the right place.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes v1 -> v2: none

 debian/pve-ha-manager.install |  1 +
 src/PVE/HA/Config.pm          | 31 ++++-----------------
 src/PVE/HA/Helpers.pm         | 52 +++++++++++++++++++++++++++++++++++
 src/PVE/HA/Makefile           |  1 +
 src/PVE/HA/Manager.pm         | 45 +++++++++++++-----------------
 5 files changed, 78 insertions(+), 52 deletions(-)
 create mode 100644 src/PVE/HA/Helpers.pm

diff --git a/debian/pve-ha-manager.install b/debian/pve-ha-manager.install
index bdb1feeb..6ee0ee5d 100644
--- a/debian/pve-ha-manager.install
+++ b/debian/pve-ha-manager.install
@@ -27,6 +27,7 @@
 /usr/share/perl5/PVE/HA/FenceConfig.pm
 /usr/share/perl5/PVE/HA/Groups.pm
 /usr/share/perl5/PVE/HA/HashTools.pm
+/usr/share/perl5/PVE/HA/Helpers.pm
 /usr/share/perl5/PVE/HA/LRM.pm
 /usr/share/perl5/PVE/HA/Manager.pm
 /usr/share/perl5/PVE/HA/NodeStatus.pm
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 3a4b7019..c85ec994 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -8,9 +8,9 @@ use JSON;
 use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
 
 use PVE::HA::Groups;
+use PVE::HA::Helpers;
 use PVE::HA::Resources;
 use PVE::HA::Rules;
-use PVE::HA::Rules::ResourceAffinity qw(get_affinitive_resources);
 use PVE::HA::Tools;
 
 my $manager_status_filename = "ha/manager_status";
@@ -402,34 +402,13 @@ sub get_resource_motion_info {
         my $manager_status = read_manager_status();
         my $ss = $manager_status->{service_status};
         my $ns = $manager_status->{node_status};
+        # get_resource_motion_info expects a hashset of all nodes with status 'online'
+        my $online_nodes = { map { $ns->{$_} eq 'online' ? ($_ => 1) : () } keys %$ns };
 
         my $compiled_rules = read_and_compile_rules_config();
-        my $resource_affinity = $compiled_rules->{'resource-affinity'};
-        my ($together, $separate) = get_affinitive_resources($resource_affinity, $sid);
 
-        for my $csid (sort keys %$together) {
-            next if !defined($ss->{$csid});
-            next if $ss->{$csid}->{state} eq 'ignored';
-
-            push @$dependent_resources, $csid;
-        }
-
-        for my $node (keys %$ns) {
-            next if $ns->{$node} ne 'online';
-
-            for my $csid (sort keys %$separate) {
-                next if !defined($ss->{$csid});
-                next if $ss->{$csid}->{state} eq 'ignored';
-                next if $ss->{$csid}->{node} && $ss->{$csid}->{node} ne $node;
-                next if $ss->{$csid}->{target} && $ss->{$csid}->{target} ne $node;
-
-                push $blocking_resources_by_node->{$node}->@*,
-                    {
-                        sid => $csid,
-                        cause => 'resource-affinity',
-                    };
-            }
-        }
+        ($dependent_resources, $blocking_resources_by_node) =
+            PVE::HA::Helpers::get_resource_motion_info($ss, $sid, $online_nodes, $compiled_rules);
     }
 
     return ($dependent_resources, $blocking_resources_by_node);
diff --git a/src/PVE/HA/Helpers.pm b/src/PVE/HA/Helpers.pm
new file mode 100644
index 00000000..09300cd4
--- /dev/null
+++ b/src/PVE/HA/Helpers.pm
@@ -0,0 +1,52 @@
+package PVE::HA::Helpers;
+
+use v5.36;
+
+use PVE::HA::Rules::ResourceAffinity qw(get_affinitive_resources);
+
+=head3 get_resource_motion_info
+
+Gathers which other HA resources in C<$ss> put a node placement dependency or
+node placement restriction on C<$sid> according to the compiled rules in
+C<$compiled_rules> and the online nodes in C<$online_nodes>.
+
+Returns a list of two elements, where the first element is a list of HA resource
+ids which are dependent on the node placement of C<$sid>, and the second element
+is a hash of nodes blocked for C<$sid>, where each entry value is a list of the
+causes that make the node unavailable to C<$sid>.
+
+=cut
+
+sub get_resource_motion_info($ss, $sid, $online_nodes, $compiled_rules) {
+    my $dependent_resources = [];
+    my $blocking_resources_by_node = {};
+
+    my $resource_affinity = $compiled_rules->{'resource-affinity'};
+    my ($together, $separate) = get_affinitive_resources($resource_affinity, $sid);
+
+    for my $csid (sort keys %$together) {
+        next if !defined($ss->{$csid});
+        next if $ss->{$csid}->{state} eq 'ignored';
+
+        push @$dependent_resources, $csid;
+    }
+
+    for my $node (keys %$online_nodes) {
+        for my $csid (sort keys %$separate) {
+            next if !defined($ss->{$csid});
+            next if $ss->{$csid}->{state} eq 'ignored';
+            next if $ss->{$csid}->{node} && $ss->{$csid}->{node} ne $node;
+            next if $ss->{$csid}->{target} && $ss->{$csid}->{target} ne $node;
+
+            push $blocking_resources_by_node->{$node}->@*,
+                {
+                    sid => $csid,
+                    cause => 'resource-affinity',
+                };
+        }
+    }
+
+    return ($dependent_resources, $blocking_resources_by_node);
+}
+
+1;
diff --git a/src/PVE/HA/Makefile b/src/PVE/HA/Makefile
index 1aeb976b..57871b29 100644
--- a/src/PVE/HA/Makefile
+++ b/src/PVE/HA/Makefile
@@ -2,6 +2,7 @@ SIM_SOURCES=CRM.pm \
 	    Env.pm \
 	    Groups.pm \
 	    HashTools.pm \
+	    Helpers.pm \
 	    Rules.pm \
 	    Resources.pm \
 	    LRM.pm \
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index a0016a3f..4e15b401 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -8,11 +8,12 @@ use Digest::MD5 qw(md5_base64);
 use PVE::Tools;
 
 use PVE::HA::Groups;
+use PVE::HA::Helpers;
 use PVE::HA::NodeStatus;
 use PVE::HA::Rules;
 use PVE::HA::Rules::NodeAffinity qw(get_node_affinity);
 use PVE::HA::Rules::ResourceAffinity
-    qw(get_affinitive_resources get_resource_affinity apply_positive_resource_affinity apply_negative_resource_affinity);
+    qw(get_resource_affinity apply_positive_resource_affinity apply_negative_resource_affinity);
 use PVE::HA::Tools ':exit_codes';
 use PVE::HA::Usage::Basic;
 
@@ -388,43 +389,35 @@ sub read_lrm_status {
 sub execute_migration {
     my ($self, $cmd, $task, $sid, $target) = @_;
 
-    my ($haenv, $ss) = $self->@{qw(haenv ss)};
+    my ($haenv, $ss, $ns, $compiled_rules) = $self->@{qw(haenv ss ns compiled_rules)};
+    my $online_nodes = { map { $_ => 1 } $self->{ns}->list_online_nodes()->@* };
 
-    my $resource_affinity = $self->{compiled_rules}->{'resource-affinity'};
-    my ($together, $separate) = get_affinitive_resources($resource_affinity, $sid);
+    my ($dependent_resources, $blocking_resources_by_node) =
+        PVE::HA::Helpers::get_resource_motion_info($ss, $sid, $online_nodes, $compiled_rules);
 
-    my $blocked_from_migration;
-    for my $csid (sort keys %$separate) {
-        next if !defined($ss->{$csid});
-        next if $ss->{$csid}->{state} eq 'ignored';
-        next if $ss->{$csid}->{node} && $ss->{$csid}->{node} ne $target;
-        next if $ss->{$csid}->{target} && $ss->{$csid}->{target} ne $target;
+    if (my $blocking_resources = $blocking_resources_by_node->{$target}) {
+        for my $blocking_resource (@$blocking_resources) {
+            my $err_msg = "unknown migration blocker reason";
+            my ($csid, $cause) = $blocking_resource->@{qw(sid cause)};
 
-        $haenv->log(
-            'err',
-            "crm command '$cmd' error - service '$csid' on node '$target' in"
-                . " negative affinity with service '$sid'",
-        );
+            if ($cause eq 'resource-affinity') {
+                $err_msg = "service '$csid' on node '$target' in negative"
+                    . " affinity with service '$sid'";
+            }
 
-        $blocked_from_migration = 1;
+            $haenv->log('err', "crm command '$cmd' error - $err_msg");
+        }
+
+        return; # do not queue migration if there are blockers
     }
 
-    return if $blocked_from_migration;
-
     $haenv->log('info', "got crm command: $cmd");
     $ss->{$sid}->{cmd} = [$task, $target];
 
-    my $resources_to_migrate = [];
-    for my $csid (sort keys %$together) {
-        next if !defined($ss->{$csid});
-        next if $ss->{$csid}->{state} eq 'ignored';
+    for my $csid (@$dependent_resources) {
         next if $ss->{$csid}->{node} && $ss->{$csid}->{node} eq $target;
         next if $ss->{$csid}->{target} && $ss->{$csid}->{target} eq $target;
 
-        push @$resources_to_migrate, $csid;
-    }
-
-    for my $csid (@$resources_to_migrate) {
         $haenv->log(
             'info',
             "crm command '$cmd' - $task service '$csid' to node '$target'"
-- 
2.47.3



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