djuntgen@juntgen.com
← all posts

Building a Homelab with AI · part 26

Part 26: The SDE/SDM Review — Bugs Hiding in Plain Sight

#homelab#monitoring#ansible#docker#code-review#loki#promtail

After the SRE review in the previous session hardened our alerting and healthchecks, we decided to run a second review before starting Phase 4. The SRE review had asked “will it alert when things break?” This review asked a different question: “will the code itself break, and will you know when it does?”

The answer, it turned out, was no.

Why Two Reviews?

An SRE review and an SDE/SDM review look at the same system from different angles.

The SRE review focused on detection and recovery: are the alerts correct, are the healthchecks present, will the dead man’s switch work? It produced healthchecks on all seven containers, inhibit rules in Alertmanager, a VictoriaMetricsDown alert, and cAdvisor pinned to a digest. All good.

The SDE/SDM review focused on correctness, maintainability, and deployment safety. It reads the code the way a senior engineer would during a pull request — looking for bugs, configuration errors, operational footguns, and things that will cause problems three months from now when you’ve forgotten the context.

We found fifteen issues. Three of them were critical.

Critical Bug: Promtail Was Collecting Zero Docker Logs

This was the worst finding because it was completely invisible.

Promtail’s configuration file defines two scrape jobs. The docker job uses docker_sd_configs to discover containers via the Docker socket and collect their logs. The syslog job reads /var/log/syslog from the host. Both were configured correctly in promtail.yml:

scrape_configs:
  - job_name: docker
    docker_sd_configs:
      - host: unix:///var/run/docker.sock
        refresh_interval: 5s
    relabel_configs:
      - source_labels: [__meta_docker_container_name]
        regex: "/(.*)"
        target_label: container

The problem was in docker-compose.yml. Promtail’s volume mounts included /var/log, /var/lib/docker/containers, and the positions file — but not the Docker socket:

promtail:
  volumes:
    - promtail_positions:/var/lib/promtail
    - /etc/monitoring/promtail.yml:/etc/promtail/promtail.yml:ro
    - /var/log:/var/log:ro
    - /var/lib/docker/containers:/var/lib/docker/containers:ro
    # Docker socket was never mounted

Without /var/run/docker.sock, Promtail’s Docker service discovery had no way to connect to the Docker daemon. It silently discovered zero containers. The syslog job worked fine, so Promtail reported healthy. The healthcheck passed. No errors in the logs. Loki received syslog data and appeared to be working.

We verified this live on docker-host:

Promtail targets: docker/unix:///var/run/docker.sock:80 (0/0 ready)
Loki jobs: ["syslog"]
Loki labels: filename, host, job, service_name

No container, no service, no stack labels. Zero Docker container logs in Loki since the monitoring stack was first deployed. The entire container log pipeline — the thing that was supposed to let us pivot from a metrics spike into a container’s log stream — had never worked.

The fix was one line:

- /var/run/docker.sock:/var/run/docker.sock:ro

After deploying:

Promtail targets: docker/unix:///var/run/docker.sock:80 (9/9 ready)
Loki labels: container, filename, host, job, service, service_name, stack
Loki containers: alertmanager, cadvisor, grafana, juntgen-com, loki,
                  openwebui-openwebui-1, promtail, victoriametrics, vmalert

Nine containers, all streaming logs.

Critical Bug: Loki Retention Was a No-Op

The Loki configuration had retention_period: 30d in limits_config. This looks correct and is documented in every Loki tutorial. What those tutorials often omit: the limits_config setting only defines the policy. The compactor — a separate Loki component — is what actually deletes old data. Without configuring the compactor with retention_enabled: true, the 30-day limit is advisory at best.

Our loki.yml had no compactor block. Data would grow until the volume filled up. On a homelab with a 32GB disk for docker-host, this was a time bomb.

The fix:

compactor:
  working_directory: /loki/compactor
  compaction_interval: 10m
  retention_enabled: true
  retention_delete_delay: 2h
  retention_delete_worker_count: 150
  delete_request_store: filesystem

The SRE review had flagged “Loki storage grows unbounded” as a medium-priority observation. This review identified the root cause: the feature that enforces retention was simply never turned on.

Critical Bug: Secrets Were World-Readable

Two files deployed by Ansible contained vault-backed secrets:

  • alertmanager.yml — SMTP username and password for sending alert emails
  • monitoring.env — Grafana admin password

Both were deployed with mode: "0644" — readable by any user on docker-host. The fix was changing the mode to 0600 on the Ansible template tasks. Docker containers run as root, so they can still read the files.

This is the kind of thing that passes every functional test. The alerts send. Grafana loads. Nothing is broken. But any process or user on docker-host could read the SMTP credentials.

Reclassifying juntgencom

In Part 25 we moved juntgencom from docker_build_stacks to docker_image_stacks, reasoning that it lived in the monorepo alongside monitoring and openwebui. We also added --build to the image stack deploy command so juntgencom would rebuild on every deploy.

The review flagged two problems with this:

  1. The --build flag is meaningless for actual image-based stacks (monitoring, openwebui). It implies something is being built when nothing is.
  2. juntgencom has a Dockerfile and a build: directive in its compose file. By definition, it is a build-based stack. The categorization was based on where the code lives (the monorepo), not what the deployment does (builds an image). Those are different things.

We moved juntgencom back to docker_build_stacks and removed --build from image stack deploys. The build_stacks.yml task already handles compose files with relative paths — juntgencom’s build: { context: ../.. } works correctly with the existing task structure.

This is a small correction, but it matters for the same reason we replaced Portainer: deployment semantics should be explicit and correct, not “it works because of a compensating workaround.”

Engineering Hygiene

The remaining fixes were smaller but collectively important:

Resource limits. None of the seven monitoring containers had memory limits. We added mem_limit to each, calibrated at 4x current usage from docker stats. VictoriaMetrics gets 512MB (using ~104MB), Grafana and Loki get 256MB, everything else gets 128MB. These prevent a single container from OOM-killing the Docker daemon and taking down all stacks on docker-host.

Deploy wrapper playbook. The monitoring stack requires two playbooks in sequence: docker-stacks.yml (containers) then monitoring.yml (configs). Running one without the other creates version skew. We added deploy-monitoring.yml that imports both in order.

Hardcoded IPs replaced. The scrape config template had three IPs hardcoded (10.0.0.4 for PVE, 10.0.0.65 for HomeAssistant). These are defined in the Ansible inventory. We replaced them with {{ hostvars['pve']['ansible_host'] }} so the scrape config stays in sync with inventory automatically.

Fragile relative paths. The monitoring role used {{ playbook_dir }}/../../stacks/monitoring/ in six places. We extracted this into a monitoring_stacks_dir variable in defaults/main.yml — one place to update if the repo layout ever changes.

Dead config removed. evaluation_interval: 30s in the VictoriaMetrics scrape config is a Prometheus concept that VM ignores (vmalert handles evaluation). A duplicate alertmanager_smtp_host in the role defaults was shadowed by the inventory variable.

cAdvisor mount tightened. cAdvisor was mounting all of /var/run when it only needs the Docker socket. Changed to /var/run/docker.sock:/var/run/docker.sock:ro.

Handler reporting fixed. All four monitoring handlers had changed_when: true, making Ansible output unreliable for auditing. Changed to register + check return code.

The Verification

After deploying all fifteen fixes:

$ ssh docker-host 'docker ps --format "table {{.Names}}\t{{.Status}}" | sort'
alertmanager    Up About a minute (healthy)
cadvisor        Up About a minute (healthy)
grafana         Up About a minute (healthy)
loki            Up About a minute (healthy)
promtail        Up About a minute (healthy)
victoriametrics Up 7 seconds (healthy)
vmalert         Up About a minute (healthy)

$ ssh docker-host 'ls -la /etc/monitoring/alertmanager.yml'
-rw------- 1 root root 1548 Mar 23 11:46 /etc/monitoring/alertmanager.yml

$ ssh docker-host 'docker logs loki 2>&1 | grep compactor'
level=info msg=starting module=compactor

$ ssh docker-host 'curl -s http://localhost:3100/loki/api/v1/label/container/values'
["alertmanager","cadvisor","grafana","juntgen-com","loki",
 "openwebui-openwebui-1","promtail","victoriametrics","vmalert"]

All containers healthy. Secrets restricted. Compactor running. Nine containers streaming logs.

Lessons Learned

Silent failures are the most dangerous kind. The Promtail bug is the canonical example. Everything looked correct: the config file was right, the healthcheck passed, Loki was receiving data, Grafana showed logs. The only signal that something was wrong was the absence of Docker container labels in Loki — something you’d only notice if you specifically checked for it. No error, no warning, no alert. The system worked; it just didn’t work completely.

Configuration and runtime are different domains. A YAML file can be syntactically perfect and semantically useless. Loki’s retention_period: 30d reads like it does something. It doesn’t — not without the compactor. The Promtail docker_sd_configs section reads like it discovers containers. It can’t — not without the socket. Configuration correctness requires understanding the runtime behavior of the system, not just the schema of the config file.

Reviews should ask “prove it works,” not “does it look right?” Every bug in this review passed visual inspection. The configs looked correct. The containers were healthy. The fix for each was trivial. What caught them was verification: checking the Promtail targets endpoint, querying Loki’s labels API, examining file permissions on disk. The lesson isn’t “review harder” — it’s “verify differently.” Look at the system from the output side, not the input side.

Two reviews are better than one. The SRE review and the SDE/SDM review found completely different issues despite examining the same codebase. The SRE caught missing alerts and healthchecks. The SDE caught broken log collection and unenforced retention. Neither review’s findings overlapped. If we’d stopped after the SRE review, we’d have excellent alerting for a system that was silently dropping all container logs.