Merge pull request #1069 from oracle/readhealthstep_logging

rjeberhard · web-flow · commit 7e7715aee8af · 2019-05-09T06:03:52.000-04:00
ReadHealthStep - Logging improvement and handling of server not available (OWLS-73109)
diff --git a/docs/domains/Domain.json b/docs/domains/Domain.json
@@ -407,7 +407,7 @@
       "type": "object",
       "properties": {
         "overallHealth": {
-          "description": "Server health of this WebLogic server.",
+          "description": "Server health of this WebLogic server. If the value is \"Not available\", the operator has failed to read the health. If the value is \"Not available (possibly overloaded)\", the operator has failed to read the health of the server possibly due to the server is in overloaded state",
           "type": "string"
         },
         "activationTime": {
diff --git a/docs/domains/Domain.md b/docs/domains/Domain.md
@@ -173,7 +173,7 @@ Shutdown describes the configuration for shutting down a server instance.
 | Name | Type | Description |
 | --- | --- | --- |
 | `activationTime` | DateTime | RFC 3339 date and time at which the server started. |
-| `overallHealth` | string | Server health of this WebLogic server. |
+| `overallHealth` | string | Server health of this WebLogic server. If the value is "Not available", the operator has failed to read the health. If the value is "Not available (possibly overloaded)", the operator has failed to read the health of the server possibly due to the server is in overloaded state |
 | `subsystems` | array of [Subsystem Health](#subsystem-health) | Status of unhealthy subsystems, if any. |
 
 ### Channel
diff --git a/docs/domains/index.html b/docs/domains/index.html
@@ -1327,7 +1327,7 @@
       "type": "object",
       "properties": {
         "overallHealth": {
-          "description": "Server health of this WebLogic server.",
+          "description": "Server health of this WebLogic server. If the value is \"Not available\", the operator has failed to read the health. If the value is \"Not available (possibly overloaded)\", the operator has failed to read the health of the server possibly due to the server is in overloaded state",
           "type": "string"
         },
         "activationTime": {
diff --git a/model/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerHealth.java b/model/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerHealth.java
@@ -23,7 +23,11 @@ public class ServerHealth {
   @Expose
   private DateTime activationTime;
 
-  @Description("Server health of this WebLogic server.")
+  @Description(
+      "Server health of this WebLogic server. If the value is \"Not available\", the operator has "
+          + "failed to read the health. If the value is \"Not available (possibly overloaded)\", the "
+          + "operator has failed to read the health of the server possibly due to the server is "
+          + "in overloaded state")
   @SerializedName("overallHealth")
   @Expose
   private String overallHealth;
diff --git a/operator/src/main/java/oracle/kubernetes/operator/DomainProcessorImpl.java b/operator/src/main/java/oracle/kubernetes/operator/DomainProcessorImpl.java
@@ -21,6 +21,7 @@
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 import javax.annotation.Nullable;
 import oracle.kubernetes.operator.TuningParameters.MainTuning;
 import oracle.kubernetes.operator.calls.CallResponse;
@@ -314,7 +315,9 @@ public void run() {
                   new CompletionCallback() {
                     @Override
                     public void onCompletion(Packet packet) {
-                      if (Boolean.TRUE.equals(packet.get(ProcessingConstants.SERVER_HEALTH_READ))) {
+                      AtomicInteger serverHealthRead =
+                          packet.getValue(ProcessingConstants.REMAINING_SERVERS_HEALTH_TO_READ);
+                      if (serverHealthRead == null || serverHealthRead.get() == 0) {
                         loggingFilter.setFiltering(false).resetLogHistory();
                       } else {
                         loggingFilter.setFiltering(true);
diff --git a/operator/src/main/java/oracle/kubernetes/operator/ProcessingConstants.java b/operator/src/main/java/oracle/kubernetes/operator/ProcessingConstants.java
@@ -32,5 +32,5 @@ public interface ProcessingConstants {
   public static final String DOMAIN_INTROSPECTOR_LOG_RESULT = "domainIntrospectorLogResult";
   public static final String SIT_CONFIG_MAP = "sitConfigMap";
 
-  public static final String SERVER_HEALTH_READ = "serverHealthRead";
+  public static final String REMAINING_SERVERS_HEALTH_TO_READ = "serverHealthRead";
 }
diff --git a/operator/src/main/java/oracle/kubernetes/operator/ServerStatusReader.java b/operator/src/main/java/oracle/kubernetes/operator/ServerStatusReader.java
@@ -21,6 +21,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import oracle.kubernetes.operator.helpers.ClientPool;
@@ -68,6 +69,9 @@ public NextAction apply(Packet packet) {
       packet.put(SERVER_STATE_MAP, new ConcurrentHashMap<String, String>());
       packet.put(SERVER_HEALTH_MAP, new ConcurrentHashMap<String, ServerHealth>());
 
+      AtomicInteger remainingServerHealthToRead = new AtomicInteger();
+      packet.put(ProcessingConstants.REMAINING_SERVERS_HEALTH_TO_READ, remainingServerHealthToRead);
+
       Collection<StepAndPacket> startDetails =
           info.getServerPods()
               .map(pod -> createStatusReaderStep(packet, pod))
@@ -76,6 +80,7 @@ public NextAction apply(Packet packet) {
       if (startDetails.isEmpty()) {
         return doNext(packet);
       } else {
+        remainingServerHealthToRead.set(startDetails.size());
         return doForkJoin(getNext(), packet, startDetails);
       }
     }
diff --git a/operator/src/main/java/oracle/kubernetes/operator/http/HttpClient.java b/operator/src/main/java/oracle/kubernetes/operator/http/HttpClient.java
@@ -36,10 +36,6 @@ public class HttpClient {
   private static final String HTTP_PROTOCOL = "http://";
   private static final String HTTPS_PROTOCOL = "https://";
 
-  // for debugging
-  private static final String SERVICE_URL =
-      System.getProperty("oracle.kubernetes.operator.http.HttpClient.SERVICE_URL");
-
   // Please use one of the factory methods to get an instance of HttpClient.
   // Constructor is package access for unit testing
   HttpClient(Client httpClient, String encodedCredentials) {
diff --git a/operator/src/main/java/oracle/kubernetes/operator/http/Result.java b/operator/src/main/java/oracle/kubernetes/operator/http/Result.java
@@ -1,4 +1,4 @@
-// Copyright 2018, Oracle Corporation and/or its affiliates.  All rights reserved.
+// Copyright 2018, 2019 Oracle Corporation and/or its affiliates.  All rights reserved.
 // Licensed under the Universal Permissive License v 1.0 as shown at
 // http://oss.oracle.com/licenses/upl.
 
@@ -37,6 +37,14 @@ public boolean isSuccessful() {
     return successful;
   }
 
+  /**
+   * @return true if the HTTP status code from the REST request indicates that the server may be
+   *     overloaded
+   */
+  public boolean isServerOverloaded() {
+    return status == 500 || status == 503;
+  }
+
   @Override
   public String toString() {
     return "Result{"
diff --git a/operator/src/main/java/oracle/kubernetes/operator/steps/ReadHealthStep.java b/operator/src/main/java/oracle/kubernetes/operator/steps/ReadHealthStep.java
@@ -17,10 +17,13 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicInteger;
 import oracle.kubernetes.operator.Pair;
 import oracle.kubernetes.operator.ProcessingConstants;
+import oracle.kubernetes.operator.WebLogicConstants;
 import oracle.kubernetes.operator.helpers.DomainPresenceInfo;
 import oracle.kubernetes.operator.http.HttpClient;
+import oracle.kubernetes.operator.http.Result;
 import oracle.kubernetes.operator.logging.LoggingFacade;
 import oracle.kubernetes.operator.logging.LoggingFactory;
 import oracle.kubernetes.operator.logging.LoggingFilter;
@@ -43,6 +46,10 @@ public class ReadHealthStep extends Step {
 
   private static final LoggingFacade LOGGER = LoggingFactory.getLogger("Operator", "Operator");
 
+  public static final String OVERALL_HEALTH_NOT_AVAILABLE = "Not available";
+  public static final String OVERALL_HEALTH_FOR_SERVER_OVERLOADED =
+      OVERALL_HEALTH_NOT_AVAILABLE + " (possibly overloaded)";
+
   private ReadHealthStep(Step next) {
     super(next);
   }
@@ -139,16 +146,14 @@ public NextAction apply(Packet packet) {
                   serverConfig.getAdminProtocolChannelName(),
                   serverConfig.getListenPort());
           if (serviceURL != null) {
-            String jsonResult =
-                httpClient
-                    .executePostUrlOnServiceClusterIP(
-                        getRetrieveHealthSearchUrl(),
-                        serviceURL,
-                        getRetrieveHealthSearchPayload(),
-                        true)
-                    .getResponse();
+            Result result =
+                httpClient.executePostUrlOnServiceClusterIP(
+                    getRetrieveHealthSearchUrl(),
+                    serviceURL,
+                    getRetrieveHealthSearchPayload(),
+                    false);
 
-            Pair<String, ServerHealth> pair = parseServerHealthJson(jsonResult);
+            Pair<String, ServerHealth> pair = createServerHealthFromResult(result);
 
             String state = pair.getLeft();
             if (state != null && !state.isEmpty()) {
@@ -162,9 +167,12 @@ public NextAction apply(Packet packet) {
             ConcurrentMap<String, ServerHealth> serverHealthMap =
                 (ConcurrentMap<String, ServerHealth>)
                     packet.get(ProcessingConstants.SERVER_HEALTH_MAP);
+
             serverHealthMap.put(
                 (String) packet.get(ProcessingConstants.SERVER_NAME), pair.getRight());
-            packet.put(ProcessingConstants.SERVER_HEALTH_READ, Boolean.TRUE);
+            AtomicInteger remainingServersHealthToRead =
+                packet.getValue(ProcessingConstants.REMAINING_SERVERS_HEALTH_TO_READ);
+            remainingServersHealthToRead.getAndDecrement();
           }
         }
         return doNext(packet);
@@ -179,6 +187,20 @@ public NextAction apply(Packet packet) {
       }
     }
 
+    private Pair<String, ServerHealth> createServerHealthFromResult(Result restResult)
+        throws IOException {
+      if (restResult.isSuccessful()) {
+        return parseServerHealthJson(restResult.getResponse());
+      }
+      return new Pair<>(
+          WebLogicConstants.UNKNOWN_STATE,
+          new ServerHealth()
+              .withOverallHealth(
+                  restResult.isServerOverloaded()
+                      ? OVERALL_HEALTH_FOR_SERVER_OVERLOADED
+                      : OVERALL_HEALTH_NOT_AVAILABLE));
+    }
+
     private Pair<String, ServerHealth> parseServerHealthJson(String jsonResult) throws IOException {
       if (jsonResult == null) return null;
 
diff --git a/operator/src/test/java/oracle/kubernetes/operator/ServerStatusReaderTest.java b/operator/src/test/java/oracle/kubernetes/operator/ServerStatusReaderTest.java
@@ -9,6 +9,7 @@
 import static oracle.kubernetes.operator.ProcessingConstants.SERVER_STATE_MAP;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.hasEntry;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.junit.MatcherAssert.assertThat;
 
 import com.meterware.pseudoserver.HttpUserAgentTest;
@@ -27,6 +28,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import oracle.kubernetes.TestUtils;
 import oracle.kubernetes.operator.helpers.DomainPresenceInfo;
@@ -119,6 +121,19 @@ public void whenServerPodsReturnNodeManagerStatus_recordInStateMap() {
     assertThat(serverStates, hasEntry("server2", "server2 status"));
   }
 
+  @Test
+  public void createDomainStatusReaderStep_initializesRemainingServersHealthRead_withNumServers() {
+    info.setServerPod("server1", createPod("server1"));
+    info.setServerPod("server2", createPod("server2"));
+
+    Packet packet =
+        testSupport.runSteps(ServerStatusReader.createDomainStatusReaderStep(info, 0, endStep));
+
+    assertThat(
+        ((AtomicInteger) packet.get(ProcessingConstants.REMAINING_SERVERS_HEALTH_TO_READ)).get(),
+        is(2));
+  }
+
   @SuppressWarnings("unchecked")
   private Map<String, String> getServerStates(Packet packet) {
     return (Map<String, String>) packet.get(SERVER_STATE_MAP);
diff --git a/operator/src/test/java/oracle/kubernetes/operator/http/HttpClientStub.java b/operator/src/test/java/oracle/kubernetes/operator/http/HttpClientStub.java
@@ -6,7 +6,9 @@
 
 public abstract class HttpClientStub extends HttpClient {
 
-  final String RESPONSE = "{}";
+  String response = "{}";
+  int status = 200;
+  boolean successful = true;
 
   public HttpClientStub() {
     super(null, null);
@@ -16,6 +18,21 @@ public HttpClientStub() {
   public Result executePostUrlOnServiceClusterIP(
       String requestUrl, String serviceURL, String payload, boolean throwOnFailure)
       throws HTTPException {
-    return new Result(RESPONSE, 200, true);
+    return new Result(response, status, successful);
+  }
+
+  public HttpClientStub withResponse(String response) {
+    this.response = response;
+    return this;
+  }
+
+  public HttpClientStub withStatus(int status) {
+    this.status = status;
+    return this;
+  }
+
+  public HttpClientStub withSuccessful(boolean successful) {
+    this.successful = successful;
+    return this;
   }
 }
diff --git a/operator/src/test/java/oracle/kubernetes/operator/http/ResultTest.java b/operator/src/test/java/oracle/kubernetes/operator/http/ResultTest.java
@@ -0,0 +1,31 @@
+// Copyright 2019 Oracle Corporation and/or its affiliates.  All rights reserved.
+// Licensed under the Universal Permissive License v 1.0 as shown at
+// http://oss.oracle.com/licenses/upl.
+
+package oracle.kubernetes.operator.http;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+public class ResultTest {
+
+  @Test
+  public void verifyIsServerOverloadedReturnsTrue() {
+    Result result500 = new Result("response", 500, false);
+    assertThat(result500.isServerOverloaded(), is(true));
+
+    Result result503 = new Result("response", 503, false);
+    assertThat(result503.isServerOverloaded(), is(true));
+  }
+
+  @Test
+  public void verifyIsOverloadedReturnsFalseForOtherResponseCode() {
+    Result result404 = new Result("response", 404, false);
+    assertThat(result404.isServerOverloaded(), is(false));
+
+    Result result200 = new Result("response", 200, false);
+    assertThat(result200.isServerOverloaded(), is(false));
+  }
+}
diff --git a/operator/src/test/java/oracle/kubernetes/operator/steps/ReadHealthStepTest.java b/operator/src/test/java/oracle/kubernetes/operator/steps/ReadHealthStepTest.java

Original file line number	Diff line number	Diff line change
`@@ -32,5 +32,5 @@ public interface ProcessingConstants {`
`32`	`32`	`public static final String DOMAIN_INTROSPECTOR_LOG_RESULT = "domainIntrospectorLogResult";`
`33`	`33`	`public static final String SIT_CONFIG_MAP = "sitConfigMap";`
`34`	`34`
`35`		`- public static final String SERVER_HEALTH_READ = "serverHealthRead";`
	`35`	`+ public static final String REMAINING_SERVERS_HEALTH_TO_READ = "serverHealthRead";`
`36`	`36`	`}`