From 66e6228ae5f84672137c8f9a822d8c86bd9857c7 Mon Sep 17 00:00:00 2001 From: Richard Williams Date: Wed, 16 Jul 2025 16:44:12 +0100 Subject: [PATCH 1/6] Move migrations into dokku `release` phase - Removed the existing `prod.sh` file which ran migrations and executed the gunicorn command as part of the `ENTRYPOINT` docker command. This caused the migrations to run every time the container was run. - Created a shell script `release.sh` which currently just does the migrations, but could do anything we need during the dokku release phase - Added a Procfile to run the release script in the release phase, and the gunicorn web application in the web phase --- Procfile | 2 ++ deploy/release.sh | 10 ++++++++++ docker/Dockerfile | 2 -- docker/entrypoints/prod.sh | 8 -------- 4 files changed, 12 insertions(+), 10 deletions(-) create mode 100644 Procfile create mode 100755 deploy/release.sh delete mode 100755 docker/entrypoints/prod.sh diff --git a/Procfile b/Procfile new file mode 100644 index 000000000..584ab1a40 --- /dev/null +++ b/Procfile @@ -0,0 +1,2 @@ +release: /usr/bin/env bash /app/deploy/release.sh +web: gunicorn --config /app/deploy/gunicorn/conf.py opencodelists.wsgi diff --git a/deploy/release.sh b/deploy/release.sh new file mode 100755 index 000000000..94b3e695a --- /dev/null +++ b/deploy/release.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash + +# Gets executed during dokku's release phase as specified in Procfile. +# Currently just runs manage.py commands to check and migrate the database. +# This is where dokku recommends running db migrations. + +set -euo pipefail + +./manage.py check --deploy +./manage.py migrate diff --git a/docker/Dockerfile b/docker/Dockerfile index 93e84a98f..a0818bb55 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -171,8 +171,6 @@ COPY . /app RUN TRUD_API_KEY=dummy-key SECRET_KEY=dummy-key \ python /app/manage.py collectstatic --no-input -ENTRYPOINT ["/app/docker/entrypoints/prod.sh"] - # We set command rather than entrypoint, to make it easier to run different # things from the cli CMD ["gunicorn", "--config", "/app/deploy/gunicorn/conf.py", "opencodelists.wsgi"] diff --git a/docker/entrypoints/prod.sh b/docker/entrypoints/prod.sh deleted file mode 100755 index f330899f5..000000000 --- a/docker/entrypoints/prod.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash - -set -euo pipefail - -./manage.py check --deploy -./manage.py migrate - -exec "$@" From b27998e9dbc9fbdeadac0b68c4ee658727741de4 Mon Sep 17 00:00:00 2001 From: Richard Williams Date: Thu, 17 Jul 2025 14:17:50 +0100 Subject: [PATCH 2/6] Fix CI yml files to output logs on failure docker_prod_1 seems to now be docker-prod-1, so changed this to spit out all docker logs on smoke test failure. In practice this will just be the prod image created in the `just docker-serve prod -d` line. --- .github/workflows/deploy.yml | 9 ++++++++- .github/workflows/main.yml | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 527015042..178741404 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -50,7 +50,14 @@ jobs: run: | SKIP_BUILD=1 just docker-serve prod -d sleep 5 - just docker-smoke-test || { docker logs docker_prod_1; exit 1; } + just docker-smoke-test || { + echo "Smoke test failed, attempting to show logs for all containers:" + for c in $(docker ps -a --format '{{.Names}}'); do + echo "Logs for $c:"; + docker logs "$c" || true; + done + exit 1 + } - name: Publish docker image run: | diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b9efa4c47..fdc183e6e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -72,7 +72,14 @@ jobs: run: | just docker-serve prod -d sleep 5 - just docker-smoke-test || { docker logs docker_prod_1; exit 1; } + just docker-smoke-test || { + echo "Smoke test failed, attempting to show logs for all containers:" + for c in $(docker ps -a --format '{{.Names}}'); do + echo "Logs for $c:"; + docker logs "$c" || true; + done + exit 1 + } - name: Save docker image run: | From 91a3e9fe9272cd198b0c3a24dce5ebf4f2be1601 Mon Sep 17 00:00:00 2001 From: Richard Williams Date: Wed, 16 Jul 2025 16:47:38 +0100 Subject: [PATCH 3/6] Fix the smoke test - Now that migrations are not run when the container is created, but in dokku, if we ever use the container outside of dokku it fails. - The only place this seems to be is in the smoke test which attempts to start the docker container, and check that the app is served correctly - Updated the smoke test to call the migration (release) script - Also parameterized it so you can run it with different environments (dev, prod) --- docker/justfile | 7 ++++++- justfile | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docker/justfile b/docker/justfile index 6579e56f5..133d94b33 100644 --- a/docker/justfile +++ b/docker/justfile @@ -86,7 +86,12 @@ exec env="dev" *args="bash": # run a basic functional smoke test against a running opencodelists -smoke-test host="http://localhost:7000": +smoke-test host="http://localhost:7000" env="prod": #!/bin/bash set -eu + # First run the release script to perform checks and migrations + echo "Running release script (check and migrations)..." + docker compose exec {{ env }} bash /app/deploy/release.sh + # Then check if the service is responding + echo "Testing service connectivity..." curl -I {{ host }} -s --compressed --fail --retry 20 --retry-delay 1 --retry-all-errors diff --git a/justfile b/justfile index bcae3e0b5..e3e89b4c3 100644 --- a/justfile +++ b/justfile @@ -342,8 +342,8 @@ docker-exec *args="bash": _env # run tests in docker container -docker-smoke-test host="http://localhost:7000": _env - {{ just_executable() }} docker/smoke-test {{ host }} +docker-smoke-test host="http://localhost:7000" env="prod": _env + {{ just_executable() }} docker/smoke-test {{ host }} {{env}} # check migrations in the dev docker container From 28c1e03983c912d17724cc33a3c4d8a186f9b16c Mon Sep 17 00:00:00 2001 From: Richard Williams Date: Wed, 16 Jul 2025 17:10:56 +0100 Subject: [PATCH 4/6] Fix docker/storage dir to always have correct owner - previously if docker/storage already existed, but with a different owner, then the chown was never run and the db could not be accessed --- docker/justfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/justfile b/docker/justfile index 133d94b33..f33fa4e8b 100644 --- a/docker/justfile +++ b/docker/justfile @@ -65,8 +65,8 @@ _create_storage env: SCRIPT_DIR={{justfile_directory()}} if ! test -d "$SCRIPT_DIR/storage"; then mkdir -p "$SCRIPT_DIR/storage" - sudo chown 10003:10003 "$SCRIPT_DIR/storage" fi + sudo chown 10003:10003 "$SCRIPT_DIR/storage" fi From d2b6ab04de7330ffe82accabcb1c8e772ef19bb9 Mon Sep 17 00:00:00 2001 From: Richard Williams Date: Thu, 17 Jul 2025 16:05:22 +0100 Subject: [PATCH 5/6] Added ADR --- ...igrations-during-dokku-in-release-phase.md | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 docs/adr/0005-migrations-during-dokku-in-release-phase.md diff --git a/docs/adr/0005-migrations-during-dokku-in-release-phase.md b/docs/adr/0005-migrations-during-dokku-in-release-phase.md new file mode 100644 index 000000000..6dd547691 --- /dev/null +++ b/docs/adr/0005-migrations-during-dokku-in-release-phase.md @@ -0,0 +1,62 @@ +# ADR: Run Django Migrations in Dokku Release Phase + +Date: 2025-07 + +## Status + +Draft + +## Context + +Previously, we had a `prod.sh` script that ran the database migrations and then called `exec "$@"` in order to execute whatever command was passed as arguments to the script. The Dockerfile was + +```docker +ENTRYPOINT ["/app/docker/entrypoints/prod.sh"] +CMD ["gunicorn", "--config", "/app/deploy/gunicorn/conf.py", "opencodelists.wsgi"] +``` + +Meaning that when the docker container was run, without additional arguments, this command would execute: + +```bash +prod.sh gunicorn --config /app/deploy/gunicorn/conf.py opencodelists.wsgi +``` +Causing the migrations to run, and gunicorn to start. You could also pass a command to `docker run` which would then execute (instead of gunicorn) after the migrations. + +With this setup, migrations were run every time the container started, including during the web process startup and health checks. This approach had caused issues, including an outage to job-server where migrations were not fully applied. + +[Dokku recommend](https://dokku.com/docs/advanced-usage/deployment-tasks/) running database migrations in the `release` phase, which is executed before the new web process is started. This ensures migrations are applied atomically and safely, and that the web process only starts after the database schema is up to date. + +## Decision + +- **Create** a `release.sh` script that runs the migrations and checks: + ```bash + #!/bin/bash + set -e + + ./manage.py check --deploy + ./manage.py migrate + ``` +- **Move** the migration and check logic to the `release` phase in the `Procfile` and add the `web` command: + ``` + release: /usr/bin/env bash /app/deploy/release.sh + web: gunicorn --config /app/deploy/gunicorn/conf.py opencodelists.wsgi + ``` +- **Remove** the `ENTRYPOINT` from the production Docker image. +- **Delete** the now-obsolete `prod.sh` entrypoint script. +- **Keep** the `CMD` in the Dockerfile so `docker run` will still start Gunicorn by default, but can be overriden + +## Consequences + +- Migrations are now run in the correct place (Dokku's release phase), before the web process starts. +- Deploys are safer and more predictable, with no risk of web processes starting before migrations are complete. +- No scripts or documentation referenced or used the old `prod.sh` entrypoint, so no further changes were needed following its deletion. +- The dev docker image overwrites the ENTRYPOINT and so is unaffected by its removal +- The `CMD` in the Dockerfile remains, allowing for overriding of the command when running the container. + + +## Changes + +- `docker/Dockerfile`: Removed ENTRYPOINT, kept CMD for Gunicorn. +- `docker/entrypoints/prod.sh`: Deleted. +- `deploy/release.sh`: Created for running migrations and checks. +- `Procfile`: Added release phase for migrations and checks, and web phase for Gunicorn. From 569792bcc62bb110d79b3dd96c600fa025bc4ec0 Mon Sep 17 00:00:00 2001 From: Richard Williams Date: Fri, 18 Jul 2025 08:42:13 +0100 Subject: [PATCH 6/6] Tidy up following review - Removed redundant comments (where there was also an `echo` message to the same effect - Removed comment that likely to go out of date - Clarified in the ADR the circumstances that led to us changing --- deploy/release.sh | 1 - docker/justfile | 2 -- docs/adr/0005-migrations-during-dokku-in-release-phase.md | 6 +++--- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/deploy/release.sh b/deploy/release.sh index 94b3e695a..757d4fd78 100755 --- a/deploy/release.sh +++ b/deploy/release.sh @@ -1,7 +1,6 @@ #!/usr/bin/env bash # Gets executed during dokku's release phase as specified in Procfile. -# Currently just runs manage.py commands to check and migrate the database. # This is where dokku recommends running db migrations. set -euo pipefail diff --git a/docker/justfile b/docker/justfile index f33fa4e8b..60aea37a1 100644 --- a/docker/justfile +++ b/docker/justfile @@ -89,9 +89,7 @@ exec env="dev" *args="bash": smoke-test host="http://localhost:7000" env="prod": #!/bin/bash set -eu - # First run the release script to perform checks and migrations echo "Running release script (check and migrations)..." docker compose exec {{ env }} bash /app/deploy/release.sh - # Then check if the service is responding echo "Testing service connectivity..." curl -I {{ host }} -s --compressed --fail --retry 20 --retry-delay 1 --retry-all-errors diff --git a/docs/adr/0005-migrations-during-dokku-in-release-phase.md b/docs/adr/0005-migrations-during-dokku-in-release-phase.md index 6dd547691..80b56c0d3 100644 --- a/docs/adr/0005-migrations-during-dokku-in-release-phase.md +++ b/docs/adr/0005-migrations-during-dokku-in-release-phase.md @@ -4,7 +4,7 @@ Date: 2025-07 ## Status -Draft +Accepted ## Context @@ -22,7 +22,7 @@ prod.sh gunicorn --config /app/deploy/gunicorn/conf.py opencodelists.wsgi ``` Causing the migrations to run, and gunicorn to start. You could also pass a command to `docker run` which would then execute (instead of gunicorn) after the migrations. -With this setup, migrations were run every time the container started, including during the web process startup and health checks. This approach had caused issues, including an outage to job-server where migrations were not fully applied. +With this setup, migrations were run every time the container started, including during the web process startup and health checks. This approach had caused issues, including an outage to job-server where migrations were not fully applied before the health checks timed out. [Dokku recommend](https://dokku.com/docs/advanced-usage/deployment-tasks/) running database migrations in the `release` phase, which is executed before the new web process is started. This ensures migrations are applied atomically and safely, and that the web process only starts after the database schema is up to date. @@ -48,7 +48,7 @@ With this setup, migrations were run every time the container started, including ## Consequences - Migrations are now run in the correct place (Dokku's release phase), before the web process starts. -- Deploys are safer and more predictable, with no risk of web processes starting before migrations are complete. +- Deploys are safer and more predictable, with no risk of the health checks failing because they start at the same time as the container starts. - No scripts or documentation referenced or used the old `prod.sh` entrypoint, so no further changes were needed following its deletion. - The dev docker image overwrites the ENTRYPOINT and so is unaffected by its removal - The `CMD` in the Dockerfile remains, allowing for overriding of the command when running the container.