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

Use generated hostname when shared network is enabled #42065

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

cristianonicolai
Copy link
Contributor

When using MongoDB DevService and Jib container build, the generated connection-string to run integration tests points to localhost, ex: QUARKUS_MONGODB_CONNECTION_STRING=mongodb://localhost:63109/test. The container will fail to start since localhost is not the correct host for the MongoDB service. Instead, the shared network hostname should be used.

@geoand
Copy link
Contributor

geoand commented Jul 23, 2024

Thanks for this!

Do you have a sample project that exhibits the problem this PR is solving?

@cristianonicolai
Copy link
Contributor Author

@geoand simply add jib to mongodb-client quickstart and try to run IT tests with -Dquarkus.container-image.build=true, otherwise I can create a reproducer but not much different than that.

@geoand
Copy link
Contributor

geoand commented Jul 23, 2024

Thanks, I'll give it a shot soon

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 23, 2024

Status for workflow Quarkus CI

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

✅ 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.

@gsmet gsmet merged commit fc6adf5 into quarkusio:main Jul 23, 2024
22 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Jul 23, 2024
@cristianonicolai cristianonicolai deleted the fix/mongo_main branch July 23, 2024 13:43
@gsmet
Copy link
Member

gsmet commented Jul 23, 2024

Ah sorry, I was too fast to merge as I was planning the backports. @geoand if something is wrong, please ping me and I'll revert it.

@gsmet
Copy link
Member

gsmet commented Jul 23, 2024

If all is good, we can backport it to the upcoming 3.13.

@geoand
Copy link
Contributor

geoand commented Jul 23, 2024

Ah sorry, I was too fast to merge as I was planning the backports. @geoand if something is wrong, please ping me and I'll revert it.

No problem, I'll check this tomorrow to be on the same side, but it's very likely the correct fix

@gsmet gsmet modified the milestones: 3.14 - main, 3.13.0 Jul 23, 2024
@gsmet
Copy link
Member

gsmet commented Jul 23, 2024

Ideally if you can get me the confirmation in the morning, that will be helpful for me to include it in 3.13.0 and make @cristianonicolai happier.

@geoand
Copy link
Contributor

geoand commented Jul 23, 2024

Sure, I'll try in the morning

@geoand
Copy link
Contributor

geoand commented Jul 24, 2024

Works like a charm

@gsmet
Copy link
Member

gsmet commented Jul 24, 2024

Thanks for the confirmation!

gsmet added a commit to gsmet/quarkus that referenced this pull request Aug 11, 2024
The hostname was fixed in quarkusio#42065 but when using a shared network and a
container build, the application will connect directly to the container
port so we shouldn't use the exposed port but the original one.

Fixes quarkusio#42453
frne pushed a commit to frne/quarkus that referenced this pull request Aug 13, 2024
The hostname was fixed in quarkusio#42065 but when using a shared network and a
container build, the application will connect directly to the container
port so we shouldn't use the exposed port but the original one.

Fixes quarkusio#42453
gsmet added a commit to gsmet/quarkus that referenced this pull request Aug 19, 2024
The hostname was fixed in quarkusio#42065 but when using a shared network and a
container build, the application will connect directly to the container
port so we shouldn't use the exposed port but the original one.

Fixes quarkusio#42453

(cherry picked from commit e853d39)
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.

3 participants