Skip to content

Fix health-check false-green and unsafe runtime migrations - #108

Merged
saqibmanan merged 4 commits into
devfrom
fix/health-check-status-code
Aug 18, 2026
Merged

Fix health-check false-green and unsafe runtime migrations#108
saqibmanan merged 4 commits into
devfrom
fix/health-check-status-code

Conversation

@saqibmanan

Copy link
Copy Markdown
Contributor

Summary

First slice of the ECS CD-pipeline work (Phase 0+1 of the plan). Independent value on its own — fixes two real bugs found while auditing deploy-to-ecs.yml.

  • api/views/health.py: health_check returned HTTP 200 unconditionally (JsonResponse(data), no status=), even when overall_status was False. The ECS task definition's container healthcheck (curl -f http://localhost:8000/health/) and any future smoke gate hitting this endpoint would both false-green a container with a dead DB/ES/Redis/telemetry connection. Now returns 503 when unhealthy.
  • Added a git_sha field to the same response, sourced from a new GIT_COMMIT_SHA env var (plumbed via ARG/ENV in the Dockerfile, same pattern already used for other build-time config). This is the primitive a later smoke gate needs to verify "the code that's live is the code that was just pushed."
  • docker-entrypoint.sh: dropped python manage.py makemigrations --noinput, which ran on every container boot. Generating migration files at deploy time instead of using committed ones is unsafe under Fargate's rolling deployment (briefly 2 tasks live) and means the schema landing in prod was never reviewed. migrate --noinput stays in the entrypoint for now — moving it to an explicit one-off pipeline step is a separate follow-up PR.
  • Also deleted a local, never-committed deploy-backend.yml draft (EC2 + GHCR + docker compose) that had no matching secrets/infra behind it — confirmed the real deploy target is ECS Fargate (CloudFormation-provisioned RDS/ElastiCache/managed-ES, already-configured development environment secrets), not EC2.

Test plan

  • tests/test_health.py (new, 5 tests): 200 when all deps healthy, 503 when ES down, 503 when Redis down, git_sha defaults to "unknown", git_sha reflects GIT_COMMIT_SHA
  • Full suite run locally (pytest tests/): 100 passed vs. the pre-existing baseline's 95 passed — no new failures. The 22 failed / 7 errored tests in charts/test_dataset_schema/test_geography_hierarchy are pre-existing on dev (confirmed by stashing this branch's changes and re-running against unmodified dev — identical failure set)
  • bash -n + shellcheck clean on docker-entrypoint.sh
  • python -m py_compile clean on health.py

Next in this series

Moving migrate out of the entrypoint into an explicit ECS one-off task, fixing deploy-to-ecs.yml's if: always() and fragile infra-change condition, wiring a smoke-test gate, and adding rollback-on-failure — tracked as follow-up PRs.

health_check previously returned JsonResponse(data) unconditionally,
so the ECS container healthcheck (curl -f) and any future smoke gate
would false-green a container with a dead DB/ES/Redis/telemetry
connection. Also exposes git_sha so a deploy can verify the running
code matches what was just pushed.
Follows the same ARG->ENV pattern the container already uses for
other build-time config. Feeds health_check's new git_sha field.
Generating migration files at deploy time instead of using committed
ones is unsafe under a rolling deployment (briefly 2 tasks live) and
means the schema that lands in prod was never reviewed. migrate itself
stays here for now; moving it to an explicit one-off step is next.
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.

1 participant