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

Disable Vertx DNS resolver when deploying to kubernetes #30952

Merged
merged 2 commits into from
Feb 8, 2023

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Feb 7, 2023

Fixes: #30951

P.S. This is the same fix we have used in other extensions (like Spring Cloud Config Client) that need a Vertx client based on a Vertx instance (that differs from the managed one Quarkus provides at runtime)

Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes do work, but only when the quarkus-kubernetes-client extension is present.
If we're using only the quarkus-openshift extension, it keeps failing.

@geoand
Copy link
Contributor Author

geoand commented Feb 7, 2023

Very interesting!

I'll have a closer look

@geoand
Copy link
Contributor Author

geoand commented Feb 7, 2023

@manusa can you remind me what the rationale was for putting QuarkusHttpClientFactory in quarkus-kubernetes-client instead of quarkus-kubernetes-client-internal?

It seems to me like the later would make more sense as it would mean that it's always picked up.

This ensures that the client will always use our factory,
whether it's being used in deployment or runtime code.
@geoand
Copy link
Contributor Author

geoand commented Feb 7, 2023

I pushed a commit which makes the move I mentione above and it seems to work.

@manusa @Sgitario can you guys confirm that the client behaves as expected now (for both deployment and runtime code)?

@manusa
Copy link
Contributor

manusa commented Feb 7, 2023

@manusa can you remind me what the rationale was for putting QuarkusHttpClientFactory in quarkus-kubernetes-client instead of quarkus-kubernetes-client-internal?

It seems to me like the later would make more sense as it would mean that it's always picked up.

This was based on our private conversation, but there we didn't discuss about internal vs. non-internal. So probably I decided upon my understanding.

I'm not sure about what the -internal module is used for (I don't recall seeing it in the guidelines). I guess my rationale is/was that users should be aware of the SPI-based HttpClient factory that Quarkus provides hence making it public (pure semantics). If moving it to -internal serves another purpose and fixes other issues, then sounds good.

@geoand
Copy link
Contributor Author

geoand commented Feb 7, 2023 via email

@quarkus-bot
Copy link

quarkus-bot bot commented Feb 7, 2023

✔️ 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.

@gsmet gsmet requested a review from Sgitario February 7, 2023 18:14
Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, it works perfectly, many thanks!

@geoand
Copy link
Contributor Author

geoand commented Feb 8, 2023

❤️

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.

Cannot deploy applications into OpenShift using quarkus.kubernetes.deploy=true
3 participants