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;
+ }
+}