diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b4ef1d3342..db0d2f826ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * Fix #4823: (java-generator) handle special characters in field names * Fix #4723: [java-generator] Fix a race in the use of JavaParser hitting large CRDs * Fix #4885: addresses a potential hang in the jdk client with exec stream reading +* Fix #4888: narrowing where the 0 initial list resourceVersion is used for informers - in particular if a limit is set or initialState is used, then we should not use 0. Additionally for the informOnCondition / wait methods we'll also not use 0 - it's not expected that the user should test any state prior to the latest. * Fix #4891: address vertx not completely reading exec streams * Fix #4899: BuildConfigs.instantiateBinary().fromFile() does not time out * Fix #4908: using the response headers in the vertx response diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java index e23414ee426..ef48f5e3ea5 100755 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java @@ -902,6 +902,8 @@ public CompletableFuture> informOnCondition(Predicate> condition // create an informer that supplies the tester with events and empty list handling SharedIndexInformer informer = this.createInformer(0, Runnable::run); + informer.initialState(Stream.empty()); + // prevent unnecessary watches and handle closure future.whenComplete((r, t) -> informer.stop()); diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/DefaultSharedIndexInformer.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/DefaultSharedIndexInformer.java index c4bfb651da1..4e01004232d 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/DefaultSharedIndexInformer.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/DefaultSharedIndexInformer.java @@ -155,6 +155,7 @@ public CompletableFuture start() { if (initialState != null) { initialState.forEach(indexer::put); + reflector.usingInitialState(); } } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/cache/Reflector.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/cache/Reflector.java index 3399438a2ae..f2e8bf6ef33 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/cache/Reflector.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/cache/Reflector.java @@ -60,6 +60,8 @@ public class Reflector timeoutFuture; + private boolean cachedListing = true; + public Reflector(ListerWatcher listerWatcher, SyncableStore store) { this.listerWatcher = listerWatcher; this.store = store; @@ -167,10 +169,8 @@ private CompletableFuture processList(Set nextKeys, String continueVa CompletableFuture futureResult = listerWatcher .submitList( new ListOptionsBuilder() - // start with 0 - meaning any cached version is fine for the initial listing - // but if we've already synced, then we have to get the latest as the version we're on - // is no longer valid - .withResourceVersion(lastSyncResourceVersion == null && continueVal == null ? "0" : null) + // if caching is allowed, start with 0 - meaning any cached version is fine for the initial listing + .withResourceVersion(isCachedListing(continueVal) ? "0" : null) .withLimit(listerWatcher.getLimit()).withContinue(continueVal) .build()); @@ -188,6 +188,11 @@ private CompletableFuture processList(Set nextKeys, String continueVa }); } + private boolean isCachedListing(String continueVal) { + // allow an initial cached listing only if there's no initial state, no limit, we haven't already sync'd, and this isn't a continue request + return cachedListing && listerWatcher.getLimit() == null && lastSyncResourceVersion == null && continueVal == null; + } + private void stopWatch(Watch w) { log.debug("Stopping watcher for {} at v{}", this, lastSyncResourceVersion); w.close(); @@ -320,4 +325,8 @@ public void setExceptionHandler(ExceptionHandler handler) { this.handler = handler; } + public void usingInitialState() { + this.cachedListing = false; + } + } diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/InformTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/InformTest.java index 2501b9edf84..ea60c563b03 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/InformTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/InformTest.java @@ -253,7 +253,7 @@ void testRunnableInformer() throws InterruptedException { .build(); server.expect() - .withPath("/api/v1/namespaces/test/pods?labelSelector=my-label&resourceVersion=0") + .withPath("/api/v1/namespaces/test/pods?labelSelector=my-label") .andReturn(HttpURLConnection.HTTP_OK, new PodListBuilder().withNewMetadata().withResourceVersion("1").endMetadata().withItems(pod1).build()) .once(); @@ -320,7 +320,7 @@ void testListLimit() throws InterruptedException { .build(); server.expect() - .withPath("/api/v1/namespaces/test/pods?limit=1&resourceVersion=0") + .withPath("/api/v1/namespaces/test/pods?limit=1") .andReturn(HttpURLConnection.HTTP_OK, new PodListBuilder().withNewMetadata() .withResourceVersion("2") @@ -337,7 +337,8 @@ void testListLimit() throws InterruptedException { .once(); server.expect() - .withPath("/api/v1/namespaces/test/pods?resourceVersion=2&timeoutSeconds=600&allowWatchBookmarks=true&watch=true") + .withPath( + "/api/v1/namespaces/test/pods?resourceVersion=2&timeoutSeconds=600&allowWatchBookmarks=true&watch=true") .andUpgradeToWebSocket() .open() .done() @@ -387,7 +388,8 @@ void testInformWithAlternativeKeyFunction() throws InterruptedException { .once(); server.expect() - .withPath("/api/v1/namespaces/test/pods?resourceVersion=1&timeoutSeconds=600&allowWatchBookmarks=true&watch=true") + .withPath( + "/api/v1/namespaces/test/pods?resourceVersion=1&timeoutSeconds=600&allowWatchBookmarks=true&watch=true") .andUpgradeToWebSocket() .open() .done() @@ -453,7 +455,8 @@ void testInformWithMinimalState() throws InterruptedException { .once(); server.expect() - .withPath("/api/v1/namespaces/test/pods?resourceVersion=1&timeoutSeconds=600&allowWatchBookmarks=true&watch=true") + .withPath( + "/api/v1/namespaces/test/pods?resourceVersion=1&timeoutSeconds=600&allowWatchBookmarks=true&watch=true") .andUpgradeToWebSocket() .open() .done() diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PodTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PodTest.java index a76d4816405..88f39ac2b83 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PodTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PodTest.java @@ -705,7 +705,7 @@ void testWait() throws InterruptedException { server.expect() .get() - .withPath("/api/v1/namespaces/test/pods?fieldSelector=metadata.name%3Dpod1&resourceVersion=0") + .withPath("/api/v1/namespaces/test/pods?fieldSelector=metadata.name%3Dpod1") .andReturn(200, notReady) .once(); diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicaSetTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicaSetTest.java index 145018abebf..c7fe14105b1 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicaSetTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicaSetTest.java @@ -208,7 +208,7 @@ void testScaleAndWait() { // list for waiting server.expect() - .withPath("/apis/apps/v1/namespaces/test/replicasets?fieldSelector=metadata.name%3Drepl1&resourceVersion=0") + .withPath("/apis/apps/v1/namespaces/test/replicasets?fieldSelector=metadata.name%3Drepl1") .andReturn(200, new ReplicaSetListBuilder().withItems(scaled).withMetadata(new ListMetaBuilder().build()).build()) .always(); diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicationControllerTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicationControllerTest.java index df9f7d815c7..cdd03aae276 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicationControllerTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ReplicationControllerTest.java @@ -212,7 +212,7 @@ void testScaleAndWait() { // list for waiting server.expect() - .withPath("/api/v1/namespaces/test/replicationcontrollers?fieldSelector=metadata.name%3Drepl1&resourceVersion=0") + .withPath("/api/v1/namespaces/test/replicationcontrollers?fieldSelector=metadata.name%3Drepl1") .andReturn(200, new ReplicationControllerListBuilder().withItems(scaled).withMetadata(new ListMetaBuilder().build()).build()) .always(); 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 index 2061755e43b..2f137a7cf1b 100644 --- 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 @@ -159,12 +159,12 @@ void testCreateOrReplaceWithoutDeleteExisting() throws Exception { void testCreateOrReplaceWithDeleteExisting() throws Exception { server.expect().delete().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(HTTP_OK, service).once(); server.expect().delete().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(HTTP_OK, configMap).once(); - server.expect().get().withPath("/api/v1/namespaces/ns1/services?fieldSelector=metadata.name%3Dmy-service&resourceVersion=0") + server.expect().get().withPath("/api/v1/namespaces/ns1/services?fieldSelector=metadata.name%3Dmy-service") .andReturn(HTTP_OK, new KubernetesListBuilder().withNewMetadata().endMetadata().build()) .once(); server.expect().get() - .withPath("/api/v1/namespaces/ns1/configmaps?fieldSelector=metadata.name%3Dmy-configmap&resourceVersion=0") + .withPath("/api/v1/namespaces/ns1/configmaps?fieldSelector=metadata.name%3Dmy-configmap") .andReturn(HTTP_OK, new KubernetesListBuilder().withNewMetadata().endMetadata().build()) .once(); @@ -202,8 +202,8 @@ void testSuccessfulWaitUntilCondition() throws InterruptedException { .anyMatch(c -> "True".equals(c.getStatus())); // The pods are never ready if you request them directly. - ResourceTest.list(server, noReady1, "0"); - ResourceTest.list(server, noReady2, "0"); + ResourceTest.list(server, noReady1, null); + ResourceTest.list(server, noReady2, null); server.expect().get().withPath( "/api/v1/namespaces/ns1/pods?fieldSelector=metadata.name%3Dpod1&resourceVersion=1&timeoutSeconds=600&allowWatchBookmarks=true&watch=true") @@ -247,8 +247,8 @@ void testPartialSuccessfulWaitUntilCondition() { .anyMatch(c -> "True".equals(c.getStatus())); // The pods are never ready if you request them directly. - ResourceTest.list(server, noReady1, "0"); - ResourceTest.list(server, noReady2, "0"); + ResourceTest.list(server, noReady1, null); + ResourceTest.list(server, noReady2, null); 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 index 28ce30b728e..9387dc043b2 100644 --- 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 @@ -327,7 +327,7 @@ void testWaitUntilReady() throws InterruptedException { * @param pod */ private void list(Pod pod) { - list(server, pod, "0"); + list(server, pod, null); } static void list(KubernetesMockServer server, Pod pod, String resourceVersion) { @@ -355,7 +355,7 @@ void testWaitUntilExistsThenReady() throws InterruptedException { // and again so that "periodicWatchUntilReady" successfully begins server.expect() .get() - .withPath("/api/v1/namespaces/test/pods?fieldSelector=metadata.name%3Dpod1&resourceVersion=0") + .withPath("/api/v1/namespaces/test/pods?fieldSelector=metadata.name%3Dpod1") .andReturn(200, noReady) .times(2); @@ -715,7 +715,7 @@ void testFromServerWaitUntilConditionAlwaysGetsResourceFromServer() throws Excep void testWaitNullDoesntExist() throws InterruptedException { server.expect() .get() - .withPath("/api/v1/namespaces/test/pods?fieldSelector=metadata.name%3Dpod1&resourceVersion=0") + .withPath("/api/v1/namespaces/test/pods?fieldSelector=metadata.name%3Dpod1") .andReturn(200, new PodListBuilder().withNewMetadata().withResourceVersion("1").endMetadata().build()) .once(); diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java index 60f5997cbaa..dee06e39505 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java @@ -210,11 +210,11 @@ void testWaitUntilReady() throws InterruptedException { .addToPorts(new EndpointPortBuilder().withPort(8443).build()) .build()) .build(); - server.expect().get().withPath("/api/v1/namespaces/ns1/endpoints?fieldSelector=metadata.name%3Dsvc1&resourceVersion=0") + server.expect().get().withPath("/api/v1/namespaces/ns1/endpoints?fieldSelector=metadata.name%3Dsvc1") .andReturn(HttpURLConnection.HTTP_OK, new EndpointsListBuilder().withItems(endpoint).withNewMetadata().withResourceVersion("1").endMetadata().build()) .once(); - server.expect().get().withPath("/api/v1/namespaces/ns1/services?fieldSelector=metadata.name%3Dsvc1&resourceVersion=0") + server.expect().get().withPath("/api/v1/namespaces/ns1/services?fieldSelector=metadata.name%3Dsvc1") .andReturn(HttpURLConnection.HTTP_OK, new ServiceListBuilder().withItems(svc1).withNewMetadata().withResourceVersion("1").endMetadata().build()) .once(); diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/StatefulSetTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/StatefulSetTest.java index a6bb49e7607..faf662fb0d9 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/StatefulSetTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/StatefulSetTest.java @@ -250,7 +250,7 @@ public void testScaleAndWait() { // list for waiting server.expect() - .withPath("/apis/apps/v1/namespaces/test/statefulsets?fieldSelector=metadata.name%3Drepl1&resourceVersion=0") + .withPath("/apis/apps/v1/namespaces/test/statefulsets?fieldSelector=metadata.name%3Drepl1") .andReturn(200, new StatefulSetListBuilder().withItems(scaled).withMetadata(new ListMetaBuilder().build()).build()) .always(); diff --git a/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/DeploymentConfigTest.java b/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/DeploymentConfigTest.java index fc0c698f790..50569f568d0 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/DeploymentConfigTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/DeploymentConfigTest.java @@ -406,7 +406,7 @@ void testWaitUntilReady() throws InterruptedException { .endStatus().build(); server.expect().get() .withPath( - "/apis/apps.openshift.io/v1/namespaces/ns1/deploymentconfigs?fieldSelector=metadata.name%3Ddc1&resourceVersion=0") + "/apis/apps.openshift.io/v1/namespaces/ns1/deploymentconfigs?fieldSelector=metadata.name%3Ddc1") .andReturn(HttpURLConnection.HTTP_OK, new DeploymentConfigListBuilder().withItems(deploymentConfig).withMetadata(new ListMeta()).build()) .always();