Skip to content

Commit

Permalink
fix fabric8io#3307: updating the isReady method to not log a warning
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed Jul 10, 2021
1 parent 386fc4a commit 728040a
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 23 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#### Improvements
* Fix #3284: refined how builders are obtained / used by HasMetadataOperation
* Fix #3307: refined the support for `Readiable.isReady`

#### Dependency Upgrade
* Update Tekton Pipeline Model to v0.25.0
Expand All @@ -17,6 +18,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ public interface VisitFromServerGetWatchDeleteRecreateWaitApplicable<T> extends
Watchable<Watcher<T>>,
Waitable<T, T>,
VisitFromServerWritable<T>,
DryRunable<VisitFromServerWritable<T>> {
DryRunable<VisitFromServerWritable<T>>,
Readiable {
}
Original file line number Diff line number Diff line change
Expand Up @@ -953,8 +953,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<HasMetadata, ?> 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
Expand Down Expand Up @@ -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<HasMetadata, ?> h = handlerOf(meta);
try {
return h.waitUntilReady(client, config, meta.getMetadata().getNamespace(), meta, amount, timeUnit);
Expand All @@ -267,7 +264,7 @@ public VisitFromServerWritable<HasMetadata> dryRun(boolean isDryRun) {
@Override
public HasMetadata waitUntilCondition(Predicate<HasMetadata> condition, long amount,
TimeUnit timeUnit) {
HasMetadata meta = acceptVisitors(asHasMetadata(get()), visitors);
HasMetadata meta = get();
ResourceHandler<HasMetadata, ?> h = handlerOf(meta);
try {
return h.waitUntilCondition(client, config, meta.getMetadata().getNamespace(), meta, condition, amount, timeUnit);
Expand All @@ -277,7 +274,6 @@ public HasMetadata waitUntilCondition(Predicate<HasMetadata> condition, long amo
}
}


private static HasMetadata acceptVisitors(HasMetadata item, List<Visitor> visitors) {
ResourceHandler<HasMetadata, ?> h = handlerOf(item);
VisitableBuilder<HasMetadata, ?> builder = h.edit(item);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 728040a

Please sign in to comment.