preserve prior server state if new is unknown

Author: russgold

operator/src/main/java/oracle/kubernetes/operator/DomainStatusUpdater.java

@@ -426,10 +426,7 @@ private ServerStatus createServerStatus(String serverName, boolean isAdminServer
       }
 
       private String getRunningState(String serverName) {
-        if (serverState != null) {
-          return serverState.getOrDefault(serverName, SHUTDOWN_STATE);
-        }
-        return SHUTDOWN_STATE;
+        return Optional.ofNullable(serverState).map(m -> m.get(serverName)).orElse(null);
       }
 
       private String getDesiredState(String serverName, String clusterName, boolean isAdminServer) {

operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainStatus.java

@@ -25,6 +25,7 @@
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.joda.time.DateTime;
 
+import static oracle.kubernetes.operator.WebLogicConstants.SHUTDOWN_STATE;
 import static oracle.kubernetes.weblogic.domain.model.ObjectPatch.createObjectPatch;
 
 /**
@@ -51,7 +52,7 @@ public class DomainStatus {
   @Description("Status of WebLogic Servers in this domain.")
   @Valid
   // sorted list of ServerStatus
-  List<ServerStatus> servers = new ArrayList<>();
+  private final List<ServerStatus> servers;
 
   @Description("Status of WebLogic clusters in this domain.")
   @Valid
@@ -72,6 +73,7 @@ public class DomainStatus {
   private Integer replicas;
 
   public DomainStatus() {
+    servers = new ArrayList<>();
   }
 
   /**
@@ -293,28 +295,48 @@ public List<ServerStatus> getServers() {
    */
   public void setServers(List<ServerStatus> servers) {
     synchronized (this.servers) {
-      if (isServersEqualIgnoringOrder(servers, this.servers)) {
+      if (this.servers.equals(servers)) {
         return;
       }
-      List<ServerStatus> sortedServers = new ArrayList<>(servers);
-      sortedServers.sort(Comparator.naturalOrder());
 
-      this.servers = sortedServers;
+      List<ServerStatus> newServers = servers
+            .stream()
+            .map(ServerStatus::new)
+            .map(this::adjust)
+            .sorted(Comparator.naturalOrder())
+            .collect(Collectors.toList());
+
+      this.servers.clear();
+      this.servers.addAll(newServers);
     }
   }
 
-  private boolean isServersEqualIgnoringOrder(List<ServerStatus> servers1, List<ServerStatus> servers2) {
-    return new HashSet<>(servers1).equals(new HashSet<>(servers2));
+  private ServerStatus adjust(ServerStatus server) {
+    if (server.getState() == null) {
+      ServerStatus oldServer = getMatchingServer(server);
+      server.setState(oldServer == null ? SHUTDOWN_STATE : oldServer.getState());
+    }
+    return server;
   }
 
+  private ServerStatus getMatchingServer(ServerStatus server) {
+    return getServers()
+          .stream()
+          .filter(s -> s.getClusterName().equals(server.getClusterName()))
+          .filter(s -> s.getServerName().equals(server.getServerName()))
+          .findFirst()
+          .orElse(null);
+  }
+
+
   /**
    * Status of WebLogic servers in this domain.
    *
    * @param servers servers
    * @return this
    */
   public DomainStatus withServers(List<ServerStatus> servers) {
-    this.servers = servers;
+    setServers(servers);
     return this;
   }
 

operator/src/test/java/oracle/kubernetes/weblogic/domain/model/DomainStatusTest.java

@@ -14,6 +14,7 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import static oracle.kubernetes.operator.WebLogicConstants.SHUTDOWN_STATE;
 import static oracle.kubernetes.weblogic.domain.model.DomainConditionMatcher.hasCondition;
 import static oracle.kubernetes.weblogic.domain.model.DomainConditionType.Available;
 import static oracle.kubernetes.weblogic.domain.model.DomainConditionType.Failed;
@@ -259,25 +260,86 @@ public void verifyThat_addServers_serverSortedInExpectedOrdering() {
     domainStatus.addServer(cluster1Server1).addServer(cluster2Server1)
         .addServer(cluster1Server2).addServer(standAloneServerA).addServer(adminServer);
 
-    assertThat(domainStatus.servers,
+    assertThat(domainStatus.getServers(),
         contains(adminServer, standAloneServerA, cluster1Server1, cluster1Server2, cluster2Server1));
   }
 
   @Test
   public void verifyThat_setServers_serverSortedInExpectedOrdering() {
-    ServerStatus cluster1Server1 = new ServerStatus().withClusterName("cluster-1").withServerName("cluster1-server1");
-    ServerStatus cluster1Server2 = new ServerStatus().withClusterName("cluster-1").withServerName("cluster1-server2");
-    ServerStatus cluster2Server1 = new ServerStatus().withClusterName("cluster-2").withServerName("cluster2-server1");
-    ServerStatus adminServer = new ServerStatus().withServerName("admin-server").withIsAdminServer(true);
-    ServerStatus standAloneServerA = new ServerStatus().withServerName("a");
+    ServerStatus cluster1Server1 = createStatus().withClusterName("cluster-1").withServerName("cluster1-server1");
+    ServerStatus cluster1Server2 = createStatus().withClusterName("cluster-1").withServerName("cluster1-server2");
+    ServerStatus cluster2Server1 = createStatus().withClusterName("cluster-2").withServerName("cluster2-server1");
+    ServerStatus adminServer = createStatus().withServerName("admin-server").withIsAdminServer(true);
+    ServerStatus standAloneServerA = createStatus().withServerName("a");
 
     domainStatus.setServers(Arrays.asList(cluster1Server1,
         cluster2Server1, cluster1Server2, standAloneServerA, adminServer));
 
-    assertThat(domainStatus.servers,
+    assertThat(domainStatus.getServers(),
         contains(adminServer, standAloneServerA, cluster1Server1, cluster1Server2, cluster2Server1));
   }
 
+  private ServerStatus createStatus() {
+    return new ServerStatus().withState("a");
+  }
+
+  @Test
+  public void whenMatchingServersExist_setServersUpdatesState() {
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("1").withState("state1"));
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("2").withState("state1"));
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("3").withState("state1"));
+    
+    domainStatus.setServers(Arrays.asList(
+          new ServerStatus().withClusterName("1").withServerName("1").withState("state1"),
+          new ServerStatus().withClusterName("1").withServerName("2").withState("state1"),
+          new ServerStatus().withClusterName("1").withServerName("3").withState("state2")
+    ));
+
+    assertThat(getServer("1", "1").getState(), equalTo("state1"));
+    assertThat(getServer("1", "2").getState(), equalTo("state1"));
+    assertThat(getServer("1", "3").getState(), equalTo("state2"));
+  }
+
+  @Test
+  public void whenSetServerIncludesServerWithoutStateAndNoExistingState_defaultToSHUTDOWN() {
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("1").withState("state1"));
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("2").withState("state1"));
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("3").withState("state1"));
+
+    domainStatus.setServers(Arrays.asList(
+          new ServerStatus().withClusterName("1").withServerName("1").withState("state1"),
+          new ServerStatus().withClusterName("1").withServerName("2").withState("state1"),
+          new ServerStatus().withClusterName("1").withServerName("3").withState("state2"),
+          new ServerStatus().withClusterName("2").withServerName("1")
+    ));
+
+    assertThat(getServer("2", "1").getState(), equalTo(SHUTDOWN_STATE));
+  }
+
+  @Test
+  public void whenSetServerIncludesServerWithoutStateAndHasExistingState_preserveIt() {
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("1").withState("state1"));
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("2").withState("state1"));
+    domainStatus.addServer(new ServerStatus().withClusterName("1").withServerName("3").withState("state1"));
+
+    domainStatus.setServers(Arrays.asList(
+          new ServerStatus().withClusterName("1").withServerName("1").withState("state1"),
+          new ServerStatus().withClusterName("1").withServerName("2").withState("state1"),
+          new ServerStatus().withClusterName("1").withServerName("3")
+    ));
+
+    assertThat(getServer("1", "3").getState(), equalTo("state1"));
+  }
+
+  private ServerStatus getServer(String clusterName, String serverName) {
+    return domainStatus.getServers()
+          .stream()
+          .filter(s -> s.getClusterName().equals(clusterName))
+          .filter(s -> s.getServerName().equals(serverName))
+          .findFirst()
+          .orElse(null);
+  }
+
   @Test
   public void verifyThat_getServers_serverInExpectedOrdering() {
     ServerStatus cluster1Server1 = new ServerStatus().withClusterName("cluster-1").withServerName("cluster1-server1");
@@ -303,7 +365,7 @@ public void verifyThat_addClusters_clustersSortedInExpectedOrdering() {
 
     domainStatus.addCluster(cluster10).addCluster(cluster1).addCluster(cluster2);
 
-    assertThat(domainStatus.clusters, contains(cluster1, cluster2, cluster10));
+    assertThat(domainStatus.getClusters(), contains(cluster1, cluster2, cluster10));
   }
 
   @Test
@@ -314,9 +376,7 @@ public void verifyThat_setClusters_clustersSortedInExpectedOrdering() {
 
     domainStatus.setClusters(Arrays.asList(cluster10, cluster1, cluster2));
 
-    List<ClusterStatus> clusterStatuses = domainStatus.clusters;
-
-    assertThat(clusterStatuses, contains(cluster1, cluster2, cluster10));
+    assertThat(domainStatus.getClusters(), contains(cluster1, cluster2, cluster10));
   }
 
   @Test