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

Cannot connect to database from other processes since 1.0.0 #172

Closed
Notakas opened this issue Feb 1, 2022 · 8 comments · May be fixed by #173
Closed

Cannot connect to database from other processes since 1.0.0 #172

Notakas opened this issue Feb 1, 2022 · 8 comments · May be fixed by #173

Comments

@Notakas
Copy link

Notakas commented Feb 1, 2022

Describe the bug

After upgrading otj-pg-embedded to 1.0.0 we were forced to remove the setPort invocation while building the EmbeddedPostgres object. Now the database is not reachable at localhost:5432 anymore which was needed for running integration tests from another Java process

Error thrown when trying to connect: org.postgresql.util.PSQLException: Connection to localhost:5432 refused. Check that the hostname and port are correct and that the postmaster is accepting TCP/IP connections.

To Reproduce

  1. Creating a database with the EmbeddedPostgres Builder.
  2. Waiting for it to be running
  3. Try to reach the database from another process or pgAdmin, resulting in a connection error

Expected behavior
Database is reachable from other system processes.

Screenshots
If applicable, add screenshots to help explain your problem.

Machine (please complete the following information):

  • OS: Mac
  • OS Version: Monterey 12.1
  • Docker Version: 20.10.11

Additional context
Add any other context about the problem here.

@mikebell90
Copy link
Contributor

Working as designed. TestContainers generates ephmeral ports, and you must use getDatasource, getUri, or getHost/getPort

@mikebell90
Copy link
Contributor

You cannot assume static port mapping. In general this is a feature. Feel free to submit a PR to do an optional static port (IIRC testcontainers supports this but recommends against it)

@mikebell90
Copy link
Contributor

mikebell90 commented Feb 1, 2022

  • From the same java process, modify your usage as described above. That's really the main focus of this library
  • From PGAdmin, issue a docker containers ls and you can figure out the port. That's probably fine there since non automated. Perhaps annoying.
  • From a separate process (which I admit sounds weird to me - why not just launch postgres from a bash script then?), you'd indeed have to grep the docker container ls or use the docker api. Which sounds awful.

For both of the latter two, feel free to submit a PR to optionally force static ports. In general these do not work well in scenario 1, which is why we went with the default.

See also https://bsideup.github.io/posts/testcontainers_fixed_ports/

@mikebell90
Copy link
Contributor

Feel free to check out https://github.com/opentable/otj-pg-embedded/tree/FixedPort - I can't guarantee it will be merged, but I believe it is what you wanted.

@mikebell90 mikebell90 linked a pull request Feb 1, 2022 that will close this issue
@Notakas
Copy link
Author

Notakas commented Feb 2, 2022

Feel free to check out https://github.com/opentable/otj-pg-embedded/tree/FixedPort - I can't guarantee it will be merged, but I believe it is what you wanted.

Yes, this is what we could use.

  • From a separate process (which I admit sounds weird to me - why not just launch postgres from a bash script then?), you'd indeed have to grep the docker container ls or use the docker api. Which sounds awful.

Because many of the products we develop and maintain already use this library to set a Postgres database up from the integration test launcher, which does not run in the same process the app does.

I wouldn't mind staying in a 0.13.4 patched revision if we had a fix for the Monterey 12.1 bug, which would guarantee we could keep running our stack the way we currently do without adding static ports, but I can't currently run 0.13.4 in my machine without workarounds.

@mikebell90
Copy link
Contributor

mikebell90 commented Feb 2, 2022

I hear all your points, but that's not the direction this library is going. I see you have the following options

  1. Redo your entire test suite. That sounds horrible
  2. Figure out how to call the test suite and get the port. That's probably as bad as number 1
  3. Switch to Zonky's fork of pg-embedded - it's listed in the README.
  4. Provide a PR and volunteer to maintain the legacy branch of pg-embedded. The current maintainers will not be providing any more patches for Monterey (not a bug actually, but its academic). No more builds will be released by them for the pre 1.0.0 versions. Realize one of the reasons we switched was exactly this - multi platform and multi os versions make maintenance extremely difficult.
  5. Wait for a while and see if other community people also think a Fixed Port support would be a good idea. You could help your case a bit by cloning the branch, building the snapshot jar, and testing it (it's not clear from your wording that you did). I will be honest and say I'm currently not particularly keen on the idea, but I could be persuaded by general community consensus.

@Notakas
Copy link
Author

Notakas commented Feb 3, 2022

Thank you for your breaking down our options and for your help in general, it's been very informative. I understand your point of view and I believe it's completely fair to draw boundaries on where the library is going.

Yeah, I've been testing out the branch you facilitated earlier. Although it was working well on my machine, I was afraid it would not work with our Jenkins pipelines because we'd run into a Docker-in-Docker scenario, but so far my tests have been successful, so I'd say it might be safe to upgrade versions for us.

Thanks again for your support! I'll keep an eye on the PR in case it gets merged so we can continue using official releases.

@mikebell90
Copy link
Contributor

No worries, I'll wait for more feedback from folks, but as you can see, I think I layered that, so maintaining a fork and merging up shouldn't be too horrible, regardless of what the PR decision is.

@Notakas Notakas closed this as completed Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants