fix fabric8io#3598: the wrong future was being cancelled, which did n…

…ot cleanup

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Fix #3538: Update Plural rule to work with Prometheus
 * Fix #3555: using the new builder for generic resources
 * Fix #3535: ensure clientKeyAlgo is set properly when loading config YAML from `fromKubeconfig`
+* Fix #3598: applying cancel to the correct future for waitUntilCondition and waitUntilReady
 
 #### Improvements
 

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java

@@ -15,10 +15,6 @@
  */
 package io.fabric8.kubernetes.client.dsl.base;
 
-import io.fabric8.kubernetes.api.model.ObjectReference;
-import io.fabric8.kubernetes.client.dsl.WritableOperation;
-import io.fabric8.kubernetes.client.utils.CreateOrReplaceHelper;
-
 import io.fabric8.kubernetes.api.builder.TypedVisitor;
 import io.fabric8.kubernetes.api.builder.Visitor;
 import io.fabric8.kubernetes.api.model.DeletionPropagation;
@@ -28,7 +24,6 @@
 import io.fabric8.kubernetes.api.model.ListOptions;
 import io.fabric8.kubernetes.api.model.ListOptionsBuilder;
 import io.fabric8.kubernetes.api.model.ObjectReference;
-import io.fabric8.kubernetes.api.model.RootPaths;
 import io.fabric8.kubernetes.api.model.Status;
 import io.fabric8.kubernetes.api.model.autoscaling.v1.Scale;
 import io.fabric8.kubernetes.api.model.extensions.DeploymentRollback;
@@ -74,7 +69,6 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Arrays;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -86,7 +80,6 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.UnaryOperator;
-import java.util.stream.Collectors;
 
 public class BaseOperation<T extends HasMetadata, L extends KubernetesResourceList<T>, R extends Resource<T>>
   extends CreateOnlyResourceOperation<T, T>
@@ -851,13 +844,13 @@ public T waitUntilReady(long amount, TimeUnit timeUnit) {
 
   @Override
   public T waitUntilCondition(Predicate<T> condition, long amount, TimeUnit timeUnit) {
-    CompletableFuture<T> futureCondition = informOnCondition(l -> {
+    CompletableFuture<List<T>> futureCondition = informOnCondition(l -> {
       if (l.isEmpty()) {
         return condition.test(null);
       }
       return condition.test(l.get(0));
-    }).thenApply(l -> l.isEmpty() ? null : l.get(0));
-
+    });
+    
     if (!Utils.waitUntilReady(futureCondition, amount, timeUnit)) {
       futureCondition.cancel(true);
       T i = getItem();
@@ -866,7 +859,7 @@ public T waitUntilCondition(Predicate<T> condition, long amount, TimeUnit timeUn
       }
       throw new KubernetesClientTimeoutException(getKind(), getName(), getNamespace(), amount, timeUnit);
     }
-    return futureCondition.getNow(null);
+    return futureCondition.thenApply(l -> l.isEmpty() ? null : l.get(0)).getNow(null);
   }
 
   @Override

kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java

@@ -23,6 +23,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
@@ -33,9 +34,14 @@
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Predicate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import io.fabric8.kubernetes.api.model.DeletionPropagation;
 import io.fabric8.kubernetes.api.model.ListOptionsBuilder;
@@ -46,6 +52,7 @@
 import io.fabric8.kubernetes.client.dsl.EditReplacePatchDeletable;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.fabric8.kubernetes.client.KubernetesClientException;
+import io.fabric8.kubernetes.client.KubernetesClientTimeoutException;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import io.fabric8.kubernetes.client.utils.URLUtils;
 import io.fabric8.kubernetes.client.utils.Utils;
@@ -320,4 +327,35 @@ void testHttpRetryWithLessFailuresThanRetries() throws MalformedURLException, IO
     assertNotNull(result);
     assertEquals("Expected 3 calls: 2 failures and 1 success!", 3, httpExecutionCounter.get());
   }
+
+  @Test
+  void testWaitUntilFailureCompletion() throws MalformedURLException, IOException {
+    final AtomicInteger httpExecutionCounter = new AtomicInteger(0);
+    OkHttpClient mockClient = newHttpClientWithSomeFailures(httpExecutionCounter, 2);
+    CompletableFuture<List<Pod>> future = new CompletableFuture<>();
+    BaseOperation<Pod, PodList, Resource<Pod>> baseOp = new BaseOperation(new OperationContext()
+      .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").build())
+      .withPlural("pods")
+      .withName("test-pod")
+      .withOkhttpClient(mockClient)) {
+
+      @Override
+      public CompletableFuture<List<Pod>> informOnCondition(Predicate condition) {
+        return future;
+      }
+
+    };
+    baseOp.setType(Pod.class);
+
+    // When
+    try {
+      baseOp.waitUntilCondition(Objects::isNull, 1, TimeUnit.MILLISECONDS);
+      fail("should timeout");
+    } catch (KubernetesClientTimeoutException e) {
+
+    }
+
+    // Then
+    assertTrue(future.isCancelled());
+  }
 }