Skip to content

Commit 928266d

Browse files
SgitarioSgitario
committed
Fix randomly failures when deploying to OpenShift/K8s on JVM/Native
There were several places where a new KubernetesClient was being created: - Some places use `Clients.fromConfig`. - Other places use `KubernetesClientUtils.createConfig`. - And other places use `KubernetesClientBuildItem.getClient` And the problem is that every time we invoke any of these methods, Fabric8 KubernetesClient tries to locate one HttpClient.Factory and it seems that the logic to get one HttpClient.Factory sometimes gets the `VertxHttpClientFactory` implementation over the expected one `QuarkusHttpClientFactory` With this solution, it avoids to find a HttpClient.Factory using the ServiceLoader logic in Fabric8 Kubernetes Client, but it provides the expected `QuarkusHttpClientFactory` implementation always. Fix #31476
1 parent 9579d9d commit 928266d

File tree

11 files changed

+84
-55
lines changed

11 files changed

+84
-55
lines changed

‎extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,17 @@
2828

2929
import org.jboss.logging.Logger;
3030

31-
import io.dekorate.utils.Clients;
3231
import io.dekorate.utils.Packaging;
3332
import io.dekorate.utils.Serialization;
3433
import io.fabric8.kubernetes.api.model.HasMetadata;
3534
import io.fabric8.kubernetes.api.model.KubernetesList;
3635
import io.fabric8.kubernetes.api.model.Secret;
3736
import io.fabric8.kubernetes.client.Config;
3837
import io.fabric8.kubernetes.client.KubernetesClient;
38+
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
3939
import io.fabric8.kubernetes.client.KubernetesClientException;
4040
import io.fabric8.kubernetes.client.dsl.LogWatch;
41+
import io.fabric8.kubernetes.client.http.HttpClient;
4142
import io.fabric8.openshift.api.model.Build;
4243
import io.fabric8.openshift.api.model.BuildConfig;
4344
import io.fabric8.openshift.api.model.ImageStream;
@@ -231,7 +232,7 @@ public void openshiftRequirementsNative(OpenshiftConfig openshiftConfig,
231232
public void openshiftBuildFromJar(OpenshiftConfig openshiftConfig,
232233
S2iConfig s2iConfig,
233234
ContainerImageConfig containerImageConfig,
234-
KubernetesClientBuildItem kubernetesClientSupplier,
235+
KubernetesClientBuildItem kubernetesClientBuilder,
235236
ContainerImageInfoBuildItem containerImage,
236237
ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
237238
List<GeneratedFileSystemResourceBuildItem> generatedResources,
@@ -263,7 +264,7 @@ public void openshiftBuildFromJar(OpenshiftConfig openshiftConfig,
263264
return;
264265
}
265266

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

324-
try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
327+
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.buildClient()) {
325328
String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
326329

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

350-
public static void createContainerImage(KubernetesClient kubernetesClient,
355+
public static void createContainerImage(KubernetesClientBuilder kubernetesClientBuilder,
351356
GeneratedFileSystemResourceBuildItem openshiftManifests,
352357
OpenshiftConfig openshiftConfig,
353358
String base,
@@ -364,10 +369,7 @@ public static void createContainerImage(KubernetesClient kubernetesClient,
364369
throw new RuntimeException("Error creating the openshift binary build archive.", e);
365370
}
366371

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

538+
private static KubernetesClientBuilder newClientBuilderWithoutHttp2(Config configuration,
539+
HttpClient.Factory httpClientFactory) {
540+
//Let's disable http2 as it causes issues with duplicate build triggers.
541+
configuration.setHttp2Disable(true);
542+
543+
return new KubernetesClientBuilder().withConfig(configuration).withHttpClientFactory(httpClientFactory);
544+
}
545+
536546
// visible for test
537547
static String concatUnixPaths(String... elements) {
538548
StringBuilder result = new StringBuilder();

‎extensions/container-image/container-image-s2i/deployment/src/main/java/io/quarkus/container/image/s2i/deployment/S2iProcessor.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public void s2iRequirementsNative(S2iConfig s2iConfig,
171171

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

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

237-
try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
237+
try (KubernetesClient kubernetesClient = kubernetesClientBuilder.buildClient()) {
238238
String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
239239
LOG.info("Performing s2i binary build with native image on server: " + kubernetesClient.getMasterUrl()
240240
+ " in namespace:" + namespace + ".");

‎extensions/kubernetes-client/deployment-internal/src/main/java/io/quarkus/kubernetes/client/deployment/KubernetesClientBuildStep.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package io.quarkus.kubernetes.client.deployment;
22

3-
import static io.quarkus.kubernetes.client.runtime.KubernetesClientUtils.*;
3+
import static io.quarkus.kubernetes.client.runtime.KubernetesClientUtils.createConfig;
44

55
import io.quarkus.deployment.annotations.BuildStep;
6+
import io.quarkus.deployment.builditem.QuarkusBuildCloseablesBuildItem;
67
import io.quarkus.kubernetes.client.runtime.KubernetesClientBuildConfig;
8+
import io.quarkus.kubernetes.client.runtime.QuarkusHttpClientFactory;
79
import io.quarkus.kubernetes.client.spi.KubernetesClientBuildItem;
810
import io.quarkus.runtime.TlsConfig;
911

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

1416
@BuildStep
15-
public KubernetesClientBuildItem process(TlsConfig tlsConfig) {
16-
return new KubernetesClientBuildItem(createConfig(buildConfig, tlsConfig));
17+
public KubernetesClientBuildItem process(TlsConfig tlsConfig, QuarkusBuildCloseablesBuildItem closeablesBuildItem) {
18+
QuarkusHttpClientFactory httpClientFactory = new QuarkusHttpClientFactory();
19+
closeablesBuildItem.add(httpClientFactory);
20+
return new KubernetesClientBuildItem(createConfig(buildConfig, tlsConfig), httpClientFactory);
1721
}
1822
}

‎extensions/kubernetes-client/runtime-internal/src/main/java/io/quarkus/kubernetes/client/runtime/QuarkusHttpClientFactory.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

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

5+
import java.io.Closeable;
6+
57
import jakarta.enterprise.inject.spi.CDI;
68

79
import io.fabric8.kubernetes.client.Config;
@@ -10,17 +12,20 @@
1012
import io.vertx.core.Vertx;
1113
import io.vertx.core.VertxOptions;
1214

13-
public class QuarkusHttpClientFactory implements HttpClient.Factory {
15+
public class QuarkusHttpClientFactory implements HttpClient.Factory, Closeable {
1416

1517
private Vertx vertx;
18+
private boolean closeVertxOnExit;
1619

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

@@ -59,4 +64,11 @@ public VertxHttpClientBuilder<QuarkusHttpClientFactory> newBuilder() {
5964
public int priority() {
6065
return 1;
6166
}
67+
68+
@Override
69+
public void close() {
70+
if (closeVertxOnExit) {
71+
vertx.close();
72+
}
73+
}
6274
}

‎extensions/kubernetes-client/runtime-internal/src/main/resources/META-INF/services/io.fabric8.kubernetes.client.http.HttpClient$Factory‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎extensions/kubernetes-client/spi/src/main/java/io/quarkus/kubernetes/client/spi/KubernetesClientBuildItem.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,30 @@
11
package io.quarkus.kubernetes.client.spi;
22

3-
import java.util.function.Supplier;
4-
53
import io.fabric8.kubernetes.client.Config;
64
import io.fabric8.kubernetes.client.KubernetesClient;
75
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
6+
import io.fabric8.kubernetes.client.http.HttpClient;
87
import io.quarkus.builder.item.SimpleBuildItem;
98

109
public final class KubernetesClientBuildItem extends SimpleBuildItem {
1110

1211
private final Config config;
12+
private final HttpClient.Factory httpClientFactory;
1313

14-
public KubernetesClientBuildItem(Config config) {
14+
public KubernetesClientBuildItem(Config config, HttpClient.Factory httpClientFactory) {
1515
this.config = config;
16-
}
17-
18-
public Supplier<KubernetesClient> getClient() {
19-
return () -> new KubernetesClientBuilder().withConfig(config).build();
16+
this.httpClientFactory = httpClientFactory;
2017
}
2118

2219
public Config getConfig() {
2320
return config;
2421
}
2522

23+
public HttpClient.Factory getHttpClientFactory() {
24+
return httpClientFactory;
25+
}
26+
27+
public KubernetesClient buildClient() {
28+
return new KubernetesClientBuilder().withConfig(config).withHttpClientFactory(httpClientFactory).build();
29+
}
2630
}

‎extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/KnativeDeployer.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@ public class KnativeDeployer {
1717
@BuildStep
1818
public void checkEnvironment(Optional<SelectedKubernetesDeploymentTargetBuildItem> selectedDeploymentTarget,
1919
List<GeneratedKubernetesResourceBuildItem> resources,
20-
KubernetesClientBuildItem clientSupplier, BuildProducer<KubernetesDeploymentClusterBuildItem> deploymentCluster) {
20+
KubernetesClientBuildItem kubernetesClientBuilder,
21+
BuildProducer<KubernetesDeploymentClusterBuildItem> deploymentCluster) {
2122
selectedDeploymentTarget.ifPresent(target -> {
22-
if (!KubernetesDeploy.INSTANCE.checkSilently()) {
23+
if (!KubernetesDeploy.INSTANCE.checkSilently(kubernetesClientBuilder)) {
2324
return;
2425
}
2526
if (target.getEntry().getName().equals(KNATIVE)) {
26-
try (DefaultKnativeClient client = clientSupplier.getClient().get().adapt(DefaultKnativeClient.class)) {
27+
try (DefaultKnativeClient client = kubernetesClientBuilder.buildClient().adapt(DefaultKnativeClient.class)) {
2728
if (client.isSupported()) {
2829
deploymentCluster.produce(new KubernetesDeploymentClusterBuildItem(KNATIVE));
2930
} else {

‎extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/KubernetesDeploy.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
import io.fabric8.kubernetes.client.KubernetesClient;
1111
import io.fabric8.kubernetes.client.VersionInfo;
12-
import io.quarkus.kubernetes.client.runtime.KubernetesClientUtils;
12+
import io.quarkus.kubernetes.client.spi.KubernetesClientBuildItem;
1313

1414
public class KubernetesDeploy {
1515

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

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

48-
private Result doCheck() {
48+
private Result doCheck(KubernetesClientBuildItem clientBuilder) {
4949
if (!KubernetesConfigUtil.isDeploymentEnabled()) {
5050
return Result.notConfigured();
5151
}
@@ -55,9 +55,8 @@ private Result doCheck() {
5555
return Result.enabled();
5656
}
5757

58-
KubernetesClient client = KubernetesClientUtils.createClient();
59-
String masterURL = client.getConfiguration().getMasterUrl();
60-
try {
58+
String masterURL = clientBuilder.getConfig().getMasterUrl();
59+
try (KubernetesClient client = clientBuilder.buildClient()) {
6160
//Let's check if we can connect.
6261
VersionInfo version = client.getVersion();
6362
if (version == null) {
@@ -83,8 +82,6 @@ private Result doCheck() {
8382
+ masterURL + "'",
8483
e));
8584
}
86-
} finally {
87-
client.close();
8885
}
8986
}
9087

0 commit comments

Comments
 (0)