service/start-odk: expose postgres connection errors & allow failure

[alxndrsn]

• replace psql with pg_isready
• don't suppress useful output

What has been done to verify that this works as intended?

☑️ if pg_isready is missing, it fails with:

service-1   | waiting for PostgreSQL to become connectable to...
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | sleeping 1 second waiting for a database connection
service-1   | ./start-odk.sh: line 48: pg_isreadyxxx: command not found
service-1   | Postgres not available after 10 attempts.
service-1 exited with code 1 (restarting)

☑️ If host is incorrect if fails with:

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | postgres.bad.example.test:5432:5432 - no response
service-1  | sleeping 1 second waiting for a database connection
service-1  | Postgres not available after 10 attempts.
service-1 exited with code 1 (restarting)

☑️ If database name is incorrect if fails with:

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | postgres14:5432 - accepting connections
service-1  | running migrations..
service-1  | Error: database "bad_db_name" does not exist
service-1 exited with code 1 (restarting)

☑️ If username is incorrect if fails with:

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | postgres14:5432 - accepting connections
service-1  | running migrations..
service-1  | Error: password authentication failed for user "not_a_real_user"
service-1 exited with code 1 (restarting)

☑️ If password is incorrect if fails with:

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | postgres14:5432 - accepting connections
service-1  | running migrations..
service-1  | Error: password authentication failed for user "odk"

Why is this the best possible solution? Were any other approaches considered?

• specifically written for this job
• provided by postgres team
• idiomatic

See: https://www.postgresql.org/docs/current/app-pg-isready.html

How does this change impact users? Describe intentional behavior changes from code updates. What are the regression risks?

Low chance of regression, but perhaps the current approach is doing something non-obvious.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

[matthew-white]

Low chance of regression, but perhaps the current approach is doing something non-obvious.

Wicher and I did discuss psql vs. pg_isready at #1671 (comment). My understanding is that pg_isready checks that the server can be connected to, validating the host and port. Whereas psql will also validate the database name. I guess pg_isready would pass successfully if someone mistyped the database name?

[matthew-white]

That understanding is just based on Wicher's comment, I haven't actually confirmed it myself.

[alxndrsn]

@matthew-white thanks for the explanation. In light of that discussion, I think either:

A. the messaging needs updating:

"waiting for PostgreSQL to become connectable to..." is accurate for pg_isready, but inaccurate for psql, or

B. switching to pg_isready

As touched on in the discussion, this would be more similar to the previous behaviour with wait-for-it, but it changed incidentally in:

• DB config, including port, through libpq env vars #1647

It would be helpful to get a clear understanding of what failure actually looks like if postgres is available, but the db name or other details are incorrect.

[matthew-white]

Not sure if this is a good idea, but what about an option C of using both? Using pg_isready + sleep in a loop to do the waiting. Then once the waiting is done, running psql just once to confirm that the database name is valid and to avoid the sort of boot loop that Wicher mentioned in his comment.

[alxndrsn]

My understanding is that pg_isready checks that the server can be connected to, validating the host and port. Whereas psql will also validate the database name. I guess pg_isready would pass successfully if someone mistyped the database name?

The current psql setup swallows the error if the db name is incorrect:

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | sleeping 1 second waiting for a database connection
service-1  | sleeping 1 second waiting for a database connection
service-1  | sleeping 1 second waiting for a database connection

Switching to pg_isready does "boot loop":

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | running migrations..
service-1  | Error: database "central_bad" does not exist
service-1 exited with code 1
service-1  | waiting for PostgreSQL to become connectable to...
service-1  | running migrations..
service-1  | Error: database "central_bad" does not exist
service-1 exited with code 1

Not sure if this is a good idea, but what about an option C of using both? Using pg_isready + sleep in a loop to do the waiting. Then once the waiting is done, running psql just once to confirm that the database name is valid and to avoid the sort of boot loop that Wicher mentioned in his comment.

A hybrid approach could look something like:

log "waiting for PostgreSQL to become connectable to..."
while ! pg_isready --quiet; do
  log "sleeping 1 second waiting for a database connection"
  sleep 1
done
log "testing database connection..."
if ! psql --no-password --command ""; then
  sleep 1 # reduce thrashing
  exit 1
fi

output:

service-1  | [start-odk] waiting for PostgreSQL to become connectable to...
service-1  | [start-odk] testing database connection...
service-1  | psql: error: connection to server at "example.com" (123.123.123.123), port 5432 failed: FATAL:  database "central_bad" does not exist
service-1 exited with code 1
service-1  | [start-odk] waiting for PostgreSQL to become connectable to...
service-1  | [start-odk] testing database connection...
service-1  | psql: error: connection to server at "example.com" (123.123.123.123), port 5432 failed: FATAL:  database "central_bad" does not exist
service-1 exited with code 1

This will still restart the container after failure.

Per the comment at #1671 (comment):

not bootlooping makes other things (such as docker exec) easier.

If avoiding restarting the container is an explicit goal, then I think this PR should be closed and replaced with one which:

1. corrects log messages to reflect what's actually happening, and
2. stops swallowing the cause of failures

[matthew-white]

Switching to pg_isready does "boot loop"

A hybrid approach could look something like: … This will still restart the container after failure.

If avoiding restarting the container is an explicit goal, then I think this PR should be closed and replaced with one which:

1. corrects log messages to reflect what's actually happening, and
2. stops swallowing the cause of failures

I feel like I'm getting to the limit of my knowledge of Docker best practice. Wicher seems to have wanted to avoid restarting the container. Is that a good idea / a beneficial approach? If so, I like the idea of exploring an alternative PR that corrects log messages and stops swallowing errors.

[alxndrsn]

wanted to avoid restarting the container. Is that a good idea / a beneficial approach?

Probably not. If the database is available, then presumably the hostname is correct. pg_isready should pass.

If the connection fails due to other misconfiguration (db name, username, password) then this can be resolved by:

1. changing values in .env or docker-compose.yml, or
2. updating/creating relevant db or user on the postgres side

Any changes made via 1 will only be available once the container has restarted.

Additionally, any healthchecks looking at container liveness or lifetime won't see any problems with a container that never restarts on connection failure.

[matthew-white]

Sounds good, I'm convinced that we don't need to avoid restarting the container. 👍

I kind of like the look of your hybrid approach above. I like that it fails fast, before reaching "running migrations.." Do you have a preference between the current approach of this PR vs. a hybrid approach?

[alxndrsn]

Do you have a preference between the current approach of this PR vs. a hybrid approach?

I prefer this PR as it's less code.

Comparing the failure messages, I don't think there's much in it:

1. with psql:

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | testing database connection...
service-1  | psql: error: connection to server at "example.com" (123.123.123.123), port 5432 failed: FATAL:  database "central_bad" does not exist
service-1 exited with code 1

2. without psql:

service-1  | waiting for PostgreSQL to become connectable to...
service-1  | running migrations..
service-1  | Error: database "central_bad" does not exist
service-1 exited with code 1

If the psql approach might be preferred, maybe we can debate + add in a separate PR?

[alxndrsn]

@matthew-white if the PR direction is accepted, I'll update the title to:

service/start-odk: expose postgres connection errors & allow failure

[matthew-white]

I prefer this PR as it's less code.

Let's go with that then. 👍 I don't have a strong preference for the hybrid approach. I agree that the error messaging is pretty similar either way.