Skip to content

fix: Support Postgres ready_conditions for already mounted volumes #416

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+22 −3
Open

fix: Support Postgres ready_conditions for already mounted volumes #416

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f4f05f
fix: Support Postgres ready_conditions for already mounted volumes
cbrevik Oct 17, 2025
acb12e9
fix: Use truncated listening log line
cbrevik Oct 18, 2025
f63cad0
fix: Use pg_isready to check for readiness
cbrevik Oct 21, 2025
59b26fa
nit
cbrevik Oct 21, 2025
01a59a0
fix: Forgot dbname arg
cbrevik Oct 21, 2025
0b92a8c
fix: Hybrid strategy, wait for pg_isready to be ready
cbrevik Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/postgres/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ impl Image for Postgres {

fn ready_conditions(&self) -> Vec<WaitFor> {
vec![
WaitFor::message_on_stderr("database system is ready to accept connections"),
WaitFor::message_on_stdout("database system is ready to accept connections"),
Comment on lines -128 to -129
Copy link
Contributor

@DDtKey DDtKey Oct 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It's intentional to wait the message against both - as in case of not initialized DB it may lead to flakiness

See
#158
and my comment:
#415 (comment)

we may introduce separate method to specify that DB already initialized or allow configuring named volume on Image side (so it will automatically change the set of conditions)

But I don't think we should change these expectations

Copy link
Author

@cbrevik cbrevik Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Would possibly doing it like testcontainers-dotnet (using pg_isready, introduced in psql 9.3, in 2013) be a way to support both cases without custom set of conditions for different setups?:

https://github.com/testcontainers/testcontainers-dotnet/blob/1cfac505ebf33f6abb30f55c92a1839101fcb34b/src/Testcontainers.PostgreSql/PostgreSqlBuilder.cs#L138C44-L138C54

Copy link
Author

@cbrevik cbrevik Oct 21, 2025

Choose a reason for hiding this comment

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

Updated PR with suggestion on how to do it with pg_isready. Followed the same pattern as the Kafka-module and did it in exec_after_start:

testcontainers-rs-modules-community/src/kafka/apache.rs

Line 126 in a47c5d6

// container will be started with custom command which will wait

Copy link
Author

@cbrevik cbrevik Oct 21, 2025

Choose a reason for hiding this comment

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

Hmm, but running tests it seems like it does not actually wait 🙈 So I guess this doesn't work like I thought it would.

Copy link
Author

@cbrevik cbrevik Oct 21, 2025

Choose a reason for hiding this comment

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

Figured out the issue, container was not ready to accept connections. So adopting a hybrid strategy of checking just once for database system is ready to accept connections also then enables checking for final readiness with pg_isready afterwards

Copy link
Contributor

@DDtKey DDtKey Oct 22, 2025

Choose a reason for hiding this comment

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

I'm afraid that first occurrence of database system is ready to accept connections still may lead to flakiness, it might be a log of initialization and pg_isready can be called between init and actual start

Copy link
Author

@cbrevik cbrevik Oct 22, 2025

Choose a reason for hiding this comment

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

Okay, I thought the point of pg_isready that as soon as the image is loaded that tool makes it easier to avoid flakiness. Would it be possible to have a "ready condition" that uses a ExecCommand (sort of like testcontainers-dotnet) that can be retried until exit code is a specific one? I'm thinking since that works for testcontainers-dotnet.

I see it is also used in testcontainers-elixir: https://github.com/testcontainers/testcontainers-elixir/blob/62cef58c518f1ab63859d9327825db5b9a8f21d8/lib/container/postgres_container.ex#L228

In testcontainers-node: https://github.com/testcontainers/testcontainers-node/blob/075ade49273d553d00bed5fb418de2db3ffb1c6c/packages/modules/postgresql/src/postgresql-container.ts#L42

And in testcontainers-php: https://github.com/testcontainers/testcontainers-php/blob/8602d6545713bd91d6d875de55be9db660f5b68c/src/Modules/PostgresContainer.php#L25

Copy link
Contributor

@DDtKey DDtKey Oct 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

AFAIK pg_isready is still flaky thing during init scenario

  1. from pg docs:

pg_isready returns 0 to the shell if the server is accepting connections normally, 1 if the server is rejecting connections (for example during startup), 2 if there was no response to the connection attempt, and 3 if no attempt was made (for example due to invalid parameters).

  1. How to determine when new container is truly "ready" docker-library/postgres#146 (comment)
  2. Confirm a container is ready to run after init docker-library/postgres#326
  1. testcontainers-java still rely on 2 log entries instead of pg_isready

Would it be possible to have a "ready condition" that uses a ExecCommand

We can, but it requires separate PR and interface proposal. We already have functionality that allows to reach similar behavior via healthcheck override - it's different, but it's docker-way for similar needs.
The only issue is that we don't expose it as part of Image itself, only as an extension to override for now (we can extend in fact)
We also had another PR that attempted to support this - perhaps I need to re-consider this (for example testcontainers-node also uses healthcheck functionality for that)

However, considering the same flakiness problem with pg_isready, it doesn’t appear to be a crucial functionality for this PR.

WaitFor::message_on_either_std("listening on IPv4 address"),
WaitFor::message_on_either_std("database system is ready to accept connections"),
]
}

Expand Down