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

Add healthcheck for database container #80

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

Phu2
Copy link
Contributor

@Phu2 Phu2 commented Feb 4, 2025

With this change strapi will only start up if database is healthy/ready.
If the database is not ready starting up using docker compose up will end with

✔ strapi-rpb Built 0.0s
✔ Network strapi-rpb Created 0.1s
✘ Container strapi-rpbDB Error 1.1s
✔ Container strapi-rpb Created 0.1s

whereas a successful start up will be confirmed by

✔ strapi-rpb Built 0.0s
✔ Network strapi-rpb Created 0.2s
✔ Container strapi-rpbDB Healthy 5.9s
✔ Container strapi-rpb Started 6.0s

Why?
As just happened the other day, creating a file data/postgresql.conf.orig on the host machine as root will make strapi start with errors like:

strapi-rpbDB | chown: /var/lib/postgresql/data/postgresql.conf.orig: Operation not permitted strapi-rpb | [2025-02-04 07:03:20.350] debug: ⛔️ Server wasn't able to start properly. strapi-rpb | [2025-02-04 07:03:20.352] error: getaddrinfo EAI_AGAIN strapi-rpbDB strapi-rpb | Error: getaddrinfo EAI_AGAIN strapi-rpbDB strapi-rpb | at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:109:26)

It is not immediately obvious what went wrong because the the docker compose up command doesn't show any errors.

strapi will only start up if database is healthy/ready
@Phu2 Phu2 requested a review from fsteeg February 4, 2025 14:06
@fsteeg
Copy link
Member

fsteeg commented Feb 14, 2025

We should merge the current main branch here, apply these changes to the new docker-compose-test.yml, and deploy to test.

@Phu2
Copy link
Contributor Author

Phu2 commented Feb 14, 2025

Deployed to test, docker compose -f docker-compose-test.yml ps shows:

NAME                   IMAGE                  COMMAND                  SERVICE        CREATED          STATUS                    PORTS
strapi-rpb             strapi-rpb:latest      "docker-entrypoint.s…"   strapi-rpb     18 seconds ago   Up 13 seconds             0.0.0.0:1339->1337/tcp
strapi-rpb-adminer-1   adminer                "entrypoint.sh php -…"   adminer        18 seconds ago   Up 13 seconds             0.0.0.0:8080->8080/tcp
strapi-rpbDB           postgres:12.0-alpine   "docker-entrypoint.s…"   strapi-rpbDB   18 seconds ago   Up 15 seconds (healthy)   0.0.0.0:5434->5432/tcp

See the healthy status of the database container.

@fsteeg
Copy link
Member

fsteeg commented Feb 14, 2025

I don't understand why the changes in the three files are so different: no condition: service_healthy in docker-compose-test, no healthcheck in docker-compose-prod, both condition: service_healthy and healthcheck in docker-compose.

@Phu2
Copy link
Contributor Author

Phu2 commented Feb 15, 2025

You are spot on. Somehow i've messed up the files when editing in compare mode of my editor. Re-deployed to test.

Copy link
Member

@fsteeg fsteeg left a comment

Choose a reason for hiding this comment

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

Looks good, will deploy on prod with next deployment there.

(One note on our test deployment process: I prefer to merge the feature branch into main on test, instead of switching the branch on test. That way, we can deploy multiple feature branches at the same time, without losing changes from another branch.)

@fsteeg fsteeg assigned fsteeg and unassigned Phu2 Feb 18, 2025
@Phu2
Copy link
Contributor Author

Phu2 commented Feb 18, 2025

(One note on our test deployment process: I prefer to merge the feature branch into main on test, instead of switching the branch on test. That way, we can deploy multiple feature branches at the same time, without losing changes from another branch.)

Alright, i will merge into main from now on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants