Let container logs reach journald, so the Logs tab has something to show - #414
Conversation
The per-app Logs tab was empty on every real box, for every app, since it shipped: it opened the stream and waited forever. host-agent-real tails an app by running `journalctl CONTAINER_NAME=<container> -f`, and CONTAINER_NAME is a field only Docker's journald log driver sets. LOGGING.md calls that driver switch "the single biggest configuration decision", but no image ever wrote /etc/docker/daemon.json, so both real profiles ran Docker's json-file default and the match returned nothing. Two blind spots hid it, both the same mistake: the inner loop's fake host-agent reads `docker logs -f`, and the cloud boot proof greps the brain through `docker logs`. Both work on any driver, so nothing exercised the query the feature depends on. Ship the driver on both images, disable journald's per-unit rate limit for dockerd (every container now shares docker.service's one bucket, so a chatty container would silently starve the rest), and assert in the boot proof that `journalctl CONTAINER_NAME=malmo-brain` returns lines -- not via docker logs, since reading it that way is why this shipped. The journal is still volatile; persistence is the next entry.
Confidence Score: 4/5The migration gap should be fixed before merging because existing boxes and their existing app containers can remain on json-file and keep showing empty Logs tabs. The new image configuration works for freshly created containers and is covered by the cloud boot assertion, but no host-update or reconciliation path installs the configuration and recreates containers that already carry json-file LogConfig. Files Needing Attention: dev/cloud/mkosi.extra/etc/docker/daemon.json, dev/test-qemu/bootstrap.sh
|
| Filename | Overview |
|---|---|
| dev/cloud/mkosi.extra/etc/docker/daemon.json | Sets journald as the default for newly created containers, but provides no migration for deployed boxes or their existing containers. |
| dev/cloud/mkosi.extra/etc/systemd/system/docker.service.d/10-malmo-logging.conf | Disables Docker's shared journald rate limit as an explicitly documented tradeoff while retaining journald's bounded storage behavior. |
| dev/cloud/cloud-assertions.sh | Correctly verifies both the configured driver and the exact CONTAINER_NAME journal query on a freshly booted cloud image. |
| dev/test-qemu/bootstrap.sh | Stages matching logging configuration into the medium test image and invalidates its cache, but this does not migrate existing production containers. |
| dev/cloud/mkosi.conf | Documents why the committed ExtraTrees logging files are required by the host-agent log source. |
| docs/progress/container-logs-journald-driver.md | Records the implementation, testing limits, volatile-journal gap, and intentionally accepted rate-limit tradeoff. |
| docs/specs/LOGGING.md | Synchronizes the documented Docker drop-in filename with the implemented configuration. |
Reviews (1): Last reviewed commit: "Let container logs reach journald, so th..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| { | ||
| "log-driver": "journald" | ||
| } |
There was a problem hiding this comment.
When an existing box receives this daemon default without recreating its app containers, those containers retain their creation-time json-file logging configuration, causing journalctl CONTAINER_NAME=<container> to return nothing and their Logs tabs to remain empty. The update and startup-reconciliation paths do not inspect or migrate container LogConfig, so the fix currently covers fresh containers and images only.
1 participant
No issue — maintainer-driven fix for a fully broken user-facing feature, found while debugging a real box. Branch name carries no issue number for that reason.
The bug
The dashboard's per-app Logs tab is empty on every real box — hosted and appliance, every app, since the tail shipped. It opens the stream and sits on "Waiting for log output…" forever.
It is a missing image file, not a code bug.
LOGGING.md# Docker daemon uses thejournaldlog driver calls the driver switch "the single biggest configuration decision", and host-agent-real is built straight on it —internal/hostagent/journalsourcerunsjournalctl CONTAINER_NAME=<container> -f -o json, andCONTAINER_NAMEis a field only the journald driver sets. But no image ever wrote/etc/docker/daemon.json. Both real profiles ran Docker'sjson-filedefault, the match returned nothing, andjournalctl -fblocked on a stream that could never produce a line.Why nothing caught it
Two blind spots, both the same mistake — reading logs a way that works on any driver, so the query the feature actually depends on was never exercised:
docker logs -f(cmd/host-agent/dockerlogsource.go), somake devshows logs fine;docker logs, andcloud-assertions.sh:373even documents the daemon as being on json-file.The change
dev/cloud/mkosi.extra/etc/docker/daemon.json—{"log-driver": "journald"}. Committed, so the boot-proof image inherits it (dev/cloud/test/mkosi.confdoesInclude=..)..../docker.service.d/10-malmo-logging.conf—LogRateLimitIntervalSec=0/LogRateLimitBurst=0, perLOGGING.md# Tuning. journald counts against_SYSTEMD_UNIT, so routing every container through dockerd puts them all in one bucket; under the 10000-per-30s default a single chatty container silently starves every other container's lines. Disabled per-unit, not raised globally, so sshd brute-force spam still caps. (My first pass raised the global journald burst instead — the spec's approach is better and I switched to it.)dev/test-qemu/bootstrap.sh— stages both byte-identically into the appliance lane's generated tree.CANARY_VERSIONv26 → v27 so the medium lane rebuilds instead of booting a cached image without them.dev/cloud/cloud-assertions.sh— new assertion 5b:docker inforeportsjournald, andjournalctl CONTAINER_NAME=malmo-brainreturns lines. Deliberately not adocker logsread — that is exactly how this shipped. Polled, for the journald-ingest lagwait_brain_logalready documents.dev/cloud/mkosi.conf— theExtraTrees=mkosi.extracomment now explains the logging wiring, since JSON takes no comments anddaemon.jsonis two lines with no hint of what depends on it.docs/specs/LOGGING.md— drop-in filename synced to the10-prefix used everywhere else.No Go changes. No new dependencies.
Testing
make checkgreen.CI / Cloud imagewithpublish=false— the real proof, since assertion 5b only runs on a booted image./dev/kvm). That change is a byte-identical copy of the hosted one and the canary is bumped, but it is unbooted. Called out in the progress entry.Known gaps
Storage=persistentor creates/var/log/journal, so scrollback dies on reboot while# Per-app logspromises "scrollback up to journald's cap". Left out deliberately: it is a disk-sizing decision on a root partition that grows at first boot, and it would have held up a fix for a fully broken feature. Queued as Up next Per-issue health audit records #1.SystemMaxUseis therefore unset, so the size-based backpressure# Tuningassumes once docker's rate limit is off is journald's default runtime cap instead. Bounded and self-rotating, not a disk-fill risk.main_serviceonly. Separate decision, noted in the entry's "what's next".Full detail in
docs/progress/container-logs-journald-driver.md.