Rolling fixes

Author: rjeberhard

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

@@ -487,11 +487,6 @@ public void makeRightDomainPresence(
       boolean isDeleting,
       boolean isWillInterrupt) {
     Domain domain = info.getDomain();
-    DomainSpec spec = null;
-    if (domain != null) {
-      spec = domain.getSpec();
-      DomainPresenceControl.normalizeDomainSpec(spec);
-    }
     String ns = info.getNamespace();
     String domainUid = info.getDomainUid();
 
@@ -513,6 +508,7 @@ public void makeRightDomainPresence(
           // change just by looking at metadata.generation.
 
           // Has the spec actually changed? We will get watch events for status updates
+          DomainSpec spec = domain != null ? domain.getSpec() : null;
           if (!explicitRecheck && spec != null && spec.equals(current.getSpec())) {
             // nothing in the spec has changed, but status likely did; update current
             existing.setDomain(domain);

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

@@ -5,12 +5,13 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Queue;
 import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.TimeUnit;
 
 import io.kubernetes.client.openapi.models.V1Pod;
 import oracle.kubernetes.operator.ProcessingConstants;
@@ -35,6 +36,7 @@
  */
 public class RollingHelper {
   private static final LoggingFacade LOGGER = LoggingFactory.getLogger("Operator", "Operator");
+  private static long DELAY_IN_SECONDS = 10;
 
   private RollingHelper() {
   }
@@ -208,31 +210,20 @@ public NextAction apply(Packet packet) {
 
       LOGGER.info(MessageKeys.ROLLING_SERVERS, dom.getDomainUid(), servers, readyServers);
 
-      int countToRestartNow = Math.max(1, countReady - dom.getMinAvailable(clusterName));
+      int countToRestartNow = countReady - dom.getMinAvailable(clusterName);
       Collection<StepAndPacket> restarts = new ArrayList<>();
       for (int i = 0; i < countToRestartNow; i++) {
-        restarts.add(new StepAndPacket(new RestartOneClusteredServerStep(servers, null), packet));
+        Optional.ofNullable(servers.poll())
+            .ifPresent(serverToRestart -> restarts.add(serverToRestart));
       }
-      return doForkJoin(getNext(), packet, restarts);
-    }
-  }
-
-  private static class RestartOneClusteredServerStep extends Step {
-    private final Queue<StepAndPacket> servers;
-
-    public RestartOneClusteredServerStep(Queue<StepAndPacket> servers, Step next) {
-      super(next);
-      this.servers = servers;
-    }
 
-    @Override
-    public NextAction apply(Packet packet) {
-      StepAndPacket serverToRestart = servers.poll();
-      if (serverToRestart != null) {
-        Collection<StepAndPacket> col = Collections.singleton(serverToRestart);
-        return doForkJoin(this, packet, col);
+      if (!restarts.isEmpty()) {
+        return doForkJoin(this, packet, restarts);
+      } else if (!servers.isEmpty()) {
+        return doDelay(this, packet, DELAY_IN_SECONDS, TimeUnit.SECONDS);
+      } else {
+        return doNext(packet);
       }
-      return doNext(packet);
     }
   }
-}
+}

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

@@ -27,7 +27,10 @@
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 
+import static oracle.kubernetes.operator.KubernetesConstants.ALWAYS_IMAGEPULLPOLICY;
 import static oracle.kubernetes.operator.KubernetesConstants.DEFAULT_ALLOW_REPLICAS_BELOW_MIN_DYN_CLUSTER_SIZE;
+import static oracle.kubernetes.operator.KubernetesConstants.DEFAULT_IMAGE;
+import static oracle.kubernetes.operator.KubernetesConstants.IFNOTPRESENT_IMAGEPULLPOLICY;
 
 /** DomainSpec is a description of a domain. */
 @Description("DomainSpec is a description of a domain.")
@@ -338,9 +341,8 @@ public DomainSpec withWebLogicCredentialsSecret(V1SecretReference webLogicCreden
     return this;
   }
 
-  @Nullable
   public String getImage() {
-    return image;
+    return Optional.ofNullable(image).orElse(DEFAULT_IMAGE);
   }
 
   public void setImage(@Nullable String image) {
@@ -349,7 +351,15 @@ public void setImage(@Nullable String image) {
 
   @Nullable
   public String getImagePullPolicy() {
-    return imagePullPolicy;
+    return Optional.ofNullable(imagePullPolicy).orElse(getInferredPullPolicy());
+  }
+
+  private String getInferredPullPolicy() {
+    return useLatestImage() ? ALWAYS_IMAGEPULLPOLICY : IFNOTPRESENT_IMAGEPULLPOLICY;
+  }
+
+  private boolean useLatestImage() {
+    return getImage().endsWith(KubernetesConstants.LATEST_IMAGE_SUFFIX);
   }
 
   public void setImagePullPolicy(@Nullable String imagePullPolicy) {
@@ -639,8 +649,8 @@ public boolean equals(Object other) {
             .append(domainHomeInImage, rhs.domainHomeInImage)
             .append(serverStartPolicy, rhs.serverStartPolicy)
             .append(webLogicCredentialsSecret, rhs.webLogicCredentialsSecret)
-            .append(image, rhs.image)
-            .append(imagePullPolicy, rhs.imagePullPolicy)
+            .append(getImage(), rhs.getImage())
+            .append(getImagePullPolicy(), rhs.getImagePullPolicy())
             .append(imagePullSecrets, rhs.imagePullSecrets)
             .append(adminServer, rhs.adminServer)
             .append(managedServers, rhs.managedServers)

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

@@ -6,7 +6,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import javax.annotation.Nonnull;
 
 import io.kubernetes.client.openapi.models.V1Affinity;
@@ -16,15 +15,10 @@
 import io.kubernetes.client.openapi.models.V1ResourceRequirements;
 import io.kubernetes.client.openapi.models.V1SecurityContext;
 import io.kubernetes.client.openapi.models.V1Toleration;
-import oracle.kubernetes.operator.KubernetesConstants;
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 
-import static oracle.kubernetes.operator.KubernetesConstants.ALWAYS_IMAGEPULLPOLICY;
-import static oracle.kubernetes.operator.KubernetesConstants.DEFAULT_IMAGE;
-import static oracle.kubernetes.operator.KubernetesConstants.IFNOTPRESENT_IMAGEPULLPOLICY;
-
 /** Represents the effective configuration for a server, as seen by the operator runtime. */
 @SuppressWarnings("WeakerAccess")
 public abstract class ServerSpecBase implements ServerSpec {
@@ -37,26 +31,14 @@ public ServerSpecBase(DomainSpec domainSpec) {
 
   @Override
   public String getImage() {
-    return Optional.ofNullable(domainSpec.getImage()).orElse(DEFAULT_IMAGE);
+    return domainSpec.getImage();
   }
 
   @Override
   public String getImagePullPolicy() {
-    return Optional.ofNullable(getConfiguredImagePullPolicy()).orElse(getInferredPullPolicy());
-  }
-
-  protected String getConfiguredImagePullPolicy() {
     return domainSpec.getImagePullPolicy();
   }
 
-  private String getInferredPullPolicy() {
-    return useLatestImage() ? ALWAYS_IMAGEPULLPOLICY : IFNOTPRESENT_IMAGEPULLPOLICY;
-  }
-
-  private boolean useLatestImage() {
-    return getImage().endsWith(KubernetesConstants.LATEST_IMAGE_SUFFIX);
-  }
-
   @Override
   public List<V1LocalObjectReference> getImagePullSecrets() {
     return domainSpec.getImagePullSecrets();

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

@@ -89,6 +89,7 @@ public void setUp() throws Exception {
     configSupport.addWlsServer(ADMIN_SERVER, ADMIN_PORT);
     configSupport.setAdminServerName(ADMIN_SERVER);
     SERVER_NAMES.forEach(s -> configSupport.addWlsServer(s, LISTEN_PORT));
+    configSupport.addWlsCluster(CLUSTER_NAME, SERVER10_NAME, SERVER1_NAME, SERVER2_NAME);
 
     testSupport.defineResources(domain);
     domainTopology = configSupport.createDomainConfig();

operator/src/test/java/oracle/kubernetes/operator/helpers/RollingHelperTest.java

@@ -0,0 +1,67 @@
+// Copyright (c) 2020, Oracle Corporation and/or its affiliates.
+// Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
+
+package oracle.kubernetes.weblogic.domain.model;
+
+import oracle.kubernetes.operator.KubernetesConstants;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.junit.MatcherAssert.assertThat;
+
+public class DomainSpecTest {
+
+  @Test
+  public void whenEffectiveDomainSpecsWithDefaultImage_domainSpecEquals() {
+    DomainSpec spec1 = new DomainSpec().withImage(KubernetesConstants.DEFAULT_IMAGE);
+    DomainSpec spec2 = new DomainSpec();
+    assertThat("Expected effective image values to be equal", spec1, equalTo(spec2));
+  }
+
+  @Test
+  public void whenEffectiveDomainSpecsWithLatestImage_domainSpecNotEquals() {
+    DomainSpec spec1 = new DomainSpec().withImage(getLatestDefaultImage());
+    DomainSpec spec2 = new DomainSpec();
+    assertThat("Expected effective image values to NOT be equal", spec1, not(equalTo(spec2)));
+  }
+
+  @Test
+  public void whenEffectiveImagePullPolicyWithDefaultImage_domainSpecImagePullPolicyEquals() {
+    DomainSpec spec1 = new DomainSpec().withImage(KubernetesConstants.DEFAULT_IMAGE);
+    DomainSpec spec2 = new DomainSpec();
+    assertThat("Expected effective image pull policy values to be equal",
+        spec1.getImagePullPolicy(), equalTo(spec2.getImagePullPolicy()));
+  }
+
+  @Test
+  public void whenEffectiveImagePullPolicyWithLatestImage_domainSpecImagePullPolicyNotEquals() {
+    DomainSpec spec1 = new DomainSpec().withImage(getLatestDefaultImage());
+    DomainSpec spec2 = new DomainSpec();
+    assertThat("Expected effective image pull policy values to NOT be equal",
+        spec1.getImagePullPolicy(), not(equalTo(spec2.getImagePullPolicy())));
+  }
+
+  @Test
+  public void whenEffectiveImageWithDefaultImage_imageEquals() {
+    DomainSpec spec1 = new DomainSpec().withImage(KubernetesConstants.DEFAULT_IMAGE);
+    DomainSpec spec2 = new DomainSpec();
+    assertThat("Expected effective image values to be equal",
+        spec1.getImage(), equalTo(spec2.getImage()));
+  }
+
+  @Test
+  public void whenEffectiveImageWithLatestImage_imageNotEquals() {
+    DomainSpec spec1 = new DomainSpec().withImage(getLatestDefaultImage());
+    DomainSpec spec2 = new DomainSpec();
+    assertThat("Expected effective image pull policy values to NOT be equal",
+        spec1.getImage(), not(equalTo(spec2.getImage())));
+  }
+
+  private String getLatestDefaultImage() {
+    String defaultImageName = KubernetesConstants.DEFAULT_IMAGE
+        .substring(0, KubernetesConstants.DEFAULT_IMAGE.indexOf(':'));
+    String latestImage = defaultImageName + KubernetesConstants.LATEST_IMAGE_SUFFIX;
+    return latestImage;
+  }
+}