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

Quarkus Operator SDK 6.1.1 breaks operator.start() in QuarkusTests #659

Closed
Donnerbart opened this issue Jul 18, 2023 · 6 comments · Fixed by #660
Closed

Quarkus Operator SDK 6.1.1 breaks operator.start() in QuarkusTests #659

Donnerbart opened this issue Jul 18, 2023 · 6 comments · Fixed by #660

Comments

@Donnerbart
Copy link
Contributor

Donnerbart commented Jul 18, 2023

Hey folks, when updating the Quarkus Operator SDK to 6.1.1 or newer, we cannot start an injected operator in a @QuarkusTest anymore.

@QuarkusTest
@QuarkusTestResource(KubernetesServerTestResource.class)
class TestControllerTest {

    private static final @NotNull Logger LOG = LoggerFactory.getLogger(TestControllerTest.class);

    @Inject
    protected @NotNull Operator operator;

    @BeforeEach
    void setUp() {
        operator.start();
        assertThat(operator.getRuntimeInfo().isStarted()).isTrue();
    }

    ...
}

Expected Behavior

The CRD can be applied and the operator is started (works with 6.1.0 or older).

08:40:54.393 [Test worker] INFO  i.q.o.runtime.OperatorProducer - Quarkus Java Operator SDK extension 6.1.0
  (commit: f79f1b6 on branch: f79f1b6ffcb49e453b697f37aebc27b34d45482c) built on Thu May 25 00:28:33 CEST 2023

Jul 18, 2023 8:40:54 AM okhttp3.mockwebserver.MockWebServer$3 processOneRequest
INFO: MockWebServer[39547] received request: POST /apis/apiextensions.k8s.io/v1/customresourcedefinitions
  HTTP/1.1 and responded: HTTP/1.1 201 OK

Jul 18, 2023 8:40:54 AM io.quarkiverse.operatorsdk.runtime.CRDUtils lambda$applyCRD$0
INFO: Applied v1 CRD named 'test-resources.donnerbart.de' from
  /home/donnerbart/IdeaProjects/operator-test/build/kubernetes/test-resources.donnerbart.de-v1.yml

Actual Behavior

The CRD cannot be applied and the operator cannot be started (fails with 6.1.1 or newer).

08:41:40.530 [Test worker] INFO  i.q.o.runtime.OperatorProducer - Quarkus Java Operator SDK extension 6.1.1
  (commit: 42b0ce2 on branch: 42b0ce29cb78360650c4c409dab43bfe752b5f1a) built on Fri Jun 02 19:11:25 CEST 2023

Jul 18, 2023 8:41:40 AM okhttp3.mockwebserver.MockWebServer$3 processOneRequest
INFO: MockWebServer[54939] received request: PATCH
  /apis/apiextensions.k8s.io/v1/customresourcedefinitions/test-resources.donnerbart.de?fieldManager=fabric8
  HTTP/1.1 and responded: HTTP/1.1 404 Client Error

Root Cause

The root cause seems to be this change in CRDUtils.apply() from:

client.apiextensions().v1().customResourceDefinitions()
        .resource((CustomResourceDefinition) crd).createOrReplace();

to

client.apiextensions().v1().customResourceDefinitions()
        .resource((CustomResourceDefinition) crd)
        .serverSideApply();

Reproducer

When we switched our operator to Quarkus 3 and fixed the K8s client deprecations in our codebase, the serverSideApply() method didn't work out for us (at least not with the Kubernetes Mock Server in unit tests). We replaced everything with either create() or update() when the state/existence of a resource is known, or a get() and then either update() or create() when the state/existence of a resource is unknown.

Are we using the framework in a wrong way or is there a problem with serverSideApply() in unit tests?

@csviri
Copy link
Contributor

csviri commented Jul 18, 2023

Hi @Donnerbart , AFAIK the Kubernetes Mock Server does not implement server side apply.

maybe @shawkins or @manusa can confirm.

@Donnerbart
Copy link
Contributor Author

To my understanding CRDUtils can also be used in production when quarkus.operator-sdk.crd.apply=true is configured, so I guess we shouldn't remove SSA completely, right?

Would #660 be a correct fix then? I would just need to figure out how to configure quarkus.operator-sdk.enable-ssa just for tests (or maybe this could automatically be set when KubernetesServerTestResource is used).

@Donnerbart
Copy link
Contributor Author

It's at least a "works on my machine" 😅

10:05:44.702 [Test worker] INFO  i.q.o.runtime.OperatorProducer - Quarkus Java Operator SDK extension 6.2.2-SNAPSHOT
(commit: d73e005 on branch: bugfix/659-fix-crdutils-apply) built on Tue Jul 18 10:02:49 CEST 2023

Jul 18, 2023 10:05:44 AM okhttp3.mockwebserver.MockWebServer$3 processOneRequest
INFO: MockWebServer[49695] received request: GET
  /apis/apiextensions.k8s.io/v1/customresourcedefinitions/test-resources.donnerbart.de
  HTTP/1.1 and responded: HTTP/1.1 404 Client Error

Jul 18, 2023 10:05:44 AM okhttp3.mockwebserver.MockWebServer$3 processOneRequest
INFO: MockWebServer[49695] received request: POST
  /apis/apiextensions.k8s.io/v1/customresourcedefinitions
  HTTP/1.1 and responded: HTTP/1.1 201 OK

10:05:44.917 [Test worker] INFO  i.q.operatorsdk.runtime.CRDUtils - Applied v1 CRD named
  'test-resources.donnerbart.de' from
  /home/donnerbart/IdeaProjects/operator-test/build/kubernetes/test-resources.donnerbart.de-v1.yml

using this Gradle configuration

tasks.test {
    useJUnitPlatform()
    testLogging {
        events("passed", "skipped", "failed")
    }
    reports {
        junitXml.isOutputPerTestCase = true
    }
    jvmArgs("-Dquarkus.operator-sdk.enable-ssa=false")
}

@metacosm
Copy link
Member

Ideally, the mock server shouldn't be used anymore with Quarkus and replaced by using the Kubernetes Dev Services since this will bring up a full cluster instead of an incomplete and possibly flawed mock version of the Kubernetes API.

@Donnerbart
Copy link
Contributor Author

We already have a working setup with a (modified) K3sContainer for our integration tests. The use case for the mock server is primarily code coverage with a quick unit testsuite. I'll look into the Dev Services, but if the lifecycle takes too much time to setup/teardown or if we have to refactor a lot to isolate unit tests when a shared cluster is used, we would still be blocked to update from Quarkus 6.1.0 :(

@shawkins
Copy link

AFAIK the Kubernetes Mock Server does not implement server side apply.

It does not in crud mode, and likely never will. You can of course add specific expectations if needed.

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