[pve-devel] [PATCH ha-manager v2 2/2] cleanup backup & mounted locks after recovery (fixes #1100)

Thomas Lamprecht t.lamprecht at proxmox.com
Mon Sep 12 16:15:25 CEST 2016


This cleans up the backup and mounted locks after a service is
recovered from a failed node, else it may not start if a locked
action occurred during the node failure.

We allow deletion of backup and mounted locks as they are secure
to do so and may happen automated (backup jobs, mount a CT and
forget to unmount it).

We do not allow snapshot lock deletion as this needs more manual
clean up, also they are normally triggered manually.

Further ignore migration locks for now, we should over think that but
as its a manual triggered action for now it should be OK to not
auto delete it.

Add the two log.expect out puts from the added test to enable
regression testing this issue.

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---

changes since v1:
* instead of removing all locks just remove the 'backup' and the 'mounted'
  one
* move out code from plugin specifc functions to an private sub in the Manager
  class, reduces code reuse and increases regression test coverage.


 src/PVE/HA/Manager.pm                    | 25 ++++++++++++
 src/PVE/HA/Resources.pm                  |  7 ++++
 src/PVE/HA/Resources/PVECT.pm            | 10 +++++
 src/PVE/HA/Resources/PVEVM.pm            |  9 +++++
 src/PVE/HA/Sim/Resources.pm              | 11 ++++++
 src/test/test-locked-service1/log.expect | 44 +++++++++++++++++++++
 src/test/test-locked-service2/log.expect | 68 ++++++++++++++++++++++++++++++++
 7 files changed, 174 insertions(+)
 create mode 100644 src/test/test-locked-service1/log.expect
 create mode 100644 src/test/test-locked-service2/log.expect

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index e3d6ffa..e4113ad 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -238,6 +238,29 @@ my $change_service_state = sub {
 		" to '${new_state}'$text_state");
 };
 
+# clean up a possible bad state from a recovered service to allow its start
+my $after_recovery_cleanup = sub {
+    my ($self, $sid) = @_;
+
+    my $haenv = $self->{haenv};
+
+    my (undef, $type, $id) = PVE::HA::Tools::parse_sid($sid);
+    my $plugin = PVE::HA::Resources->lookup($type);
+
+    die "unknown resource type '$type'" if !$plugin;
+
+    # locks may block a recovery, cleanup those which are safe to remove
+    my @safe_locks = qw(backup mounted);
+
+    foreach my $lock (@safe_locks) {
+	if ($plugin->remove_lock($haenv, $id, $lock)) {
+	    $haenv->log('warning', "removed leftover lock '$lock' from " .
+	                "recovered service '$sid' to allow its start.");
+	    last;
+	}
+    }
+};
+
 # after a node was fenced this recovers the service to a new node
 my $recover_fenced_service = sub {
     my ($self, $sid, $cd) = @_;
@@ -266,6 +289,8 @@ my $recover_fenced_service = sub {
 
 	$haenv->steal_service($sid, $sd->{node}, $recovery_node);
 
+	&$after_recovery_cleanup($self, $sid);
+
 	# $sd *is normally read-only*, fencing is the exception
 	$cd->{node} = $sd->{node} = $recovery_node;
 	&$change_service_state($self, $sid, 'started', node => $recovery_node);
diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
index 3836fc8..e4e4cd6 100644
--- a/src/PVE/HA/Resources.pm
+++ b/src/PVE/HA/Resources.pm
@@ -124,6 +124,13 @@ sub check_running {
     die "implement in subclass";
 }
 
+sub remove_lock {
+    my ($self, $haenv, $id, $lock) = @_;
+
+    die "implement in subclass";
+}
+
+
 
 # package PVE::HA::Resources::IPAddr;
 
diff --git a/src/PVE/HA/Resources/PVECT.pm b/src/PVE/HA/Resources/PVECT.pm
index 0b44c70..59f3263 100644
--- a/src/PVE/HA/Resources/PVECT.pm
+++ b/src/PVE/HA/Resources/PVECT.pm
@@ -6,6 +6,7 @@ use warnings;
 use PVE::HA::Tools;
 
 use PVE::LXC;
+use PVE::LXC::Config;
 use PVE::API2::LXC;
 use PVE::API2::LXC::Status;
 
@@ -113,4 +114,13 @@ sub check_running {
     return PVE::LXC::check_running($vmid);
 }
 
+sub remove_lock {
+    my ($self, $haenv, $id, $lock) = @_;
+
+    eval { die if !defined(PVE::LXC::Config->remove_lock($id, $lock)); };
+    return undef if $@;
+
+    return 1;
+}
+
 1;
diff --git a/src/PVE/HA/Resources/PVEVM.pm b/src/PVE/HA/Resources/PVEVM.pm
index 4c06df9..db62ee5 100644
--- a/src/PVE/HA/Resources/PVEVM.pm
+++ b/src/PVE/HA/Resources/PVEVM.pm
@@ -116,4 +116,13 @@ sub check_running {
     return PVE::QemuServer::check_running($vmid, 1, $nodename);
 }
 
+sub remove_lock {
+    my ($self, $haenv, $id, $lock) = @_;
+
+    eval { die if !defined(PVE::QemuConfig->remove_lock($id, $lock)); };
+    return undef if $@;
+
+    return 1;
+}
+
 1;
diff --git a/src/PVE/HA/Sim/Resources.pm b/src/PVE/HA/Sim/Resources.pm
index ec3d775..cc7b7e2 100644
--- a/src/PVE/HA/Sim/Resources.pm
+++ b/src/PVE/HA/Sim/Resources.pm
@@ -113,5 +113,16 @@ sub migrate {
     return defined($ss->{$sid}) ? 0 : 1;
 }
 
+sub remove_lock {
+    my ($self, $haenv, $id, $lock) = @_;
+
+    my $sid = $self->type() . ":$id";
+    my $hardware = $haenv->hardware();
+
+    eval { die if !$hardware->unlock_service($sid, $lock); };
+    return undef if $@;
+
+    return 1;
+}
 
 1;
diff --git a/src/test/test-locked-service1/log.expect b/src/test/test-locked-service1/log.expect
new file mode 100644
index 0000000..6bc04a1
--- /dev/null
+++ b/src/test/test-locked-service1/log.expect
@@ -0,0 +1,44 @@
+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 'vm:103' on node 'node3'
+info     22    node2/crm: status change wait_for_quorum => slave
+info     24    node3/crm: status change wait_for_quorum => slave
+info     25    node3/lrm: got lock 'ha_agent_node3_lock'
+info     25    node3/lrm: status change wait_for_agent_lock => active
+info     25    node3/lrm: starting service vm:103
+info     25    node3/lrm: service status vm:103 started
+info    120      cmdlist: execute service vm:103 lock
+info    220      cmdlist: execute network node3 off
+info    220    node1/crm: node 'node3': state changed from 'online' => 'unknown'
+info    224    node3/crm: status change slave => wait_for_quorum
+info    225    node3/lrm: status change active => lost_agent_lock
+info    260    node1/crm: service 'vm:103': state changed from 'started' to 'fence'
+info    260    node1/crm: node 'node3': state changed from 'unknown' => 'fence'
+info    266     watchdog: execute power node3 off
+info    265    node3/crm: killed by poweroff
+info    266    node3/lrm: killed by poweroff
+info    266     hardware: server 'node3' stopped by poweroff (watchdog)
+info    340    node1/crm: got lock 'ha_agent_node3_lock'
+info    340    node1/crm: fencing: acknowleged - got agent lock for node 'node3'
+info    340    node1/crm: node 'node3': state changed from 'fence' => 'unknown'
+info    340    node1/crm: recover service 'vm:103' from fenced node 'node3' to node 'node1'
+warn    340    node1/crm: removed leftover lock 'backup' from recovered service 'vm:103' to allow its start.
+info    340    node1/crm: service 'vm:103': state changed from 'fence' to 'started'  (node = node1)
+info    341    node1/lrm: got lock 'ha_agent_node1_lock'
+info    341    node1/lrm: status change wait_for_agent_lock => active
+info    341    node1/lrm: starting service vm:103
+info    341    node1/lrm: service status vm:103 started
+info    820     hardware: exit simulation - done
diff --git a/src/test/test-locked-service2/log.expect b/src/test/test-locked-service2/log.expect
new file mode 100644
index 0000000..90d830f
--- /dev/null
+++ b/src/test/test-locked-service2/log.expect
@@ -0,0 +1,68 @@
+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 'vm:103' on node 'node3'
+info     22    node2/crm: status change wait_for_quorum => slave
+info     24    node3/crm: status change wait_for_quorum => slave
+info     25    node3/lrm: got lock 'ha_agent_node3_lock'
+info     25    node3/lrm: status change wait_for_agent_lock => active
+info     25    node3/lrm: starting service vm:103
+info     25    node3/lrm: service status vm:103 started
+info    120      cmdlist: execute service vm:103 lock snapshot
+info    220      cmdlist: execute network node3 off
+info    220    node1/crm: node 'node3': state changed from 'online' => 'unknown'
+info    224    node3/crm: status change slave => wait_for_quorum
+info    225    node3/lrm: status change active => lost_agent_lock
+info    260    node1/crm: service 'vm:103': state changed from 'started' to 'fence'
+info    260    node1/crm: node 'node3': state changed from 'unknown' => 'fence'
+info    266     watchdog: execute power node3 off
+info    265    node3/crm: killed by poweroff
+info    266    node3/lrm: killed by poweroff
+info    266     hardware: server 'node3' stopped by poweroff (watchdog)
+info    340    node1/crm: got lock 'ha_agent_node3_lock'
+info    340    node1/crm: fencing: acknowleged - got agent lock for node 'node3'
+info    340    node1/crm: node 'node3': state changed from 'fence' => 'unknown'
+info    340    node1/crm: recover service 'vm:103' from fenced node 'node3' to node 'node1'
+info    340    node1/crm: service 'vm:103': state changed from 'fence' to 'started'  (node = node1)
+info    341    node1/lrm: got lock 'ha_agent_node1_lock'
+info    341    node1/lrm: status change wait_for_agent_lock => active
+info    341    node1/lrm: starting service vm:103
+err     341    node1/lrm: service 'vm:103' locked, unable to start!
+warn    341    node1/lrm: unable to start service vm:103
+warn    341    node1/lrm: restart policy: retry number 1 for service 'vm:103'
+info    361    node1/lrm: starting service vm:103
+err     361    node1/lrm: service 'vm:103' locked, unable to start!
+warn    361    node1/lrm: unable to start service vm:103
+err     361    node1/lrm: unable to start service vm:103 on local node after 1 retries
+warn    380    node1/crm: starting service vm:103 on node 'node1' failed, relocating service.
+info    380    node1/crm: migrate service 'vm:103' to node 'node2' (running)
+info    380    node1/crm: service 'vm:103': state changed from 'started' to 'migrate'  (node = node1, target = node2)
+info    381    node1/lrm: service vm:103 - start migrate to node 'node2'
+info    381    node1/lrm: service vm:103 - end migrate to node 'node2'
+info    400    node1/crm: service 'vm:103': state changed from 'migrate' to 'started'  (node = node2)
+info    403    node2/lrm: got lock 'ha_agent_node2_lock'
+info    403    node2/lrm: status change wait_for_agent_lock => active
+info    403    node2/lrm: starting service vm:103
+err     403    node2/lrm: service 'vm:103' locked, unable to start!
+warn    403    node2/lrm: unable to start service vm:103
+warn    403    node2/lrm: restart policy: retry number 1 for service 'vm:103'
+info    423    node2/lrm: starting service vm:103
+err     423    node2/lrm: service 'vm:103' locked, unable to start!
+warn    423    node2/lrm: unable to start service vm:103
+err     423    node2/lrm: unable to start service vm:103 on local node after 1 retries
+err     440    node1/crm: recovery policy for service vm:103 failed, entering error state. Failed nodes: node1, node2
+info    440    node1/crm: service 'vm:103': state changed from 'started' to 'error'
+err     443    node2/lrm: service vm:103 is in an error state and needs manual intervention. Look up 'ERROR RECOVERY' in the documentation.
+info    820     hardware: exit simulation - done
-- 
2.1.4





More information about the pve-devel mailing list