From e92f7971c0c37bacd4c350bcb26e2a8fd39c549a Mon Sep 17 00:00:00 2001 From: attilapiros Date: Fri, 7 May 2021 21:25:03 +0200 Subject: [PATCH 1/3] Retry HTTP for status >= 500 (exponential backoff) --- CHANGELOG.md | 1 + README.md | 3 + .../io/fabric8/kubernetes/client/Config.java | 46 +++++++- .../kubernetes/client/RequestConfig.java | 39 ++++++- .../client/dsl/base/OperationSupport.java | 23 +++- .../client/dsl/base/BaseOperationTest.java | 102 +++++++++++++++++- 6 files changed, 205 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e1e09c26f0..1612e089313 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ * Fix #3166: Add DSL Support for `machineconfiguration.openshift.io/v1` resources in OpenShiftClient * Fix #3142: Add DSL support for missing resources in `operator.openshift.io` and `monitoring.coreos.com` apiGroups * Add DSL support for missing resources in `template.openshift.io`, `helm.openshift.io`, `network.openshift.io`, `user.openshift.io` apigroups +* Fix #3087: Support HTTP operation retry with exponential backoff (for status code >= 500) #### _**Note**_: Breaking changes in the API ##### DSL Changes: diff --git a/README.md b/README.md index 4b290b83835..12d4ae5fd8d 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,9 @@ System properties are preferred over environment variables. The following system | `kubernetes.watch.reconnectLimit` / `KUBERNETES_WATCH_RECONNECTLIMIT` | Number of reconnect attempts (-1 for infinite) | `-1` | | `kubernetes.connection.timeout` / `KUBERNETES_CONNECTION_TIMEOUT` | Connection timeout in ms (0 for no timeout) | `10000` | | `kubernetes.request.timeout` / `KUBERNETES_REQUEST_TIMEOUT` | Read timeout in ms | `10000` | +| `kubernetes.request.retry.backoff.count` / `KUBERNETES_REQUEST_RETRY_BACKOFF_COUNT_SYSTEM_PROPERTY` | Retry count | `0` | +| `kubernetes.request.retry.backoff.initial` / `KUBERNETES_REQUEST_RETRY_BACKOFF_INITIAL_SYSTEM_PROPERTY` | Retry initial backoff in ms | `1000` | +| `kubernetes.request.retry.backoff.multipier` / `KUBERNETES_REQUEST_RETRY_BACKOFF_MULTIPIER_SYSTEM_PROPERTY` | Retry multipliler | `2` | | `kubernetes.rolling.timeout` / `KUBERNETES_ROLLING_TIMEOUT` | Rolling timeout in ms | `900000` | | `kubernetes.logging.interval` / `KUBERNETES_LOGGING_INTERVAL` | Logging interval in ms | `20000` | | `kubernetes.scale.timeout` / `KUBERNETES_SCALE_TIMEOUT` | Scale timeout in ms | `600000` | diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java index edf674cf136..6cd97ef0947 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java @@ -87,6 +87,9 @@ public class Config { public static final String KUBERNETES_WATCH_RECONNECT_LIMIT_SYSTEM_PROPERTY = "kubernetes.watch.reconnectLimit"; public static final String KUBERNETES_CONNECTION_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.connection.timeout"; public static final String KUBERNETES_REQUEST_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.request.timeout"; + public static final String KUBERNETES_REQUEST_RETRY_BACKOFF_COUNT_SYSTEM_PROPERTY = "kubernetes.request.retry.count"; + public static final String KUBERNETES_REQUEST_RETRY_BACKOFF_INITIAL_SYSTEM_PROPERTY = "kubernetes.request.retry.backoff.initial"; + public static final String KUBERNETES_REQUEST_RETRY_BACKOFF_MULTIPLIER_SYSTEM_PROPERTY = "kubernetes.request.retry.backoff.multiplier"; public static final String KUBERNETES_ROLLING_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.rolling.timeout"; public static final String KUBERNETES_LOGGING_INTERVAL_SYSTEM_PROPERTY = "kubernetes.logging.interval"; public static final String KUBERNETES_SCALE_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.scale.timeout"; @@ -135,6 +138,10 @@ public class Config { public static final Integer DEFAULT_MAX_CONCURRENT_REQUESTS = 64; public static final Integer DEFAULT_MAX_CONCURRENT_REQUESTS_PER_HOST = 5; + public static final Integer DEFAULT_REQUEST_RETRY_BACKOFF_COUNT = 0; + public static final Integer DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL = 1000; + public static final Integer DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER = 2; + public static final String HTTP_PROTOCOL_PREFIX = "http://"; public static final String HTTPS_PROTOCOL_PREFIX = "https://"; @@ -173,6 +180,9 @@ public class Config { private int watchReconnectInterval = 1000; private int watchReconnectLimit = -1; private int connectionTimeout = 10 * 1000; + private int requestRetryBackoffCount; + private int requestRetryBackoffInitial; + private int requestRetryBackoffMultiplier; private int requestTimeout = 10 * 1000; private long rollingTimeout = DEFAULT_ROLLING_TIMEOUT; private long scaleTimeout = DEFAULT_SCALE_TIMEOUT; @@ -289,11 +299,11 @@ private static String ensureHttps(String masterUrl, Config config) { @Deprecated public Config(String masterUrl, String apiVersion, String namespace, boolean trustCerts, boolean disableHostnameVerification, String caCertFile, String caCertData, String clientCertFile, String clientCertData, String clientKeyFile, String clientKeyData, String clientKeyAlgo, String clientKeyPassphrase, String username, String password, String oauthToken, int watchReconnectInterval, int watchReconnectLimit, int connectionTimeout, int requestTimeout, long rollingTimeout, long scaleTimeout, int loggingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost, String httpProxy, String httpsProxy, String[] noProxy, Map errorMessages, String userAgent, TlsVersion[] tlsVersions, long websocketTimeout, long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras) { - this(masterUrl, apiVersion, namespace, trustCerts, disableHostnameVerification, caCertFile, caCertData, clientCertFile, clientCertData, clientKeyFile, clientKeyData, clientKeyAlgo, clientKeyPassphrase, username, password, oauthToken, watchReconnectInterval, watchReconnectLimit, connectionTimeout, requestTimeout, rollingTimeout, scaleTimeout, loggingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, false, httpProxy, httpsProxy, noProxy, errorMessages, userAgent, tlsVersions, websocketTimeout, websocketPingInterval, proxyUsername, proxyPassword, trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups, impersonateExtras, null,null); + this(masterUrl, apiVersion, namespace, trustCerts, disableHostnameVerification, caCertFile, caCertData, clientCertFile, clientCertData, clientKeyFile, clientKeyData, clientKeyAlgo, clientKeyPassphrase, username, password, oauthToken, watchReconnectInterval, watchReconnectLimit, connectionTimeout, requestTimeout, rollingTimeout, scaleTimeout, loggingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, false, httpProxy, httpsProxy, noProxy, errorMessages, userAgent, tlsVersions, websocketTimeout, websocketPingInterval, proxyUsername, proxyPassword, trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups, impersonateExtras, null,null, DEFAULT_REQUEST_RETRY_BACKOFF_COUNT, DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL, DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER); } @Buildable(builderPackage = "io.fabric8.kubernetes.api.builder", editableEnabled = false) - public Config(String masterUrl, String apiVersion, String namespace, boolean trustCerts, boolean disableHostnameVerification, String caCertFile, String caCertData, String clientCertFile, String clientCertData, String clientKeyFile, String clientKeyData, String clientKeyAlgo, String clientKeyPassphrase, String username, String password, String oauthToken, int watchReconnectInterval, int watchReconnectLimit, int connectionTimeout, int requestTimeout, long rollingTimeout, long scaleTimeout, int loggingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost, boolean http2Disable, String httpProxy, String httpsProxy, String[] noProxy, Map errorMessages, String userAgent, TlsVersion[] tlsVersions, long websocketTimeout, long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras, OAuthTokenProvider oauthTokenProvider,Map customHeaders) { + public Config(String masterUrl, String apiVersion, String namespace, boolean trustCerts, boolean disableHostnameVerification, String caCertFile, String caCertData, String clientCertFile, String clientCertData, String clientKeyFile, String clientKeyData, String clientKeyAlgo, String clientKeyPassphrase, String username, String password, String oauthToken, int watchReconnectInterval, int watchReconnectLimit, int connectionTimeout, int requestTimeout, long rollingTimeout, long scaleTimeout, int loggingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost, boolean http2Disable, String httpProxy, String httpsProxy, String[] noProxy, Map errorMessages, String userAgent, TlsVersion[] tlsVersions, long websocketTimeout, long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras, OAuthTokenProvider oauthTokenProvider,Map customHeaders, int requestRetryBackoffCount, int requestRetryBackoffInitial, int requestRetryBackoffMultiplier) { this.masterUrl = masterUrl; this.apiVersion = apiVersion; this.namespace = namespace; @@ -308,7 +318,7 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru this.clientKeyAlgo = clientKeyAlgo; this.clientKeyPassphrase = clientKeyPassphrase; - this.requestConfig = new RequestConfig(username, password, oauthToken, watchReconnectLimit, watchReconnectInterval, connectionTimeout, rollingTimeout, requestTimeout, scaleTimeout, loggingInterval, websocketTimeout, websocketPingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, oauthTokenProvider); + this.requestConfig = new RequestConfig(username, password, oauthToken, watchReconnectLimit, watchReconnectInterval, connectionTimeout, rollingTimeout, requestTimeout, scaleTimeout, loggingInterval, websocketTimeout, websocketPingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, oauthTokenProvider, requestRetryBackoffCount, requestRetryBackoffInitial, requestRetryBackoffMultiplier); this.requestConfig.setImpersonateUsername(impersonateUsername); this.requestConfig.setImpersonateGroups(impersonateGroups); this.requestConfig.setImpersonateExtras(impersonateExtras); @@ -399,6 +409,9 @@ public static void configFromSysPropsOrEnvVars(Config config) { config.setConnectionTimeout(Utils.getSystemPropertyOrEnvVar(KUBERNETES_CONNECTION_TIMEOUT_SYSTEM_PROPERTY, config.getConnectionTimeout())); config.setRequestTimeout(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_TIMEOUT_SYSTEM_PROPERTY, config.getRequestTimeout())); + config.setRequestRetryBackoffCount(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFF_COUNT_SYSTEM_PROPERTY, config.getRequestRetryBackoffCount())); + config.setRequestRetryBackoffInitial(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFF_INITIAL_SYSTEM_PROPERTY, config.getRequestRetryBackoffInitial())); + config.setRequestRetryBackoffMultiplier(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFF_MULTIPLIER_SYSTEM_PROPERTY, config.getRequestRetryBackoffMultiplier())); String configuredWebsocketTimeout = Utils.getSystemPropertyOrEnvVar(KUBERNETES_WEBSOCKET_TIMEOUT_SYSTEM_PROPERTY, String.valueOf(config.getWebsocketTimeout())); if (configuredWebsocketTimeout != null) { @@ -1036,6 +1049,33 @@ public void setRequestTimeout(int requestTimeout) { this.requestConfig.setRequestTimeout(requestTimeout); } + @JsonProperty("requestRetryBackoffCount") + public int getRequestRetryBackoffCount() { + return getRequestConfig().getRequestRetryBackoffCount(); + } + + public void setRequestRetryBackoffCount(int requestRetryBackoffCount) { + requestConfig.setRequestRetryBackoffCount(requestRetryBackoffCount); + } + + @JsonProperty("requestRetryBackoffInitial") + public int getRequestRetryBackoffInitial() { + return getRequestConfig().getRequestRetryBackoffInitial(); + } + + public void setRequestRetryBackoffInitial(int requestRetryBackoffInitial) { + requestConfig.setRequestRetryBackoffInitial(requestRetryBackoffInitial); + } + + @JsonProperty("requestRetryBackoffMultiplier") + public int getRequestRetryBackoffMultiplier() { + return getRequestConfig().getRequestRetryBackoffMultiplier(); + } + + public void setRequestRetryBackoffMultiplier(int requestRetryBackoffMultiplier) { + requestConfig.setRequestRetryBackoffMultiplier(requestRetryBackoffMultiplier); + } + @JsonProperty("rollingTimeout") public long getRollingTimeout() { return getRequestConfig().getRollingTimeout(); diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java index 12b08c3685f..413259aebd5 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java @@ -26,6 +26,9 @@ import static io.fabric8.kubernetes.client.Config.DEFAULT_LOGGING_INTERVAL; import static io.fabric8.kubernetes.client.Config.DEFAULT_MAX_CONCURRENT_REQUESTS; import static io.fabric8.kubernetes.client.Config.DEFAULT_MAX_CONCURRENT_REQUESTS_PER_HOST; +import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFF_COUNT; +import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL; +import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER; import static io.fabric8.kubernetes.client.Config.DEFAULT_ROLLING_TIMEOUT; import static io.fabric8.kubernetes.client.Config.DEFAULT_SCALE_TIMEOUT; import static io.fabric8.kubernetes.client.Config.DEFAULT_WEBSOCKET_PING_INTERVAL; @@ -45,6 +48,9 @@ public class RequestConfig { private int watchReconnectInterval = 1000; private int watchReconnectLimit = -1; private int connectionTimeout = 10 * 1000; + private int requestRetryBackoffCount = DEFAULT_REQUEST_RETRY_BACKOFF_COUNT; + private int requestRetryBackoffInitial = DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL; + private int requestRetryBackoffMultiplier = DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER; private int requestTimeout = 10 * 1000; private long rollingTimeout = DEFAULT_ROLLING_TIMEOUT; private long scaleTimeout = DEFAULT_SCALE_TIMEOUT; @@ -83,7 +89,8 @@ public RequestConfig(String username, String password, String oauthToken, long websocketTimeout, long websocketPingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost) { this(username, password, oauthToken, watchReconnectLimit, watchReconnectInterval, connectionTimeout, rollingTimeout, requestTimeout, scaleTimeout, loggingInterval, - websocketTimeout, websocketPingInterval,maxConcurrentRequests, maxConcurrentRequestsPerHost, null); + websocketTimeout, websocketPingInterval,maxConcurrentRequests, maxConcurrentRequestsPerHost, null, DEFAULT_REQUEST_RETRY_BACKOFF_COUNT, DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL, + DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER); } @Buildable(builderPackage = "io.fabric8.kubernetes.api.builder", editableEnabled = false) @@ -91,7 +98,8 @@ public RequestConfig(String username, String password, String oauthToken, int watchReconnectLimit, int watchReconnectInterval, int connectionTimeout, long rollingTimeout, int requestTimeout, long scaleTimeout, int loggingInterval, long websocketTimeout, long websocketPingInterval, - int maxConcurrentRequests, int maxConcurrentRequestsPerHost, OAuthTokenProvider oauthTokenProvider) { + int maxConcurrentRequests, int maxConcurrentRequestsPerHost, OAuthTokenProvider oauthTokenProvider, + int requestRetryBackoffCount, int requestRetryBackoffInitial, int requestRetryBackoffMultiplier) { this.username = username; this.oauthToken = oauthToken; this.password = password; @@ -107,6 +115,9 @@ public RequestConfig(String username, String password, String oauthToken, this.maxConcurrentRequests = maxConcurrentRequests; this.maxConcurrentRequestsPerHost = maxConcurrentRequestsPerHost; this.oauthTokenProvider = oauthTokenProvider; + this.requestRetryBackoffCount = requestRetryBackoffCount; + this.requestRetryBackoffInitial = requestRetryBackoffInitial; + this.requestRetryBackoffMultiplier = requestRetryBackoffMultiplier; } public String getUsername() { @@ -168,6 +179,30 @@ public void setRequestTimeout(int requestTimeout) { this.requestTimeout = requestTimeout; } + public int getRequestRetryBackoffCount() { + return requestRetryBackoffCount; + } + + public void setRequestRetryBackoffCount(int requestRetryBackoffCount) { + this.requestRetryBackoffCount = requestRetryBackoffCount; + } + + public int getRequestRetryBackoffInitial() { + return requestRetryBackoffInitial; + } + + public void setRequestRetryBackoffInitial(int requestRetryBackoffInitial) { + this.requestRetryBackoffInitial = requestRetryBackoffInitial; + } + + public int getRequestRetryBackoffMultiplier() { + return requestRetryBackoffMultiplier; + } + + public void setRequestRetryBackoffMultiplier(int requestRetryBackoffMultiplier) { + this.requestRetryBackoffMultiplier = requestRetryBackoffMultiplier; + } + public int getConnectionTimeout() { return connectionTimeout; } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java index 89736efdb86..820ad9ee230 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java @@ -39,6 +39,8 @@ import okhttp3.RequestBody; import okhttp3.Response; import okhttp3.ResponseBody; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.InputStream; @@ -58,6 +60,7 @@ public class OperationSupport { public static final MediaType JSON_MERGE_PATCH = MediaType.parse("application/merge-patch+json"); protected static final ObjectMapper JSON_MAPPER = Serialization.jsonMapper(); protected static final ObjectMapper YAML_MAPPER = Serialization.yamlMapper(); + private static final Logger LOG = LoggerFactory.getLogger(OperationSupport.class); private static final String CLIENT_STATUS_FLAG = "CLIENT_STATUS_FLAG"; protected OperationContext context; @@ -538,7 +541,7 @@ protected T handleResponse(OkHttpClient client, Request.Builder requestBuild protected T handleResponse(OkHttpClient client, Request.Builder requestBuilder, Class type, Map parameters) throws ExecutionException, InterruptedException, IOException { VersionUsageUtils.log(this.resourceT, this.apiGroupVersion); Request request = requestBuilder.build(); - Response response = client.newCall(request).execute(); + Response response = retryWithExponentialBackoff(client, request); try (ResponseBody body = response.body()) { assertResponseCode(request, response); if (type != null) { @@ -560,6 +563,24 @@ protected T handleResponse(OkHttpClient client, Request.Builder requestBuild } } + protected Response retryWithExponentialBackoff(OkHttpClient client, Request request) throws InterruptedException, IOException { + int numRetries = config.getRequestRetryBackoffCount(); + Response response; + long currentBackOff = config.getRequestRetryBackoffInitial(); + boolean doRetry; + do { + response = client.newCall(request).execute(); + numRetries--; + doRetry = numRetries != -1 && response.code() >= 500; + if (doRetry) { + LOG.debug("HTTP operation on url: {} should be retried as the response code was {}, retrying after {} millis", request.url(), response.code(), currentBackOff); + Thread.sleep(currentBackOff); + currentBackOff *= config.getRequestRetryBackoffMultiplier(); + } + } while(doRetry); + return response; + } + /** * Checks if the response status code is the expected and throws the appropriate KubernetesClientException if not. * diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java index 67cac389af9..aaf1ee54eab 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java @@ -15,17 +15,25 @@ */ package io.fabric8.kubernetes.client.dsl.base; +import static okhttp3.Protocol.HTTP_1_1; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.mockito.Mockito.verify; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import java.io.IOException; +import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; +import java.util.concurrent.atomic.AtomicInteger; import java.util.HashMap; import java.util.Map; @@ -39,14 +47,19 @@ import io.fabric8.kubernetes.client.ConfigBuilder; import io.fabric8.kubernetes.client.dsl.EditReplacePatchDeletable; import io.fabric8.kubernetes.client.dsl.Resource; +import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.client.utils.Serialization; import io.fabric8.kubernetes.client.utils.URLUtils; +import okhttp3.Call; +import okhttp3.MediaType; +import okhttp3.OkHttpClient; import okhttp3.Request; -import org.junit.jupiter.api.Assertions; +import okhttp3.Response; +import okhttp3.ResponseBody; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.client.dsl.internal.PodOperationContext; import io.fabric8.kubernetes.client.dsl.internal.core.v1.PodOperationsImpl; -import org.mockito.ArgumentCaptor; class BaseOperationTest { @@ -209,4 +222,87 @@ void testGetWriteOperationUrlWithDryRunDisabled() throws MalformedURLException { assertNotNull(result); assertEquals("https://172.17.0.2:8443/api/v1/namespaces/ns1/pods/foo", result.toString()); } + + private OkHttpClient newHttpClientWithSomeFailures(final AtomicInteger httpExecutionCounter, final int numFailures) { + OkHttpClient mockClient = mock(OkHttpClient.class); + when(mockClient.newCall(any())).thenAnswer( + invocation -> { + Call mockCall = mock(Call.class); + Request req = invocation.getArgument(0); + when(mockCall.execute()).thenAnswer(i -> { + int count = httpExecutionCounter.getAndIncrement(); + if (count < numFailures) { + return new Response.Builder().request(req).message("Internal Server Error").protocol(HTTP_1_1).code(500).build(); + } else { + Pod podNoLabels = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build(); + ResponseBody body = ResponseBody.create(MediaType.get("application/json"), Serialization.asJson(podNoLabels)); + return new Response.Builder().request(req).protocol(HTTP_1_1).body(body).message("OK").code(HttpURLConnection.HTTP_OK).build(); + } + }); + return mockCall; + } + ); + return mockClient; + } + + @Test + void testNoHttpRetryWithDefaultConfig() throws MalformedURLException, IOException { + final AtomicInteger httpExecutionCounter = new AtomicInteger(0); + OkHttpClient mockClient = newHttpClientWithSomeFailures(httpExecutionCounter, 1000); + BaseOperation> baseOp = new BaseOperation(new OperationContext() + .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").build()) + .withPlural("pods") + .withName("test-pod") + .withOkhttpClient(mockClient)); + baseOp.setType(Pod.class); + + // When + Exception exception = assertThrows(KubernetesClientException.class, () -> { + Pod result = baseOp.get(); + }); + + // Then + assertTrue(exception.getMessage().contains("Internal Server Error")); + assertEquals(1, httpExecutionCounter.get()); + } + + @Test + void testHttpRetryWithMoreFailuresThanRetries() throws MalformedURLException, IOException { + final AtomicInteger httpExecutionCounter = new AtomicInteger(0); + OkHttpClient mockClient = newHttpClientWithSomeFailures(httpExecutionCounter, 1000); + BaseOperation> baseOp = new BaseOperation(new OperationContext() + .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").withRequestRetryBackoffCount(3).build()) + .withPlural("pods") + .withName("test-pod") + .withOkhttpClient(mockClient)); + baseOp.setType(Pod.class); + + // When + Exception exception = assertThrows(KubernetesClientException.class, () -> { + Pod result = baseOp.get(); + }); + + // Then + assertTrue(exception.getMessage().contains("Internal Server Error")); + assertEquals("Expected 4 calls: one normal try and 3 backoff retries!", 4, httpExecutionCounter.get()); + } + + @Test + void testHttpRetryWithLessFailuresThanRetries() throws MalformedURLException, IOException { + final AtomicInteger httpExecutionCounter = new AtomicInteger(0); + OkHttpClient mockClient = newHttpClientWithSomeFailures(httpExecutionCounter, 2); + BaseOperation> baseOp = new BaseOperation(new OperationContext() + .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").withRequestRetryBackoffCount(3).build()) + .withPlural("pods") + .withName("test-pod") + .withOkhttpClient(mockClient)); + baseOp.setType(Pod.class); + + // When + Pod result = baseOp.get(); + + // Then + assertNotNull(result); + assertEquals("Expected 3 calls: 2 failures and 1 success!", 3, httpExecutionCounter.get()); + } } From 3c95a6f7da2271392a76ae806d475018d33b66d3 Mon Sep 17 00:00:00 2001 From: attilapiros Date: Fri, 28 May 2021 09:17:09 +0200 Subject: [PATCH 2/3] Extract and reuse retry interval calculation --- README.md | 5 +- .../io/fabric8/kubernetes/client/Config.java | 55 +++++++------------ .../kubernetes/client/RequestConfig.java | 46 ++++++---------- .../client/dsl/base/OperationSupport.java | 27 ++++++--- .../dsl/internal/AbstractWatchManager.java | 11 ++-- .../ExponentialBackoffIntervalCalculator.java | 36 ++++++++++++ .../client/dsl/base/BaseOperationTest.java | 4 +- 7 files changed, 101 insertions(+), 83 deletions(-) create mode 100644 kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ExponentialBackoffIntervalCalculator.java diff --git a/README.md b/README.md index 12d4ae5fd8d..04ff839ff39 100644 --- a/README.md +++ b/README.md @@ -92,9 +92,8 @@ System properties are preferred over environment variables. The following system | `kubernetes.watch.reconnectLimit` / `KUBERNETES_WATCH_RECONNECTLIMIT` | Number of reconnect attempts (-1 for infinite) | `-1` | | `kubernetes.connection.timeout` / `KUBERNETES_CONNECTION_TIMEOUT` | Connection timeout in ms (0 for no timeout) | `10000` | | `kubernetes.request.timeout` / `KUBERNETES_REQUEST_TIMEOUT` | Read timeout in ms | `10000` | -| `kubernetes.request.retry.backoff.count` / `KUBERNETES_REQUEST_RETRY_BACKOFF_COUNT_SYSTEM_PROPERTY` | Retry count | `0` | -| `kubernetes.request.retry.backoff.initial` / `KUBERNETES_REQUEST_RETRY_BACKOFF_INITIAL_SYSTEM_PROPERTY` | Retry initial backoff in ms | `1000` | -| `kubernetes.request.retry.backoff.multipier` / `KUBERNETES_REQUEST_RETRY_BACKOFF_MULTIPIER_SYSTEM_PROPERTY` | Retry multipliler | `2` | +| `kubernetes.request.retry.backoffLimit` / `KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY` | Number of retry attempts | `0` | +| `kubernetes.request.retry.backoffInterval` / `KUBERNETES_REQUEST_RETRY_BACKOFFINTERVAL_SYSTEM_PROPERTY` | Retry initial backoff interval in ms | `1000` | | `kubernetes.rolling.timeout` / `KUBERNETES_ROLLING_TIMEOUT` | Rolling timeout in ms | `900000` | | `kubernetes.logging.interval` / `KUBERNETES_LOGGING_INTERVAL` | Logging interval in ms | `20000` | | `kubernetes.scale.timeout` / `KUBERNETES_SCALE_TIMEOUT` | Scale timeout in ms | `600000` | diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java index 6cd97ef0947..afae4ddffc7 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java @@ -87,9 +87,8 @@ public class Config { public static final String KUBERNETES_WATCH_RECONNECT_LIMIT_SYSTEM_PROPERTY = "kubernetes.watch.reconnectLimit"; public static final String KUBERNETES_CONNECTION_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.connection.timeout"; public static final String KUBERNETES_REQUEST_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.request.timeout"; - public static final String KUBERNETES_REQUEST_RETRY_BACKOFF_COUNT_SYSTEM_PROPERTY = "kubernetes.request.retry.count"; - public static final String KUBERNETES_REQUEST_RETRY_BACKOFF_INITIAL_SYSTEM_PROPERTY = "kubernetes.request.retry.backoff.initial"; - public static final String KUBERNETES_REQUEST_RETRY_BACKOFF_MULTIPLIER_SYSTEM_PROPERTY = "kubernetes.request.retry.backoff.multiplier"; + public static final String KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY = "kubernetes.request.retry.backoffLimit"; + public static final String KUBERNETES_REQUEST_RETRY_BACKOFFINTERVAL_SYSTEM_PROPERTY = "kubernetes.request.retry.backoffInterval"; public static final String KUBERNETES_ROLLING_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.rolling.timeout"; public static final String KUBERNETES_LOGGING_INTERVAL_SYSTEM_PROPERTY = "kubernetes.logging.interval"; public static final String KUBERNETES_SCALE_TIMEOUT_SYSTEM_PROPERTY = "kubernetes.scale.timeout"; @@ -138,9 +137,8 @@ public class Config { public static final Integer DEFAULT_MAX_CONCURRENT_REQUESTS = 64; public static final Integer DEFAULT_MAX_CONCURRENT_REQUESTS_PER_HOST = 5; - public static final Integer DEFAULT_REQUEST_RETRY_BACKOFF_COUNT = 0; - public static final Integer DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL = 1000; - public static final Integer DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER = 2; + public static final Integer DEFAULT_REQUEST_RETRY_BACKOFFLIMIT = 0; + public static final Integer DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL = 1000; public static final String HTTP_PROTOCOL_PREFIX = "http://"; public static final String HTTPS_PROTOCOL_PREFIX = "https://"; @@ -180,9 +178,8 @@ public class Config { private int watchReconnectInterval = 1000; private int watchReconnectLimit = -1; private int connectionTimeout = 10 * 1000; - private int requestRetryBackoffCount; - private int requestRetryBackoffInitial; - private int requestRetryBackoffMultiplier; + private int requestRetryBackoffLimit; + private int requestRetryBackoffInterval; private int requestTimeout = 10 * 1000; private long rollingTimeout = DEFAULT_ROLLING_TIMEOUT; private long scaleTimeout = DEFAULT_SCALE_TIMEOUT; @@ -299,11 +296,11 @@ private static String ensureHttps(String masterUrl, Config config) { @Deprecated public Config(String masterUrl, String apiVersion, String namespace, boolean trustCerts, boolean disableHostnameVerification, String caCertFile, String caCertData, String clientCertFile, String clientCertData, String clientKeyFile, String clientKeyData, String clientKeyAlgo, String clientKeyPassphrase, String username, String password, String oauthToken, int watchReconnectInterval, int watchReconnectLimit, int connectionTimeout, int requestTimeout, long rollingTimeout, long scaleTimeout, int loggingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost, String httpProxy, String httpsProxy, String[] noProxy, Map errorMessages, String userAgent, TlsVersion[] tlsVersions, long websocketTimeout, long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras) { - this(masterUrl, apiVersion, namespace, trustCerts, disableHostnameVerification, caCertFile, caCertData, clientCertFile, clientCertData, clientKeyFile, clientKeyData, clientKeyAlgo, clientKeyPassphrase, username, password, oauthToken, watchReconnectInterval, watchReconnectLimit, connectionTimeout, requestTimeout, rollingTimeout, scaleTimeout, loggingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, false, httpProxy, httpsProxy, noProxy, errorMessages, userAgent, tlsVersions, websocketTimeout, websocketPingInterval, proxyUsername, proxyPassword, trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups, impersonateExtras, null,null, DEFAULT_REQUEST_RETRY_BACKOFF_COUNT, DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL, DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER); + this(masterUrl, apiVersion, namespace, trustCerts, disableHostnameVerification, caCertFile, caCertData, clientCertFile, clientCertData, clientKeyFile, clientKeyData, clientKeyAlgo, clientKeyPassphrase, username, password, oauthToken, watchReconnectInterval, watchReconnectLimit, connectionTimeout, requestTimeout, rollingTimeout, scaleTimeout, loggingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, false, httpProxy, httpsProxy, noProxy, errorMessages, userAgent, tlsVersions, websocketTimeout, websocketPingInterval, proxyUsername, proxyPassword, trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups, impersonateExtras, null,null, DEFAULT_REQUEST_RETRY_BACKOFFLIMIT, DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL); } @Buildable(builderPackage = "io.fabric8.kubernetes.api.builder", editableEnabled = false) - public Config(String masterUrl, String apiVersion, String namespace, boolean trustCerts, boolean disableHostnameVerification, String caCertFile, String caCertData, String clientCertFile, String clientCertData, String clientKeyFile, String clientKeyData, String clientKeyAlgo, String clientKeyPassphrase, String username, String password, String oauthToken, int watchReconnectInterval, int watchReconnectLimit, int connectionTimeout, int requestTimeout, long rollingTimeout, long scaleTimeout, int loggingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost, boolean http2Disable, String httpProxy, String httpsProxy, String[] noProxy, Map errorMessages, String userAgent, TlsVersion[] tlsVersions, long websocketTimeout, long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras, OAuthTokenProvider oauthTokenProvider,Map customHeaders, int requestRetryBackoffCount, int requestRetryBackoffInitial, int requestRetryBackoffMultiplier) { + public Config(String masterUrl, String apiVersion, String namespace, boolean trustCerts, boolean disableHostnameVerification, String caCertFile, String caCertData, String clientCertFile, String clientCertData, String clientKeyFile, String clientKeyData, String clientKeyAlgo, String clientKeyPassphrase, String username, String password, String oauthToken, int watchReconnectInterval, int watchReconnectLimit, int connectionTimeout, int requestTimeout, long rollingTimeout, long scaleTimeout, int loggingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost, boolean http2Disable, String httpProxy, String httpsProxy, String[] noProxy, Map errorMessages, String userAgent, TlsVersion[] tlsVersions, long websocketTimeout, long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras, OAuthTokenProvider oauthTokenProvider,Map customHeaders, int requestRetryBackoffLimit, int requestRetryBackoffInterval) { this.masterUrl = masterUrl; this.apiVersion = apiVersion; this.namespace = namespace; @@ -318,7 +315,7 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru this.clientKeyAlgo = clientKeyAlgo; this.clientKeyPassphrase = clientKeyPassphrase; - this.requestConfig = new RequestConfig(username, password, oauthToken, watchReconnectLimit, watchReconnectInterval, connectionTimeout, rollingTimeout, requestTimeout, scaleTimeout, loggingInterval, websocketTimeout, websocketPingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, oauthTokenProvider, requestRetryBackoffCount, requestRetryBackoffInitial, requestRetryBackoffMultiplier); + this.requestConfig = new RequestConfig(username, password, oauthToken, watchReconnectLimit, watchReconnectInterval, connectionTimeout, rollingTimeout, requestTimeout, scaleTimeout, loggingInterval, websocketTimeout, websocketPingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, oauthTokenProvider, requestRetryBackoffLimit, requestRetryBackoffInterval); this.requestConfig.setImpersonateUsername(impersonateUsername); this.requestConfig.setImpersonateGroups(impersonateGroups); this.requestConfig.setImpersonateExtras(impersonateExtras); @@ -409,9 +406,8 @@ public static void configFromSysPropsOrEnvVars(Config config) { config.setConnectionTimeout(Utils.getSystemPropertyOrEnvVar(KUBERNETES_CONNECTION_TIMEOUT_SYSTEM_PROPERTY, config.getConnectionTimeout())); config.setRequestTimeout(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_TIMEOUT_SYSTEM_PROPERTY, config.getRequestTimeout())); - config.setRequestRetryBackoffCount(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFF_COUNT_SYSTEM_PROPERTY, config.getRequestRetryBackoffCount())); - config.setRequestRetryBackoffInitial(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFF_INITIAL_SYSTEM_PROPERTY, config.getRequestRetryBackoffInitial())); - config.setRequestRetryBackoffMultiplier(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFF_MULTIPLIER_SYSTEM_PROPERTY, config.getRequestRetryBackoffMultiplier())); + config.setRequestRetryBackoffLimit(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY, config.getRequestRetryBackoffLimit())); + config.setRequestRetryBackoffInterval(Utils.getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFFINTERVAL_SYSTEM_PROPERTY, config.getRequestRetryBackoffInterval())); String configuredWebsocketTimeout = Utils.getSystemPropertyOrEnvVar(KUBERNETES_WEBSOCKET_TIMEOUT_SYSTEM_PROPERTY, String.valueOf(config.getWebsocketTimeout())); if (configuredWebsocketTimeout != null) { @@ -1049,31 +1045,22 @@ public void setRequestTimeout(int requestTimeout) { this.requestConfig.setRequestTimeout(requestTimeout); } - @JsonProperty("requestRetryBackoffCount") - public int getRequestRetryBackoffCount() { - return getRequestConfig().getRequestRetryBackoffCount(); + @JsonProperty("requestRetryBackoffLimit") + public int getRequestRetryBackoffLimit() { + return getRequestConfig().getRequestRetryBackoffLimit(); } - public void setRequestRetryBackoffCount(int requestRetryBackoffCount) { - requestConfig.setRequestRetryBackoffCount(requestRetryBackoffCount); + public void setRequestRetryBackoffLimit(int requestRetryBackoffLimit) { + requestConfig.setRequestRetryBackoffLimit(requestRetryBackoffLimit); } - @JsonProperty("requestRetryBackoffInitial") - public int getRequestRetryBackoffInitial() { - return getRequestConfig().getRequestRetryBackoffInitial(); + @JsonProperty("requestRetryBackoffInterval") + public int getRequestRetryBackoffInterval() { + return getRequestConfig().getRequestRetryBackoffInterval(); } - public void setRequestRetryBackoffInitial(int requestRetryBackoffInitial) { - requestConfig.setRequestRetryBackoffInitial(requestRetryBackoffInitial); - } - - @JsonProperty("requestRetryBackoffMultiplier") - public int getRequestRetryBackoffMultiplier() { - return getRequestConfig().getRequestRetryBackoffMultiplier(); - } - - public void setRequestRetryBackoffMultiplier(int requestRetryBackoffMultiplier) { - requestConfig.setRequestRetryBackoffMultiplier(requestRetryBackoffMultiplier); + public void setRequestRetryBackoffInterval(int requestRetryBackoffInterval) { + requestConfig.setRequestRetryBackoffInterval(requestRetryBackoffInterval); } @JsonProperty("rollingTimeout") diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java index 413259aebd5..7fa01a60761 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/RequestConfig.java @@ -26,9 +26,8 @@ import static io.fabric8.kubernetes.client.Config.DEFAULT_LOGGING_INTERVAL; import static io.fabric8.kubernetes.client.Config.DEFAULT_MAX_CONCURRENT_REQUESTS; import static io.fabric8.kubernetes.client.Config.DEFAULT_MAX_CONCURRENT_REQUESTS_PER_HOST; -import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFF_COUNT; -import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL; -import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER; +import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFFLIMIT; +import static io.fabric8.kubernetes.client.Config.DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL; import static io.fabric8.kubernetes.client.Config.DEFAULT_ROLLING_TIMEOUT; import static io.fabric8.kubernetes.client.Config.DEFAULT_SCALE_TIMEOUT; import static io.fabric8.kubernetes.client.Config.DEFAULT_WEBSOCKET_PING_INTERVAL; @@ -48,9 +47,8 @@ public class RequestConfig { private int watchReconnectInterval = 1000; private int watchReconnectLimit = -1; private int connectionTimeout = 10 * 1000; - private int requestRetryBackoffCount = DEFAULT_REQUEST_RETRY_BACKOFF_COUNT; - private int requestRetryBackoffInitial = DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL; - private int requestRetryBackoffMultiplier = DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER; + private int requestRetryBackoffLimit = DEFAULT_REQUEST_RETRY_BACKOFFLIMIT; + private int requestRetryBackoffInterval = DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL; private int requestTimeout = 10 * 1000; private long rollingTimeout = DEFAULT_ROLLING_TIMEOUT; private long scaleTimeout = DEFAULT_SCALE_TIMEOUT; @@ -78,8 +76,6 @@ public class RequestConfig { * @param scaleTimeout scale timeout * @param loggingInterval logging interval * @param websocketTimeout web socket timeout - * @param websocketPingInterval web socket ping interval - * @param maxConcurrentRequests max concurrent requests * @param maxConcurrentRequestsPerHost max concurrent requests per host */ @Deprecated @@ -89,8 +85,7 @@ public RequestConfig(String username, String password, String oauthToken, long websocketTimeout, long websocketPingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost) { this(username, password, oauthToken, watchReconnectLimit, watchReconnectInterval, connectionTimeout, rollingTimeout, requestTimeout, scaleTimeout, loggingInterval, - websocketTimeout, websocketPingInterval,maxConcurrentRequests, maxConcurrentRequestsPerHost, null, DEFAULT_REQUEST_RETRY_BACKOFF_COUNT, DEFAULT_REQUEST_RETRY_BACKOFF_INITIAL, - DEFAULT_REQUEST_RETRY_BACKOFF_MULTIPIER); + websocketTimeout, websocketPingInterval,maxConcurrentRequests, maxConcurrentRequestsPerHost, null, DEFAULT_REQUEST_RETRY_BACKOFFLIMIT, DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL); } @Buildable(builderPackage = "io.fabric8.kubernetes.api.builder", editableEnabled = false) @@ -99,7 +94,7 @@ public RequestConfig(String username, String password, String oauthToken, int connectionTimeout, long rollingTimeout, int requestTimeout, long scaleTimeout, int loggingInterval, long websocketTimeout, long websocketPingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost, OAuthTokenProvider oauthTokenProvider, - int requestRetryBackoffCount, int requestRetryBackoffInitial, int requestRetryBackoffMultiplier) { + int requestRetryBackoffLimit, int requestRetryBackoffInterval) { this.username = username; this.oauthToken = oauthToken; this.password = password; @@ -115,9 +110,8 @@ public RequestConfig(String username, String password, String oauthToken, this.maxConcurrentRequests = maxConcurrentRequests; this.maxConcurrentRequestsPerHost = maxConcurrentRequestsPerHost; this.oauthTokenProvider = oauthTokenProvider; - this.requestRetryBackoffCount = requestRetryBackoffCount; - this.requestRetryBackoffInitial = requestRetryBackoffInitial; - this.requestRetryBackoffMultiplier = requestRetryBackoffMultiplier; + this.requestRetryBackoffLimit = requestRetryBackoffLimit; + this.requestRetryBackoffInterval = requestRetryBackoffInterval; } public String getUsername() { @@ -179,28 +173,20 @@ public void setRequestTimeout(int requestTimeout) { this.requestTimeout = requestTimeout; } - public int getRequestRetryBackoffCount() { - return requestRetryBackoffCount; + public int getRequestRetryBackoffLimit() { + return requestRetryBackoffLimit; } - public void setRequestRetryBackoffCount(int requestRetryBackoffCount) { - this.requestRetryBackoffCount = requestRetryBackoffCount; + public void setRequestRetryBackoffLimit(int requestRetryBackoffLimit) { + this.requestRetryBackoffLimit = requestRetryBackoffLimit; } - public int getRequestRetryBackoffInitial() { - return requestRetryBackoffInitial; + public int getRequestRetryBackoffInterval() { + return requestRetryBackoffInterval; } - public void setRequestRetryBackoffInitial(int requestRetryBackoffInitial) { - this.requestRetryBackoffInitial = requestRetryBackoffInitial; - } - - public int getRequestRetryBackoffMultiplier() { - return requestRetryBackoffMultiplier; - } - - public void setRequestRetryBackoffMultiplier(int requestRetryBackoffMultiplier) { - this.requestRetryBackoffMultiplier = requestRetryBackoffMultiplier; + public void setRequestRetryBackoffInterval(int requestRetryBackoffInterval) { + this.requestRetryBackoffInterval = requestRetryBackoffInterval; } public int getConnectionTimeout() { diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java index 820ad9ee230..0af6cadd633 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java @@ -29,6 +29,7 @@ import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.internal.VersionUsageUtils; +import io.fabric8.kubernetes.client.utils.ExponentialBackoffIntervalCalculator; import io.fabric8.kubernetes.client.utils.Serialization; import io.fabric8.kubernetes.client.utils.URLUtils; import io.fabric8.kubernetes.client.utils.Utils; @@ -62,6 +63,7 @@ public class OperationSupport { protected static final ObjectMapper YAML_MAPPER = Serialization.yamlMapper(); private static final Logger LOG = LoggerFactory.getLogger(OperationSupport.class); private static final String CLIENT_STATUS_FLAG = "CLIENT_STATUS_FLAG"; + private static final int maxRetryIntervalExponent = 5; protected OperationContext context; protected final OkHttpClient client; @@ -72,6 +74,8 @@ public class OperationSupport { protected String apiGroupName; protected String apiGroupVersion; protected boolean dryRun; + private final ExponentialBackoffIntervalCalculator retryIntervalCalculator; + private final int requestRetryBackoffLimit; public OperationSupport() { this (new OperationContext()); @@ -101,6 +105,16 @@ public OperationSupport(OperationContext ctx) { } else { this.apiGroupVersion = "v1"; } + + final int requestRetryBackoffInterval; + if (ctx.getConfig() != null) { + requestRetryBackoffInterval = ctx.getConfig().getRequestRetryBackoffInterval(); + this.requestRetryBackoffLimit = ctx.getConfig().getRequestRetryBackoffLimit(); + } else { + requestRetryBackoffInterval = Config.DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL; + this.requestRetryBackoffLimit = Config.DEFAULT_REQUEST_RETRY_BACKOFFLIMIT; + } + this.retryIntervalCalculator = new ExponentialBackoffIntervalCalculator(requestRetryBackoffInterval, maxRetryIntervalExponent); } public String getAPIGroup() { @@ -564,18 +578,17 @@ protected T handleResponse(OkHttpClient client, Request.Builder requestBuild } protected Response retryWithExponentialBackoff(OkHttpClient client, Request request) throws InterruptedException, IOException { - int numRetries = config.getRequestRetryBackoffCount(); Response response; - long currentBackOff = config.getRequestRetryBackoffInitial(); boolean doRetry; + int numRetries = 0; do { response = client.newCall(request).execute(); - numRetries--; - doRetry = numRetries != -1 && response.code() >= 500; + doRetry = numRetries < requestRetryBackoffLimit && response.code() >= 500; if (doRetry) { - LOG.debug("HTTP operation on url: {} should be retried as the response code was {}, retrying after {} millis", request.url(), response.code(), currentBackOff); - Thread.sleep(currentBackOff); - currentBackOff *= config.getRequestRetryBackoffMultiplier(); + long retryInterval= retryIntervalCalculator.getInterval(numRetries); + LOG.debug("HTTP operation on url: {} should be retried as the response code was {}, retrying after {} millis", request.url(), response.code(), retryInterval); + Thread.sleep(retryInterval); + numRetries++; } } while(doRetry); return response; diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManager.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManager.java index e54f54a9689..33bfa259ade 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManager.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManager.java @@ -16,6 +16,7 @@ package io.fabric8.kubernetes.client.dsl.internal; import io.fabric8.kubernetes.api.model.ListOptions; +import io.fabric8.kubernetes.client.utils.ExponentialBackoffIntervalCalculator; import io.fabric8.kubernetes.client.Watch; import io.fabric8.kubernetes.client.Watcher; import io.fabric8.kubernetes.client.WatcherException; @@ -41,8 +42,7 @@ public abstract class AbstractWatchManager implements Watch { final AtomicBoolean forceClosed; private final int reconnectLimit; - private final int reconnectInterval; - private final int maxIntervalExponent; + private final ExponentialBackoffIntervalCalculator retryIntervalCalculator; final AtomicInteger currentReconnectAttempt; private ScheduledFuture reconnectAttempt; @@ -56,8 +56,7 @@ public abstract class AbstractWatchManager implements Watch { ) { this.watcher = watcher; this.reconnectLimit = reconnectLimit; - this.reconnectInterval = reconnectInterval; - this.maxIntervalExponent = maxIntervalExponent; + this.retryIntervalCalculator = new ExponentialBackoffIntervalCalculator(reconnectInterval, maxIntervalExponent); this.resourceVersion = new AtomicReference<>(listOptions.getResourceVersion()); this.currentReconnectAttempt = new AtomicInteger(0); this.forceClosed = new AtomicBoolean(); @@ -126,9 +125,7 @@ final boolean cannotReconnect() { final long nextReconnectInterval() { int exponentOfTwo = currentReconnectAttempt.getAndIncrement(); - if (exponentOfTwo > maxIntervalExponent) - exponentOfTwo = maxIntervalExponent; - long ret = (long)reconnectInterval * (1 << exponentOfTwo); + long ret = retryIntervalCalculator.getInterval(exponentOfTwo); logger.debug("Current reconnect backoff is {} milliseconds (T{})", ret, exponentOfTwo); return ret; } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ExponentialBackoffIntervalCalculator.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ExponentialBackoffIntervalCalculator.java new file mode 100644 index 00000000000..db91356769d --- /dev/null +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ExponentialBackoffIntervalCalculator.java @@ -0,0 +1,36 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client.utils; + +public class ExponentialBackoffIntervalCalculator { + + private final int initialInterval; + private final int maxRetryIntervalExponent; + + public ExponentialBackoffIntervalCalculator(int initialInterval, int maxRetryIntervalExponent) { + this.initialInterval = initialInterval; + this.maxRetryIntervalExponent = maxRetryIntervalExponent; + } + + public long getInterval(int retryIndex) { + int exponentOfTwo = retryIndex; + if (exponentOfTwo > maxRetryIntervalExponent) { + exponentOfTwo = maxRetryIntervalExponent; + } + return (long)initialInterval * (1 << exponentOfTwo); + } + +} diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java index aaf1ee54eab..5dd71375431 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java @@ -271,7 +271,7 @@ void testHttpRetryWithMoreFailuresThanRetries() throws MalformedURLException, IO final AtomicInteger httpExecutionCounter = new AtomicInteger(0); OkHttpClient mockClient = newHttpClientWithSomeFailures(httpExecutionCounter, 1000); BaseOperation> baseOp = new BaseOperation(new OperationContext() - .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").withRequestRetryBackoffCount(3).build()) + .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").withRequestRetryBackoffLimit(3).build()) .withPlural("pods") .withName("test-pod") .withOkhttpClient(mockClient)); @@ -292,7 +292,7 @@ void testHttpRetryWithLessFailuresThanRetries() throws MalformedURLException, IO final AtomicInteger httpExecutionCounter = new AtomicInteger(0); OkHttpClient mockClient = newHttpClientWithSomeFailures(httpExecutionCounter, 2); BaseOperation> baseOp = new BaseOperation(new OperationContext() - .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").withRequestRetryBackoffCount(3).build()) + .withConfig(new ConfigBuilder().withMasterUrl("https://172.17.0.2:8443").withRequestRetryBackoffLimit(3).build()) .withPlural("pods") .withName("test-pod") .withOkhttpClient(mockClient)); From 278ce6ddcf665c196756cea49b21d623e798cc21 Mon Sep 17 00:00:00 2001 From: Attila Zsolt Piros <2017933+attilapiros@users.noreply.github.com> Date: Wed, 2 Jun 2021 15:42:09 +0200 Subject: [PATCH 3/3] Update README.md Co-authored-by: Marc Nuri --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 04ff839ff39..d0fc649478c 100644 --- a/README.md +++ b/README.md @@ -92,8 +92,8 @@ System properties are preferred over environment variables. The following system | `kubernetes.watch.reconnectLimit` / `KUBERNETES_WATCH_RECONNECTLIMIT` | Number of reconnect attempts (-1 for infinite) | `-1` | | `kubernetes.connection.timeout` / `KUBERNETES_CONNECTION_TIMEOUT` | Connection timeout in ms (0 for no timeout) | `10000` | | `kubernetes.request.timeout` / `KUBERNETES_REQUEST_TIMEOUT` | Read timeout in ms | `10000` | -| `kubernetes.request.retry.backoffLimit` / `KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY` | Number of retry attempts | `0` | -| `kubernetes.request.retry.backoffInterval` / `KUBERNETES_REQUEST_RETRY_BACKOFFINTERVAL_SYSTEM_PROPERTY` | Retry initial backoff interval in ms | `1000` | +| `kubernetes.request.retry.backoffLimit` / `KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT` | Number of retry attempts | `0` | +| `kubernetes.request.retry.backoffInterval` / `KUBERNETES_REQUEST_RETRY_BACKOFFINTERVAL` | Retry initial backoff interval in ms | `1000` | | `kubernetes.rolling.timeout` / `KUBERNETES_ROLLING_TIMEOUT` | Rolling timeout in ms | `900000` | | `kubernetes.logging.interval` / `KUBERNETES_LOGGING_INTERVAL` | Logging interval in ms | `20000` | | `kubernetes.scale.timeout` / `KUBERNETES_SCALE_TIMEOUT` | Scale timeout in ms | `600000` |