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.

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

Fix randomly failures when deploying to OpenShift/K8s on JVM/Native #31503

Merged
merged 1 commit into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@

import org.jboss.logging.Logger;

import io.dekorate.utils.Clients;
import io.dekorate.utils.Packaging;
import io.dekorate.utils.Serialization;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.KubernetesList;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.dsl.LogWatch;
import io.fabric8.kubernetes.client.http.HttpClient;
import io.fabric8.openshift.api.model.Build;
import io.fabric8.openshift.api.model.BuildConfig;
import io.fabric8.openshift.api.model.ImageStream;
Expand Down Expand Up @@ -231,7 +232,7 @@ public void openshiftRequirementsNative(OpenshiftConfig openshiftConfig,
public void openshiftBuildFromJar(OpenshiftConfig openshiftConfig,
S2iConfig s2iConfig,
ContainerImageConfig containerImageConfig,
KubernetesClientBuildItem kubernetesClientSupplier,
KubernetesClientBuildItem kubernetesClientBuilder,
ContainerImageInfoBuildItem containerImage,
ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
List<GeneratedFileSystemResourceBuildItem> generatedResources,
Expand Down Expand Up @@ -263,7 +264,7 @@ public void openshiftBuildFromJar(OpenshiftConfig openshiftConfig,
return;
}

try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.buildClient()) {
String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
LOG.info("Starting (in-cluster) container image build for jar using: " + config.buildStrategy + " on server: "
+ kubernetesClient.getMasterUrl() + " in namespace:" + namespace + ".");
Expand All @@ -272,14 +273,16 @@ public void openshiftBuildFromJar(OpenshiftConfig openshiftConfig,
//For s2i kind of builds where jars are expected directly in the '/' we have to use null.
String outputDirName = out.getOutputDirectory().getFileName().toString();
String contextRoot = getContextRoot(outputDirName, packageConfig.isFastJar(), config.buildStrategy);
KubernetesClientBuilder clientBuilder = newClientBuilderWithoutHttp2(kubernetesClient.getConfiguration(),
kubernetesClientBuilder.getHttpClientFactory());
if (packageConfig.isFastJar()) {
createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
createContainerImage(clientBuilder, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
jar.getPath().getParent());
} else if (jar.getLibraryDir() != null) { //When using uber-jar the libraryDir is going to be null, potentially causing NPE.
createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
createContainerImage(clientBuilder, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
jar.getPath(), jar.getLibraryDir());
} else {
createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
createContainerImage(clientBuilder, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
jar.getPath());
}
artifactResultProducer.produce(new ArtifactResultBuildItem(null, "jar-container", Collections.emptyMap()));
Expand All @@ -300,7 +303,7 @@ private String getContextRoot(String outputDirName, boolean isFastJar, BuildStra
@BuildStep(onlyIf = { IsNormalNotRemoteDev.class, OpenshiftBuild.class, NativeBuild.class })
public void openshiftBuildFromNative(OpenshiftConfig openshiftConfig, S2iConfig s2iConfig,
ContainerImageConfig containerImageConfig,
KubernetesClientBuildItem kubernetesClientSupplier,
KubernetesClientBuildItem kubernetesClientBuilder,
ContainerImageInfoBuildItem containerImage,
ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
List<GeneratedFileSystemResourceBuildItem> generatedResources,
Expand All @@ -321,7 +324,7 @@ public void openshiftBuildFromNative(OpenshiftConfig openshiftConfig, S2iConfig
return;
}

try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.buildClient()) {
String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");

LOG.info("Starting (in-cluster) container image build for jar using: " + config.buildStrategy + " on server: "
Expand All @@ -340,14 +343,16 @@ public void openshiftBuildFromNative(OpenshiftConfig openshiftConfig, S2iConfig
//For docker kind of builds where we use instructions like: `COPY target/*.jar /deployments` it using '/target' is a requirement.
//For s2i kind of builds where jars are expected directly in the '/' we have to use null.
String contextRoot = config.buildStrategy == BuildStrategy.DOCKER ? "target" : null;
createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, out.getOutputDirectory(),
nativeImage.getPath());
createContainerImage(
newClientBuilderWithoutHttp2(kubernetesClient.getConfiguration(),
kubernetesClientBuilder.getHttpClientFactory()),
openshiftYml.get(), config, contextRoot, out.getOutputDirectory(), nativeImage.getPath());
artifactResultProducer.produce(new ArtifactResultBuildItem(null, "native-container", Collections.emptyMap()));
containerImageBuilder.produce(new ContainerImageBuilderBuildItem(OPENSHIFT));
}
}

public static void createContainerImage(KubernetesClient kubernetesClient,
public static void createContainerImage(KubernetesClientBuilder kubernetesClientBuilder,
GeneratedFileSystemResourceBuildItem openshiftManifests,
OpenshiftConfig openshiftConfig,
String base,
Expand All @@ -364,10 +369,7 @@ public static void createContainerImage(KubernetesClient kubernetesClient,
throw new RuntimeException("Error creating the openshift binary build archive.", e);
}

Config config = kubernetesClient.getConfiguration();
//Let's disable http2 as it causes issues with duplicate build triggers.
config.setHttp2Disable(true);
try (KubernetesClient client = Clients.fromConfig(config)) {
try (KubernetesClient client = kubernetesClientBuilder.build()) {
OpenShiftClient openShiftClient = toOpenshiftClient(client);
KubernetesList kubernetesList = Serialization
.unmarshalAsList(new ByteArrayInputStream(openshiftManifests.getData()));
Expand Down Expand Up @@ -533,6 +535,14 @@ private static void display(Reader logReader, Logger.Level level) throws IOExcep
}
}

private static KubernetesClientBuilder newClientBuilderWithoutHttp2(Config configuration,
HttpClient.Factory httpClientFactory) {
//Let's disable http2 as it causes issues with duplicate build triggers.
configuration.setHttp2Disable(true);

return new KubernetesClientBuilder().withConfig(configuration).withHttpClientFactory(httpClientFactory);
}

// visible for test
static String concatUnixPaths(String... elements) {
StringBuilder result = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public void s2iRequirementsNative(S2iConfig s2iConfig,

@BuildStep(onlyIf = { IsNormalNotRemoteDev.class, S2iBuild.class }, onlyIfNot = NativeBuild.class)
public void s2iBuildFromJar(S2iConfig s2iConfig, ContainerImageConfig containerImageConfig,
KubernetesClientBuildItem kubernetesClientSupplier,
KubernetesClientBuildItem kubernetesClientBuilder,
ContainerImageInfoBuildItem containerImage,
ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
List<GeneratedFileSystemResourceBuildItem> generatedResources,
Expand Down Expand Up @@ -201,7 +201,7 @@ public void s2iBuildFromJar(S2iConfig s2iConfig, ContainerImageConfig containerI
"No Openshift manifests were generated so no s2i process will be taking place");
return;
}
try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.buildClient()) {
String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
LOG.info("Performing s2i binary build with jar on server: " + kubernetesClient.getMasterUrl()
+ " in namespace:" + namespace + ".");
Expand All @@ -215,7 +215,7 @@ public void s2iBuildFromJar(S2iConfig s2iConfig, ContainerImageConfig containerI

@BuildStep(onlyIf = { IsNormalNotRemoteDev.class, S2iBuild.class, NativeBuild.class })
public void s2iBuildFromNative(S2iConfig s2iConfig, ContainerImageConfig containerImageConfig,
KubernetesClientBuildItem kubernetesClientSupplier,
KubernetesClientBuildItem kubernetesClientBuilder,
ContainerImageInfoBuildItem containerImage,
ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
List<GeneratedFileSystemResourceBuildItem> generatedResources,
Expand All @@ -234,7 +234,7 @@ public void s2iBuildFromNative(S2iConfig s2iConfig, ContainerImageConfig contain
return;
}

try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.buildClient()) {
String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
LOG.info("Performing s2i binary build with native image on server: " + kubernetesClient.getMasterUrl()
+ " in namespace:" + namespace + ".");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package io.quarkus.kubernetes.client.deployment;

import static io.quarkus.kubernetes.client.runtime.KubernetesClientUtils.*;
import static io.quarkus.kubernetes.client.runtime.KubernetesClientUtils.createConfig;

import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.builditem.QuarkusBuildCloseablesBuildItem;
import io.quarkus.kubernetes.client.runtime.KubernetesClientBuildConfig;
import io.quarkus.kubernetes.client.runtime.QuarkusHttpClientFactory;
import io.quarkus.kubernetes.client.spi.KubernetesClientBuildItem;
import io.quarkus.runtime.TlsConfig;

Expand All @@ -12,7 +14,9 @@ public class KubernetesClientBuildStep {
private KubernetesClientBuildConfig buildConfig;

@BuildStep
public KubernetesClientBuildItem process(TlsConfig tlsConfig) {
return new KubernetesClientBuildItem(createConfig(buildConfig, tlsConfig));
public KubernetesClientBuildItem process(TlsConfig tlsConfig, QuarkusBuildCloseablesBuildItem closeablesBuildItem) {
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL about QuarkusBuildCloseablesBuildItem :)

QuarkusHttpClientFactory httpClientFactory = new QuarkusHttpClientFactory();
closeablesBuildItem.add(httpClientFactory);
return new KubernetesClientBuildItem(createConfig(buildConfig, tlsConfig), httpClientFactory);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static io.vertx.core.spi.resolver.ResolverProvider.DISABLE_DNS_RESOLVER_PROP_NAME;

import java.io.Closeable;

import jakarta.enterprise.inject.spi.CDI;

import io.fabric8.kubernetes.client.Config;
Expand All @@ -10,17 +12,20 @@
import io.vertx.core.Vertx;
import io.vertx.core.VertxOptions;

public class QuarkusHttpClientFactory implements HttpClient.Factory {
public class QuarkusHttpClientFactory implements HttpClient.Factory, Closeable {

private Vertx vertx;
private boolean closeVertxOnExit;

public QuarkusHttpClientFactory() {
// The client might get initialized outside a Quarkus context that can provide the Vert.x instance
// This is the case for the MockServer / @KubernetesTestServer where the server provides the KubernetesClient instance
try {
this.vertx = CDI.current().select(Vertx.class).get();
this.closeVertxOnExit = false;
} catch (Exception e) {
this.vertx = createVertxInstance();
this.closeVertxOnExit = true;
}
}

Expand Down Expand Up @@ -59,4 +64,11 @@ public VertxHttpClientBuilder<QuarkusHttpClientFactory> newBuilder() {
public int priority() {
return 1;
}

@Override
public void close() {
if (closeVertxOnExit) {
vertx.close();
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,26 +1,30 @@
package io.quarkus.kubernetes.client.spi;

import java.util.function.Supplier;

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
import io.fabric8.kubernetes.client.http.HttpClient;
import io.quarkus.builder.item.SimpleBuildItem;

public final class KubernetesClientBuildItem extends SimpleBuildItem {

private final Config config;
private final HttpClient.Factory httpClientFactory;

public KubernetesClientBuildItem(Config config) {
public KubernetesClientBuildItem(Config config, HttpClient.Factory httpClientFactory) {
this.config = config;
}

public Supplier<KubernetesClient> getClient() {
return () -> new KubernetesClientBuilder().withConfig(config).build();
this.httpClientFactory = httpClientFactory;
}

public Config getConfig() {
return config;
}

public HttpClient.Factory getHttpClientFactory() {
return httpClientFactory;
}

public KubernetesClient buildClient() {
return new KubernetesClientBuilder().withConfig(config).withHttpClientFactory(httpClientFactory).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ public class KnativeDeployer {
@BuildStep
public void checkEnvironment(Optional<SelectedKubernetesDeploymentTargetBuildItem> selectedDeploymentTarget,
List<GeneratedKubernetesResourceBuildItem> resources,
KubernetesClientBuildItem clientSupplier, BuildProducer<KubernetesDeploymentClusterBuildItem> deploymentCluster) {
KubernetesClientBuildItem kubernetesClientBuilder,
BuildProducer<KubernetesDeploymentClusterBuildItem> deploymentCluster) {
selectedDeploymentTarget.ifPresent(target -> {
if (!KubernetesDeploy.INSTANCE.checkSilently()) {
if (!KubernetesDeploy.INSTANCE.checkSilently(kubernetesClientBuilder)) {
return;
}
if (target.getEntry().getName().equals(KNATIVE)) {
try (DefaultKnativeClient client = clientSupplier.getClient().get().adapt(DefaultKnativeClient.class)) {
try (DefaultKnativeClient client = kubernetesClientBuilder.buildClient().adapt(DefaultKnativeClient.class)) {
if (client.isSupported()) {
deploymentCluster.produce(new KubernetesDeploymentClusterBuildItem(KNATIVE));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.VersionInfo;
import io.quarkus.kubernetes.client.runtime.KubernetesClientUtils;
import io.quarkus.kubernetes.client.spi.KubernetesClientBuildItem;

public class KubernetesDeploy {

Expand All @@ -27,8 +27,8 @@ private KubernetesDeploy() {
*
* @throws RuntimeException if there was an error while communicating with the Kubernetes API server
*/
public boolean check() {
Result result = doCheck();
public boolean check(KubernetesClientBuildItem clientBuilder) {
Result result = doCheck(clientBuilder);

if (result.getException().isPresent()) {
throw result.getException().get();
Expand All @@ -41,11 +41,11 @@ public boolean check() {
* @return {@code true} if @{code quarkus.kubernetes.deploy=true} AND the target Kubernetes API server is reachable
* {@code false} otherwise or if there was an error while communicating with the Kubernetes API server
*/
public boolean checkSilently() {
return doCheck().isAllowed();
public boolean checkSilently(KubernetesClientBuildItem clientBuilder) {
return doCheck(clientBuilder).isAllowed();
}

private Result doCheck() {
private Result doCheck(KubernetesClientBuildItem clientBuilder) {
if (!KubernetesConfigUtil.isDeploymentEnabled()) {
return Result.notConfigured();
}
Expand All @@ -55,9 +55,8 @@ private Result doCheck() {
return Result.enabled();
}

KubernetesClient client = KubernetesClientUtils.createClient();
String masterURL = client.getConfiguration().getMasterUrl();
try {
String masterURL = clientBuilder.getConfig().getMasterUrl();
try (KubernetesClient client = clientBuilder.buildClient()) {
//Let's check if we can connect.
VersionInfo version = client.getVersion();
if (version == null) {
Expand All @@ -83,8 +82,6 @@ private Result doCheck() {
+ masterURL + "'",
e));
}
} finally {
client.close();
}
}

Expand Down
Loading