CRM: refactor check if state transition to active is ok

Mainly addresses a problem where we read the manager status without
catching any possible exceptions.

As this was done only to check if our node has active fencing jobs,
which tells us that it makes no sense to even try to acquire the
manager lock - as we're fenced soon anyway.
Besides this check we always checked if we're quorate and if there
are services configured, so move
both checks in the new 'can_get_active' method, which replaces the
check_pending_fencing and the has_services method.

Move the quorum check in front and catch a possible error from the
following manager status read.
As a side effect the state transition code gets a bit shorter without
hiding the check intention.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

diff --git a/src/PVE/HA/CRM.pm b/src/PVE/HA/CRM.pm
index 3dac3acf5cf3da4560406276ef5fed8f82282a0e..2d70570d52fef21b83d8902d5f65df8323f61682 100644 (file)
--- a/src/PVE/HA/CRM.pm
+++ b/src/PVE/HA/CRM.pm
@@ -111,14 +111,32 @@ sub get_protected_ha_manager_lock {
     return 0;
 }
 
-sub check_pending_fencing {
-    my ($manager_status, $node) = @_;
+# checks quorum, for no active pending fence jobs and if services are configured
+sub can_get_active {
+    my ($self, $allow_no_service) = @_;
 
+    my $haenv = $self->{haenv};
+
+    return 0 if !$haenv->quorate();
+
+    my $manager_status = eval { $haenv->read_manager_status() };
+    if (my $err = $@) {
+       $haenv->log('err', "could not read manager status: $err");
+       return 0;
+    }
     my $ss = $manager_status->{service_status};
+    return 0 if PVE::HA::Tools::count_fenced_services($ss, $haenv->nodename());
 
-    return 1 if PVE::HA::Tools::count_fenced_services($ss, $node);
+    if (!$allow_no_service) {
+       my $conf = eval { $haenv->read_service_config() };
+       if (my $err = $@) {
+           $haenv->log('err', "could not read service config: $err");
+           return undef;
+       }
+       return 0 if !scalar(%{$conf});
+    }
 
-    return 0;
+    return 1;
 }
 
 sub do_one_iteration {
@@ -129,15 +147,11 @@ sub do_one_iteration {
     my $status = $self->get_local_status();
     my $state = $status->{state};
 
-    my $manager_status = $haenv->read_manager_status();
-    my $pending_fencing = check_pending_fencing($manager_status, $haenv->nodename());
-    
     # do state changes first 
 
     if ($state eq 'wait_for_quorum') {
 
-       if (!$pending_fencing && $haenv->quorate() &&
-           PVE::HA::Tools::has_services($haenv)) {
+       if ($self->can_get_active()) {
            if ($self->get_protected_ha_manager_lock()) {
                $self->set_local_status({ state => 'master' });
            } else {
@@ -147,8 +161,7 @@ sub do_one_iteration {
 
     } elsif ($state eq 'slave') {
 
-       if (!$pending_fencing && $haenv->quorate() &&
-           PVE::HA::Tools::has_services($haenv)) {
+       if ($self->can_get_active()) {
            if ($self->get_protected_ha_manager_lock()) {
                $self->set_local_status({ state => 'master' });
            }
@@ -158,7 +171,7 @@ sub do_one_iteration {
 
     } elsif ($state eq 'lost_manager_lock') {
 
-       if (!$pending_fencing && $haenv->quorate()) {
+       if ($self->can_get_active(1)) {
            if ($self->get_protected_ha_manager_lock()) {
                $self->set_local_status({ state => 'master' });
            }

diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
index 0f49bc5e8986772d58139b2eb72daafe5bfbeff9..88f775eadc63c79f534b6c914fa543c96d4375e3 100644 (file)
--- a/src/PVE/HA/Tools.pm
+++ b/src/PVE/HA/Tools.pm
@@ -148,21 +148,6 @@ sub write_json_to_file {
     PVE::Tools::file_set_contents($filename, $raw);
 }
 
-sub has_services {
-    my ($haenv, $node) = @_;
-
-    my $conf = $haenv->read_service_config();
-
-    # if no node defined any service count is fine
-    return scalar(%{$conf}) if !defined($node);
-
-    foreach my $d (values %$conf) {
-       return 1 if $d->{node} eq $node;
-    }
-
-    return undef;
-}
-
 sub count_fenced_services {
     my ($ss, $node) = @_;