always queue service stop if node shuts down

Commit 61ae38eb6fc5ab351fb61f2323776819e20538b7 which ensured that
services get freezed on a node reboot had a side effect where running
services did not get gracefully shutdown on node reboot.
This may lead to data loss as the services then get hard killed, or
they may even prevent a node reboot because a storage cannot get
unmounted as a service still access it.

This commits addresses this issue but does not changes behavior of
the freeze logic for now, but we should evaluate if a freeze makes
really sense here or at least make it configurable.

The changed regression test is a result of the fact that we did not
adapt the correct behavior for the is_node_shutdown command in the
problematic commit. The simulation envrionment returned true
everytime a node shutdown (reboot and poweroff) and the real world
environment just returned true if a poweroff happened but not on a
reboot.

Now the simulation acts the same way as the real environment.
Further I moved the simulation implemenentation to the base class so
that both simulator and regression test system behave the same.

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

diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index 382b61ce56638d5148b7132ca5d2e51faed90c40..fdfadd7ca3efacad38e01ad47e4386380b4dd4b2 100644 (file)
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -85,18 +85,21 @@ sub is_node_shutdown {
     my ($self) = @_;
 
     my $shutdown = 0;
+    my $reboot = 0;
 
     my $code = sub {
        my $line = shift;
 
        # ensure we match the full unit name by matching /^JOB_ID UNIT /
-       $shutdown = 1 if ($line =~ m/^\d+\s+(poweroff|halt)\.target\s+/);
+       # see: man systemd.special
+       $shutdown = 1 if ($line =~ m/^\d+\s+shutdown\.target\s+/);
+       $reboot = 1 if ($line =~ m/^\d+\s+reboot\.target\s+/);
     };
 
     my $cmd = ['/bin/systemctl', '--full', 'list-jobs'];
     eval { PVE::Tools::run_command($cmd, outfunc => $code, noerr => 1); };
 
-    return $shutdown;
+    return ($shutdown, $reboot);
 }
 
 sub queue_crm_commands {

diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index 26c5c8986e952b469f8828a1562b6fb4e0795d25..98055ba659deca1a1b427e92742e9e46adf02bf1 100644 (file)
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -51,14 +51,12 @@ sub shutdown_request {
 
     my $nodename = $haenv->nodename();
 
-    my $shutdown = $haenv->is_node_shutdown();
+    my ($shutdown, $reboot) = $haenv->is_node_shutdown();
 
     if ($shutdown) {
-       $haenv->log('info', "shutdown LRM, stop all services");
-       $self->{mode} = 'shutdown';
-
-       # queue stop jobs for all services
-
+       # *always* queue stop jobs for all services if the node shuts down,
+       # independent if it's a reboot or a poweroff, else we may corrupt
+       # services or hinder node shutdown
        my $ss = $self->{service_status};
 
        foreach my $sid (keys %$ss) {
@@ -68,7 +66,16 @@ sub shutdown_request {
            # Note: use undef uid to mark shutdown/stop jobs
            $self->queue_resource_command($sid, undef, 'request_stop');
        }
+    }
 
+    if ($shutdown) {
+       if ($reboot) {
+           $haenv->log('info', "reboot LRM, stop and freeze all services");
+           $self->{mode} = 'restart';
+       } else {
+           $haenv->log('info', "shutdown LRM, stop all services");
+           $self->{mode} = 'shutdown';
+       }
     } else {
        $haenv->log('info', "restart LRM, freeze all services");
        $self->{mode} = 'restart';

diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index cd1574c1c6db0094a0c74222881a96afaada7a1d..fce688a375c11fc19d883660835dbb62e0db1bbf 100644 (file)
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -158,7 +158,25 @@ sub write_lrm_status {
 sub is_node_shutdown {
     my ($self) = @_;
 
-    return 0; # default to freezing services if not overwritten by subclass
+    my $node = $self->{nodename};
+    my $cstatus = $self->{hardware}->read_hardware_status_nolock();
+
+    die "undefined node status for node '$node'" if !defined($cstatus->{$node});
+
+    my ($shutdown, $reboot) = (0, 0);
+
+    if (my $target = $cstatus->{$node}->{shutdown}) {
+       if ($target eq 'shutdown') {
+           $shutdown = 1;
+       } elsif ($target eq 'reboot') {
+           $shutdown = 1;
+           $reboot = 1;
+       } else {
+           die "unknown shutdown target '$target'";
+       }
+    }
+
+    return ($shutdown, $reboot);
 }
 
 sub read_service_config {

diff --git a/src/PVE/HA/Sim/TestEnv.pm b/src/PVE/HA/Sim/TestEnv.pm
index fe08be310b98d7072861cf09bf959c876f467657..0e6eced119979b7fc4cea30e300c2f2d328b4f03 100644 (file)
--- a/src/PVE/HA/Sim/TestEnv.pm
+++ b/src/PVE/HA/Sim/TestEnv.pm
@@ -112,17 +112,6 @@ sub loop_end_hook {
     $self->{cur_time} += 1; # easier for simulation
 }
 
-sub is_node_shutdown {
-    my ($self) = @_;
-
-    my $node = $self->{nodename};
-    my $cstatus = $self->{hardware}->read_hardware_status_nolock();
-
-    die "undefined node status for node '$node'" if !defined($cstatus->{$node});
-
-    return defined($cstatus->{$node}->{shutdown}) ? 1 : 0;
-}
-
 # must be 0 as we do not want to fork in the regression tests
 sub get_max_workers {
     my ($self) = @_;

diff --git a/src/test/test-reboot1/log.expect b/src/test/test-reboot1/log.expect
index 840f56d1bd958a373137eee0c0ac0f445c4e4ae5..733e715637a0a15ce8e069c1478b5322f0b27643 100644 (file)
--- a/src/test/test-reboot1/log.expect
+++ b/src/test/test-reboot1/log.expect
@@ -21,7 +21,8 @@ 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 reboot node3
-info    120    node3/lrm: shutdown LRM, stop all services
+info    120    node3/lrm: reboot LRM, stop and freeze all services
+info    120    node1/crm: service 'vm:103': state changed from 'started' to 'freeze'
 info    125    node3/lrm: stopping service vm:103
 info    125    node3/lrm: service status vm:103 stopped
 info    126    node3/lrm: exit (loop end)
@@ -31,9 +32,10 @@ info    145       reboot: execute power node3 off
 info    145       reboot: execute power node3 on
 info    145    node3/crm: status change startup => wait_for_quorum
 info    140    node3/lrm: status change startup => wait_for_agent_lock
-info    145    node3/lrm: got lock 'ha_agent_node3_lock'
-info    145    node3/lrm: status change wait_for_agent_lock => active
-info    145    node3/lrm: starting service vm:103
-info    145    node3/lrm: service status vm:103 started
+info    160    node1/crm: service 'vm:103': state changed from 'freeze' to 'started'
 info    164    node3/crm: status change wait_for_quorum => slave
+info    165    node3/lrm: got lock 'ha_agent_node3_lock'
+info    165    node3/lrm: status change wait_for_agent_lock => active
+info    165    node3/lrm: starting service vm:103
+info    165    node3/lrm: service status vm:103 started
 info    720     hardware: exit simulation - done