Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover Kubernetes connection after watching builds logs timeout in OCP #34374

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Recover Kubernetes connection after watching builds logs timeout in OCP #34374

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 62 additions & 55 deletions ...ent/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.jboss.logging.Logger;

Expand Down Expand Up @@ -385,7 +384,7 @@ public static void createContainerImage(KubernetesClientBuilder kubernetesClient
.collect(Collectors.toList());

applyOpenshiftResources(openShiftClient, buildResources);
openshiftBuild(openShiftClient, buildResources, tar, openshiftConfig);
openshiftBuild(buildResources, tar, openshiftConfig, kubernetesClientBuilder);
}
}

Expand Down Expand Up @@ -423,73 +422,85 @@ private static void applyOpenshiftResources(OpenShiftClient client, List<HasMeta
}
}

private static void openshiftBuild(OpenShiftClient client, List<HasMetadata> buildResources, File binaryFile,
OpenshiftConfig openshiftConfig) {
private static void openshiftBuild(List<HasMetadata> buildResources, File binaryFile,
OpenshiftConfig openshiftConfig, KubernetesClientBuilder kubernetesClientBuilder) {
distinct(buildResources).stream().filter(i -> i instanceof BuildConfig).map(i -> (BuildConfig) i)
.forEach(bc -> openshiftBuild(client, bc, binaryFile, openshiftConfig));
.forEach(bc -> {
Build build = startOpenshiftBuild(bc, binaryFile, openshiftConfig, kubernetesClientBuilder);
waitForOpenshiftBuild(build, openshiftConfig, kubernetesClientBuilder);
});
}

/**
* Performs the binary build of the specified {@link BuildConfig} with the given
* binary input.
*
* @param client The openshift client instance
* @param buildConfig The build config
* @param binaryFile The binary file
* @param openshiftConfig The openshift configuration
* @param kubernetesClientBuilder The kubernetes client builder
*/
private static void openshiftBuild(OpenShiftClient client, BuildConfig buildConfig, File binaryFile,
OpenshiftConfig openshiftConfig) {
Build build;
try {
build = client.buildConfigs().withName(buildConfig.getMetadata().getName())
.instantiateBinary()
.withTimeoutInMillis(openshiftConfig.buildTimeout.toMillis())
.fromFile(binaryFile);
} catch (Exception e) {
Optional<Build> running = runningBuildsOf(client, buildConfig).findFirst();
if (running.isPresent()) {
LOG.warn("An exception: '" + e.getMessage()
+ " ' occurred while instantiating the build, however the build has been started.");
build = running.get();
} else {
throw openshiftException(e);
private static Build startOpenshiftBuild(BuildConfig buildConfig, File binaryFile,
OpenshiftConfig openshiftConfig, KubernetesClientBuilder kubernetesClientBuilder) {
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.build()) {
OpenShiftClient client = toOpenshiftClient(kubernetesClient);
try {
return client.buildConfigs().withName(buildConfig.getMetadata().getName())
.instantiateBinary()
.withTimeoutInMillis(openshiftConfig.buildTimeout.toMillis())
.fromFile(binaryFile);
} catch (Exception e) {
Optional<Build> running = buildsOf(client, buildConfig).stream().findFirst();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also changed runningBuildsOf by buildsOf because there is a chance that the build is completed or failed when calling this method. The state is properly read in the next method.
The method runningBuildsOf was removed because there were no more usages.

if (running.isPresent()) {
LOG.warn("An exception: '" + e.getMessage()
+ " ' occurred while instantiating the build, however the build has been started.");
return running.get();
} else {
throw openshiftException(e);
}
}
}
}

private static void waitForOpenshiftBuild(Build build, OpenshiftConfig openshiftConfig,
KubernetesClientBuilder kubernetesClientBuilder) {

while (isNew(build) || isPending(build) || isRunning(build)) {
final String buildName = build.getMetadata().getName();
Build updated = client.builds().withName(buildName).get();
if (updated == null) {
throw new IllegalStateException("Build:" + build.getMetadata().getName() + " is no longer present!");
} else if (updated.getStatus() == null) {
throw new IllegalStateException("Build:" + build.getMetadata().getName() + " has no status!");
} else if (isNew(updated) || isPending(updated) || isRunning(updated)) {
build = updated;
try (LogWatch w = client.builds().withName(buildName).withPrettyOutput().watchLog();
Reader reader = new InputStreamReader(w.getOutput())) {
display(reader, openshiftConfig.buildLogLevel);
} catch (IOException e) {
// This may happen if the LogWatch is closed while we are still reading.
// We shouldn't let the build fail, so let's log a warning and display last few lines of the log
LOG.warn("Log stream closed, redisplaying last " + LOG_TAIL_SIZE + " entries:");
try {
display(client.builds().withName(buildName).tailingLines(LOG_TAIL_SIZE).getLogReader(),
Logger.Level.WARN);
} catch (IOException ex) {
// Let's ignore this.
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.build()) {
Copy link
Contributor Author

@Sgitario Sgitario Jun 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's open a new connection every time we iterate (there should not be many times).
This is what actually fixes the issue.

OpenShiftClient client = toOpenshiftClient(kubernetesClient);
Build updated = client.builds().withName(buildName).get();
if (updated == null) {
throw new IllegalStateException("Build:" + build.getMetadata().getName() + " is no longer present!");
} else if (updated.getStatus() == null) {
throw new IllegalStateException("Build:" + build.getMetadata().getName() + " has no status!");
} else if (isNew(updated) || isPending(updated) || isRunning(updated)) {
build = updated;
try (LogWatch w = client.builds().withName(buildName).withPrettyOutput().watchLog();
Reader reader = new InputStreamReader(w.getOutput())) {
display(reader, openshiftConfig.buildLogLevel);
} catch (IOException | KubernetesClientException ex) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's catch both exceptions, not only IOException.

// This may happen if the LogWatch is closed while we are still reading.
// We shouldn't let the build fail, so let's log a warning and display last few lines of the log
LOG.warn("Log stream closed, redisplaying last " + LOG_TAIL_SIZE + " entries:");
try {
display(client.builds().withName(buildName).tailingLines(LOG_TAIL_SIZE).getLogReader(),
Logger.Level.WARN);
} catch (IOException | KubernetesClientException ignored) {
// Let's ignore this.
}
}
} else if (isComplete(updated)) {
return;
} else if (isCancelled(updated)) {
throw new IllegalStateException("Build:" + buildName + " cancelled!");
} else if (isFailed(updated)) {
throw new IllegalStateException(
"Build:" + buildName + " failed! " + updated.getStatus().getMessage());
} else if (isError(updated)) {
throw new IllegalStateException(
"Build:" + buildName + " encountered error! " + updated.getStatus().getMessage());
}
} else if (isComplete(updated)) {
return;
} else if (isCancelled(updated)) {
throw new IllegalStateException("Build:" + buildName + " cancelled!");
} else if (isFailed(updated)) {
throw new IllegalStateException(
"Build:" + buildName + " failed! " + updated.getStatus().getMessage());
} else if (isError(updated)) {
throw new IllegalStateException(
"Build:" + buildName + " encountered error! " + updated.getStatus().getMessage());
}
}
}
Expand All @@ -508,10 +519,6 @@ private static List<Build> buildsOf(OpenShiftClient client, BuildConfig config)
return client.builds().withLabel(BUILD_CONFIG_NAME, config.getMetadata().getName()).list().getItems();
}

private static Stream<Build> runningBuildsOf(OpenShiftClient client, BuildConfig config) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed because of https://github.com/quarkusio/quarkus/pull/34374/files#r1245146619

return buildsOf(client, config).stream().filter(b -> RUNNING.equalsIgnoreCase(b.getStatus().getPhase()));
}

private static RuntimeException openshiftException(Throwable t) {
if (t instanceof KubernetesClientException) {
KubernetesClientErrorHandler.handle((KubernetesClientException) t);
Expand Down