addressing code smells and adding more changelog

fabric8io · Jun 26, 2021 · 1b1a26f · 1b1a26f
1 parent 70206c1
commit 1b1a26f
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,7 +55,8 @@
 ##### DSL Changes:
 - #3127 `StatusUpdatable.updateStatus` deprecated, please use patchStatus, editStatus, or replaceStatus
 - #3239 deprecated methods on SharedInformerFactory directly dealing with the OperationContext, withName, and withNamespace - the Informable interface should be used instead.
-- #3271 waitUntilReady and waitUntilCondition with throw a KubernetesClientTimeoutException instead of an IllegalArgumentException on timeout
+- #3271 `Waitable.waitUntilReady` and `Waitable.waitUntilCondition` with throw a KubernetesClientTimeoutException instead of an IllegalArgumentException on timeout.  
+  `Waitable.withWaitRetryBackoff` and the associated constants are now deprecated.
 
 ##### Util Changes:
 - #3197 `Utils.waitUntilReady` now accepts a Future, rather than a BlockingQueue

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/Waitable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/Waitable.java
@@ -20,8 +20,14 @@
 
 public interface Waitable<T, P> {
 
+  /**
+   * @deprecated no longer used
+   */
   @Deprecated
   long DEFAULT_INITIAL_BACKOFF_MILLIS = 5L;
+  /**
+   * @deprecated no longer used
+   */
   @Deprecated
   double DEFAULT_BACKOFF_MULTIPLIER = 2d;
 

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java
@@ -1161,9 +1161,9 @@ public void onDelete(T obj, boolean deletedFinalStateUnknown) {
     } catch (ExecutionException e) {
       throw KubernetesClientException.launderThrowable(e.getCause());
     } catch (TimeoutException e) {
-      T item = getItem();
-      if (item != null) {
-        throw new KubernetesClientTimeoutException(item, amount, timeUnit);
+      T i = getItem();
+      if (i != null) {
+        throw new KubernetesClientTimeoutException(i, amount, timeUnit);
       }
       throw new KubernetesClientTimeoutException(getKind(), getName(), getNamespace(), amount, timeUnit);
     }
@@ -1196,13 +1196,15 @@ public Informable<T> withIndexers(Map<String, Function<T, List<String>>> indexer
   @Override
   public SharedIndexInformer<T> inform(ResourceEventHandler<T> handler, long resync) {
     DefaultSharedIndexInformer<T, L> result = createInformer(resync, null, handler);
+    // synchronous start list/watch must succeed in the calling thread
+    // initial add events will be processed in the calling thread as well
     result.run();
     return result;
   }
 
   private DefaultSharedIndexInformer<T, L> createInformer(long resync, Consumer<L> onList, ResourceEventHandler<T> handler) {
-    T item = getItem();
-    String name = (Utils.isNotNullOrEmpty(getName()) || item != null) ? checkName(item) : null;
+    T i = getItem();
+    String name = (Utils.isNotNullOrEmpty(getName()) || i != null) ? checkName(i) : null;
 
     // use the local context / namespace
     DefaultSharedIndexInformer<T, L> informer = new DefaultSharedIndexInformer<>(getType(), new ListerWatcher<T, L>() {

diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java
@@ -185,8 +185,8 @@ void testSuccessfulWaitUntilCondition() throws InterruptedException {
       .anyMatch(c -> "True".equals(c.getStatus()));
 
     // The pods are never ready if you request them directly.
-    server.expect().get().withPath("/api/v1/namespaces/ns1/pods?fieldSelector=metadata.name%3Dpod1&watch=false").andReturn(HTTP_OK, noReady1).once();
-    server.expect().get().withPath("/api/v1/namespaces/ns1/pods?fieldSelector=metadata.name%3Dpod2&watch=false").andReturn(HTTP_OK, noReady2).once();
+    ResourceTest.list(server, noReady1);
+    ResourceTest.list(server, noReady2);
 
     server.expect().get().withPath("/api/v1/namespaces/ns1/pods?fieldSelector=metadata.name%3Dpod1&resourceVersion=1&watch=true").andUpgradeToWebSocket()
       .open()
@@ -226,8 +226,8 @@ void testPartialSuccessfulWaitUntilCondition() {
       .anyMatch(c -> "True".equals(c.getStatus()));
 
     // The pods are never ready if you request them directly.
-    server.expect().get().withPath("/api/v1/namespaces/ns1/pods?fieldSelector=metadata.name%3Dpod1&watch=false").andReturn(HTTP_OK, noReady1).once();
-    server.expect().get().withPath("/api/v1/namespaces/ns1/pods?fieldSelector=metadata.name%3Dpod2&watch=false").andReturn(HTTP_OK, noReady2).once();
+    ResourceTest.list(server, noReady1);
+    ResourceTest.list(server, noReady2);
 
     Status gone = new StatusBuilder()
       .withCode(HTTP_GONE)

diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java
@@ -237,9 +237,13 @@ void testWaitUntilReady() throws InterruptedException {
   }
 
   private void list(Pod pod) {
+    list(server, pod);
+  }
+
+  static void list(KubernetesMockServer server, Pod pod) {
     server.expect()
         .get()
-        .withPath("/api/v1/namespaces/test/pods?fieldSelector=metadata.name%3D"+pod.getMetadata().getName()+"&watch=false")
+        .withPath("/api/v1/namespaces/"+pod.getMetadata().getNamespace()+"/pods?fieldSelector=metadata.name%3D"+pod.getMetadata().getName()+"&watch=false")
         .andReturn(200,
             new PodListBuilder().withItems(pod).withNewMetadata().withResourceVersion("1").endMetadata().build())
         .once();