[PATCH-SERIES ha-manager v2 0/2] fix #6084: allow dropping maintenance state with active HA resources

[PATCH-SERIES ha-manager v2 0/2] fix #6084: allow dropping maintenance state with active HA resources

[Daniel Kral]

changes v1 -> v2:
 - add extra context to comment in condition (as suggested by @Michael)
 - add R-b + T-b from Michael

As reported in BZ #6804 [0], if a node held active HA resources, that
could not be moved to any other node, while a node maintenance request
has been placed and dropped again, the node's LRM would stay in
maintenance state even though the maintenance mode was lifted.

This small patch series fixes the issue by also allowing LRMs with
active assigned HA resources to be moved out of maintenance state again.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6084


Daniel Kral (2):
  test: add maintenance test case with stuck HA resource
  fix #6084: lrm: allow dropping maintenance with active HA resources

 src/PVE/HA/LRM.pm                             | 10 +++-
 src/test/test-crm-maintenance-request5/README |  6 ++
 .../test-crm-maintenance-request5/cmdlist     |  5 ++
 .../hardware_status                           |  5 ++
 .../test-crm-maintenance-request5/log.expect  | 58 +++++++++++++++++++
 .../manager_status                            |  1 +
 .../service_config                            |  3 +
 7 files changed, 85 insertions(+), 3 deletions(-)
 create mode 100644 src/test/test-crm-maintenance-request5/README
 create mode 100644 src/test/test-crm-maintenance-request5/cmdlist
 create mode 100644 src/test/test-crm-maintenance-request5/hardware_status
 create mode 100644 src/test/test-crm-maintenance-request5/log.expect
 create mode 100644 src/test/test-crm-maintenance-request5/manager_status
 create mode 100644 src/test/test-crm-maintenance-request5/service_config

-- 
2.47.3

[PATCH ha-manager v2 1/2] test: add maintenance test case with stuck HA resource

[Daniel Kral]

This test case documents the current behavior of a node being stuck in
maintenance state even after lifting the maintenance request on that
node if at least one HA resource is kept stuck on the node as it cannot
migrate to another node.

This behavior will be changed by an upcoming patch.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/test/test-crm-maintenance-request5/README |  7 +++
 .../test-crm-maintenance-request5/cmdlist     |  5 ++
 .../hardware_status                           |  5 ++
 .../test-crm-maintenance-request5/log.expect  | 57 +++++++++++++++++++
 .../manager_status                            |  1 +
 .../service_config                            |  3 +
 6 files changed, 78 insertions(+)
 create mode 100644 src/test/test-crm-maintenance-request5/README
 create mode 100644 src/test/test-crm-maintenance-request5/cmdlist
 create mode 100644 src/test/test-crm-maintenance-request5/hardware_status
 create mode 100644 src/test/test-crm-maintenance-request5/log.expect
 create mode 100644 src/test/test-crm-maintenance-request5/manager_status
 create mode 100644 src/test/test-crm-maintenance-request5/service_config

diff --git a/src/test/test-crm-maintenance-request5/README b/src/test/test-crm-maintenance-request5/README
new file mode 100644
index 00000000..e7fe0d5e
--- /dev/null
+++ b/src/test/test-crm-maintenance-request5/README
@@ -0,0 +1,7 @@
+Test setting the maintenance mode for a node where an HA resource cannot be
+moved to any other node because of migration errors and moving it out of
+maintenance mode again.
+
+Currently, the node will move out of the maintenance mode, but is left stuck in
+maintenance state as it never reached the required state to be empty to be able
+to move to active state again after the maintenance request is lifted.
diff --git a/src/test/test-crm-maintenance-request5/cmdlist b/src/test/test-crm-maintenance-request5/cmdlist
new file mode 100644
index 00000000..5aa42a6c
--- /dev/null
+++ b/src/test/test-crm-maintenance-request5/cmdlist
@@ -0,0 +1,5 @@
+[
+    [ "power node1 on", "power node2 on", "power node3 on" ],
+    [ "crm node1 enable-node-maintenance" ],
+    [ "crm node1 disable-node-maintenance" ]
+]
diff --git a/src/test/test-crm-maintenance-request5/hardware_status b/src/test/test-crm-maintenance-request5/hardware_status
new file mode 100644
index 00000000..451beb13
--- /dev/null
+++ b/src/test/test-crm-maintenance-request5/hardware_status
@@ -0,0 +1,5 @@
+{
+  "node1": { "power": "off", "network": "off" },
+  "node2": { "power": "off", "network": "off" },
+  "node3": { "power": "off", "network": "off" }
+}
diff --git a/src/test/test-crm-maintenance-request5/log.expect b/src/test/test-crm-maintenance-request5/log.expect
new file mode 100644
index 00000000..90326ef5
--- /dev/null
+++ b/src/test/test-crm-maintenance-request5/log.expect
@@ -0,0 +1,57 @@
+info      0     hardware: starting simulation
+info     20      cmdlist: execute power node1 on
+info     20    node1/crm: status change startup => wait_for_quorum
+info     20    node1/lrm: status change startup => wait_for_agent_lock
+info     20      cmdlist: execute power node2 on
+info     20    node2/crm: status change startup => wait_for_quorum
+info     20    node2/lrm: status change startup => wait_for_agent_lock
+info     20      cmdlist: execute power node3 on
+info     20    node3/crm: status change startup => wait_for_quorum
+info     20    node3/lrm: status change startup => wait_for_agent_lock
+info     20    node1/crm: got lock 'ha_manager_lock'
+info     20    node1/crm: status change wait_for_quorum => master
+info     20    node1/crm: node 'node1': state changed from 'unknown' => 'online'
+info     20    node1/crm: node 'node2': state changed from 'unknown' => 'online'
+info     20    node1/crm: node 'node3': state changed from 'unknown' => 'online'
+info     20    node1/crm: adding new service 'fa:109' on node 'node1'
+info     20    node1/crm: service 'fa:109': state changed from 'request_start' to 'started'  (node = node1)
+info     21    node1/lrm: got lock 'ha_agent_node1_lock'
+info     21    node1/lrm: status change wait_for_agent_lock => active
+info     21    node1/lrm: starting service fa:109
+info     21    node1/lrm: service status fa:109 started
+info     22    node2/crm: status change wait_for_quorum => slave
+info     24    node3/crm: status change wait_for_quorum => slave
+info    120      cmdlist: execute crm node1 enable-node-maintenance
+info    121    node1/lrm: status change active => maintenance
+info    140    node1/crm: node 'node1': state changed from 'online' => 'maintenance'
+info    140    node1/crm: relocate service 'fa:109' to node 'node2'
+info    140    node1/crm: service 'fa:109': state changed from 'started' to 'relocate'  (node = node1, target = node2)
+err     141    node1/lrm: service fa:109 not moved (migration error)
+info    143    node2/lrm: got lock 'ha_agent_node2_lock'
+info    143    node2/lrm: status change wait_for_agent_lock => active
+err     160    node1/crm: service 'fa:109' - migration failed (exit code 1)
+info    160    node1/crm: service 'fa:109': state changed from 'relocate' to 'started'  (node = node1)
+info    160    node1/crm: relocate service 'fa:109' to node 'node2'
+info    160    node1/crm: service 'fa:109': state changed from 'started' to 'relocate'  (node = node1, target = node2)
+err     161    node1/lrm: service fa:109 not moved (migration error)
+err     180    node1/crm: service 'fa:109' - migration failed (exit code 1)
+info    180    node1/crm: service 'fa:109': state changed from 'relocate' to 'started'  (node = node1)
+info    180    node1/crm: relocate service 'fa:109' to node 'node2'
+info    180    node1/crm: service 'fa:109': state changed from 'started' to 'relocate'  (node = node1, target = node2)
+err     181    node1/lrm: service fa:109 not moved (migration error)
+err     200    node1/crm: service 'fa:109' - migration failed (exit code 1)
+info    200    node1/crm: service 'fa:109': state changed from 'relocate' to 'started'  (node = node1)
+info    200    node1/crm: relocate service 'fa:109' to node 'node2'
+info    200    node1/crm: service 'fa:109': state changed from 'started' to 'relocate'  (node = node1, target = node2)
+err     201    node1/lrm: service fa:109 not moved (migration error)
+info    220      cmdlist: execute crm node1 disable-node-maintenance
+err     220    node1/crm: service 'fa:109' - migration failed (exit code 1)
+info    220    node1/crm: service 'fa:109': state changed from 'relocate' to 'started'  (node = node1)
+info    220    node1/crm: relocate service 'fa:109' to node 'node2'
+info    220    node1/crm: service 'fa:109': state changed from 'started' to 'relocate'  (node = node1, target = node2)
+err     221    node1/lrm: service fa:109 not moved (migration error)
+info    240    node1/crm: node 'node1': state changed from 'maintenance' => 'online'
+err     240    node1/crm: service 'fa:109' - migration failed (exit code 1)
+info    240    node1/crm: service 'fa:109': state changed from 'relocate' to 'started'  (node = node1)
+info    240    node1/crm: service 'fa:109': clearing stale maintenance node 'node1' setting (is current node)
+info    820     hardware: exit simulation - done
diff --git a/src/test/test-crm-maintenance-request5/manager_status b/src/test/test-crm-maintenance-request5/manager_status
new file mode 100644
index 00000000..0967ef42
--- /dev/null
+++ b/src/test/test-crm-maintenance-request5/manager_status
@@ -0,0 +1 @@
+{}
diff --git a/src/test/test-crm-maintenance-request5/service_config b/src/test/test-crm-maintenance-request5/service_config
new file mode 100644
index 00000000..78879193
--- /dev/null
+++ b/src/test/test-crm-maintenance-request5/service_config
@@ -0,0 +1,3 @@
+{
+    "fa:109": { "node": "node1", "state": "started" }
+}
-- 
2.47.3

[PATCH ha-manager v2 2/2] fix #6084: lrm: allow dropping maintenance with active HA resources

[Daniel Kral]

If a node's LRM is in the maintenance state and one of the HA resources
could not be moved to another node, the node's LRM would be kept in the
maintenance state even as the maintenance request on the node is lifted
again.

This happens because the LRM maintenance state branch for any active HA
resources or workers will catch these cases and not advance to the
branch which puts nodes without maintenance requests back into active
state.

Therefore, do advance to the next branch if the maintenance request is
already lifted and the node's LRM can go back to the active state.

The check for is_maintenance_requested() is intentionally put last to
not change the existing execution chain as run_workers() might have some
side-effects on the LRM workers itself.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/HA/LRM.pm                                 | 10 +++++++---
 src/test/test-crm-maintenance-request5/README     |  5 ++---
 src/test/test-crm-maintenance-request5/log.expect |  1 +
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index 34425653..72e37e69 100644
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -456,13 +456,17 @@ sub work {
             $haenv->release_ha_agent_lock();
             give_up_watchdog_protection($self);
             $self->set_local_status({ state => 'wait_for_agent_lock' });
-        } elsif ($self->active_service_count() || $self->run_workers()) {
-            # keep the lock and watchdog as long as not all services cleared the node
+        } elsif (
+            ($self->active_service_count() || $self->run_workers())
+            && $self->is_maintenance_requested()
+        ) {
+            # keep the lock and watchdog as long as not all active services
+            # cleared the node and the maintenance request is upheld
             if (!$self->get_protected_ha_agent_lock()) {
                 $self->set_local_status({ state => 'lost_agent_lock' });
             }
         } elsif (!$self->is_maintenance_requested()) {
-            # empty && no maintenance mode && not exited -> need to switch active again
+            # no node maintenance request && not exited -> need to switch active again
             if ($self->get_protected_ha_agent_lock()) {
                 $self->set_local_status({ state => 'active' });
             } else {
diff --git a/src/test/test-crm-maintenance-request5/README b/src/test/test-crm-maintenance-request5/README
index e7fe0d5e..99612321 100644
--- a/src/test/test-crm-maintenance-request5/README
+++ b/src/test/test-crm-maintenance-request5/README
@@ -2,6 +2,5 @@ Test setting the maintenance mode for a node where an HA resource cannot be
 moved to any other node because of migration errors and moving it out of
 maintenance mode again.
 
-Currently, the node will move out of the maintenance mode, but is left stuck in
-maintenance state as it never reached the required state to be empty to be able
-to move to active state again after the maintenance request is lifted.
+The node's LRM is expected to move out of maintenance mode and state again
+after the maintenance request is lifted from the node.
diff --git a/src/test/test-crm-maintenance-request5/log.expect b/src/test/test-crm-maintenance-request5/log.expect
index 90326ef5..a30be92d 100644
--- a/src/test/test-crm-maintenance-request5/log.expect
+++ b/src/test/test-crm-maintenance-request5/log.expect
@@ -49,6 +49,7 @@ err     220    node1/crm: service 'fa:109' - migration failed (exit code 1)
 info    220    node1/crm: service 'fa:109': state changed from 'relocate' to 'started'  (node = node1)
 info    220    node1/crm: relocate service 'fa:109' to node 'node2'
 info    220    node1/crm: service 'fa:109': state changed from 'started' to 'relocate'  (node = node1, target = node2)
+info    221    node1/lrm: status change maintenance => active
 err     221    node1/lrm: service fa:109 not moved (migration error)
 info    240    node1/crm: node 'node1': state changed from 'maintenance' => 'online'
 err     240    node1/crm: service 'fa:109' - migration failed (exit code 1)
-- 
2.47.3