OWLS-86407 - Fixed the logic to start clustered managed server pods when admin server pod not running (#2093)

* OWLS-86407 - Fixed logic to start clustered managed server pods when admin server pod is not running

* minor change

* Add a unit test case for OWLS-86407

* changes to address PR review comments.

* PR review comments.

* Remove redundant condition.

* Refactoring change as per the review comment.

* Chnaged method name to be consistent.

Author: ankedia

operator/src/main/java/oracle/kubernetes/operator/helpers/DomainPresenceInfo.java

@@ -34,6 +34,7 @@
 
 import static java.lang.System.lineSeparator;
 import static oracle.kubernetes.operator.helpers.PodHelper.hasClusterNameOrNull;
+import static oracle.kubernetes.operator.helpers.PodHelper.isNotAdminServer;
 
 /**
  * Operator's mapping between custom resource Domain and runtime details about that domain,
@@ -101,27 +102,60 @@ private static <K, V> boolean removeIfPresentAnd(
    */
   public long getNumScheduledServers(String clusterName) {
     return getServersInNoOtherCluster(clusterName)
+            .filter(PodHelper::isScheduled)
+            .count();
+  }
+
+  /**
+   * Counts the number of unclustered managed servers and managed servers in the specified cluster that are scheduled.
+   * @param clusterName cluster name of the pod server
+   * @param adminServerName Name of the admin server
+   * @return Number of scheduled managed servers
+   */
+  public long getNumScheduledManagedServers(String clusterName, String adminServerName) {
+    return getManagedServersInNoOtherCluster(clusterName, adminServerName)
           .filter(PodHelper::isScheduled)
           .count();
   }
 
   /**
-   * Counts the number of unclustered servers and servers in the specified cluster that are ready.
+   * Counts the number of unclustered servers (including admin) and servers in the specified cluster that are ready.
    * @param clusterName cluster name of the pod server
    * @return Number of ready servers
    */
   public long getNumReadyServers(String clusterName) {
     return getServersInNoOtherCluster(clusterName)
+            .filter(PodHelper::hasReadyServer)
+            .count();
+  }
+
+  /**
+   * Counts the number of unclustered managed servers and managed servers in the specified cluster that are ready.
+   * @param clusterName cluster name of the pod server
+   * @return Number of ready servers
+   */
+  public long getNumReadyManagedServers(String clusterName, String adminServerName) {
+    return getManagedServersInNoOtherCluster(clusterName, adminServerName)
           .filter(PodHelper::hasReadyServer)
           .count();
   }
 
   @Nonnull
   private Stream<V1Pod> getServersInNoOtherCluster(String clusterName) {
+    return getServers().values().stream()
+            .map(ServerKubernetesObjects::getPod)
+            .map(AtomicReference::get)
+            .filter(this::isNotDeletingPod)
+            .filter(p -> hasClusterNameOrNull(p, clusterName));
+  }
+
+  @Nonnull
+  private Stream<V1Pod> getManagedServersInNoOtherCluster(String clusterName, String adminServerName) {
     return getServers().values().stream()
           .map(ServerKubernetesObjects::getPod)
           .map(AtomicReference::get)
           .filter(this::isNotDeletingPod)
+          .filter(p -> isNotAdminServer(p, adminServerName))
           .filter(p -> hasClusterNameOrNull(p, clusterName));
   }
 
@@ -755,4 +789,4 @@ public int hashCode() {
               .toHashCode();
     }
   }
-}
+}

operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java

@@ -39,6 +39,7 @@
 import oracle.kubernetes.weblogic.domain.model.Shutdown;
 
 import static oracle.kubernetes.operator.LabelConstants.CLUSTERNAME_LABEL;
+import static oracle.kubernetes.operator.LabelConstants.SERVERNAME_LABEL;
 import static oracle.kubernetes.operator.ProcessingConstants.SERVERS_TO_ROLL;
 
 public class PodHelper {
@@ -114,6 +115,22 @@ private static String getClusterName(@Nonnull Map<String,String> labels) {
     return labels.get(CLUSTERNAME_LABEL);
   }
 
+  static boolean isNotAdminServer(@Nullable V1Pod pod, String adminServerName) {
+    return Optional.ofNullable(getServerName(pod)).map(s -> !s.equals(adminServerName)).orElse(true);
+  }
+
+  private static String getServerName(@Nullable V1Pod pod) {
+    return Optional.ofNullable(pod)
+            .map(V1Pod::getMetadata)
+            .map(V1ObjectMeta::getLabels)
+            .map(PodHelper::getServerName)
+            .orElse(null);
+  }
+
+  private static String getServerName(@Nonnull Map<String,String> labels) {
+    return labels.get(SERVERNAME_LABEL);
+  }
+
   /**
    * get if pod is in ready state.
    * @param pod pod

operator/src/main/java/oracle/kubernetes/operator/steps/ManagedServerUpIteratorStep.java

@@ -24,11 +24,14 @@
 import oracle.kubernetes.operator.helpers.ServiceHelper;
 import oracle.kubernetes.operator.logging.LoggingFacade;
 import oracle.kubernetes.operator.logging.LoggingFactory;
+import oracle.kubernetes.operator.wlsconfig.WlsDomainConfig;
 import oracle.kubernetes.operator.work.NextAction;
 import oracle.kubernetes.operator.work.Packet;
 import oracle.kubernetes.operator.work.Step;
 import oracle.kubernetes.weblogic.domain.model.Domain;
 
+import static oracle.kubernetes.operator.ProcessingConstants.DOMAIN_TOPOLOGY;
+
 /**
  * A step which will bring up the specified managed servers in parallel.
  * Adds to packet:
@@ -168,22 +171,35 @@ public NextAction apply(Packet packet) {
 
       if (startDetailsQueue.isEmpty()) {
         return doNext(new ManagedServerUpAfterStep(getNext()), packet);
-      } else if (hasServerAvailableToStart(packet.getSpi(DomainPresenceInfo.class))) {
+      } else if (hasServerAvailableToStart(packet)) {
         numStarted.getAndIncrement();
         return doForkJoin(this, packet, Collections.singletonList(startDetailsQueue.poll()));
       } else {
         return doDelay(this, packet, SCHEDULING_DETECTION_DELAY, TimeUnit.MILLISECONDS);
       }
     }
 
-    private boolean hasServerAvailableToStart(DomainPresenceInfo info) {
-      return ((numStarted.get() < info.getNumScheduledServers(clusterName))
-              && (canStartConcurrently(info.getNumReadyServers(clusterName))));
+    private boolean hasServerAvailableToStart(Packet packet) {
+      DomainPresenceInfo info = packet.getSpi(DomainPresenceInfo.class);
+      String adminServerName = ((WlsDomainConfig) packet.get(DOMAIN_TOPOLOGY)).getAdminServerName();
+      return ((getNumServersStarted() <= info.getNumScheduledManagedServers(clusterName, adminServerName)
+              && (canStartConcurrently(info.getNumReadyManagedServers(clusterName, adminServerName)))));
     }
 
     private boolean canStartConcurrently(long numReady) {
-      return ((this.maxConcurrency > 0) && (numStarted.get() < (this.maxConcurrency + numReady - 1)))
-          || (this.maxConcurrency == 0);
+      return (ignoreConcurrencyLimits() || numNotReady(numReady) < this.maxConcurrency);
+    }
+
+    private long numNotReady(long numReady) {
+      return getNumServersStarted() - numReady;
+    }
+
+    private int getNumServersStarted() {
+      return numStarted.get();
+    }
+
+    private boolean ignoreConcurrencyLimits() {
+      return this.maxConcurrency == 0;
     }
   }
 

operator/src/test/java/oracle/kubernetes/operator/steps/ManagedServerUpIteratorStepTest.java

@@ -94,7 +94,7 @@ private static String getManagedServerName(int n) {
   private final Step nextStep = new TerminalStep();
   private final KubernetesTestSupport testSupport = new KubernetesTestSupport();
   private final List<Memento> mementos = new ArrayList<>();
-  private final DomainPresenceInfo domainPresenceInfo = createDomainPresenceInfoWithServers();
+  private DomainPresenceInfo domainPresenceInfo = createDomainPresenceInfoWithAdminServer();
   private final WlsDomainConfig domainConfig = createDomainConfig();
   private final Collection<ServerStartupInfo> startupInfos = new ArrayList<>();
 
@@ -108,10 +108,9 @@ private static WlsDomainConfig createDomainConfig() {
             .withCluster(clusterConfig);
   }
 
-  private DomainPresenceInfo createDomainPresenceInfoWithServers(String... serverNames) {
+  private DomainPresenceInfo createDomainPresenceInfoWithAdminServer() {
     DomainPresenceInfo dpi = new DomainPresenceInfo(domain);
     addServer(dpi, ADMIN);
-    Arrays.asList(serverNames).forEach(serverName -> addServer(dpi, serverName));
     return dpi;
   }
 
@@ -249,6 +248,23 @@ public void whenConcurrencyLimitIs1_secondClusteredServerDoesNotStartIfFirstIsNo
     assertThat(getStartedManagedServers(), hasSize(1));
   }
 
+  @Test
+  public void whileAdminServerStopped_canStartManagedServer() {
+    createDomainPresenceInfoWithNoAdminServer();
+    addWlsCluster(CLUSTER1, MS1);
+
+    invokeStepWithServerStartupInfos();
+
+    assertThat(getStartedManagedServers(), hasSize(1));
+  }
+
+  private void createDomainPresenceInfoWithNoAdminServer() {
+    domainPresenceInfo = new DomainPresenceInfo(domain);
+    testSupport
+            .addToPacket(ProcessingConstants.DOMAIN_TOPOLOGY, domainConfig)
+            .addDomainPresenceInfo(domainPresenceInfo);
+  }
+
   @Test
   public void whenConcurrencyLimitIs1_secondClusteredServerStartsAfterFirstIsReady() {
     configureCluster(CLUSTER1).withMaxConcurrentStartup(1);