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

operator-framework-junit-5 version 4.4.0 doesn't work with fabric8 version 6.8.0 #1978

Closed
bachmanity1 opened this issue Jul 25, 2023 · 25 comments · Fixed by #1979
Closed

operator-framework-junit-5 version 4.4.0 doesn't work with fabric8 version 6.8.0 #1978

bachmanity1 opened this issue Jul 25, 2023 · 25 comments · Fixed by #1979
Assignees

Comments

@bachmanity1
Copy link
Contributor

bachmanity1 commented Jul 25, 2023

Bug Report

I'm using josdk version 4.4.0 with fabric8 version 6.8.0 overall everything works as expected except for e2e tests. Running e2e tests gives this error:

image

I think this is related to the fact that josdk v4.4.0 internally depends on fabric8 v6.7.2. I'm not sure why this problem happens because my project has a direct dependency on fabric8 and I thought all transitive dependencies will use version specified by the direct dependency.

Here is relevant part of the dependency tree:

image.

@csviri I think to reproduce this problem you should edit pom file of example operators to use fabric8 v6.8.0 and josdk v.4.4.0.

The problem is resolved when I run mvn install -Dmaven.test.skip=true using branch fabric-6.8 and use version 4.4.1-SNAPSHOT in my project. So, maybe I just should wait for the release of next version.

What did you do?

What did you expect to see?

What did you see instead? Under which circumstances?

Environment

Kubernetes cluster type:

$ Mention java-operator-sdk version from pom.xml file

$ java -version

$ kubectl version

Possible Solution

Additional context

@csviri
Copy link
Collaborator

csviri commented Jul 25, 2023

The error happens in the Junit extension, the thing is that it not happens for the internal ITs at least when the client is upgraded:
#1975

If you could create/opensource a sample to reproduce that would be a big help

@csviri csviri self-assigned this Jul 25, 2023
@bachmanity1
Copy link
Contributor Author

If you could create/opensource a sample to reproduce that would be a big help

I think to reproduce this error you could just use any of the sample operators under sample-operators directory, just make sure that pom file uses josdk version 4.4.0 and fabric8 version 6.8.0 and then try to run the e2e test.

@csviri
Copy link
Collaborator

csviri commented Jul 25, 2023

As you can see this build passes, that uses those version: https://github.com/operator-framework/java-operator-sdk/pull/1975/files
All the samples have end to end tests using that junit extension, and it does not fail at all.

@bachmanity1
Copy link
Contributor Author

As you can see this build passes, that uses those version: https://github.com/operator-framework/java-operator-sdk/pull/1975/files All the samples have end to end tests using that junit extension, and it does not fail at all.

yes, I saw that but I don't think that's what I'm trying to tell you.
In the e2e tests mentioned by you, josdk has an internal dependency on f8 version 6.8.0 but to reproduce this bug it should have internal dependency on versioin 6.7.2 and sample operator should have direct dependency on version 6.8.0.

@csviri
Copy link
Collaborator

csviri commented Jul 25, 2023

Yes, but that we don't support, so out of the box what is supported is the fabric8 client version comes with the framework. We will release new version with v6.8.0 soonish (will see based also on Quarkus side).

If you provide a minimalist example, I can still take a quick look to help with that..

@bachmanity1
Copy link
Contributor Author

@csviri I've added a simple example that reproduces this error.
https://github.com/bachmanity1/josdk-test

@csviri csviri linked a pull request Jul 25, 2023 that will close this issue
@csviri
Copy link
Collaborator

csviri commented Jul 25, 2023

I added a PR:

changing usage of builder from:

     new NamespaceBuilder()
               .withNewMetadata().withName(namespace).endMetadata()
               .build()

to:

new NamespaceBuilder().withMetadata(
                    new ObjectMetaBuilder()
                            .withName(namespace)
                            .build())
                .build())

That seems to be fixing this problem.

tbh I still not fully understand why this happens, and certainly not happens in the tests.

@manusa @shawkins might be interested.

@manusa
Copy link
Contributor

manusa commented Jul 25, 2023

If I'm understand correctly this might happen when mixing different Fabric8 Kubernetes Client versions (more specifically 6.7.2 and 6.8.0) but it's not reproducible when the Operator is directly using 6.8.0, right?

The problem might be explained through the changes in sundrio and the removal/refactor of the Fluent interface. So probably the nested buildables are incompatible.

If it does happen when the client is only 6.8.0, then we might have a big problem. However, I haven't seen this behavior reproduced elsewhere and it does seem to be a simple and common use case.

@csviri
Copy link
Collaborator

csviri commented Jul 25, 2023

If I'm understand correctly this might happen when mixing different Fabric8 Kubernetes Client versions (more specifically 6.7.2 and 6.8.0) but it's not reproducible when the Operator is directly using 6.8.0, right?

no, or at least no in our samples

@shawkins
Copy link
Collaborator

If it does happen when the client is only 6.8.0, then we might have a big problem.

I can confirm it does not.

So probably the nested buildables are incompatible.

It's hard wrap my head around what would be happening here - all of the relevant classes would be coming from a single jar
kubernetes-model-core, that would have the model objects, the sundrio package, and the generated annotation classes.

@manusa
Copy link
Contributor

manusa commented Jul 25, 2023

It's hard wrap my head around what would be happening here - all of the relevant classes would be coming from a single jar
kubernetes-model-core, that would have the model objects, the sundrio package, and the generated annotation classes.

I only trust maven-enforcer-plugin for that :)

Although it's not exactly the same, I do see similar problems when mixing buildables created from different sundrio versions in this PR: dekorateio/dekorate#1233 (check description bullet point 1).

@shawkins
Copy link
Collaborator

@csviri are you sure about committing this fix? It seems like if this were happening for the namespace builder it could happen all of the place with mixing versions.

@csviri
Copy link
Collaborator

csviri commented Jul 25, 2023

@shawkins the thing is that on all other places I use this style of metadata creation, so I think it is not a sin to merge this :)

@csviri
Copy link
Collaborator

csviri commented Jul 25, 2023

But as a I mentioned above, we don't support this scenario explicitly.

@tomdw
Copy link

tomdw commented Jul 31, 2023

We will release new version with v6.8.0 soonish (will see based also on Quarkus side).

@csviri when is a version with this upgrade planned? will also the kubernetes-webhooks-framework-core be upgraded then?

@csviri
Copy link
Collaborator

csviri commented Jul 31, 2023

Hi @tomdw plan to do both next week.

@csviri
Copy link
Collaborator

csviri commented Aug 8, 2023

@tomdw regarding the JOSDK and QOSDK there are issues with fabric8 v6.8.0 this might be delayed, on webhook side, I will take a look soon.

@tomdw
Copy link

tomdw commented Aug 9, 2023

@csviri thanks for the update, hope it gets fixed soon

@csviri
Copy link
Collaborator

csviri commented Aug 9, 2023

our best men are working on it :)
currently looking into admission controller, but might stay on f8 v6.7.* until that time.

@csviri
Copy link
Collaborator

csviri commented Aug 10, 2023

@tomdw
Copy link

tomdw commented Aug 27, 2023

@csviri thank for the web hook update, hopefully the operator sdk upgrade itself gets resolved too

@tomdw
Copy link

tomdw commented Sep 27, 2023

@csviri this issue is closed but the pom still shows that version 6.8.0 is not yet used in the latest releases, see

<fabric8-client.version>6.7.2</fabric8-client.version>

should it be reopened?

@csviri
Copy link
Collaborator

csviri commented Sep 27, 2023

Hi, will, be in next release. V4.5

@pmig
Copy link
Contributor

pmig commented Oct 31, 2023

@csviri it seems that the fabric8 version upgrade to 6.8 didn't make it into v4.5 any plans on inlcuding it in a bugfix version as 6.9.1 is already released?

@csviri
Copy link
Collaborator

csviri commented Oct 31, 2023

Hi yes, it should come soon.

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

Successfully merging a pull request may close this issue.

6 participants