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
Bump kubernetes-client-bom from 5.7.2 to 5.7.3 #20016
Conversation
I'm having issues with it for the |
I'll hold off merging until you folks figure this one out |
I could reproduce the error with 2.2.1.Final:
In my branch I did the following:
Changed dependencies of reproducer project to:
Original issue should be gone, but getting this instead (Same error Chris reports):
|
For some reason, the knative dependency gets loaded from Dekorate instead. A possible fix might be adding |
Have you tried the proposed fix?
…On Thu, Sep 9, 2021, 13:22 Marc Nuri ***@***.***> wrote:
For some reason, the knative dependency gets loaded from Dekorate instead.
A possible fix might be adding io.fabric8:knative-model to the Quarkus
dependencyManagement section.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#20016 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBMDP4YFHT5ZEWMM37LSJLUBCDEZANCNFSM5DWPNGPQ>
.
|
I tried, but doesn't work either. The Quarkus Maven Plugin is using QuarkusClassLoader, and apparently the loaded jar/dependency might be different than that defined in the pom.xml. Since I'm only partially building the project, this might be the cause for that. Does the CI output any artifacts I can reuse to overwrite my local maven repo? |
No, not really
…On Thu, Sep 9, 2021, 14:05 Marc Nuri ***@***.***> wrote:
Have you tried the proposed fix?
I tried, but doesn't work either.
The Quarkus Maven Plugin is using QuarkusClassLoader, and *apparently*
the loaded jar/dependency might be different than that defined in the
pom.xml. Since I'm only partially building the project, this might be the
cause for that.
Does the CI output any artifacts I can reuse to overwrite my local maven
repo?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#20016 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBMDP5UYLDO4ZQRI7EFPRLUBCIHXANCNFSM5DWPNGPQ>
.
|
@aloubyansky is the expert on how dependencies of extensions are resolved. |
@manusa have you checked the output of |
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building bf4d499
Failures⚙️ JVM Tests - JDK 11 #- Failing: integration-tests/kubernetes/maven-invoker-way integration-tests/kubernetes/quarkus-standard-way integration-tests/kubernetes/quarkus-standard-way-kafka and 1 more
📦 integration-tests/kubernetes/maven-invoker-way✖ 📦 integration-tests/kubernetes/quarkus-standard-way✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ 📦 integration-tests/kubernetes/quarkus-standard-way-kafka✖ ✖ ✖ 📦 integration-tests/micrometer-prometheus✖ ⚙️ JVM Tests - JDK 11 Windows #- Failing: integration-tests/kubernetes/quarkus-standard-way integration-tests/kubernetes/quarkus-standard-way-kafka integration-tests/micrometer-prometheus
📦 integration-tests/kubernetes/quarkus-standard-way✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ 📦 integration-tests/kubernetes/quarkus-standard-way-kafka✖ ✖ ✖ 📦 integration-tests/micrometer-prometheus✖ ⚙️ JVM Tests - JDK 16 #- Failing: integration-tests/kubernetes/maven-invoker-way integration-tests/kubernetes/quarkus-standard-way integration-tests/kubernetes/quarkus-standard-way-kafka and 1 more
📦 integration-tests/kubernetes/maven-invoker-way✖ 📦 integration-tests/kubernetes/quarkus-standard-way✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ 📦 integration-tests/kubernetes/quarkus-standard-way-kafka✖ ✖ ✖ 📦 integration-tests/micrometer-prometheus✖ ⚙️ Native Tests - Misc4 #- Failing: integration-tests/micrometer-prometheus
📦 integration-tests/micrometer-prometheus✖ |
From the looks of it, there is something pretty weird with the Kubernetes Client release compared to the previous one. |
All the test failures are related with the issue at hand. Somewhere there's a reference to the EditableService class. |
Which I assume is a class that has been removed? |
This is an excerpt of the relevant output of the
Everything points to the right version. |
The latest release of Dekorate depends on kubernetes-client 5.1.1 https://github.com/dekorateio/dekorate/blob/2.4.1/pom.xml#L62 |
It hasn't changed since the release we are currently using https://github.com/dekorateio/dekorate/blob/2.3.0/pom.xml#L62 |
I guess we need to upgrade Dekorate to the same kubernetes-client version. |
Yes, but dependency tree, etc. show a dependency to the 5.7.3 version of the io.fabric8 modules. This is the odd part. |
Which appears to be binary incompatible with the Dekorate version we are using. |
I see that we are using dekorate |
It's not odd, this is what we enforce in the BOM. |
What I mean is that the bom enforces the new client version, as you say, but somehow it appears that dekorate is still loading the older version (I'm not sure if I explain myself correctly). |
The issue is that the latest release of Dekorate (released 8 days ago) still depends on 5.1.1. So upgrading to the Dekorate 2.4.1 won't help. |
Not really. What is happening is what Alexey describes. Quarkus is correctly using the kubernetes client version but dekorate is built against an old version of the client which is not binary compatible |
It's classloading. Dekorate classes reference classes that are not present in the kubernetes client version we are upgrading to. |
OK; I know I might be slow 😅, but how is the EditableService class reference ending there anyway. There's no such reference in the Dekorate module to that class. |
And this is exactly what I'm saying is odd. Because Dekorate doesn't reference that EditableService class. |
When the JVM tries to load |
This is what I've been trying to figure out for the past few hours. The quickest solution is to release a dekorate 2.3.4 version (with updated client reference), if possible. However, I still have this unresolved mystery. |
Does using the |
Relevant parts:
|
So the generic type is loaded properly, although that doesn't help narrow down the source much :) |
So this confirms that somehow Dekorate has an embedded reference to the EditableService. However, as I stated before, I haven't found any trace of this reference in the decompiled classes of the published 2.3.0 Dekorate version Patched files in Dekorate 2.3.0:
|
@manusa looks like this is also being done in dekorateio/dekorate#786, maybe it would be nice if @iocanel and @metacosm could review that and release a new Dekorate version 👍🏻 UPDATE: Wow, the PR was already merged, that was fast 😄 |
@iocanel is too cool. No comment, simply getting things fixed :) |
|
Well, the problem is that while the issue seems fixed, which is very cool, we still have no idea what caused it in the first place, i.e. where is the |
I'll have a look and see if I can dig up anything |
I don't see Quarkus doing anything special here, so I'll leave it up to you folks you have knowledge of the internals of these tools to figure out what the problem was if you think it's worth it. |
Signed-off-by: Marc Nuri <marc@marcnuri.com>
bf4d499
to
43aa47e
Compare
So I finally found the root cause of the problem. It all boils down to Java compiler bytecode optimization. In my previous iterations, I never saw the reference to the However, when inspecting the bytecode of the packaged class in the knative-annotations-2.3.0-noapt.jar jar, a reference to the EditableService class does exist:
|
Kubernetes Client 5.7.3 was just released: https://github.com/fabric8io/kubernetes-client/releases/tag/v5.7.3
Just speeding up the dependabot process to see if CI reports any issue.
This should take care of #19950. It's still compatible with the older Jandex version (2.3), so should be back-portable to 2.2.x
/cc @metacosm @gastaldi
Fixes: #19950