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.

Sign up for GitHub

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

Jump to bottom

io.fabric8:kubernetes-client in main is not binary compatible with the one in Quarkus 3.13 #42656

Closed
mjurc opened this issue Aug 20, 2024 · 11 comments
Closed

io.fabric8:kubernetes-client in main is not binary compatible with the one in Quarkus 3.13 #42656

mjurc opened this issue Aug 20, 2024 · 11 comments
Labels
area/kubernetes kind/bug Something isn't working

Comments

@mjurc
Copy link

mjurc commented Aug 20, 2024

Describe the bug

Using the client compiled with Quarkus 3.13 and main built from main after #42450 produces NoSuchMethod exceptions in runtime, e.g.

java.lang.NoSuchMethodError: 'io.fabric8.kubernetes.client.ConfigFluent io.fabric8.openshift.client.OpenShiftConfigBuilder.withNamespace(java.lang.String)'
	at io.quarkus.test.bootstrap.inject.OpenShiftClient.<init>(OpenShiftClient.java:119)
	at io.quarkus.test.bootstrap.inject.OpenShiftClient.create(OpenShiftClient.java:137)
	at io.quarkus.test.bootstrap.OpenShiftExtensionBootstrap.beforeAll(OpenShiftExtensionBootstrap.java:45)
	at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.lambda$beforeAll$0(QuarkusScenarioBootstrap.java:61)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.beforeAll(QuarkusScenarioBootstrap.java:61)
	at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.beforeAll(QuarkusScenarioBootstrap.java:50)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	Suppressed: java.lang.NullPointerException: Cannot invoke "io.quarkus.test.bootstrap.inject.OpenShiftClient.deleteProject()" because "this.client" is null
		at io.quarkus.test.bootstrap.OpenShiftExtensionBootstrap.afterAll(OpenShiftExtensionBootstrap.java:52)
		at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.lambda$afterAll$2(QuarkusScenarioBootstrap.java:88)
		at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
		at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.afterAll(QuarkusScenarioBootstrap.java:88)
		at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.afterAll(QuarkusScenarioBootstrap.java:78)
		... 1 more

The other way is broken too - compiling with Quarkus built from main and running with Quarkus 3.13 produces similar exception, albeit with different package signature:

java.lang.NoSuchMethodError: 'io.fabric8.kubernetes.client.SundrioConfigFluent io.fabric8.openshift.client.OpenShiftConfigBuilder.withNamespace(java.lang.String)'
	at io.quarkus.test.bootstrap.inject.OpenShiftClient.<init>(OpenShiftClient.java:119)
	at io.quarkus.test.bootstrap.inject.OpenShiftClient.create(OpenShiftClient.java:137)
	at io.quarkus.test.bootstrap.OpenShiftExtensionBootstrap.beforeAll(OpenShiftExtensionBootstrap.java:45)
	at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.lambda$beforeAll$0(QuarkusScenarioBootstrap.java:61)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.beforeAll(QuarkusScenarioBootstrap.java:61)
	at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.beforeAll(QuarkusScenarioBootstrap.java:50)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	Suppressed: java.lang.NullPointerException: Cannot invoke "io.quarkus.test.bootstrap.inject.OpenShiftClient.deleteProject()" because "this.client" is null
		at io.quarkus.test.bootstrap.OpenShiftExtensionBootstrap.afterAll(OpenShiftExtensionBootstrap.java:52)
		at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.lambda$afterAll$2(QuarkusScenarioBootstrap.java:88)
		at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
		at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.afterAll(QuarkusScenarioBootstrap.java:88)
		at io.quarkus.test.bootstrap.QuarkusScenarioBootstrap.afterAll(QuarkusScenarioBootstrap.java:78)
		... 1 more

Expected behavior

The client is binary compatible and no runtime exceptions are produced

Actual behavior

Runtime exceptions are produced

How to Reproduce?

  1. login to OpenShift cluster
  2. build Quarkus from main
git clone [email protected]:quarkus-qe/quarkus-test-framework.git
cd quarkus-test-framework

# Build the lib with Quarkus 3.13, run with Quarkus main
mvn clean install -Pframework && mvn clean verify -Popenshift,examples -pl examples/https/ -Dquarkus.platform.version=999-SNAPSHOT

# Build the lib with Quarkus main, run with 3.13
mvn clean install -Pframework -Dquarkus.platform.version=999-SNAPSHOT && mvn clean verify -Popenshift,examples -pl examples/https/ -Dquarkus.platform.version=3.13.2

Output of uname -a or ver

Linux tigris 6.10.5-200.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Aug 14 15:49:44 UTC 2024 x86_64 GNU/Linux

Output of java -version

openjdk version "17.0.12" 2024-07-16

Quarkus version or git rev

Since #42450

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937)

Additional information

No response
@mjurc mjurc added the kind/bug Something isn't working label Aug 20, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 20, 2024

/cc @geoand (kubernetes,openshift), @iocanel (kubernetes,openshift)

@geoand
Copy link
Contributor

geoand commented Aug 20, 2024

I'm pretty sure that @manusa is already aware (I think there are other open issues that duplicate this)

@mjurc
Copy link
Author

mjurc commented Aug 20, 2024 •
edited
Loading

Hey, I've seen fabric8io/kubernetes-client#6249 in the client lib so far and one in Quarkus quickstarts (quarkusio/quarkus-quickstarts#1442), but not one in Quarkus repo yet.

Didn't see anything in Quarkus repo itself, and it's not mentioned in migration either - but maybe it would be nice to have a note about it.

@mjurc mjurc mentioned this issue Aug 20, 2024
Closed
@geoand
Copy link
Contributor

geoand commented Aug 20, 2024

👌

@gsmet
Copy link
Member

gsmet commented Aug 20, 2024

What's the use case for this exactly?

Because I don't expect Quarkus to be binary compatible. We never promised that and never will. You need to use the BOM and use versions that are enforced in the BOM.

And as for external libraries and their binary compatibility, that's really on them.

As for the Kubernetes Client compatibility break, I agree that what they did in a micro is by far not ideal.

@mjurc
Copy link
Author

mjurc commented Aug 20, 2024

Our framework uses the bom. I also think Quarkus has a pretty good track in compatibility, as we don't usually see this kind of issue.

The use case is that we run OpenShift testing weekly with Quarkus from main, but we release our test framework from latest release to spot issues. Considering 3.14 is in CR, I think we'll be able to bump to it pretty soon, but I think it would still be good to have a notice about this change in migration guide. Despite no guarantees on binary compatibility, we usually don't encounter issues like this, and I think if we encounter it, somebody else may too, so maybe at least a mention could be made in migration.

At the same time, I don't think this setup is very common, but hard for me to tell

@gsmet
Copy link
Member

gsmet commented Aug 20, 2024

I also think Quarkus has a pretty good track in compatibility

This is very different from ensuring binary compatibility. Binary compatibility is extremely hard to obtain: it basically means that you can never change a return type for instance and that even having a method returning void changed to returning something is forbidden. And you would end up adding bridge methods to ensure you don't break existing apps.

We might have few issues but we don't give this guarantee (EAP on the other hand has some level of guarantee for the binary compatibility).

Personally, I think this is a slippery slope. All the libraries we include will break break binary compatibility one way or another at some point. We don't check it and I don't want us to be responsible of that.
It's an issue with the binary compatibility of the Kubernetes Client upstream, they need to report it there if they think it's a problem.

Note that it might end up being an issue for extensions using the Kubernetes Client such as the issue we had with Stork.

There's really nothing we can do on our side, except maybe reverting the update but I think this ship has sailed.

BTW, good opportunity to make sure our friends at Camel Quarkus are aware of the issue: /cc @ppalaga @jamesnetherton .

@gsmet
Copy link
Member

gsmet commented Aug 20, 2024

BTW, if it wasn't the middle of the summer, we would probably have pushed for a new release of the Kubernetes Client bringing back the compatibility but it's how it is, unfortunately.

@jamesnetherton
Copy link
Contributor

BTW, good opportunity to make sure our friends at Camel Quarkus are aware of the issue: /cc @ppalaga @jamesnetherton

Yeah, it was already observed on our nightly builds. The next release of Camel will be aligned to the updated k8s client, which seems to fix things for us.

@manusa
Copy link
Contributor

manusa commented Aug 20, 2024 •
edited
Loading

I'm pretty sure that @manusa is already aware (I think there are other open issues that duplicate this)

Yes, I'm aware of this.
Unfortunately, I'm away until the 26th.

As for the Kubernetes Client compatibility break, I agree that what they did in a micro is by far not ideal.

Regarding the compatibility (As far as I've gathered from the distance), 6.13.3 is source compatible (you'll notice because bumping is flaw-less).
However, there's a binary compatibility issue that has not even to do with the change of a method signature.
From what I've seen it has to do with the disappearance of one of the Sundrio generated intermediate classes that aren't even exposed. We had one ConfigFluent that was previously generated and it's no longer. The problem is that some of the other libraries that converge with the client and Quarkus itself has its compiled bytecode referencing this internal class (would need to check this, but I'd bet this is the root cause).

Binary compatibility is extremely hard to obtain: it basically means that you can never change a return type for instance and that even having a method returning void changed to returning something is forbidden.

Just to reiterate, there hasn't even been a method signature change (from the source reference perspective). A method exposed by a builder-generated-class was moved from an intermediate (generated) class to another. Note that recompiling the affected components with the lates client version should require no code changes and will fix the issue (as long as they converge)

This convergence problem has been seen before because there are many extensions that depend on the client too, so it makes the client the most delicate component. As we can see in this example, it's not even about guaranteeing backwards compatibility and non-breaking source changes, but binary/bytecode issues too.

mjurc added a commit to mjurc/quarkus-test-framework that referenced this issue Aug 20, 2024
@mjurc
Manage fabric8 client version in framework
938bec2 
* Due to quarkusio/quarkus#42656, this
  implements a workaround where we manage fabric8 version ourselves
  rather than letting Quarkus manage it, as this leads to binary
  incompatible versions of library.
mjurc added a commit to mjurc/quarkus-test-framework that referenced this issue Aug 20, 2024
@mjurc
Manage fabric8 client version in framework
916e56c 
* Due to quarkusio/quarkus#42656, this
  implements a workaround where we manage fabric8 version ourselves
  rather than letting Quarkus manage it, as this leads to binary
  incompatible versions of library.
@mjurc mjurc mentioned this issue Aug 20, 2024
Closed
11 tasks
@gsmet
Copy link
Member

gsmet commented Aug 21, 2024

I will close this one. It's good to have it around so that people can find some info about the issue if they stumble upon it.

@gsmet gsmet closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2024
mjurc added a commit to mjurc/quarkus-test-framework that referenced this issue Aug 27, 2024
@mjurc
Manage fabric8 client version in framework
ee49b7f 
* Due to quarkusio/quarkus#42656, this
  implements a workaround where we manage fabric8 version ourselves
  rather than letting Quarkus manage it, as this leads to binary
  incompatible versions of library.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/kubernetes kind/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@manusa @gsmet @geoand @jamesnetherton @mjurc