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

6.5.0 version breaks some of our tests #5738

Closed
wind57 opened this issue Feb 1, 2024 · 11 comments
Closed

6.5.0 version breaks some of our tests #5738

wind57 opened this issue Feb 1, 2024 · 11 comments

Comments

@wind57
Copy link
Contributor

wind57 commented Feb 1, 2024

Describe the bug

Version 6.5.0 changed the way @EnableKubernetesMockClient works.

Fabric8 Kubernetes Client version

6.5.1

Steps to reproduce

I have a very simple test like this (I can provide a github repo if needed):

public class AllConfigMaps {

    public List<ConfigMap> all(KubernetesClient client) {
        return client.configMaps().inNamespace("default").list().getItems();
    }

}

and a test:

@EnableKubernetesMockClient
public class SimpleTest {

    private static final String API = "/api/v1/namespaces/default/configmaps";

    private static KubernetesMockServer mockServer;

    private static KubernetesClient mockClient;

    @BeforeAll
    static void beforeAll() {
        System.setProperty(Config.KUBERNETES_MASTER_SYSTEM_PROPERTY, mockClient.getConfiguration().getMasterUrl());
        System.setProperty(Config.KUBERNETES_TRUST_CERT_SYSTEM_PROPERTY, "true");
        System.setProperty(Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
        System.setProperty(Config.KUBERNETES_AUTH_TRYSERVICEACCOUNT_SYSTEM_PROPERTY, "false");
        System.setProperty(Config.KUBERNETES_HTTP2_DISABLE, "true");
        System.setProperty(Config.KUBERNETES_REQUEST_TIMEOUT_SYSTEM_PROPERTY, "1000000");
    }

    @Test
    void testAll() {

        Map<String, String> data = new HashMap<>();
        data.put("some.prop", "theValue");
        data.put("some.number", "0");

        mockServer.expect().withPath(API).andReturn(500, "Error").times(1);
        mockServer
                .expect().withPath(API).andReturn(200, new ConfigMapListBuilder().withItems(new ConfigMapBuilder()
                        .withNewMetadata().withName("application").endMetadata().addToData(data).build()).build())
                .once();

        new AllConfigMaps().all(mockClient);

    }

}

In version 6.4.1 this test fails. In version 6.5.0 this test passes.

In version 6.5.0, I see such logs:

11:42:27.023 [OkHttp https://localhost:51509/...] DEBUG io.fabric8.kubernetes.client.utils.ExponentialBackoffIntervalCalculator -- Current reconnect backoff is 100 milliseconds (T0)
11:42:27.024 [OkHttp https://localhost:51509/...] DEBUG io.fabric8.kubernetes.client.http.StandardHttpClient -- HTTP operation on url: https://localhost:51509/api/v1/namespaces/default/configmaps should be retried as the response code was 500, retrying after 100 millis
11:42:27.134 [OkHttp https://localhost:51509/...] DEBUG io.fabric8.kubernetes.client.utils.ExponentialBackoffIntervalCalculator -- Current reconnect backoff is 200 milliseconds (T1)

So it seems that there was a decision to re-try on error code 500 in that release. Unfortunately, this breaks a lot of our tests. Fixing them is not complicated, we can change the error code, from 500 to 400 for example.

But I am still interested why this breaking change happened, and if there is a way for us to still stick to 500 without retries.

Thank you.

Expected behavior

The test should fail, imho, in both versions.

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.25.3@latest

Environment

Linux, macOS

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

@wind57
Copy link
Contributor Author

wind57 commented Feb 1, 2024

The other issue is that some of the tests that were taking milliseconds to run, now take tens of seconds, because of retries that happen, for example this is one of our logs:

2024-02-01T12:22:42.482+02:00 DEBUG 88116 --- [//masterurl/...] i.f.k.client.http.StandardHttpClient     : HTTP operation on url: http://masterURL/api/v1/namespaces/testns/secrets should be retried after 3200 millis because of IOException

Is there a way to tell the client (for testing at least) : "do not retry?"

@manusa
Copy link
Member

manusa commented Feb 1, 2024

The problem is not with the MockWebServer, but with the new retry approach.

To fix tests you need to either account for this, or to disable retry.

I think I have a few examples for the latter, let me try to find some references.

@rohanKanojia
Copy link
Member

Can we use kubernetesClientBuilderCustomizer attribute to edit the config here?

@EnableKubernetesMockClient(kubernetesClientBuilderCustomizer = DisableKubernetesClientRequestRetry.class)
class KubernetesClientDisableRetryTest {

@wind57
Copy link
Contributor Author

wind57 commented Feb 1, 2024

@manusa this:

mockClient.getConfiguration().setRequestRetryBackoffLimit(0);

did not help, unfortunately.

@manusa
Copy link
Member

manusa commented Feb 1, 2024

Do you have a working branch and a reference to one of the failing tests? I can try to take a look

@manusa
Copy link
Member

manusa commented Feb 1, 2024

kubernetesClientBuilderCustomizer

Yes, that could be used as a way to provide common initialization options for those tests.

@wind57
Copy link
Contributor Author

wind57 commented Feb 1, 2024

I have a public repository with a test : here

What I what to achieve, is for the test to fail immediately, without logs like this:

13:46:37.555 [OkHttp https://localhost:54140/...] DEBUG io.fabric8.kubernetes.client.http.StandardHttpClient -- HTTP operation on url: https://localhost:54140/api/v1/namespaces/default/configmaps should be retried as the response code was 500, retrying after 100 millis

If there is anything else needed from me, do not hesitate! Thank you

@manusa
Copy link
Member

manusa commented Feb 1, 2024

I'm not sure what might be wrong with your execution.
This is the log I get when running your test (with clutter removed):

/home/user/00-MN/bin/jdk-21+35/bin/java -ea -Didea.test.cyclic.buffer.size=1048576 -javaagent:/home/user/00-MN/bin/ideaIU-2021/lib/idea_rt.jar=41965:/home/user/00-MN/bin/ideaIU-2021/bin -Dfile.encoding=UTF-8 -Dsun.stdout.encoding=UTF-8 -Dsun.stderr.encoding=UTF-8 -classpath /home/user/.m2/repository/org/junit/platform/junit-platform-launcher/1.9.2/junit-platform-launcher-1.9.2.jar:/home/user/.m2/repository/org/junit/vintage/junit-vintage-engine/5.9.2/junit-vintage-engine-5.9.2.jar:/home/user/00-MN/bin/ideaIU-2021/lib/idea_rt.jar:/home/user/00-MN/bin/ideaIU-2021/plugins/junit/lib/junit5-rt.jar:/home/user/00-MN/bin/ideaIU-2021/plugins/junit/lib/junit-rt.jar:/home/user/Downloads/delete/fabric8-issue/target/test-classes:/home/user/Downloads/delete/fabric8-issue/target/classes:/home/user/.m2/repository/io/fabric8/kubernetes-client/6.5.0/kubernetes-client-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-client-api/6.5.0/kubernetes-client-api-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-core/6.5.0/kubernetes-model-core-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-common/6.5.0/kubernetes-model-common-6.5.0.jar:/home/user/.m2/repository/com/fasterxml/jackson/core/jackson-annotations/2.14.2/jackson-annotations-2.14.2.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-gatewayapi/6.5.0/kubernetes-model-gatewayapi-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-resource/6.5.0/kubernetes-model-resource-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-rbac/6.5.0/kubernetes-model-rbac-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-admissionregistration/6.5.0/kubernetes-model-admissionregistration-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-apps/6.5.0/kubernetes-model-apps-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-autoscaling/6.5.0/kubernetes-model-autoscaling-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-apiextensions/6.5.0/kubernetes-model-apiextensions-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-batch/6.5.0/kubernetes-model-batch-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-certificates/6.5.0/kubernetes-model-certificates-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-coordination/6.5.0/kubernetes-model-coordination-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-discovery/6.5.0/kubernetes-model-discovery-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-events/6.5.0/kubernetes-model-events-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-extensions/6.5.0/kubernetes-model-extensions-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-flowcontrol/6.5.0/kubernetes-model-flowcontrol-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-networking/6.5.0/kubernetes-model-networking-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-metrics/6.5.0/kubernetes-model-metrics-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-policy/6.5.0/kubernetes-model-policy-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-scheduling/6.5.0/kubernetes-model-scheduling-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-storageclass/6.5.0/kubernetes-model-storageclass-6.5.0.jar:/home/user/.m2/repository/io/fabric8/kubernetes-model-node/6.5.0/kubernetes-model-node-6.5.0.jar:/home/user/.m2/repository/org/snakeyaml/snakeyaml-engine/2.6/snakeyaml-engine-2.6.jar:/home/user/.m2/repository/com/fasterxml/jackson/dataformat/jackson-dataformat-yaml/2.14.2/jackson-dataformat-yaml-2.14.2.jar:/home/user/.m2/repository/org/yaml/snakeyaml/1.33/snakeyaml-1.33.jar:/home/user/.m2/repository/com/fasterxml/jackson/datatype/jackson-datatype-jsr310/2.14.2/jackson-datatype-jsr310-2.14.2.jar:/home/user/.m2/repository/com/fasterxml/jackson/core/jackson-databind/2.14.2/jackson-databind-2.14.2.jar:/home/user/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.14.2/jackson-core-2.14.2.jar:/home/user/.m2/repository/io/fabric8/kubernetes-httpclient-okhttp/6.5.0/kubernetes-httpclient-okhttp-6.5.0.jar:/home/user/.m2/repository/com/squareup/okhttp3/okhttp/3.12.12/okhttp-3.12.12.jar:/home/user/.m2/repository/com/squareup/okio/okio/1.15.0/okio-1.15.0.jar:/home/user/.m2/repository/com/squareup/okhttp3/logging-interceptor/3.12.12/logging-interceptor-3.12.12.jar:/home/user/.m2/repository/io/fabric8/zjsonpatch/0.3.0/zjsonpatch-0.3.0.jar:/home/user/.m2/repository/ch/qos/logback/logback-classic/1.4.14/logback-classic-1.4.14.jar:/home/user/.m2/repository/ch/qos/logback/logback-core/1.4.14/logback-core-1.4.14.jar:/home/user/.m2/repository/org/slf4j/slf4j-api/2.0.7/slf4j-api-2.0.7.jar:/home/user/.m2/repository/io/fabric8/kubernetes-server-mock/6.5.0/kubernetes-server-mock-6.5.0.jar:/home/user/.m2/repository/io/fabric8/mockwebserver/0.2.2/mockwebserver-0.2.2.jar:/home/user/.m2/repository/com/squareup/okhttp3/mockwebserver/3.12.12/mockwebserver-3.12.12.jar:/home/user/.m2/repository/junit/junit/4.12/junit-4.12.jar:/home/user/.m2/repository/org/hamcrest/hamcrest-core/1.3/hamcrest-core-1.3.jar:/home/user/.m2/repository/io/fabric8/servicecatalog-client/6.5.0/servicecatalog-client-6.5.0.jar:/home/user/.m2/repository/io/fabric8/servicecatalog-model/6.5.0/servicecatalog-model-6.5.0.jar:/home/user/.m2/repository/org/junit/jupiter/junit-jupiter-api/5.9.2/junit-jupiter-api-5.9.2.jar:/home/user/.m2/repository/org/opentest4j/opentest4j/1.2.0/opentest4j-1.2.0.jar:/home/user/.m2/repository/org/junit/platform/junit-platform-commons/1.9.2/junit-platform-commons-1.9.2.jar:/home/user/.m2/repository/org/apiguardian/apiguardian-api/1.1.2/apiguardian-api-1.1.2.jar:/home/user/.m2/repository/org/junit/jupiter/junit-jupiter-engine/5.9.2/junit-jupiter-engine-5.9.2.jar:/home/user/.m2/repository/org/junit/platform/junit-platform-engine/1.9.2/junit-platform-engine-1.9.2.jar com.intellij.rt.junit.JUnitStarter -ideVersion5 -junit5 wind57.test.SimpleTest,testAll
Feb 01, 2024 12:57:40 PM okhttp3.mockwebserver.MockWebServer$2 execute
INFO: MockWebServer[44197] starting to accept connections
Feb 01, 2024 12:57:40 PM okhttp3.mockwebserver.MockWebServer$2 execute
INFO: MockWebServer[39943] starting to accept connections
Feb 01, 2024 12:57:41 PM okhttp3.mockwebserver.MockWebServer$3 processOneRequest
INFO: MockWebServer[44197] received request: GET /api/v1/namespaces/default/configmaps HTTP/1.1 and responded: HTTP/1.1 500 Server Error

io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: GET at: https://localhost:44197/api/v1/namespaces/default/configmaps. Message: Internal Server Error.

	at io.fabric8.kubernetes.client.KubernetesClientException.copyAsCause(KubernetesClientException.java:238)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.waitForResult(OperationSupport.java:546)
	at io.fabric8.kubernetes.client.dsl.internal.BaseOperation.list(BaseOperation.java:424)
	at io.fabric8.kubernetes.client.dsl.internal.BaseOperation.list(BaseOperation.java:392)
	at io.fabric8.kubernetes.client.dsl.internal.BaseOperation.list(BaseOperation.java:93)
	at wind57.AllConfigMaps.all(AllConfigMaps.java:11)
	at wind57.test.SimpleTest.testAll(SimpleTest.java:51)

Caused by: io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: GET at: https://localhost:44197/api/v1/namespaces/default/configmaps. Message: Internal Server Error.
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.requestFailure(OperationSupport.java:701)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.requestFailure(OperationSupport.java:681)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.assertResponseCode(OperationSupport.java:630)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.lambda$handleResponse$0(OperationSupport.java:591)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2179)
	at io.fabric8.kubernetes.client.http.StandardHttpClient.lambda$completeOrCancel$5(StandardHttpClient.java:120)


Feb 01, 2024 12:57:41 PM okhttp3.mockwebserver.MockWebServer$2 acceptConnections
INFO: MockWebServer[39943] done accepting connections: Socket closed
12:57:41.299 [main] DEBUG io.fabric8.kubernetes.client.okhttp.OkHttpClientImpl -- Shutting down dispatcher okhttp3.Dispatcher@59a67c3a at the following call stack:
	at io.fabric8.kubernetes.client.okhttp.OkHttpClientImpl.close(OkHttpClientImpl.java:253)
	at io.fabric8.kubernetes.client.impl.BaseClient.close(BaseClient.java:133)
	at io.fabric8.kubernetes.client.server.mock.KubernetesMockServerExtension.destroy(KubernetesMockServerExtension.java:110)
	at io.fabric8.kubernetes.client.server.mock.KubernetesMockServerExtension.afterAll(KubernetesMockServerExtension.java:67)

The same test with the line

mockClient.getConfiguration().setRequestRetryBackoffLimit(0);

commented-out does show the repetition (as expected)

@wind57
Copy link
Contributor Author

wind57 commented Feb 1, 2024

Marc, you're right! I'm really sorry for being an idiot: I was looking at too many things at the same time.

Setting:

client.getConfiguration().setRequestRetryBackoffLimit(0);
client.getConfiguration().setRequestRetryBackoffInterval(0);

indeed solved the 500 issue I was seeing. Much appreciate your time here.

Awesome that you exposed also:

System.setProperty(Config.KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY, "0");
System.setProperty(Config.KUBERNETES_REQUEST_RETRY_BACKOFFINTERVAL_SYSTEM_PROPERTY, "0");

This solves the second problem of ours, with long running tests. Fabulous!

@wind57 wind57 closed this as completed Feb 1, 2024
@manusa
Copy link
Member

manusa commented Feb 1, 2024

This solves the second problem of ours, with long running tests. Fabulous!

Cool, thx 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants