From 4eafdba9fb89b7c62f0aaacb296261459b8f29ed Mon Sep 17 00:00:00 2001
From: Sam Corbett <sam.corbett@cloudsoftcorp.com>
Date: Fri, 22 Dec 2017 13:34:06 +0000
Subject: [PATCH] Fix reporting of master ID when web API used before
 persistence is ready

ServerResource#getHighAvailabilityPlaneStates obtains a new plane sync
record when the master node ID in the first one it obtained is null.
This generally only happens when the web-console tries to obtain the
HA status before the server has started up.

Fixes https://issues.apache.org/jira/browse/BROOKLYN-167.
---
 .../core/mgmt/ha/HighAvailabilityManagerImpl.java  | 14 +++++++-------
 .../brooklyn/rest/resources/ServerResource.java    |  8 ++++++--
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
index 7220091ee3..9205b0ad16 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
@@ -234,7 +234,7 @@ public HighAvailabilityManagerImpl setLocalTicker(Ticker val) {
      * Defaults to null which means to use the remote timestamp. 
      * Only for testing as this records the remote timestamp in the object.
      * <p>
-     * If this is supplied, one must also set {@link ManagementPlaneSyncRecordPersisterToObjectStore#useRemoteTimestampInMemento()}. */
+     * If this is supplied, one must also set {@link ManagementPlaneSyncRecordPersisterToObjectStore#preferRemoteTimestampInMemento()}. */
     @VisibleForTesting
     public HighAvailabilityManagerImpl setRemoteTicker(Ticker val) {
         this.optionalRemoteTickerUtc = val;
@@ -570,11 +570,6 @@ public ManagementNodeState getNodeState() {
         return myNodeState;
     }
 
-    @Override
-    public ManagementPlaneSyncRecord getLastManagementPlaneSyncRecord() {
-        return lastSyncRecord;
-    }
-    
     protected void registerPollTask() {
         final Runnable job = new Runnable() {
             private boolean lastFailed;
@@ -1003,6 +998,11 @@ public ManagementPlaneSyncRecord loadManagementPlaneSyncRecord(boolean useLocalK
         lastSyncRecord = record;
         return record; 
     }
+
+    @Override
+    public ManagementPlaneSyncRecord getLastManagementPlaneSyncRecord() {
+        return lastSyncRecord;
+    }
     
     private ManagementPlaneSyncRecord loadManagementPlaneSyncRecordInternal(boolean useLocalKnowledgeForThisNode) {
         if (disabled) {
@@ -1020,7 +1020,7 @@ private ManagementPlaneSyncRecord loadManagementPlaneSyncRecordInternal(boolean
             LOG.debug("High availablity manager has no persister; returning empty record");
             return ManagementPlaneSyncRecordImpl.builder().build();
         }
-        
+
         int maxLoadAttempts = 5;
         Exception lastException = null;
         Stopwatch timer = Stopwatch.createStarted();
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
index d1215bceae..3c59624e63 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
@@ -29,7 +29,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
-
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
@@ -52,6 +51,7 @@
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.StartableApplication;
 import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
+import org.apache.brooklyn.core.mgmt.ShutdownHandler;
 import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.mgmt.persist.BrooklynPersistenceUtils;
@@ -63,7 +63,6 @@
 import org.apache.brooklyn.rest.domain.VersionSummary;
 import org.apache.brooklyn.rest.transform.BrooklynFeatureTransformer;
 import org.apache.brooklyn.rest.transform.HighAvailabilityTransformer;
-import org.apache.brooklyn.core.mgmt.ShutdownHandler;
 import org.apache.brooklyn.rest.util.WebResourceUtils;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.ResourceUtils;
@@ -442,6 +441,11 @@ public HighAvailabilitySummary getHighAvailabilityPlaneStates() {
         ManagementPlaneSyncRecord memento = mgmt().getHighAvailabilityManager().getLastManagementPlaneSyncRecord();
         if (memento==null) memento = mgmt().getHighAvailabilityManager().loadManagementPlaneSyncRecord(true);
         if (memento==null) return null;
+        // This may be the case if this method was called before persistence was properly initialised.
+        // Retry so that the server doesn't get stuck. See https://issues.apache.org/jira/browse/BROOKLYN-167.
+        if (memento.getMasterNodeId() == null) {
+            memento = mgmt().getHighAvailabilityManager().loadManagementPlaneSyncRecord(true);
+        }
         return HighAvailabilityTransformer.highAvailabilitySummary(mgmt().getManagementNodeId(), memento);
     }