fix 'change_service_location' misuse and recovery from fencing

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@proxmox.com>

diff --git a/src/PVE/HA/Env.pm b/src/PVE/HA/Env.pm
index 3d96a6e870fff779da644115b04e41d2af427370..f6be7b243171816bcfcf91f446c5029bda32c177 100644 (file)
--- 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 7e2906da23a957e8dd43c07d164989a86fe3e375..86387863a159b7d93a29d55e5acf4bf0d35a4827 100644 (file)
--- 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 21a34dd40fa38a982940e530cf17316ae7d61a67..cab2b0a00c551a6e35c676cc7b68c9c65e34c9bc 100644 (file)
--- 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 103af50ba03f90383bf4880f904b5da9ee2393d0..5ce174a84fd845cd1005dcfd447ebb2b6099f221 100644 (file)
--- 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 f41d6eede9baf96cbd027cf5442f2ba1c10dfc41..25b034ee3dee1543196478cbd91e31942245063f 100644 (file)
--- 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 77599b3d0b47fb3fa1737e06265f64759eda6d25..5cd30f0fcc66bfaf0593f4a4d320bc649d880b6f 100644 (file)
--- 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 95346f2d7b78945d3ea4bcb654058142e2098007..72822ce48b907595073ea3df6fcaeaec6ed7eaa2 100644 (file)
--- 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 7a1782b0a2f42734255f80b4312be749ca8df70c..e640e1331d712fb662fff6e7bea15eafce05f072 100644 (file)
--- 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 76f5133054e58188bd5a11113f67e6cb0af15ce2..95937cbcc0e946f9d60466e93011b7fb215d57ec 100644 (file)
--- 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 4b90294d51ece4d508bed75adc6635e5a140990a..fb959f9f5a4aacb62068a5971245b46a5448a551 100644 (file)
--- 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 8ceb042071be7f9d5412b85644b9a7e46c7efa45..4efa3e73c0c3537b891b9dad3532afb410f5940b 100644 (file)
--- 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 c5564cc67d1380238d6aaff2f77b847a550cc0f6..aa8cfccf120514c3f96fe84990c0ecdce9b6691a 100644 (file)
--- 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