[pve-devel] [RFC ha-manager] fix 'change_service_location' misuse and recovery from fencing
Thomas Lamprecht
t.lamprecht at proxmox.com
Fri Feb 12 16:14:54 CET 2016
First rename the change_service_location method from the environment
to an more fitting name, 'steal_service'.
The 'change_service_location' from the virtual hardware class stays
at it is, because there the name fits (those function have not the
same meaning, so it's good that they named different now).
As we misused the config steal method (former
change_service_location) in the stopped state to process the
services from fenced nodes we need another way now.
This is achieved through the private method 'recover_fenced_service'
which is now the only place who has the right to steal a service
from an node.
When a node was successfully fenced we no longer change its services
state to 'stopped', rather we drop that hack and search a new node
in 'recover_fenced_service', if found we the steal the service and
move it to from the fenced to the new (recovery) node and place it
there in the 'started' state, after that the state machine is able
to handle the rest.
If we do not find a node we try again next round as that is better
then placing it in the error state, because so we have still a
chance to recover, which we do not have with the error state.
Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---
A few more notes which may not fit into the commit message:
The patch touches quite a few parts but after some evaluating
I didn't found a good way to split it.
Renaming the (haenv) 'change_service_location' gives it a lot more
weight and it should make us think twice if we want to use it somewhere
else.
The way we handled/recovered fenced services was rather a hack than nice,
the proposed should be (hopefully) a bit more sane.
I tested it by evaluating every changed regression test (so everyone
which involved fencing in some way) the single one which changed some
actual semantic behaviour was 'test-basic2' but it seems to hold it's
purpose still so I accepted it as a nuisance.
Also tested successfully on a real HA cluster.
src/PVE/HA/Env.pm | 5 +--
src/PVE/HA/Env/PVE2.pm | 3 +-
src/PVE/HA/Manager.pm | 73 +++++++++++++++++++++++---------------
src/PVE/HA/Sim/Env.pm | 3 +-
src/PVE/HA/Sim/Resources.pm | 2 +-
src/test/test-basic1/log.expect | 8 ++---
src/test/test-basic2/log.expect | 12 +++----
src/test/test-basic5/log.expect | 4 +--
src/test/test-shutdown1/log.expect | 4 +--
src/test/test-shutdown2/log.expect | 4 +--
src/test/test-shutdown3/log.expect | 4 +--
src/test/test-shutdown4/log.expect | 4 +--
12 files changed, 73 insertions(+), 53 deletions(-)
diff --git a/src/PVE/HA/Env.pm b/src/PVE/HA/Env.pm
index 3d96a6e..f6be7b2 100644
--- a/src/PVE/HA/Env.pm
+++ b/src/PVE/HA/Env.pm
@@ -87,10 +87,11 @@ sub read_service_config {
return $self->{plug}->read_service_config();
}
-sub change_service_location {
+# this is normally only allowed by the master to recover a _fenced_ service
+sub steal_service {
my ($self, $sid, $current_node, $new_node) = @_;
- return $self->{plug}->change_service_location($sid, $current_node, $new_node);
+ return $self->{plug}->steal_service($sid, $current_node, $new_node);
}
sub read_group_config {
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index 7e2906d..8638786 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -144,7 +144,8 @@ sub read_service_config {
return $conf;
}
-sub change_service_location {
+# this is only allowed by the master to recover a _fenced_ service
+sub steal_service {
my ($self, $sid, $current_node, $new_node) = @_;
my (undef, $type, $name) = PVE::HA::Tools::parse_sid($sid);
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 21a34dd..cab2b0a 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -213,6 +213,44 @@ my $change_service_state = sub {
$haenv->log('info', "service '$sid': state changed from '${old_state}' to '${new_state}' $text_state");
};
+# after a node was fenced this recovers the service to a new node
+my $recover_fenced_service = sub {
+ my ($self, $sid, $cd) = @_;
+
+ my ($haenv, $ss) = ($self->{haenv}, $self->{ss});
+
+ my $sd = $ss->{$sid};
+
+ if ($sd->{state} ne 'fence') { # should not happen
+ $haenv->log('err', "cannot recover service '$sid' from fencing," .
+ " wrong state '$sd->{state}'");
+ return;
+ }
+
+ my $fenced_node = $sd->{node}; # for logging purpose
+
+ $self->recompute_online_node_usage(); # we want the most current node state
+
+ my $recovery_node = select_service_node($self->{groups},
+ $self->{online_node_usage},
+ $cd, $sd->{node});
+
+ if ($recovery_node) {
+ $haenv->log('info', "recover service '$sid' from fenced node " .
+ "'$fenced_node' to node '$recovery_node'");
+
+ $haenv->steal_service($sid, $sd->{node}, $recovery_node);
+
+ # $sd *is normally read-only*, fencing is the exception
+ $cd->{node} = $sd->{node} = $recovery_node;
+ &$change_service_state($self, $sid, 'started', node => $recovery_node);
+ } else {
+ # no node found, let the service in 'fence' state and try again
+ $haenv->log('err', "recovering service '$sid' from fenced node " .
+ "'$fenced_node' failed, no recovery node found");
+ }
+};
+
# read LRM status for all nodes
sub read_lrm_status {
my ($self) = @_;
@@ -380,8 +418,8 @@ sub manage {
next if !$fenced_nodes->{$sd->{node}};
- # node fence was successful - mark service as stopped
- &$change_service_state($self, $sid, 'stopped');
+ # node fence was successful - recover service
+ &$recover_fenced_service($self, $sid, $sc->{$sid});
}
last if !$repeat;
@@ -471,14 +509,8 @@ sub next_state_stopped {
} elsif ($sd->{node} eq $target) {
$haenv->log('info', "ignore service '$sid' $cmd request - service already on node '$target'");
} else {
- eval {
- $haenv->change_service_location($sid, $sd->{node}, $target);
- $cd->{node} = $sd->{node} = $target; # fixme: $sd is read-only??!!
- $haenv->log('info', "$cmd service '$sid' to node '$target' (stopped)");
- };
- if (my $err = $@) {
- $haenv->log('err', "$cmd service '$sid' to node '$target' failed - $err");
- }
+ &$change_service_state($self, $sid, $cmd, node => $target);
+ return;
}
} else {
$haenv->log('err', "unknown command '$cmd' for service '$sid'");
@@ -491,24 +523,9 @@ sub next_state_stopped {
}
if ($cd->{state} eq 'enabled') {
- if (my $node = select_service_node($self->{groups}, $self->{online_node_usage}, $cd, $sd->{node})) {
- if ($node && ($sd->{node} ne $node)) {
- eval {
- $haenv->change_service_location($sid, $sd->{node}, $node);
- $cd->{node} = $sd->{node} = $node; # fixme: $sd is read-only??!!
- };
- if (my $err = $@) {
- $haenv->log('err', "move service '$sid' to node '$node' failed - $err");
- } else {
- &$change_service_state($self, $sid, 'started', node => $node);
- }
- } else {
- &$change_service_state($self, $sid, 'started', node => $node);
- }
- } else {
- # fixme: warn
- }
-
+ # simply mark it started, if it's on the wrong node
+ # next_state_started will fix that for us
+ &$change_service_state($self, $sid, 'started', node => $sd->{node});
return;
}
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index 103af50..5ce174a 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -173,7 +173,8 @@ sub read_group_config {
return $self->{hardware}->read_group_config();
}
-sub change_service_location {
+# this is normally only allowed by the master to recover a _fenced_ service
+sub steal_service {
my ($self, $sid, $current_node, $new_node) = @_;
return $self->{hardware}->change_service_location($sid, $current_node, $new_node);
diff --git a/src/PVE/HA/Sim/Resources.pm b/src/PVE/HA/Sim/Resources.pm
index f41d6ee..25b034e 100644
--- a/src/PVE/HA/Sim/Resources.pm
+++ b/src/PVE/HA/Sim/Resources.pm
@@ -98,7 +98,7 @@ sub migrate {
$haenv->sleep(2); # (live) migration time
}
- $haenv->change_service_location($sid, $nodename, $target);
+ $hardware->change_service_location($sid, $nodename, $target);
$haenv->log("info", "service $sid - end $cmd to node '$target'");
# ensure that the old node doesn't has the service anymore
delete $ss->{$sid};
diff --git a/src/test/test-basic1/log.expect b/src/test/test-basic1/log.expect
index 77599b3..5cd30f0 100644
--- a/src/test/test-basic1/log.expect
+++ b/src/test/test-basic1/log.expect
@@ -43,8 +43,8 @@ info 166 hardware: server 'node3' stopped by poweroff (watchdog)
info 240 node1/crm: got lock 'ha_agent_node3_lock'
info 240 node1/crm: fencing: acknowleged - got agent lock for node 'node3'
info 240 node1/crm: node 'node3': state changed from 'fence' => 'unknown'
-info 240 node1/crm: service 'vm:103': state changed from 'fence' to 'stopped'
-info 260 node1/crm: service 'vm:103': state changed from 'stopped' to 'started' (node = node2)
-info 263 node2/lrm: starting service vm:103
-info 263 node2/lrm: service status vm:103 started
+info 240 node1/crm: recover service 'vm:103' from fenced node 'node3' to node 'node2'
+info 240 node1/crm: service 'vm:103': state changed from 'fence' to 'started' (node = node2)
+info 243 node2/lrm: starting service vm:103
+info 243 node2/lrm: service status vm:103 started
info 720 hardware: exit simulation - done
diff --git a/src/test/test-basic2/log.expect b/src/test/test-basic2/log.expect
index 95346f2..72822ce 100644
--- a/src/test/test-basic2/log.expect
+++ b/src/test/test-basic2/log.expect
@@ -11,12 +11,12 @@ info 22 node3/crm: node 'node2': state changed from 'online' => 'unknown'
info 22 node3/crm: got lock 'ha_agent_node1_lock'
info 22 node3/crm: fencing: acknowleged - got agent lock for node 'node1'
info 22 node3/crm: node 'node1': state changed from 'fence' => 'unknown'
-info 22 node3/crm: service 'vm:101': state changed from 'fence' to 'stopped'
+info 22 node3/crm: recover service 'vm:101' from fenced node 'node1' to node 'node3'
+info 22 node3/crm: service 'vm:101': state changed from 'fence' to 'started' (node = node3)
+info 23 node3/lrm: got lock 'ha_agent_node3_lock'
+info 23 node3/lrm: status change wait_for_agent_lock => active
+info 23 node3/lrm: starting service vm:101
+info 23 node3/lrm: service status vm:101 started
info 40 node1/crm: status change wait_for_quorum => slave
info 42 node3/crm: node 'node1': state changed from 'unknown' => 'online'
-info 42 node3/crm: service 'vm:101': state changed from 'stopped' to 'started' (node = node1)
-info 161 node1/lrm: got lock 'ha_agent_node1_lock'
-info 161 node1/lrm: status change wait_for_agent_lock => active
-info 161 node1/lrm: starting service vm:101
-info 161 node1/lrm: service status vm:101 started
info 620 hardware: exit simulation - done
diff --git a/src/test/test-basic5/log.expect b/src/test/test-basic5/log.expect
index 7a1782b..e640e13 100644
--- a/src/test/test-basic5/log.expect
+++ b/src/test/test-basic5/log.expect
@@ -46,8 +46,8 @@ info 282 node3/crm: node 'node1': state changed from 'unknown' => 'fence'
info 282 node3/crm: got lock 'ha_agent_node1_lock'
info 282 node3/crm: fencing: acknowleged - got agent lock for node 'node1'
info 282 node3/crm: node 'node1': state changed from 'fence' => 'unknown'
-info 282 node3/crm: service 'vm:101': state changed from 'fence' to 'stopped'
-info 282 node3/crm: service 'vm:101': state changed from 'stopped' to 'started' (node = node2)
+info 282 node3/crm: recover service 'vm:101' from fenced node 'node1' to node 'node2'
+info 282 node3/crm: service 'vm:101': state changed from 'fence' to 'started' (node = node2)
info 301 node2/lrm: starting service vm:101
info 301 node2/lrm: service status vm:101 started
info 500 cmdlist: execute power node1 on
diff --git a/src/test/test-shutdown1/log.expect b/src/test/test-shutdown1/log.expect
index 76f5133..95937cb 100644
--- a/src/test/test-shutdown1/log.expect
+++ b/src/test/test-shutdown1/log.expect
@@ -34,8 +34,8 @@ info 200 node1/crm: node 'node3': state changed from 'unknown' => 'fence'
info 200 node1/crm: got lock 'ha_agent_node3_lock'
info 200 node1/crm: fencing: acknowleged - got agent lock for node 'node3'
info 200 node1/crm: node 'node3': state changed from 'fence' => 'unknown'
-info 200 node1/crm: service 'vm:103': state changed from 'fence' to 'stopped'
-info 200 node1/crm: service 'vm:103': state changed from 'stopped' to 'started' (node = node1)
+info 200 node1/crm: recover service 'vm:103' from fenced node 'node3' to node 'node1'
+info 200 node1/crm: service 'vm:103': state changed from 'fence' to 'started' (node = node1)
info 201 node1/lrm: got lock 'ha_agent_node1_lock'
info 201 node1/lrm: status change wait_for_agent_lock => active
info 201 node1/lrm: starting service vm:103
diff --git a/src/test/test-shutdown2/log.expect b/src/test/test-shutdown2/log.expect
index 4b90294..fb959f9 100644
--- a/src/test/test-shutdown2/log.expect
+++ b/src/test/test-shutdown2/log.expect
@@ -34,8 +34,8 @@ info 200 node1/crm: node 'node3': state changed from 'unknown' => 'fence'
info 200 node1/crm: got lock 'ha_agent_node3_lock'
info 200 node1/crm: fencing: acknowleged - got agent lock for node 'node3'
info 200 node1/crm: node 'node3': state changed from 'fence' => 'unknown'
-info 200 node1/crm: service 'vm:103': state changed from 'fence' to 'stopped'
-info 200 node1/crm: service 'vm:103': state changed from 'stopped' to 'started' (node = node1)
+info 200 node1/crm: recover service 'vm:103' from fenced node 'node3' to node 'node1'
+info 200 node1/crm: service 'vm:103': state changed from 'fence' to 'started' (node = node1)
info 201 node1/lrm: got lock 'ha_agent_node1_lock'
info 201 node1/lrm: status change wait_for_agent_lock => active
info 201 node1/lrm: starting service vm:103
diff --git a/src/test/test-shutdown3/log.expect b/src/test/test-shutdown3/log.expect
index 8ceb042..4efa3e7 100644
--- a/src/test/test-shutdown3/log.expect
+++ b/src/test/test-shutdown3/log.expect
@@ -34,8 +34,8 @@ info 200 node1/crm: node 'node3': state changed from 'unknown' => 'fence'
info 200 node1/crm: got lock 'ha_agent_node3_lock'
info 200 node1/crm: fencing: acknowleged - got agent lock for node 'node3'
info 200 node1/crm: node 'node3': state changed from 'fence' => 'unknown'
-info 200 node1/crm: service 'ct:103': state changed from 'fence' to 'stopped'
-info 200 node1/crm: service 'ct:103': state changed from 'stopped' to 'started' (node = node1)
+info 200 node1/crm: recover service 'ct:103' from fenced node 'node3' to node 'node1'
+info 200 node1/crm: service 'ct:103': state changed from 'fence' to 'started' (node = node1)
info 201 node1/lrm: got lock 'ha_agent_node1_lock'
info 201 node1/lrm: status change wait_for_agent_lock => active
info 201 node1/lrm: starting service ct:103
diff --git a/src/test/test-shutdown4/log.expect b/src/test/test-shutdown4/log.expect
index c5564cc..aa8cfcc 100644
--- a/src/test/test-shutdown4/log.expect
+++ b/src/test/test-shutdown4/log.expect
@@ -37,8 +37,8 @@ info 220 node2/crm: node 'node1': state changed from 'unknown' => 'fence'
info 220 node2/crm: got lock 'ha_agent_node1_lock'
info 220 node2/crm: fencing: acknowleged - got agent lock for node 'node1'
info 220 node2/crm: node 'node1': state changed from 'fence' => 'unknown'
-info 220 node2/crm: service 'vm:100': state changed from 'fence' to 'stopped'
-info 220 node2/crm: service 'vm:100': state changed from 'stopped' to 'started' (node = node2)
+info 220 node2/crm: recover service 'vm:100' from fenced node 'node1' to node 'node2'
+info 220 node2/crm: service 'vm:100': state changed from 'fence' to 'started' (node = node2)
info 221 node2/lrm: got lock 'ha_agent_node2_lock'
info 221 node2/lrm: status change wait_for_agent_lock => active
info 221 node2/lrm: starting service vm:100
--
2.1.4
More information about the pve-devel
mailing list