Update unit tests and simplify logic for dynamic update

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

@@ -46,17 +46,25 @@
 import oracle.kubernetes.operator.work.Packet;
 import oracle.kubernetes.operator.work.Step;
 import oracle.kubernetes.weblogic.domain.model.ClusterStatus;
+import oracle.kubernetes.weblogic.domain.model.Configuration;
 import oracle.kubernetes.weblogic.domain.model.Domain;
 import oracle.kubernetes.weblogic.domain.model.DomainCondition;
+import oracle.kubernetes.weblogic.domain.model.DomainConditionType;
+import oracle.kubernetes.weblogic.domain.model.DomainSpec;
 import oracle.kubernetes.weblogic.domain.model.DomainStatus;
+import oracle.kubernetes.weblogic.domain.model.Model;
+import oracle.kubernetes.weblogic.domain.model.OnlineUpdate;
 import oracle.kubernetes.weblogic.domain.model.ServerHealth;
 import oracle.kubernetes.weblogic.domain.model.ServerStatus;
 
 import static oracle.kubernetes.operator.LabelConstants.CLUSTERNAME_LABEL;
+import static oracle.kubernetes.operator.MIINonDynamicChangesMethod.CommitUpdateOnly;
 import static oracle.kubernetes.operator.ProcessingConstants.DOMAIN_TOPOLOGY;
 import static oracle.kubernetes.operator.ProcessingConstants.EXCEEDED_INTROSPECTOR_MAX_RETRY_COUNT_ERROR_MSG;
 import static oracle.kubernetes.operator.ProcessingConstants.FATAL_INTROSPECTOR_ERROR;
 import static oracle.kubernetes.operator.ProcessingConstants.FATAL_INTROSPECTOR_ERROR_MSG;
+import static oracle.kubernetes.operator.ProcessingConstants.MII_DYNAMIC_UPDATE;
+import static oracle.kubernetes.operator.ProcessingConstants.MII_DYNAMIC_UPDATE_RESTART_REQUIRED;
 import static oracle.kubernetes.operator.ProcessingConstants.SERVER_HEALTH_MAP;
 import static oracle.kubernetes.operator.ProcessingConstants.SERVER_STATE_MAP;
 import static oracle.kubernetes.operator.WebLogicConstants.RUNNING_STATE;
@@ -490,9 +498,13 @@ static class StatusUpdateContext extends DomainStatusUpdaterContext {
       private final WlsDomainConfig config;
       private final Map<String, String> serverState;
       private final Map<String, ServerHealth> serverHealth;
+      private final Optional<DomainPresenceInfo> info;
+      private final Packet packet;
 
       StatusUpdateContext(Packet packet, StatusUpdateStep statusUpdateStep) {
         super(packet, statusUpdateStep);
+        this.packet = packet;
+        info = DomainPresenceInfo.fromPacket(packet);
         config = packet.getValue(DOMAIN_TOPOLOGY);
         serverState = packet.getValue(SERVER_STATE_MAP);
         serverHealth = packet.getValue(SERVER_HEALTH_MAP);
@@ -523,6 +535,54 @@ void modifyStatus(DomainStatus status) {
                 .withReason(MANAGED_SERVERS_STARTING_PROGRESS_REASON));
         }
 
+        if (miiNondynamicRestartRequired() && isCommitUpdateOnly()) {
+          setOnlineUpdateNeedRestartCondition(status);
+        }
+
+      }
+
+      private boolean miiNondynamicRestartRequired() {
+        return MII_DYNAMIC_UPDATE_RESTART_REQUIRED.equals(packet.get(MII_DYNAMIC_UPDATE));
+      }
+
+      private boolean isCommitUpdateOnly() {
+        return getMiiNonDynamicChangesMethod() == CommitUpdateOnly;
+      }
+
+      private MIINonDynamicChangesMethod getMiiNonDynamicChangesMethod() {
+        return info
+            .map(DomainPresenceInfo::getDomain)
+            .map(Domain::getSpec)
+            .map(DomainSpec::getConfiguration)
+            .map(Configuration::getModel)
+            .map(Model::getOnlineUpdate)
+            .map(OnlineUpdate::getOnNonDynamicChanges)
+            .orElse(MIINonDynamicChangesMethod.CommitUpdateOnly);
+      }
+
+      private void setOnlineUpdateNeedRestartCondition(DomainStatus status) {
+        String dynamicUpdateRollBackFile = Optional.ofNullable((String)packet.get(
+            ProcessingConstants.MII_DYNAMIC_UPDATE_WDTROLLBACKFILE))
+            .orElse("");
+        String message = String.format("%s\n%s",
+            LOGGER.formatMessage(MessageKeys.MII_DOMAIN_UPDATED_POD_RESTART_REQUIRED), dynamicUpdateRollBackFile);
+        updateDomainConditions(status, message, DomainConditionType.ConfigChangesPendingRestart);
+      }
+
+      private void updateDomainConditions(DomainStatus status, String message, DomainConditionType domainSourceType) {
+        String introspectVersion = info
+            .map(DomainPresenceInfo::getDomain)
+            .map(Domain::getSpec)
+            .map(DomainSpec::getIntrospectVersion)
+            .orElse("");
+
+        DomainCondition onlineUpdateCondition = new DomainCondition(domainSourceType)
+            .withMessage(message)
+            .withReason("Online update applied, introspectVersion updated to " + introspectVersion)
+            .withStatus("True");
+
+        status.removeConditionIf(c -> c.getType() == DomainConditionType.ConfigChangesPendingRestart);
+        status.addCondition(onlineUpdateCondition);
       }
 
       private boolean stillHasPodPendingRestart(DomainStatus status) {
@@ -651,9 +711,11 @@ Map<String, ClusterStatus> getClusterStatuses() {
       private ClusterStatus createClusterStatus(String clusterName) {
         return new ClusterStatus()
             .withClusterName(clusterName)
-            .withReplicas(Optional.ofNullable(getClusterCounts().get(clusterName)).map(Long::intValue).orElse(null))
+            .withReplicas(Optional.ofNullable(getClusterCounts().get(clusterName))
+                .map(Long::intValue).orElse(null))
             .withReadyReplicas(
-                Optional.ofNullable(getClusterCounts(true).get(clusterName)).map(Long::intValue).orElse(null))
+                Optional.ofNullable(getClusterCounts(true)
+                    .get(clusterName)).map(Long::intValue).orElse(null))
             .withMaximumReplicas(getClusterMaximumSize(clusterName))
             .withMinimumReplicas(getClusterMinimumSize(clusterName))
             .withReplicasGoal(getClusterSizeGoal(clusterName));

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

@@ -183,7 +183,7 @@ V1Service removeServerService(String serverName) {
     return getSko(serverName).getService().getAndSet(null);
   }
 
-  static Optional<DomainPresenceInfo> fromPacket(Packet packet) {
+  public static Optional<DomainPresenceInfo> fromPacket(Packet packet) {
     return Optional.ofNullable(packet.getSpi(DomainPresenceInfo.class));
   }
 

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

@@ -56,14 +56,8 @@
 import oracle.kubernetes.operator.work.NextAction;
 import oracle.kubernetes.operator.work.Packet;
 import oracle.kubernetes.operator.work.Step;
-import oracle.kubernetes.weblogic.domain.model.Configuration;
 import oracle.kubernetes.weblogic.domain.model.Domain;
-import oracle.kubernetes.weblogic.domain.model.DomainCondition;
-import oracle.kubernetes.weblogic.domain.model.DomainConditionType;
-import oracle.kubernetes.weblogic.domain.model.DomainSpec;
 import oracle.kubernetes.weblogic.domain.model.DomainStatus;
-import oracle.kubernetes.weblogic.domain.model.Model;
-import oracle.kubernetes.weblogic.domain.model.OnlineUpdate;
 import oracle.kubernetes.weblogic.domain.model.ServerEnvVars;
 import oracle.kubernetes.weblogic.domain.model.ServerSpec;
 import oracle.kubernetes.weblogic.domain.model.Shutdown;
@@ -72,7 +66,8 @@
 import static oracle.kubernetes.operator.IntrospectorConfigMapConstants.NUM_CONFIG_MAPS;
 import static oracle.kubernetes.operator.LabelConstants.INTROSPECTION_STATE_LABEL;
 import static oracle.kubernetes.operator.LabelConstants.MII_UPDATED_RESTART_REQUIRED_LABEL;
-import static oracle.kubernetes.operator.logging.MessageKeys.MII_DOMAIN_DYNAMICALLY_UPDATED;
+import static oracle.kubernetes.operator.LabelConstants.MODEL_IN_IMAGE_DOMAINZIP_HASH;
+import static oracle.kubernetes.operator.ProcessingConstants.MII_DYNAMIC_UPDATE_SUCCESS;
 
 public abstract class PodStepContext extends BasePodStepContext {
 
@@ -93,6 +88,7 @@ public abstract class PodStepContext extends BasePodStepContext {
   private final String miiModelSecretsHash;
   private final String miiDomainZipHash;
   private final String domainRestartVersion;
+  private boolean addRestartRequiredLabel;
 
   PodStepContext(Step conflictStep, Packet packet) {
     super(packet.getSpi(DomainPresenceInfo.class));
@@ -393,10 +389,6 @@ private Step patchCurrentPod(V1Pod currentPod, Step next) {
     return createProgressingStep(patchPod(currentPod, next));
   }
 
-  private Step patchRunningPod(V1Pod currentPod, V1Pod updatedPod, Step next, boolean addRestartRequiredLabel) {
-    return createProgressingStep(patchPod(currentPod, updatedPod, next, addRestartRequiredLabel));
-  }
-
   protected Step patchPod(V1Pod currentPod, Step next) {
     JsonPatchBuilder patchBuilder = Json.createPatchBuilder();
     KubernetesUtils.addPatches(
@@ -408,50 +400,18 @@ protected Step patchPod(V1Pod currentPod, Step next) {
             new V1Patch(patchBuilder.build().toString()), patchResponse(next));
   }
 
-  // Method for online update
-  protected Step patchPod(V1Pod currentPod, V1Pod updatedPod, Step next, boolean addRestartRequiredLabel) {
-    JsonPatchBuilder patchBuilder = Json.createPatchBuilder();
-    Map<String, String> updatedLabels = Optional.ofNullable(updatedPod)
-        .map(V1Pod::getMetadata)
-        .map(V1ObjectMeta::getLabels)
-        .orElse(null);
-
-    Map<String, String> updatedAnnotations = Optional.ofNullable(updatedPod)
-        .map(V1Pod::getMetadata)
-        .map(V1ObjectMeta::getAnnotations)
-        .orElse(null);
-    if (updatedLabels != null) {
-      String introspectVersion = Optional.ofNullable(info)
-          .map(DomainPresenceInfo::getDomain)
-          .map(Domain::getSpec)
-          .map(DomainSpec::getIntrospectVersion)
-          .orElse(null);
-      if (introspectVersion != null) {
-        updatedLabels.put(INTROSPECTION_STATE_LABEL, introspectVersion);
-        if (addRestartRequiredLabel) {
-          updatedLabels.put(MII_UPDATED_RESTART_REQUIRED_LABEL, "true");
-        }
-      }
-
-      KubernetesUtils.addPatches(
-          patchBuilder, "/metadata/labels/", getLabels(currentPod), updatedLabels);
-    }
-    if (updatedAnnotations != null) {
-      KubernetesUtils.addPatches(
-          patchBuilder, "/metadata/annotations/", getAnnotations(currentPod),
-          updatedAnnotations);
-    }
-    return new CallBuilder()
-        .patchPodAsync(getPodName(), getNamespace(), getDomainUid(),
-            new V1Patch(patchBuilder.build().toString()), patchResponse(next));
-  }
-
   private Map<String, String> getNonHashedPodLabels() {
     Map<String,String> result = new HashMap<>(getPodLabels());
+    Optional.ofNullable(miiDomainZipHash)
+          .ifPresent(h -> result.put(MODEL_IN_IMAGE_DOMAINZIP_HASH, formatHashLabel(h)));
 
     Optional.ofNullable(getDomain().getSpec().getIntrospectVersion())
         .ifPresent(version -> result.put(INTROSPECTION_STATE_LABEL, version));
 
+    if (addRestartRequiredLabel) {
+      result.put(MII_UPDATED_RESTART_REQUIRED_LABEL, "true");
+    }
+
     return result;
   }
 
@@ -499,7 +459,8 @@ private boolean mustPatchPod(V1Pod currentPod) {
   private boolean canUseCurrentPod(V1Pod currentPod) {
 
     boolean useCurrent =
-        AnnotationHelper.getHash(getPodModel()).equals(AnnotationHelper.getHash(currentPod));
+        AnnotationHelper.getHash(getPodModel()).equals(AnnotationHelper.getHash(currentPod))
+            && canUseNewDomainZip(currentPod);
 
     if (!useCurrent && AnnotationHelper.getDebugString(currentPod).length() > 0) {
       LOGGER.fine(
@@ -511,6 +472,28 @@ private boolean canUseCurrentPod(V1Pod currentPod) {
     return useCurrent;
   }
 
+  private boolean canUseNewDomainZip(V1Pod currentPod) {
+    String dynamicUpdateResult = packet.getValue(ProcessingConstants.MII_DYNAMIC_UPDATE);
+
+    if (miiDomainZipHash == null || isDomainZipUnchanged(currentPod)) {
+      return true;
+    } else if (dynamicUpdateResult == null) {
+      return false;
+    } else if (dynamicUpdateResult.equals(MII_DYNAMIC_UPDATE_SUCCESS)) {
+      return true;
+    } else if (getDomain().getMiiNonDynamicChangesMethod() == MIINonDynamicChangesMethod.CommitUpdateOnly) {
+      addRestartRequiredLabel = true;
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  private boolean isDomainZipUnchanged(V1Pod currentPod) {
+    return formatHashLabel(miiDomainZipHash)
+        .equals(currentPod.getMetadata().getLabels().get(MODEL_IN_IMAGE_DOMAINZIP_HASH));
+  }
+
   private String getReasonToRecycle(V1Pod currentPod) {
     PodCompatibility compatibility = new PodCompatibility(getPodModel(), currentPod);
     return compatibility.getIncompatibility();
@@ -554,6 +537,8 @@ V1Pod withNonHashedElements(V1Pod pod) {
     getPodAnnotations().entrySet().stream()
         .filter(PodStepContext::isPatchableItem)
         .forEach(e -> metadata.putAnnotationsItem(e.getKey(), e.getValue()));
+    Optional.ofNullable(miiDomainZipHash)
+        .ifPresent(hash -> addHashLabel(metadata, LabelConstants.MODEL_IN_IMAGE_DOMAINZIP_HASH, hash));
 
     setTerminationGracePeriod(pod);
     getContainer(pod).map(V1Container::getEnv).ifPresent(this::updateEnv);
@@ -639,8 +624,6 @@ LabelConstants.CLUSTERRESTARTVERSION_LABEL, getServerSpec().getClusterRestartVer
         .putLabelsItem(
             LabelConstants.SERVERRESTARTVERSION_LABEL, getServerSpec().getServerRestartVersion());
 
-    Optional.ofNullable(miiDomainZipHash)
-          .ifPresent(hash -> addHashLabel(metadata, LabelConstants.MODEL_IN_IMAGE_DOMAINZIP_HASH, hash));
     Optional.ofNullable(miiModelSecretsHash)
           .ifPresent(hash -> addHashLabel(metadata, LabelConstants.MODEL_IN_IMAGE_MODEL_SECRETS_HASH, hash));
 
@@ -934,50 +917,14 @@ private class VerifyPodStep extends BaseStep {
     public NextAction apply(Packet packet) {
       V1Pod currentPod = info.getServerPod(getServerName());
       // reset introspect failure job count - if any
-
       Optional.ofNullable(packet.getSpi(DomainPresenceInfo.class))
           .map(DomainPresenceInfo::getDomain)
           .map(Domain::getStatus)
           .ifPresent(DomainStatus::resetIntrospectJobFailureCount);
 
-      String dynamicUpdateResult = Optional.ofNullable((String)packet.get(ProcessingConstants.MII_DYNAMIC_UPDATE))
-          .orElse(null);
-
       if (currentPod == null) {
         return doNext(createNewPod(getNext()), packet);
       } else if (!canUseCurrentPod(currentPod)) {
-        if (shouldNotRestartAfterOnlineUpdate(dynamicUpdateResult)) {
-          Map<String, String> labels = Optional.of(currentPod)
-              .map(V1Pod::getMetadata)
-              .map(V1ObjectMeta::getLabels)
-              .orElse(new HashMap<>());
-          String serverName = "";
-          if (labels.containsKey(LabelConstants.SERVERNAME_LABEL)) {
-            serverName = labels.get(LabelConstants.SERVERNAME_LABEL);
-          }
-          LOGGER.fine(MII_DOMAIN_DYNAMICALLY_UPDATED, info.getDomain().getDomainUid(), serverName);
-          logPodExists();
-          //Create dummy meta data for patching
-          V1Pod updatedPod = AnnotationHelper.withSha256Hash(createPodRecipe());
-          V1ObjectMeta updatedMetaData = new V1ObjectMeta();
-
-          updatedMetaData.putAnnotationsItem("weblogic.sha256",
-               updatedPod.getMetadata().getAnnotations().get("weblogic.sha256"));
-          if (miiDomainZipHash != null) {
-            updatedMetaData.putLabelsItem(LabelConstants.MODEL_IN_IMAGE_DOMAINZIP_HASH,
-                formatHashLabel(miiDomainZipHash));
-          }
-          updatedPod.setMetadata(updatedMetaData);
-          boolean addRestartRequiredLabel = false;
-          // if the introspector job tells it needs restart, update the condition with the needed attributes
-          if (ProcessingConstants.MII_DYNAMIC_UPDATE_RESTART_REQUIRED.equals(dynamicUpdateResult)) {
-            setOnlineUpdateNeedRestartCondition(packet);
-            addRestartRequiredLabel = true;
-          } else {
-            setOnlineUpdateSuccessCondition();
-          }
-          return doNext(patchRunningPod(currentPod, updatedPod, getNext(), addRestartRequiredLabel), packet);
-        }
         LOGGER.info(
             MessageKeys.CYCLING_POD,
             Objects.requireNonNull(currentPod.getMetadata()).getName(),
@@ -990,63 +937,6 @@ public NextAction apply(Packet packet) {
         return doNext(packet);
       }
     }
-
-    private boolean shouldNotRestartAfterOnlineUpdate(String dynamicUpdateResult) {
-      boolean result = false;
-      if (ProcessingConstants.MII_DYNAMIC_UPDATE_SUCCESS.equals(dynamicUpdateResult)) {
-        result = true;
-      } else if (ProcessingConstants.MII_DYNAMIC_UPDATE_RESTART_REQUIRED.equals(dynamicUpdateResult)) {
-        result = MIINonDynamicChangesMethod.CommitUpdateOnly.equals(
-            Optional.ofNullable(info)
-             .map(DomainPresenceInfo::getDomain)
-             .map(Domain::getSpec)
-             .map(DomainSpec::getConfiguration)
-             .map(Configuration::getModel)
-             .map(Model::getOnlineUpdate)
-             .map(OnlineUpdate::getOnNonDynamicChanges)
-             .orElse(MIINonDynamicChangesMethod.CommitUpdateOnly));
-      }
-      return result;
-    }
-
-    private void setOnlineUpdateNeedRestartCondition(Packet packet) {
-      String dynamicUpdateRollBackFile = Optional.ofNullable((String)packet.get(
-          ProcessingConstants.MII_DYNAMIC_UPDATE_WDTROLLBACKFILE))
-          .orElse("");
-      String message = String.format("%s\n%s",
-          LOGGER.formatMessage(MessageKeys.MII_DOMAIN_UPDATED_POD_RESTART_REQUIRED), dynamicUpdateRollBackFile);
-      updateDomainConditions(message, DomainConditionType.ConfigChangesPendingRestart);
-    }
-
-    private void setOnlineUpdateSuccessCondition() {
-    //      updateDomainConditions("Online update successful. No restart necessary",
-    //          DomainConditionType.OnlineUpdateComplete);
-    }
-
-    private void updateDomainConditions(String message, DomainConditionType domainSourceType) {
-      DomainCondition onlineUpdateCondition = new DomainCondition(domainSourceType);
-      String introspectVersion = Optional.ofNullable(info)
-            .map(DomainPresenceInfo::getDomain)
-            .map(Domain::getSpec)
-            .map(DomainSpec::getIntrospectVersion)
-            .orElse("");
-
-      onlineUpdateCondition
-          .withMessage(message)
-          .withReason("Online update applied, introspectVersion updated to " + introspectVersion)
-          .withStatus("True");
-
-      Optional.ofNullable(info)
-          .map(DomainPresenceInfo::getDomain)
-          .map(Domain::getStatus)
-          .ifPresent(o -> o.removeConditionIf(c -> c.getType() == DomainConditionType.ConfigChangesPendingRestart));
-
-      Optional.ofNullable(info)
-          .map(DomainPresenceInfo::getDomain)
-          .map(Domain::getStatus)
-          .ifPresent(o -> o.addCondition(onlineUpdateCondition));
-    }
-
   }
 
   private abstract class BaseResponseStep extends ResponseStep<V1Pod> {

operator/src/main/java/oracle/kubernetes/weblogic/domain/DomainConfigurator.java

@@ -484,7 +484,7 @@ public abstract DomainConfigurator withPodSecurityContext(
    *
    * @return this object
    */
-  public abstract DomainConfigurator withMIIOnlineUpate();
+  public abstract DomainConfigurator withMIIOnlineUpdate();
 
   /**
    * Set onNonDynamicChanges to CommitUpdateAndRoll for  MII Online Update.