owls-83918 an idle domain's resource version should stay unchanged

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

@@ -64,13 +64,15 @@
 import oracle.kubernetes.weblogic.domain.model.AdminService;
 import oracle.kubernetes.weblogic.domain.model.Channel;
 import oracle.kubernetes.weblogic.domain.model.Domain;
+import oracle.kubernetes.weblogic.domain.model.DomainStatus;
+import oracle.kubernetes.weblogic.domain.model.ServerHealth;
+import oracle.kubernetes.weblogic.domain.model.ServerStatus;
 
-import static oracle.kubernetes.operator.DomainStatusUpdater.ADMIN_SERVER_STARTING_PROGRESS_REASON;
-import static oracle.kubernetes.operator.DomainStatusUpdater.INSPECTING_DOMAIN_PROGRESS_REASON;
-import static oracle.kubernetes.operator.DomainStatusUpdater.MANAGED_SERVERS_STARTING_PROGRESS_REASON;
 import static oracle.kubernetes.operator.LabelConstants.INTROSPECTION_STATE_LABEL;
 import static oracle.kubernetes.operator.ProcessingConstants.DOMAIN_INTROSPECT_REQUESTED;
 import static oracle.kubernetes.operator.ProcessingConstants.MAKE_RIGHT_DOMAIN_OPERATION;
+import static oracle.kubernetes.operator.ProcessingConstants.SERVER_HEALTH_MAP;
+import static oracle.kubernetes.operator.ProcessingConstants.SERVER_STATE_MAP;
 import static oracle.kubernetes.operator.helpers.LegalNames.toJobIntrospectorName;
 
 public class DomainProcessorImpl implements DomainProcessor {
@@ -211,8 +213,7 @@ private static Step bringAdminServerUpSteps(
   }
 
   private static Step bringManagedServersUp(Step next) {
-    return DomainStatusUpdater.createProgressingStep(MANAGED_SERVERS_STARTING_PROGRESS_REASON, true,
-            new ManagedServersUpStep(next));
+    return new ManagedServersUpStep(next);
   }
 
   private FiberGate getMakeRightFiberGate(String ns) {
@@ -258,7 +259,7 @@ public void reportSuspendedFibers() {
             gate.getCurrentFibers().forEach(
                 (key, fiber) -> {
                   Optional.ofNullable(fiber.getSuspendedStep()).ifPresent(suspendedStep -> {
-                    try (LoggingContext stack
+                    try (LoggingContext ignored
                              = LoggingContext.setThreadContext().namespace(namespace).domainUid(getDomainUid(fiber))) {
                       LOGGER.fine("Fiber is SUSPENDED at " + suspendedStep.getName());
                     }
@@ -517,7 +518,7 @@ public void onThrowable(Packet packet, Throwable throwable) {
                           }
                         });
               } catch (Throwable t) {
-                try (LoggingContext stack
+                try (LoggingContext ignored
                          = LoggingContext.setThreadContext()
                     .namespace(info.getNamespace()).domainUid(info.getDomainUid())) {
                   LOGGER.severe(MessageKeys.EXCEPTION, t);
@@ -551,6 +552,49 @@ public MakeRightDomainOperation createMakeRightOperation(Domain liveDomain) {
     return createMakeRightOperation(new DomainPresenceInfo(liveDomain));
   }
 
+  public Step createPopulatePacketServerMapsStep(oracle.kubernetes.operator.work.Step next) {
+    return new PopulatePacketServerMapsStep(next);
+  }
+
+  public static class PopulatePacketServerMapsStep extends Step {
+    public PopulatePacketServerMapsStep(Step next) {
+      super(next);
+    }
+
+    @Override
+    public NextAction apply(Packet packet) {
+      populatePacketServerMapsFromDomain(packet);
+      return doNext(packet);
+    }
+
+    private void populatePacketServerMapsFromDomain(Packet packet) {
+      Map<String, ServerHealth> serverHealth = new ConcurrentHashMap<>();
+      Map<String, String> serverState = new ConcurrentHashMap<>();
+      Optional.ofNullable(packet.getSpi(DomainPresenceInfo.class))
+          .map(DomainPresenceInfo::getDomain)
+          .map(Domain::getStatus)
+          .map(DomainStatus::getServers)
+          .ifPresent(servers -> servers.forEach(item -> addServerToMaps(serverHealth, serverState, item)));
+      if (!serverState.isEmpty()) {
+        packet.put(SERVER_STATE_MAP, serverState);
+      }
+      if (!serverHealth.isEmpty()) {
+        packet.put(SERVER_HEALTH_MAP, serverHealth);
+      }
+    }
+
+    private void addServerToMaps(Map<String, ServerHealth> serverHealthMap,
+                                 Map<String, String> serverStateMap, ServerStatus item) {
+      if (item.getHealth() != null) {
+        serverHealthMap.put(item.getServerName(), item.getHealth());
+      }
+      if (item.getState() != null) {
+        serverStateMap.put(item.getServerName(), item.getState());
+      }
+    }
+
+  }
+
   /**
    * A factory which creates and executes steps to align the cached domain status with the value read from Kubernetes.
    */
@@ -659,12 +703,11 @@ private void internalMakeRightDomainPresence() {
               Component.createFor(liveInfo, delegate.getVersion(),
                   PodAwaiterStepFactory.class, delegate.getPodAwaiterStepFactory(getNamespace()),
                   V1SubjectRulesReviewStatus.class, delegate.getSubjectRulesReviewStatus(getNamespace())));
-
       runDomainPlan(
             getDomain(),
             getDomainUid(),
             getNamespace(),
-            new StepAndPacket(createSteps(), packet),
+            new StepAndPacket(createPopulatePacketServerMapsStep(createSteps()), packet),
             deleting,
             willInterrupt);
     }
@@ -759,7 +802,7 @@ public void onThrowable(Packet packet, Throwable throwable) {
                     () -> {
                       DomainPresenceInfo existing = getExistingDomainPresenceInfo(ns, domainUid);
                       if (existing != null) {
-                        try (LoggingContext stack =
+                        try (LoggingContext ignored =
                                  LoggingContext.setThreadContext().namespace(ns).domainUid(domainUid)) {
                           existing.setPopulated(false);
                           // proceed only if we have not already retried max number of times
@@ -801,14 +844,12 @@ Step createDomainUpPlan(DomainPresenceInfo info) {
         Step.chain(
             domainIntrospectionSteps(info),
             new DomainStatusStep(info, null),
-            DomainStatusUpdater.createProgressingStep(ADMIN_SERVER_STARTING_PROGRESS_REASON,true, null),
             bringAdminServerUp(info, delegate.getPodAwaiterStepFactory(info.getNamespace())),
             managedServerStrategy);
 
     return Step.chain(
           createDomainUpInitialStep(info),
           ConfigMapHelper.readExistingIntrospectorConfigMap(info.getNamespace(), info.getDomainUid()),
-          DomainStatusUpdater.createProgressingStep(INSPECTING_DOMAIN_PROGRESS_REASON,true, null),
           DomainPresenceStep.createDomainPresenceStep(info.getDomain(), domainUpStrategy, managedServerStrategy));
   }
 

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

@@ -591,7 +591,7 @@ private Integer getClusterSizeGoal(String clusterName) {
     }
   }
 
-  private static class ProgressingStep extends DomainStatusUpdaterStep {
+  public static class ProgressingStep extends DomainStatusUpdaterStep {
     private final String reason;
     private final boolean isPreserveAvailable;
 

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

@@ -45,6 +45,8 @@
 import oracle.kubernetes.weblogic.domain.model.ServerEnvVars;
 
 import static oracle.kubernetes.operator.DomainSourceType.FromModel;
+import static oracle.kubernetes.operator.DomainStatusUpdater.INSPECTING_DOMAIN_PROGRESS_REASON;
+import static oracle.kubernetes.operator.DomainStatusUpdater.createProgressingStep;
 import static oracle.kubernetes.operator.logging.MessageKeys.INTROSPECTOR_JOB_FAILED;
 import static oracle.kubernetes.operator.logging.MessageKeys.INTROSPECTOR_JOB_FAILED_DETAIL;
 
@@ -342,10 +344,12 @@ public NextAction apply(Packet packet) {
         packet.putIfAbsent(START_TIME, System.currentTimeMillis());
 
         return doNext(
-              context.createNewJob(
-                    readDomainIntrospectorPodLogStep(
-                          deleteDomainIntrospectorJobStep(
-                                ConfigMapHelper.createIntrospectorConfigMapStep(getNext())))),
+            Step.chain(
+                createProgressingStep(info, INSPECTING_DOMAIN_PROGRESS_REASON, true, null),
+                context.createNewJob(null),
+                readDomainIntrospectorPodLogStep(null),
+                deleteDomainIntrospectorJobStep(null),
+                ConfigMapHelper.createIntrospectorConfigMapStep(getNext())),
               packet);
       }
 

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

@@ -306,17 +306,23 @@ String getServerName() {
       return getAsName();
     }
 
+    @Override
+    Step createProgressingStep(Step actionStep) {
+      return DomainStatusUpdater.createProgressingStep(
+          DomainStatusUpdater.ADMIN_SERVER_STARTING_PROGRESS_REASON, false, actionStep);
+    }
+
     @Override
     Step createNewPod(Step next) {
-      return createPod(next);
+      return createProgressingStep(createPod(next));
     }
 
     @Override
     Step replaceCurrentPod(Step next) {
       if (MakeRightDomainOperation.isInspectionRequired(packet)) {
-        return MakeRightDomainOperation.createStepsToRerunWithIntrospection(packet);
+        return createProgressingStep(MakeRightDomainOperation.createStepsToRerunWithIntrospection(packet));
       } else {
-        return createCyclePodStep(next);
+        return createProgressingStep(createCyclePodStep(next));
       }
     }
 
@@ -464,7 +470,8 @@ private Map<String, Step.StepAndPacket> getServersToRoll() {
       return (Map<String, Step.StepAndPacket>) packet.get(SERVERS_TO_ROLL);
     }
 
-    private Step createProgressingStep(Step actionStep) {
+    @Override
+    Step createProgressingStep(Step actionStep) {
       return DomainStatusUpdater.createProgressingStep(
           DomainStatusUpdater.MANAGED_SERVERS_STARTING_PROGRESS_REASON, false, actionStep);
     }

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

@@ -322,7 +322,19 @@ private Step replacePod(Step next) {
     return createPodAsync(replaceResponse(next));
   }
 
+  /**
+   * Creates a Progressing step before an action step.
+   *
+   * @param actionStep the step to perform after the ProgressingStep.
+   * @return a step to be scheduled.
+   */
+  abstract Step createProgressingStep(Step actionStep);
+
   private Step patchCurrentPod(V1Pod currentPod, Step next) {
+    return createProgressingStep(patchPod(currentPod, next));
+  }
+
+  protected Step patchPod(V1Pod currentPod, Step next) {
     JsonPatchBuilder patchBuilder = Json.createPatchBuilder();
 
     KubernetesUtils.addPatches(
@@ -331,7 +343,7 @@ private Step patchCurrentPod(V1Pod currentPod, Step next) {
         patchBuilder, "/metadata/annotations/", getAnnotations(currentPod), getPodAnnotations());
 
     return new CallBuilder()
-        .patchPodAsync(getPodName(), getNamespace(), getDomainUid(),
+            .patchPodAsync(getPodName(), getNamespace(), getDomainUid(),
             new V1Patch(patchBuilder.build().toString()), patchResponse(next));
   }
 

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

@@ -32,6 +32,9 @@
 import oracle.kubernetes.weblogic.domain.model.Domain;
 import oracle.kubernetes.weblogic.domain.model.ServerSpec;
 
+import static oracle.kubernetes.operator.DomainStatusUpdater.MANAGED_SERVERS_STARTING_PROGRESS_REASON;
+import static oracle.kubernetes.operator.DomainStatusUpdater.createProgressingStep;
+
 public class ManagedServersUpStep extends Step {
   static final String SERVERS_UP_MSG =
       "Running servers for domain with UID: {0}, running list: {1}";
@@ -66,7 +69,9 @@ private static Step scaleDownIfNecessary(
     List<String> serversToStop = getServersToStop(info, serversToIgnore);
 
     if (!serversToStop.isEmpty()) {
-      insert(steps, new ServerDownIteratorStep(serversToStop, null));
+      insert(steps,
+          Step.chain(createProgressingStep(info, MANAGED_SERVERS_STARTING_PROGRESS_REASON, true, null),
+          new ServerDownIteratorStep(serversToStop, null)));
     }
 
     return Step.chain(steps.toArray(new Step[0]));

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

@@ -208,7 +208,7 @@ public ServerStatus withNodeName(String nodeName) {
    *
    * @return health
    */
-  ServerHealth getHealth() {
+  public ServerHealth getHealth() {
     return health;
   }
 

operator/src/test/java/oracle/kubernetes/operator/DomainProcessorTest.java

@@ -160,6 +160,12 @@ public void whenDomainSpecNotChanged_dontRunUpdateThread() {
     makeRightOperation.execute();
 
     assertThat(logRecords, containsFine(NOT_STARTING_DOMAINUID_THREAD));
+    Domain updatedDomain = testSupport.getResourceWithName(DOMAIN, domain.getDomainUid());
+    assertThat(getResourceVersion(updatedDomain), equalTo(getResourceVersion(domain)));
+  }
+
+  private String getResourceVersion(Domain domain) {
+    return Optional.of(domain).map(Domain::getMetadata).map(V1ObjectMeta::getResourceVersion).orElse("");
   }
 
   @Test
@@ -199,6 +205,7 @@ public void whenMakeRightRun_updateSDomainStatus() {
     assertThat(getDesiredState(updatedDomain, MANAGED_SERVER_NAMES[2]), equalTo(SHUTDOWN_STATE));
     assertThat(getDesiredState(updatedDomain, MANAGED_SERVER_NAMES[3]), equalTo(SHUTDOWN_STATE));
     assertThat(getDesiredState(updatedDomain, MANAGED_SERVER_NAMES[4]), equalTo(SHUTDOWN_STATE));
+    assertThat(getResourceVersion(updatedDomain), not(getResourceVersion(domain)));
   }
 
   @Test

operator/src/test/java/oracle/kubernetes/operator/DomainStatusUpdaterTest.java

@@ -71,6 +71,8 @@ public class DomainStatusUpdaterTest {
   private String reason = generator.getUniqueString();
   private RuntimeException failure = new RuntimeException(message);
   private String validationWarning = generator.getUniqueString();
+  private final DomainProcessorImpl processor =
+      new DomainProcessorImpl(DomainProcessorDelegateStub.createDelegate(testSupport));
 
   /**
    * Setup test environment.
@@ -1013,12 +1015,57 @@ public void whenDomainHasProgressingTrueCondition_failedStepRemovesIt() {
   }
 
   @Test
-  public void whenDomainHasAvailableCondition_failedStepRemovesIt() {
-    domain.getStatus().addCondition(new DomainCondition(Available));
+  public void whenPacketNotPopulatedBeforeUpdateServerStatus_resourceVersionUpdated() {
+    setupInitialServerStatus();
+    String cachedResourceVersion = getRecordedDomain().getMetadata().getResourceVersion();
 
-    testSupport.runSteps(DomainStatusUpdater.createFailedStep(failure, endStep));
+    // Clear the server maps in the packet, and run StatusUpdateStep, the domain resource
+    // version should be updated because server health information is removed from domain status.
+    clearPacketServerStatusMaps();
+    testSupport.runSteps(DomainStatusUpdater.createStatusUpdateStep(endStep));
 
-    assertThat(getRecordedDomain(), not(hasCondition(Available)));
+    assertThat(getRecordedDomain().getMetadata().getResourceVersion(), not(cachedResourceVersion));
+  }
+
+  @Test
+  public void whenPacketPopulatedBeforeUpdateServerStatus_resourceVersionNotUpdated() {
+    setupInitialServerStatus();
+    String cachedResourceVersion = getRecordedDomain().getMetadata().getResourceVersion();
+
+    // Clear the server maps in the packet, run StatusUpdateStep after running 
+    // PopulatePacketServerMapsStep, the domain resource version should NOT be updated because
+    // the server maps are populated in the packet with the existing server status
+    clearPacketServerStatusMaps();
+
+    testSupport.runSteps(
+        processor.createPopulatePacketServerMapsStep(
+            DomainStatusUpdater.createStatusUpdateStep(endStep)));
+
+    assertThat(getRecordedDomain().getMetadata().getResourceVersion(), equalTo(cachedResourceVersion));
+  }
+
+  private void setupInitialServerStatus() {
+    setClusterAndNodeName(getPod("server1"), "clusterA", "node1");
+    setClusterAndNodeName(getPod("server2"), "clusterB", "node2");
+
+    configSupport.addWlsCluster("clusterA", "server1");
+    configSupport.addWlsCluster("clusterB", "server2");
+    generateStartupInfos("server1", "server2");
+    testSupport.addToPacket(DOMAIN_TOPOLOGY, configSupport.createDomainConfig());
+
+    // Run StatusUpdateStep with server maps in the packet to set up the initial domain status
+    testSupport.addToPacket(
+        SERVER_STATE_MAP, ImmutableMap.of("server1", RUNNING_STATE, "server2", SHUTDOWN_STATE));
+    testSupport.addToPacket(
+        SERVER_HEALTH_MAP,
+        ImmutableMap.of("server1", overallHealth("health1"), "server2", overallHealth("health2")));
+
+    testSupport.runSteps(DomainStatusUpdater.createStatusUpdateStep(endStep));
+  }
+
+  private void clearPacketServerStatusMaps() {
+    testSupport.addToPacket(SERVER_STATE_MAP, null);
+    testSupport.addToPacket(SERVER_HEALTH_MAP, null);
   }
 
   private Domain getRecordedDomain() {

operator/src/test/java/oracle/kubernetes/operator/DomainUpPlanTest.java

@@ -75,6 +75,7 @@ public void setUp() throws NoSuchFieldException {
     mementos.add(InMemoryCertificates.install());
     mementos.add(TuningParametersStub.install());
 
+    testSupport.defineResources(domain);
     testSupport.addDomainPresenceInfo(domainPresenceInfo);
   }