From e4bffb722fcc844a5a1aa783687dcef182324309 Mon Sep 17 00:00:00 2001 From: shawkins Date: Sat, 10 Jul 2021 10:30:36 -0400 Subject: [PATCH] fix #3307: updating the isReady method to not log a warning --- CHANGELOG.md | 5 ++++ .../kubernetes/client/dsl/Readiable.java | 6 +++- ...rGetWatchDeleteRecreateWaitApplicable.java | 3 +- .../client/dsl/base/BaseOperation.java | 8 +++-- ...WatchDeleteRecreateWaitApplicableImpl.java | 30 ++++++++----------- ...hDeleteRecreateWaitApplicableListImpl.java | 2 +- .../server/mock/DeploymentConfigTest.java | 15 +++++++++- 7 files changed, 46 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d281b22faea..9621ae8b0e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * Fix #3285: adding waiting for list contexts `Informable.informOnCondition` * Fix #3284: refined how builders are obtained / used by HasMetadataOperation * Fix #3314: Add more detailed information about what is generated by the CRD generator +* Fix #3307: refined the support for `Readiable.isReady` #### Dependency Upgrade * Update Tekton Pipeline Model to v0.25.0 @@ -21,6 +22,10 @@ * Fix #3291: Retrying the HTTP operation in case of IOException too * Fix #2712: Add support for watching logs in multi-container Controller resources (Deployments, StatefulSets, ReplicaSet etc) +#### _**Note**_: Breaking changes in the API +##### DSL Changes: +- #3307 `Readiable.isReady` returns a boolean rather than a Boolean + ### 5.5.0 (2021-06-30) #### Bugs diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/Readiable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/Readiable.java index f80254c7828..c8726da86f4 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/Readiable.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/Readiable.java @@ -17,5 +17,9 @@ public interface Readiable { - Boolean isReady(); + /** + * Check if the resource is ready. If no readiness check exists, this is just an existence check. + * @return true if the resource exists and is ready. + */ + boolean isReady(); } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java index 073e0a826a4..4491d64287a 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java @@ -25,5 +25,6 @@ public interface VisitFromServerGetWatchDeleteRecreateWaitApplicable extends Watchable>, Waitable, VisitFromServerWritable, - DryRunable> { + DryRunable>, + Readiable { } 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 index b8303479f13..ef59c2229d2 100755 --- 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 @@ -950,8 +950,12 @@ public Readiness getReadiness() { } @Override - public final Boolean isReady() { - return getReadiness().isReady(get()); + public final boolean isReady() { + T item = fromServer().get(); + if (item == null) { + return false; + } + return getReadiness().isReady(item); } @Override diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java index 636c758e39c..e2e55020ad4 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java @@ -153,22 +153,15 @@ public Boolean delete() { @Override public HasMetadata get() { + HasMetadata meta = acceptVisitors(asHasMetadata(item), visitors); if (fromServer) { - HasMetadata meta = acceptVisitors(asHasMetadata(item), visitors); ResourceHandler h = handlerOf(meta); - HasMetadata reloaded = h.reload(client, config, meta.getMetadata().getNamespace(), meta); - if (reloaded != null) { - HasMetadata edited = reloaded; - //Let's apply any visitor that might have been specified. - for (Visitor v : visitors) { - edited = h.edit(edited).accept(v).build(); - } - return edited; + meta = h.reload(client, config, meta.getMetadata().getNamespace(), meta); + if (meta != null) { + return acceptVisitors(meta, visitors); } - return null; - } else { - return acceptVisitors(asHasMetadata(item), visitors); } + return meta; } @Override @@ -243,13 +236,17 @@ protected Readiness getReadiness() { } @Override - public final Boolean isReady() { - return getReadiness().isReady(get()); + public final boolean isReady() { + HasMetadata meta = fromServer().get(); + if (meta == null) { + return false; + } + return getReadiness().isReady(meta); } @Override public HasMetadata waitUntilReady(long amount, TimeUnit timeUnit) { - HasMetadata meta = acceptVisitors(asHasMetadata(get()), visitors); + HasMetadata meta = get(); ResourceHandler h = handlerOf(meta); return h.waitUntilReady(client, config, meta.getMetadata().getNamespace(), meta, amount, timeUnit); } @@ -262,12 +259,11 @@ public VisitFromServerWritable dryRun(boolean isDryRun) { @Override public HasMetadata waitUntilCondition(Predicate condition, long amount, TimeUnit timeUnit) { - HasMetadata meta = acceptVisitors(asHasMetadata(get()), visitors); + HasMetadata meta = get(); ResourceHandler h = handlerOf(meta); return h.waitUntilCondition(client, config, meta.getMetadata().getNamespace(), meta, condition, amount, timeUnit); } - private static HasMetadata acceptVisitors(HasMetadata item, List visitors) { ResourceHandler h = handlerOf(item); VisitableBuilder builder = h.edit(item); diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java index 1d2ca64942c..d4180ab0ad7 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java @@ -140,7 +140,7 @@ private static void logAsNotReady(Throwable t, HasMetadata meta) { } @Override - public Boolean isReady() { + public boolean isReady() { for (final HasMetadata meta : acceptVisitors(get(), visitors)) { if (!getReadiness().isReady(meta)) { return false; 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 8a7a50801c0..ecbc983d017 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 @@ -321,15 +321,28 @@ void testIsReady() { .endCondition() .withReplicas(1).withAvailableReplicas(1) .endStatus().build(); + + // When using a resource, it should still consult the server + boolean result = client.resource(deploymentConfig).isReady(); + + // Then + assertFalse(result); + server.expect().get().withPath("/apis/apps.openshift.io/v1/namespaces/ns1/deploymentconfigs/dc1") .andReturn(HttpURLConnection.HTTP_OK, deploymentConfig) .always(); // When - boolean result = client.deploymentConfigs().inNamespace("ns1").withName("dc1").isReady(); + result = client.deploymentConfigs().inNamespace("ns1").withName("dc1").isReady(); // Then assertTrue(result); + + // When missing + result = client.deploymentConfigs().inNamespace("ns1").withName("dc2").isReady(); + + // Then + assertFalse(result); } private DeploymentConfigBuilder getDeploymentConfig() {