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

Bump kubernetes-client-bom from 6.13.1 to 6.13.3 #42450

Merged
merged 2 commits into from
Aug 14, 2024

Conversation

manusa
Copy link
Contributor

@manusa manusa commented Aug 9, 2024

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Aug 10, 2024

@metacosm I let you check this one.

@metacosm
Copy link
Contributor

@gsmet
Copy link
Member

gsmet commented Aug 10, 2024

@metacosm my guess is that the version of Maven you use is too old (GitHub is known for shipping an old default version). Use ./mvnw to build Quarkus.

@metacosm
Copy link
Contributor

metacosm commented Aug 13, 2024

This is no-go for us at the moment. We're seeing issues similar to fabric8io/kubernetes-client#6249 with the upgrade. See https://github.com/quarkiverse/quarkus-operator-sdk/actions/runs/10356102901/job/28665238089#step:18:696

@metacosm
Copy link
Contributor

Note that this is an example of why version checking alignment was introduced in QOSDK: a patch version discrepancy between Quarkus, JOSDK and QOSDK breaks end user apps and this is why some of our users wanted to be able to fail a build if such a situation was detected.

@manusa manusa changed the title Bump kubernetes-client-bom from 6.13.1 to 6.13.2 Bump kubernetes-client-bom from 6.13.1 to 6.13.3 Aug 13, 2024
Copy link

quarkus-bot bot commented Aug 13, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit cda2640.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gastaldi gastaldi merged commit 19eba27 into quarkusio:main Aug 14, 2024
52 checks passed
@gastaldi
Copy link
Contributor

gastaldi commented Aug 14, 2024

Ah crap, I didn't see the comments, I'm going to revert this, sorry for the trouble.

Actually, I am confused as the tests did pass, should we revert this @metacosm @gsmet? According to the conversation in fabric8io/kubernetes-client#6249 it seems to be okay?

@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Aug 14, 2024
@manusa
Copy link
Contributor Author

manusa commented Aug 14, 2024

Note that @metacosm comments are related to 6.13.2 (prior to my last commit that bumps to 6.13.3).

Supposedly 6.13.3 addresses the issue mentioned by Chris.

I'd wait for his comments before reverting because this might be already alright.

@metacosm
Copy link
Contributor

metacosm commented Aug 14, 2024

No need to revert.

Ah crap, I didn't see the comments, I'm going to revert this, sorry for the trouble.

Actually, I am confused as the tests did pass, should we revert this @metacosm @gsmet? According to the conversation in fabric8io/kubernetes-client#6249 it seems to be okay?

The problem is that the Quarkus tests passing doesn't mean that things are OK as they do not cover many things that can break. I don't pretend that the tests QOSDK does cover all these things either but at least, they do exercise the Kubernetes support in Quarkus some more 😅. Using passing tests here does not presume of things not being broken elsewhere (in particular in QOSDK), which is why we're asking to be asked before the Fabric8 client is updated in Quarkus.

That said, https://github.com/quarkiverse/quarkus-operator-sdk/actions/runs/10373264251/job/28717966233 seems to indicate that, indeed, the initial issue we found with 6.13.2 is addressed. Now, it would be nice to figure out why the issue was happening in the first place since that hasn't been fixed (6.13.3 relies on method overloading to bypass the underlying issue).

It would be nice to have some more automated way to perform these checks (currently this relies on a manual process) and stricter gates to prevent merging when things are not supposed to be. At the very least, updates to the Fabric8 client shouldn't be merged without an approval of the PR by someone from the QOSDK team.

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

Successfully merging this pull request may close these issues.

4 participants