[PATCH ha-manager v2 5/7] manager: make HA resource bundles move back to maintenance node

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

HA resources in HA resource bundles always prefer their current, common
node as soon as at least one of their HA resources is actively assigned
to a node already. This is done by apply_positive_resource_affinity(),
which will reduce the node set to only this current, common node.

However, except the HA resource bundles' maintenance node is their
current, common node (e.g. no replacement node could be found), these
will never fallback to the maintenance node if this reduction is made
before the fallback logic.

Therefore, move the maintenance fallback before
apply_positive_resource_affinity() to make HA resources with the
maintenance node set fallback to their maintenance node, which is likely
the maintenance node of the other HA resources in the HA resource bundle
as well.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- move patch up

 src/PVE/HA/Manager.pm                         |  4 ++--
 .../README                                    |  3 ++-
 .../log.expect                                | 16 +++++++++++++
 .../README                                    |  3 ++-
 .../log.expect                                | 23 +++++++++++++++++++
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index c38b1ddd..3c768a86 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -388,11 +388,11 @@ sub select_service_node {
         }
     }
 
-    apply_positive_resource_affinity($together, $pri_nodes);
-
     return $maintenance_fallback
         if defined($maintenance_fallback) && $pri_nodes->{$maintenance_fallback};
 
+    apply_positive_resource_affinity($together, $pri_nodes);
+
     return $current_node if $node_preference eq 'none' && $pri_nodes->{$current_node};
 
     my $scores = $online_node_usage->score_nodes_to_start_service($sid, $current_node);
diff --git a/src/test/test-resource-affinity-maintenance-strict-positive1/README b/src/test/test-resource-affinity-maintenance-strict-positive1/README
index 4b62e578..ab293cc5 100644
--- a/src/test/test-resource-affinity-maintenance-strict-positive1/README
+++ b/src/test/test-resource-affinity-maintenance-strict-positive1/README
@@ -1,3 +1,4 @@
 Tests whether a strict positive resource affinity rule among two HA resources
 makes both HA resources move to the same replacement node in case their
-current, common node is put in maintenance mode.
+current, common node is put in maintenance mode and moves them back as the
+previous maintenance node is available again.
diff --git a/src/test/test-resource-affinity-maintenance-strict-positive1/log.expect b/src/test/test-resource-affinity-maintenance-strict-positive1/log.expect
index 5f91b877..91637279 100644
--- a/src/test/test-resource-affinity-maintenance-strict-positive1/log.expect
+++ b/src/test/test-resource-affinity-maintenance-strict-positive1/log.expect
@@ -48,4 +48,20 @@ info    220      cmdlist: execute crm node3 disable-node-maintenance
 info    225    node3/lrm: got lock 'ha_agent_node3_lock'
 info    225    node3/lrm: status change maintenance => active
 info    240    node1/crm: node 'node3': state changed from 'maintenance' => 'online'
+info    240    node1/crm: moving service 'vm:101' back to 'node3', node came back from maintenance.
+info    240    node1/crm: migrate service 'vm:101' to node 'node3' (running)
+info    240    node1/crm: service 'vm:101': state changed from 'started' to 'migrate'  (node = node1, target = node3)
+info    240    node1/crm: moving service 'vm:102' back to 'node3', node came back from maintenance.
+info    240    node1/crm: migrate service 'vm:102' to node 'node3' (running)
+info    240    node1/crm: service 'vm:102': state changed from 'started' to 'migrate'  (node = node1, target = node3)
+info    241    node1/lrm: service vm:101 - start migrate to node 'node3'
+info    241    node1/lrm: service vm:101 - end migrate to node 'node3'
+info    241    node1/lrm: service vm:102 - start migrate to node 'node3'
+info    241    node1/lrm: service vm:102 - end migrate to node 'node3'
+info    260    node1/crm: service 'vm:101': state changed from 'migrate' to 'started'  (node = node3)
+info    260    node1/crm: service 'vm:102': state changed from 'migrate' to 'started'  (node = node3)
+info    265    node3/lrm: starting service vm:101
+info    265    node3/lrm: service status vm:101 started
+info    265    node3/lrm: starting service vm:102
+info    265    node3/lrm: service status vm:102 started
 info    820     hardware: exit simulation - done
diff --git a/src/test/test-resource-affinity-maintenance-strict-positive2/README b/src/test/test-resource-affinity-maintenance-strict-positive2/README
index 32f0942b..dcc4c81d 100644
--- a/src/test/test-resource-affinity-maintenance-strict-positive2/README
+++ b/src/test/test-resource-affinity-maintenance-strict-positive2/README
@@ -2,7 +2,8 @@ Tests whether a strict positive resource affinity rule among three HA
 resources, where two of them are already on a common node but the other HA
 resource is still on another node, makes the former two HA resources move to
 the node of the other HA resource as their current common node is put in
-maintenance mode.
+maintenance mode and moves them back as soon as the previous maintenance node
+is available again.
 
 The "skip-round crm 1" command ensures that the HA Manager will not move the
 dislocated, third HA resource to the common node, but make the LRM acknowledge
diff --git a/src/test/test-resource-affinity-maintenance-strict-positive2/log.expect b/src/test/test-resource-affinity-maintenance-strict-positive2/log.expect
index ef63c8ca..9da6d968 100644
--- a/src/test/test-resource-affinity-maintenance-strict-positive2/log.expect
+++ b/src/test/test-resource-affinity-maintenance-strict-positive2/log.expect
@@ -43,4 +43,27 @@ info    120      cmdlist: execute crm node1 disable-node-maintenance
 info    121    node1/lrm: got lock 'ha_agent_node1_lock'
 info    121    node1/lrm: status change maintenance => active
 info    140    node1/crm: node 'node1': state changed from 'maintenance' => 'online'
+info    140    node1/crm: moving service 'vm:101' back to 'node1', node came back from maintenance.
+info    140    node1/crm: migrate service 'vm:101' to node 'node1' (running)
+info    140    node1/crm: service 'vm:101': state changed from 'started' to 'migrate'  (node = node3, target = node1)
+info    140    node1/crm: moving service 'vm:102' back to 'node1', node came back from maintenance.
+info    140    node1/crm: migrate service 'vm:102' to node 'node1' (running)
+info    140    node1/crm: service 'vm:102': state changed from 'started' to 'migrate'  (node = node3, target = node1)
+info    140    node1/crm: migrate service 'vm:103' to node 'node1' (running)
+info    140    node1/crm: service 'vm:103': state changed from 'started' to 'migrate'  (node = node3, target = node1)
+info    145    node3/lrm: service vm:101 - start migrate to node 'node1'
+info    145    node3/lrm: service vm:101 - end migrate to node 'node1'
+info    145    node3/lrm: service vm:102 - start migrate to node 'node1'
+info    145    node3/lrm: service vm:102 - end migrate to node 'node1'
+info    145    node3/lrm: service vm:103 - start migrate to node 'node1'
+info    145    node3/lrm: service vm:103 - end migrate to node 'node1'
+info    160    node1/crm: service 'vm:101': state changed from 'migrate' to 'started'  (node = node1)
+info    160    node1/crm: service 'vm:102': state changed from 'migrate' to 'started'  (node = node1)
+info    160    node1/crm: service 'vm:103': state changed from 'migrate' to 'started'  (node = node1)
+info    161    node1/lrm: starting service vm:101
+info    161    node1/lrm: service status vm:101 started
+info    161    node1/lrm: starting service vm:102
+info    161    node1/lrm: service status vm:102 started
+info    161    node1/lrm: starting service vm:103
+info    161    node1/lrm: service status vm:103 started
 info    720     hardware: exit simulation - done
-- 
2.47.3