fix #5229: allowing for other exits from the readiness check

also fixing the timeout units to match the javadocs

CHANGELOG.md

@@ -8,6 +8,7 @@
 * Fix #5218: No export for `io.fabric8.tekton.triggers.internal.knative.pkg.apis.duck.v1beta1` in tekton v1beta1 triggers model
 * Fix #5224: Ensuring jetty sets the User-Agent header
 * Fix #4225: Enum fields written in generated crd yaml
+* Fix #5229: Correcting the timeout units and behavior of withReadyWaitTimeout
 * Fix #5236: using scale v1beta1 compatible logic for DeploymentConfig
 * Fix #5235: Vert.x doesn't need to track derived HttpClients - prevents leakage
 * Fix #5238: Preserve folder structure again in PodUpload.upload()

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/ContainerResource.java

@@ -55,7 +55,7 @@ public interface ContainerResource
   CopyOrReadable dir(String path);
 
   /**
-   * How long to wait for the pod to be ready before performing an operation, such as
+   * How long to wait for the pod to be ready or terminal before performing an operation, such as
    * getting the logs, exec, attach, copy, etc.
    *
    * @param timeout in milliseconds

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/CopyOrReadable.java

@@ -41,7 +41,7 @@ public interface CopyOrReadable {
   boolean copy(Path destination);
 
   /**
-   * How long to wait for a ready pod before performing the copy or read operation.
+   * How long to wait for a ready or terminal pod before performing the copy or read operation.
    *
    * @param timeout in milliseconds
    */

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/Execable.java

@@ -36,7 +36,7 @@ public interface Execable {
   ExecWatch attach();
 
   /**
-   * How long shall we wait until a Pod is ready before attaching or execing
+   * How long shall we wait until a Pod is ready or terminal before attaching or execing
    *
    * @param timeout in milliseconds
    * @return {@link Loggable} for fetching logs

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/Loggable.java

@@ -86,7 +86,7 @@ public interface Loggable {
 
   /**
    * While waiting for Pod logs, how long shall we wait until a Pod
-   * becomes ready and starts producing logs
+   * becomes ready or terminal and starts producing logs
    *
    * @param timeout in milliseconds
    * @return {@link Loggable} for fetching logs

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/PodOperationsImpl.java

@@ -92,7 +92,7 @@ public class PodOperationsImpl extends HasMetadataOperation<Pod, PodList, PodRes
     implements PodResource, EphemeralContainersResource, CopyOrReadable {
 
   public static final int HTTP_TOO_MANY_REQUESTS = 429;
-  private static final Integer DEFAULT_POD_READY_WAIT_TIMEOUT = 5;
+  public static final int DEFAULT_POD_READY_WAIT_TIMEOUT_MS = 5000;
   private static final String[] EMPTY_COMMAND = { "/bin/sh", "-i" };
   public static final String DEFAULT_CONTAINER_ANNOTATION_NAME = "kubectl.kubernetes.io/default-container";
 
@@ -122,8 +122,8 @@ protected <T> T doGetLog(Class<T> type) {
     try {
       URL url = new URL(URLUtils.join(getResourceUrl().toString(), podOperationContext.getLogParameters()));
 
-      PodOperationUtil.waitUntilReadyOrSucceded(this,
-          getContext().getReadyWaitTimeout() != null ? getContext().getReadyWaitTimeout() : DEFAULT_POD_READY_WAIT_TIMEOUT);
+      PodOperationUtil.waitUntilReadyOrTerminal(this,
+          getContext().getReadyWaitTimeout() != null ? getContext().getReadyWaitTimeout() : DEFAULT_POD_READY_WAIT_TIMEOUT_MS);
 
       return handleRawGet(url, type);
     } catch (IOException ioException) {
@@ -176,8 +176,8 @@ private void checkForPiped(Object object) {
   public LogWatch watchLog(OutputStream out) {
     checkForPiped(out);
     try {
-      PodOperationUtil.waitUntilReadyOrSucceded(this,
-          getContext().getReadyWaitTimeout() != null ? getContext().getReadyWaitTimeout() : DEFAULT_POD_READY_WAIT_TIMEOUT);
+      PodOperationUtil.waitUntilReadyOrTerminal(this,
+          getContext().getReadyWaitTimeout() != null ? getContext().getReadyWaitTimeout() : DEFAULT_POD_READY_WAIT_TIMEOUT_MS);
       // Issue Pod Logs HTTP request
       URL url = new URL(URLUtils.join(getResourceUrl().toString(), getContext().getLogParameters() + "&follow=true"));
       final LogWatchCallback callback = new LogWatchCallback(out, context);
@@ -303,8 +303,8 @@ public ExecWatch attach() {
   }
 
   private URL getURL(String operation, String[] commands) throws MalformedURLException {
-    Pod fromServer = PodOperationUtil.waitUntilReadyOrSucceded(this,
-        getContext().getReadyWaitTimeout() != null ? getContext().getReadyWaitTimeout() : DEFAULT_POD_READY_WAIT_TIMEOUT);
+    Pod fromServer = PodOperationUtil.waitUntilReadyOrTerminal(this,
+        getContext().getReadyWaitTimeout() != null ? getContext().getReadyWaitTimeout() : DEFAULT_POD_READY_WAIT_TIMEOUT_MS);
 
     String url = URLUtils.join(getResourceUrl().toString(), operation);
     URLBuilder httpUrlBuilder = new URLBuilder(url);

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/internal/PodOperationUtil.java

@@ -18,6 +18,7 @@
 import io.fabric8.kubernetes.api.model.OwnerReference;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodList;
+import io.fabric8.kubernetes.api.model.PodStatus;
 import io.fabric8.kubernetes.client.KubernetesClientTimeoutException;
 import io.fabric8.kubernetes.client.dsl.LogWatch;
 import io.fabric8.kubernetes.client.dsl.Loggable;
@@ -34,6 +35,7 @@
 import java.io.OutputStream;
 import java.io.Reader;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -116,21 +118,27 @@ public static List<PodResource> getPodOperationsForController(PodOperationsImpl
     return PodOperationUtil.getFilteredPodsForLogs(podOperations, controllerPodList, controllerUid);
   }
 
-  public static Pod waitUntilReadyOrSucceded(PodResource podOperation, Integer logWaitTimeout) {
+  public static Pod waitUntilReadyOrTerminal(PodResource podOperation, int logWaitTimeoutMs) {
     AtomicReference<Pod> podRef = new AtomicReference<>();
     try {
       // Wait for Pod to become ready or succeeded
       podOperation.waitUntilCondition(p -> {
         podRef.set(p);
-        return p != null && (Readiness.isPodReady(p) || Readiness.isPodSucceeded(p));
+        return isReadyOrTerminal(p);
       },
-          logWaitTimeout,
-          TimeUnit.SECONDS);
+          logWaitTimeoutMs,
+          TimeUnit.MILLISECONDS);
     } catch (KubernetesClientTimeoutException timeout) {
       LOG.debug("Timed out waiting for Pod to become Ready: {}, will still proceed", timeout.getMessage());
     } catch (Exception otherException) {
       LOG.warn("Error while waiting for Pod to become Ready", otherException);
     }
     return podRef.get();
   }
+
+  static boolean isReadyOrTerminal(Pod p) {
+    // we'll treat missing as an exit condition - there's no expectation that we should wait for a pod that doesn't exist
+    return p == null || Readiness.isPodReady(p) || Optional.ofNullable(p.getStatus()).map(PodStatus::getPhase)
+        .filter(phase -> !Arrays.asList("Pending", "Unknown").contains(phase)).isPresent();
+  }
 }

kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/internal/PodOperationUtilTest.java

@@ -42,7 +42,9 @@
 
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
@@ -91,9 +93,17 @@ void testGetGenericPodOperations() {
   @Test
   void testWaitUntilReadyBeforeFetchingLogs() {
     // When
-    PodOperationUtil.waitUntilReadyOrSucceded(podOperations, 5);
+    PodOperationUtil.waitUntilReadyOrTerminal(podOperations, 5);
     // Then
-    verify(podOperations, times(1)).waitUntilCondition(any(), eq(5L), eq(TimeUnit.SECONDS));
+    verify(podOperations, times(1)).waitUntilCondition(any(), eq(5L), eq(TimeUnit.MILLISECONDS));
+  }
+
+  @Test
+  void testIsReadyOrTerminal() {
+    assertTrue(PodOperationUtil.isReadyOrTerminal(null));
+    assertFalse(PodOperationUtil.isReadyOrTerminal(new Pod()));
+    assertTrue(PodOperationUtil.isReadyOrTerminal(new PodBuilder().withNewStatus().withPhase("Failed").endStatus().build()));
+    assertFalse(PodOperationUtil.isReadyOrTerminal(new PodBuilder().withNewStatus().withPhase("Pending").endStatus().build()));
   }
 
   @Test

openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/apps/DeploymentConfigOperationsImpl.java

@@ -30,6 +30,7 @@
 import io.fabric8.kubernetes.client.dsl.internal.LogWatchCallback;
 import io.fabric8.kubernetes.client.dsl.internal.OperationContext;
 import io.fabric8.kubernetes.client.dsl.internal.PodOperationContext;
+import io.fabric8.kubernetes.client.dsl.internal.core.v1.PodOperationsImpl;
 import io.fabric8.kubernetes.client.dsl.internal.extensions.v1beta1.LegacyRollableScalableResourceOperation;
 import io.fabric8.kubernetes.client.utils.URLUtils;
 import io.fabric8.kubernetes.client.utils.internal.PodOperationUtil;
@@ -53,7 +54,6 @@ public class DeploymentConfigOperationsImpl
     extends HasMetadataOperation<DeploymentConfig, DeploymentConfigList, DeployableScalableResource<DeploymentConfig>>
     implements DeployableScalableResource<DeploymentConfig> {
 
-  private static final Integer DEFAULT_POD_LOG_WAIT_TIMEOUT = 5;
   public static final String OPENSHIFT_IO_DEPLOYMENT_CONFIG_NAME = "openshift.io/deployment-config.name";
   private final PodOperationContext rollingOperationContext;
 
@@ -185,12 +185,13 @@ private void waitUntilDeploymentConfigPodBecomesReady(DeploymentConfig deploymen
         rollingOperationContext,
         deploymentConfig.getMetadata().getUid(), getDeploymentConfigPodLabels(deploymentConfig));
 
-    waitForBuildPodToBecomeReady(podOps, podLogWaitTimeout != null ? podLogWaitTimeout : DEFAULT_POD_LOG_WAIT_TIMEOUT);
+    waitForBuildPodToBecomeReady(podOps,
+        podLogWaitTimeout != null ? podLogWaitTimeout : PodOperationsImpl.DEFAULT_POD_READY_WAIT_TIMEOUT_MS);
   }
 
   private static void waitForBuildPodToBecomeReady(List<PodResource> podOps, Integer podLogWaitTimeout) {
     for (PodResource podOp : podOps) {
-      PodOperationUtil.waitUntilReadyOrSucceded(podOp, podLogWaitTimeout);
+      PodOperationUtil.waitUntilReadyOrTerminal(podOp, podLogWaitTimeout);
     }
   }
 

openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildOperationsImpl.java

@@ -30,6 +30,7 @@
 import io.fabric8.kubernetes.client.dsl.internal.LogWatchCallback;
 import io.fabric8.kubernetes.client.dsl.internal.OperationContext;
 import io.fabric8.kubernetes.client.dsl.internal.PodOperationContext;
+import io.fabric8.kubernetes.client.dsl.internal.core.v1.PodOperationsImpl;
 import io.fabric8.kubernetes.client.utils.URLUtils;
 import io.fabric8.kubernetes.client.utils.internal.PodOperationUtil;
 import io.fabric8.openshift.api.model.Build;
@@ -52,7 +53,6 @@ public class BuildOperationsImpl extends HasMetadataOperation<Build, BuildList,
 
   public static final String OPENSHIFT_IO_BUILD_NAME = "openshift.io/build.name";
   private Integer version;
-  private static final Integer DEFAULT_POD_LOG_WAIT_TIMEOUT = 5;
   private final PodOperationContext operationContext;
 
   public BuildOperationsImpl(Client client) {
@@ -197,12 +197,13 @@ private void waitUntilBuildPodBecomesReady(Build build) {
         getBuildPodLabels(build));
 
     waitForBuildPodToBecomeReady(podOps,
-        operationContext.getReadyWaitTimeout() != null ? operationContext.getReadyWaitTimeout() : DEFAULT_POD_LOG_WAIT_TIMEOUT);
+        operationContext.getReadyWaitTimeout() != null ? operationContext.getReadyWaitTimeout()
+            : PodOperationsImpl.DEFAULT_POD_READY_WAIT_TIMEOUT_MS);
   }
 
   private static void waitForBuildPodToBecomeReady(List<PodResource> podOps, Integer podLogWaitTimeout) {
     for (PodResource podOp : podOps) {
-      PodOperationUtil.waitUntilReadyOrSucceded(podOp, podLogWaitTimeout);
+      PodOperationUtil.waitUntilReadyOrTerminal(podOp, podLogWaitTimeout);
     }
   }