fix(observability): remove impossible Loki healthcheck that hung the stack - #8
Merged
Conversation
…stack grafana/loki is built FROM gcr.io/distroless/static:nonroot — the image contains /usr/bin/loki and nothing else. No shell, no wget, no curl. A Docker healthcheck can only exec something inside the container, so the wget probe could never succeed and Loki sat in "starting" forever. That is not cosmetic. Grafana and Alloy both waited on Loki via `depends_on: condition: service_healthy`, so neither ever started and `make up` appeared to hang with no error logged anywhere. Loki itself was healthy the whole time; only the probe was impossible. - drop the healthcheck from loki, with a comment explaining why one cannot exist and where readiness is observed instead (curl :3100/ready, plus Prometheus scraping the loki job — InstanceDown and LokiIngestionStalled already cover it) - grafana and alloy now depend on loki with condition: service_started. Grafana resolves its Loki datasource lazily per query; Alloy retries pushes with backoff and buffers meanwhile. Racing Loki's startup is safe for both. Add scripts/check_compose_health.py so this cannot come back. It asserts that every service_healthy dependency targets a service that actually declares a healthcheck, and flags healthchecks that exec a binary on an image that looks distroless. `docker compose config` accepts both happily, which is why the existing CI was silent. Wired into CI, scripts/validate.sh and `make check-compose-health`. Also ignore secrets/*.sops.yaml in .yamllint.yaml. SOPS output is machine-generated ciphertext that does not follow the comment and indentation conventions and never will; committing the encrypted file turned the lint job red on main.
There was a problem hiding this comment.
Pull request overview
This PR fixes a compose startup deadlock in the observability stack by removing Loki’s unsatisfiable healthcheck and changing Grafana/Alloy to no longer wait on Loki becoming “healthy”. It also adds a CI guard to prevent unsatisfiable service_healthy dependency graphs and updates yamllint ignores for encrypted SOPS files.
Changes:
- Remove Loki healthcheck and switch Grafana/Alloy
depends_onfromservice_healthytoservice_started. - Add
scripts/check_compose_health.pyand wire it intoscripts/validate.sh, CI, andmake check-compose-health. - Ignore
secrets/*.sops.yamlin.yamllint.yamlto avoid lint failures on machine-generated ciphertext.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| stacks/observability/compose.yaml | Removes Loki healthcheck and adjusts dependency conditions to prevent stack hang. |
| scripts/validate.sh | Runs the new compose health dependency check as part of local validation. |
| scripts/check_compose_health.py | Adds a guard script to detect unsatisfiable compose health dependencies/healthchecks. |
| Makefile | Adds a check-compose-health target. |
| .yamllint.yaml | Ignores SOPS-encrypted secret YAML files. |
| .github/workflows/ci.yml | Runs the new compose health dependency guard in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+22
to
+28
| import pathlib | ||
| import sys | ||
|
|
||
| try: | ||
| import yaml | ||
| except ModuleNotFoundError: | ||
| sys.exit("PyYAML is required: python3 -m pip install pyyaml") |
|
|
||
| # Images with no shell and no userland. A healthcheck cannot exec anything in | ||
| # these beyond the service binary itself. | ||
| DISTROLESS_MARKERS = ("distroless", "/static", "scratch") |
Review caught that the guard did not actually cover its own motivating case. The distroless detection matched on substrings in the image reference — "distroless", "/static", "scratch" — but the image that caused the outage is `grafana/loki`, which contains none of them. Re-adding a wget healthcheck to Loki would have satisfied the service_healthy check and hung the stack again, with the guard silent. An image reference says nothing about its base, so known no-userland images have to be listed by name. NO_USERLAND_IMAGES holds grafana/loki, verified against its published Dockerfile, and the substring markers stay for images that name their own base. Matching is on the repository component and accepts registry-qualified forms, so docker.io/grafana/loki and registry.local:5000/grafana/loki resolve the same way. Verified with a 13-case table over the reference forms, and against a fixture that re-adds the wget healthcheck to Loki — previously silent, now: loki has a healthcheck exec'ing 'wget', but grafana/loki:3.7.4@sha256:... has no shell and no userland — the probe can never run, so loki stays 'starting' forever Also from review: - Install PyYAML if absent instead of exiting. The script now gates CI, and a clean runner without the library would have failed a perfectly good compose file. Matches what check_loki_rules.sh already does. Exercised both ways in a throwaway venv: installs and passes where pip exists, and reports "PyYAML is required and could not be installed" rather than a traceback where it does not. - Replace `int(bool(print(...))) or 1` with a plain print and return. - Rename the validate.sh label to "compose health dependencies", so the failure line no longer reads "FAIL health dependencies satisfiable".
A bytecode cache directory landed in the previous commit — importing check_compose_health.py to unit-test its image classification created scripts/__pycache__/, and it was swept in by `git add -A`. Remove it from the index and ignore __pycache__/ and *.py[cod], which the repository never had a pattern for despite carrying two Python scripts.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Loki no longer declares a healthcheck, and Grafana and Alloy now wait on it with
condition: service_startedinstead ofservice_healthy. A newscripts/check_compose_health.pyguard makes it hard to reintroduce the bug, andsecrets/*.sops.yamlis now ignored by yamllint.Why
make uphung on the monitoring host. Every container came up except Loki, which sat instartingforever with nothing logged.grafana/lokiis builtFROM gcr.io/distroless/static:nonroot— the image contains/usr/bin/lokiand nothing else. No shell, nowget, nocurl. A Docker healthcheck can only exec something inside the container, so thewget --spiderprobe could never succeed no matter how healthy Loki was.That is not cosmetic. Grafana and Alloy both waited on Loki via
depends_on: condition: service_healthy, so neither ever started, and the stack appeared to deadlock with no error surfaced anywhere. Loki itself was fine the entire time; only the probe was impossible.Readiness is now observed from outside the container instead:
curl -s localhost:3100/readyon demandloki:3100as thelokijob —InstanceDown{job="loki"}andLokiIngestionStalledboth cover itDowngrading the dependencies to
service_startedis safe for both dependants: Grafana resolves its Loki datasource lazily per query, and Alloy retries a failed push with backoff while buffering in the meantime. The worst case is an empty panel for a few seconds.Two follow-ons rode along:
scripts/check_compose_health.py—docker compose configaccepts aservice_healthydependency on a service with no healthcheck without complaint, which is exactly why CI stayed green while the stack deadlocked. The guard closes both halves of the trap: everyservice_healthytarget must actually declare a healthcheck, and no healthcheck may exec a binary on an image with no userland to run it. The second half needs an explicit list — an image reference says nothing about its base, andgrafana/lokicontains no "distroless" substring to match on. Wired into CI,scripts/validate.sh, andmake check-compose-health.secrets/*.sops.yaml— committing the encrypted secrets file turned the Lint job red onmain. SOPS output is machine-generated ciphertext; it does not follow the repo's comment and indentation conventions and never will.Blast radius
The observability stack on
prometheus(10.0.99.20, VLAN 99). Startup ordering only — no config change to any service, no data path touched.secrets/*.sops.yamlVerification
Locally, against the pinned images and real binaries:
The guard matters only if it fails when it should, so both regression paths were tested against fixtures.
Re-pointing Grafana back at
loki: condition: service_healthy:Re-adding a
wgethealthcheck to Loki — the subtler path, since it satisfies the check above and hangs the stack anyway:Image classification is covered by a 13-case table over bare, tagged, digest-pinned and registry-qualified references, asserting
grafana/lokiand explicit distroless/scratch bases are flagged while the busybox, alpine and ubuntu based images in this stack are not. The PyYAML fallback was exercised both ways in a throwaway venv: installs and passes where pip is available, and reportsPyYAML is required and could not be installedrather than a traceback where it is not.make validatepassesgit pull && make upon the monitoring host is the remaining stepcompose.yamlnext to the removed healthcheck, where the next person tempted to add one will read it)