[PATCH ha-manager v2 7/7] manager: move HA resources without failback back to maintenance node

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

If the current node of an HA resource with failback disabled is put in
maintenance mode, the HA resource will correctly move to a replacement
node. Though as the previous node is put out of maintenance mode again,
the HA resource will stay on the new node, because of the disabled
failback flag.

As HA resources should move back to their previous maintenance node
independently of the failback flag, make the HA resource move to the
previous maintenance node as soon as it is available again.

This does not move HA resources without failback back to a maintenance
node that is not part of the highest node priority class. This behavior
is still up for debate and documented with relevant test cases.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v1:
- different patch now, but has the same behavior
- the maintenance fallback is moved up here now too as its easier to
  think and reason about than the previous patch
- this also prevents the weird case where the "avoids random migrations"
  is skipped if there is a maintenance_fallback set and isn't the
  $current_node, making failbacks happen even though failback is cleared
  for $sid

 src/PVE/HA/Manager.pm                                     | 6 +++---
 src/test/test-node-affinity-maintenance-nonstrict2/README | 3 ++-
 .../test-node-affinity-maintenance-nonstrict2/log.expect  | 8 ++++++++
 src/test/test-node-affinity-maintenance-strict4/README    | 3 ++-
 .../test-node-affinity-maintenance-strict4/log.expect     | 8 ++++++++
 5 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index b2b74282..2ce7e0b1 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -376,6 +376,9 @@ sub select_service_node {
 
     return undef if !%$pri_nodes;
 
+    return $maintenance_fallback
+        if defined($maintenance_fallback) && $pri_nodes->{$maintenance_fallback};
+
     # stay on current node if possible (avoids random migrations)
     if (
         $node_preference eq 'none'
@@ -388,9 +391,6 @@ sub select_service_node {
         return $current_node;
     }
 
-    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};
diff --git a/src/test/test-node-affinity-maintenance-nonstrict2/README b/src/test/test-node-affinity-maintenance-nonstrict2/README
index aa65a55a..1958154f 100644
--- a/src/test/test-node-affinity-maintenance-nonstrict2/README
+++ b/src/test/test-node-affinity-maintenance-nonstrict2/README
@@ -1,3 +1,4 @@
 Test whether an HA resource with failback disabled on the single member node of
 its non-strict node affinity rule with a single node member will move to a
-replacement node if its current node is in maintenance mode.
+replacement node if its current node is in maintenance mode and moves back to
+the previous maintenance node as soon as it's available again.
diff --git a/src/test/test-node-affinity-maintenance-nonstrict2/log.expect b/src/test/test-node-affinity-maintenance-nonstrict2/log.expect
index 05a77a24..339ce3ab 100644
--- a/src/test/test-node-affinity-maintenance-nonstrict2/log.expect
+++ b/src/test/test-node-affinity-maintenance-nonstrict2/log.expect
@@ -37,4 +37,12 @@ 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    241    node1/lrm: service vm:101 - start migrate to node 'node3'
+info    241    node1/lrm: service vm:101 - end migrate to node 'node3'
+info    260    node1/crm: service 'vm:101': 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    820     hardware: exit simulation - done
diff --git a/src/test/test-node-affinity-maintenance-strict4/README b/src/test/test-node-affinity-maintenance-strict4/README
index 43c68463..e6ad5c7e 100644
--- a/src/test/test-node-affinity-maintenance-strict4/README
+++ b/src/test/test-node-affinity-maintenance-strict4/README
@@ -1,3 +1,4 @@
 Test whether an HA resource with failback disabled in a strict node affinity
 rule with two differently prioritized node members will move to the
-lower-priority node if its current node is in maintenance mode.
+lower-priority node if its current node is in maintenance mode and moves back
+to the previous maintenance node as soon as it's available again.
diff --git a/src/test/test-node-affinity-maintenance-strict4/log.expect b/src/test/test-node-affinity-maintenance-strict4/log.expect
index 6f19258c..0bdf4fa0 100644
--- a/src/test/test-node-affinity-maintenance-strict4/log.expect
+++ b/src/test/test-node-affinity-maintenance-strict4/log.expect
@@ -37,4 +37,12 @@ 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 = node2, target = node3)
+info    243    node2/lrm: service vm:101 - start migrate to node 'node3'
+info    243    node2/lrm: service vm:101 - end migrate to node 'node3'
+info    260    node1/crm: service 'vm:101': 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    820     hardware: exit simulation - done
-- 
2.47.3