Skip to content

Commit

Permalink
fix fabric8io#4471: kubernetesclientbuilder method for the httpclient…
Browse files Browse the repository at this point in the history
….builder
  • Loading branch information
shawkins committed Oct 12, 2022
1 parent 25d7509 commit 58dead8
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 34 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -13,6 +13,7 @@
* Fix #4473: Fix regression in backoff interval introduced in #4365

#### Improvements
* Fix #4471: Adding KubernetesClientBuilder.withHttpClientBuilderConsumer to further customize the HttpClient for any implementation.
* Fix #4348: Introduce specific annotations for the generators
* Refactor #4441: refactoring `TokenRefreshInterceptor`
* Fix #4365: The Watch retry logic will handle more cases, as well as perform an exceptional close for events that are not properly handled. Informers can directly provide those exceptional outcomes via the SharedIndexInformer.stopped CompletableFuture.
Expand Down
Expand Up @@ -18,7 +18,6 @@

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.http.HttpClient;
import io.fabric8.kubernetes.client.utils.HttpClientUtils;
import io.fabric8.kubernetes.client.utils.Utils;

import java.util.concurrent.Executor;
Expand Down Expand Up @@ -46,15 +45,6 @@ public void shutdownNow() {

}

@Override
public HttpClient createHttpClient(Config config) {
JdkHttpClientBuilderImpl builderWrapper = newBuilder();

HttpClientUtils.applyCommonConfiguration(config, builderWrapper, this);

return builderWrapper.build();
}

@Override
public JdkHttpClientBuilderImpl newBuilder() {
return new JdkHttpClientBuilderImpl(this);
Expand Down
Expand Up @@ -15,20 +15,10 @@
*/
package io.fabric8.kubernetes.client.jetty;

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.http.HttpClient;

import static io.fabric8.kubernetes.client.utils.HttpClientUtils.applyCommonConfiguration;

public class JettyHttpClientFactory implements HttpClient.Factory {

@Override
public HttpClient createHttpClient(Config config) {
final var builder = newBuilder();
applyCommonConfiguration(config, builder, this);
return builder.build();
}

@Override
public JettyHttpClientBuilder newBuilder() {
return new JettyHttpClientBuilder(this);
Expand Down
Expand Up @@ -27,7 +27,7 @@ class JettyHttpClientFactoryTest {
@DisplayName("createHttpClient instantiates a JettyHttpClient")
void createHttpClientInstantiatesJettyHttpClient() {
// When
try (var result = new JettyHttpClientFactory().createHttpClient(Config.empty())) {
try (var result = new JettyHttpClientFactory().newBuilder(Config.empty()).build()) {
// Then
assertThat(result).isNotNull().isInstanceOf(JettyHttpClient.class);
}
Expand Down
Expand Up @@ -59,13 +59,13 @@ public Builder newBuilder() {
}

/**
* Creates an HTTP client configured to access the Kubernetes API.
* Creates an HTTP client builder configured to access the Kubernetes API.
*
* @param config Kubernetes API client config
* @return returns an HTTP client
* @return returns an HTTP client builder
*/
@Override
public OkHttpClientImpl createHttpClient(Config config) {
public OkHttpClientBuilderImpl newBuilder(Config config) {
try {
OkHttpClient.Builder httpClientBuilder = newOkHttpClientBuilder();

Expand Down Expand Up @@ -101,7 +101,7 @@ public OkHttpClientImpl createHttpClient(Config config) {

additionalConfig(httpClientBuilder);

return builderWrapper.build();
return builderWrapper;
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(e);
}
Expand Down
Expand Up @@ -23,6 +23,7 @@
import java.io.InputStream;
import java.lang.reflect.InvocationTargetException;
import java.util.concurrent.Executor;
import java.util.function.Consumer;
import java.util.function.Supplier;

/**
Expand All @@ -44,6 +45,7 @@ default void onClose(Executor executor) {
private HttpClient.Factory factory;
private Class<KubernetesClient> clazz;
private ExecutorSupplier executorSupplier;
private Consumer<HttpClient.Builder> builderConsumer;

public KubernetesClientBuilder() {
// basically the same logic as in KubernetesResourceUtil for finding list types
Expand All @@ -60,6 +62,10 @@ public KubernetesClientBuilder() {
}
}

KubernetesClientBuilder(Class<KubernetesClient> clazz) {
this.clazz = clazz;
}

public KubernetesClient build() {
if (config == null) {
config = new ConfigBuilder().build();
Expand All @@ -68,7 +74,7 @@ public KubernetesClient build() {
if (factory == null) {
return clazz.getConstructor(Config.class).newInstance(config);
}
HttpClient client = factory.createHttpClient(config);
HttpClient client = getHttpClient();
return clazz.getConstructor(HttpClient.class, Config.class, ExecutorSupplier.class).newInstance(client, config,
executorSupplier);
} catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException
Expand All @@ -77,6 +83,14 @@ public KubernetesClient build() {
}
}

HttpClient getHttpClient() {
HttpClient.Builder builder = factory.newBuilder(config);
if (this.builderConsumer != null) {
this.builderConsumer.accept(builder);
}
return builder.build();
}

public KubernetesClientBuilder withConfig(Config config) {
this.config = config;
return this;
Expand All @@ -102,7 +116,7 @@ public KubernetesClientBuilder withHttpClientFactory(HttpClient.Factory factory)
* calls and writing to streams.
* <p>
* Only override if you need more control over the number of task threads used by the kubernetes client.
*
*
* @return this builder
*/
public KubernetesClientBuilder withTaskExecutor(Executor executor) {
Expand All @@ -117,12 +131,23 @@ public KubernetesClientBuilder withTaskExecutor(Executor executor) {
* There will be a call to {@link ExecutorSupplier#onClose(Executor)} when a client is closed.
* <p>
* Only override if you need more control over the number of task threads used by the kubernetes client.
*
*
* @return this builder
*/
public KubernetesClientBuilder withTaskExecutorSupplier(ExecutorSupplier executorSupplier) {
this.executorSupplier = executorSupplier;
return this;
}

/**
* Provide additional configuration for the {@link HttpClient} that is created for this {@link KubernetesClient}.
*
* @param consumer to modify the {@link HttpClient.Builder}
* @return this builder
*/
public KubernetesClientBuilder withHttpClientBuilderConsumer(Consumer<HttpClient.Builder> consumer) {
this.builderConsumer = consumer;
return this;
}

}
Expand Up @@ -18,6 +18,7 @@

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.RequestConfig;
import io.fabric8.kubernetes.client.utils.HttpClientUtils;

import java.io.BufferedReader;
import java.net.InetSocketAddress;
Expand All @@ -33,7 +34,18 @@ public interface HttpClient extends AutoCloseable {

interface Factory {

HttpClient createHttpClient(Config config);
/**
* Create a builder that is customized by the {@link Config}. By default it
* will apply the common configuration {@link HttpClientUtils#applyCommonConfiguration(Config, Builder, Factory)}
*
* @param config the configuration to apply
* @return the configured {@link Builder}
*/
default HttpClient.Builder newBuilder(Config config) {
Builder builder = newBuilder();
HttpClientUtils.applyCommonConfiguration(config, builder, this);
return builder;
}

HttpClient.Builder newBuilder();

Expand Down
Expand Up @@ -159,7 +159,7 @@ public static HttpClient createHttpClient(Config config) {
throw new KubernetesClientException(
"No httpclient implementations found on the context classloader, please ensure your classpath includes an implementation jar");
}
return factory.createHttpClient(config);
return factory.newBuilder(config).build();
}

public static void applyCommonConfiguration(Config config, HttpClient.Builder builder, HttpClient.Factory factory) {
Expand Down
@@ -0,0 +1,21 @@
package io.fabric8.kubernetes.client;

import io.fabric8.kubernetes.client.http.HttpClient;
import io.fabric8.kubernetes.client.http.HttpClient.Factory;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

public class KubernetesClientBuilderTest {

@Test
void testHttpClientConfiguration() {
KubernetesClientBuilder builder = new KubernetesClientBuilder(null);
Factory mockFactory = Mockito.mock(HttpClient.Factory.class);
HttpClient.Builder mockBuilder = Mockito.mock(HttpClient.Builder.class);
Mockito.when(mockFactory.newBuilder(Mockito.any())).thenReturn(mockBuilder);
builder.withHttpClientFactory(mockFactory).withHttpClientBuilderConsumer(b -> b.proxyAuthorization("something"));
builder.getHttpClient();
Mockito.verify(mockBuilder).proxyAuthorization("something");
}

}
Expand Up @@ -37,29 +37,29 @@ class OkHttpClientFactoryTest {

@Test
void shouldRespectMaxRequests() {
OkHttpClientImpl client = new OkHttpClientFactory().createHttpClient(new ConfigBuilder().build());
OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(new ConfigBuilder().build()).build();

assertEquals(64, client.getOkHttpClient().dispatcher().getMaxRequests());

Config config = new ConfigBuilder()
.withMaxConcurrentRequests(120)
.build();

client = new OkHttpClientFactory().createHttpClient(config);
client = new OkHttpClientFactory().newBuilder(config).build();
assertEquals(120, client.getOkHttpClient().dispatcher().getMaxRequests());
}

@Test
void shouldRespectMaxRequestsPerHost() {
OkHttpClientImpl client = new OkHttpClientFactory().createHttpClient(new ConfigBuilder().build());
OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(new ConfigBuilder().build()).build();

assertEquals(5, client.getOkHttpClient().dispatcher().getMaxRequestsPerHost());

Config config = new ConfigBuilder()
.withMaxConcurrentRequestsPerHost(20)
.build();

client = new OkHttpClientFactory().createHttpClient(config);
client = new OkHttpClientFactory().newBuilder(config).build();

assertEquals(20, client.getOkHttpClient().dispatcher().getMaxRequestsPerHost());
}
Expand Down

0 comments on commit 58dead8

Please sign in to comment.