fix(trmnl): request an explicit device LB address, document the sidekiq race #481

Merged
k.renner merged 1 commits from fix-trmnl-lb-address into master 2026-08-10 21:08:54 +00:00
Member

Follow-up to #480, from watching the first reconcile. The deployment came up healthy — this fixes one thing that came up wrong and documents one that looks wrong but isn't.

The device LoadBalancer got 10.2.99.0

Nothing asked for it. I deliberately left lbipam.cilium.io/ips off the Service in #480, reasoning that the device is provisioned with a hostname so the address could move freely. IPAM then handed out the first free address in the pool block (10.2.99.0–10.2.99.255) — the base address of the range.

Each LB address is advertised to the UDM as its own BGP route (advertisementType: Service, addresses: [LoadBalancerIP]), and the UDM's prefix-list accepts 10.2.99.0/24 le 32, so 10.2.99.0/32 is genuinely installed and routable. But it's a .0 host address about to be handed to an ESP32-class IP stack, which is the one client that has to work. Not worth the risk for an address nobody chose on purpose.

Confirmed live that external-dns faithfully published the accident:

trmnl-device.renner.uno  A     10.2.99.0
trmnl-device.renner.uno  AAAA  fd99::89

Now requests 10.2.99.89,fd99::89 explicitly:

  • .89 continues the existing sequence (.81–.88 in use)
  • matches the fd99::89 IPAM had already picked for v6
  • brings this in line with every other LB Service in the repo, all of which pin their address

The sidekiq restart is expected

On a fresh deploy the sidekiq container exits 1 with PG::UndefinedTable: relation "account" does not exist, then comes up healthy on the restart. It loses the race against the web container's APP_SETUP migration. Observed on first deploy, converged in ~1s.

Left to Kubernetes' restart deliberately: there's no upstream ordering hook, and gating it behind an initContainer probing for a named table would couple us to Terminus's schema. Documented in place so the next person seeing RESTARTS 1 can distinguish the benign case — the comment is explicit that repeated restarts, or restarts long after a rollout, are not this.

Live state verified before committing

Kustomization trmnl   Ready: Applied revision master@sha1:2d8324b3
tr-pg                 Cluster in healthy state   (PostgreSQL 18)
ExternalSecrets       hetzner-s3-creds=SecretSynced  terminus-secrets=SecretSynced
Pods                  terminus Running  terminus-valkey Running  tr-pg-1 Running

The PostgreSQL 18 deviation works — initdb completed and the cluster reports healthy, which is the thing that would have failed outright on 16.

Both image-automation policies also behaved as intended: Valkey auto-bumped 9.1-alpine → 9.1.1-alpine (commit 2d8324b3) while Terminus stayed pinned at 0.68.0. Rebased onto that commit so this PR doesn't revert it.

Device reachability — resolved, previously mis-flagged

#480 listed "the device VLAN may not resolve trmnl-device.renner.uno" as an open risk. That was wrong and is withdrawn.

external-dns-unifi runs --source=service --label-filter=dns.renner.uno/public!=true, so this Service is in scope and its records are published to the UDM — the resolver for every VLAN. Both records verified present above.

Routing is a solved problem too, with a production precedent: mosquitto-iot (10.2.99.81) is reached by the Shelly/IoT fleet on v30iot today, and its manifest states plainly that "devices reach the broker via the UDM's BGP-learned routes — no on-link L2 announce on v30iot." Same VLAN, same pool, same mechanism. That file also records the same hostname-over-literal reasoning used here, to avoid reflash sweeps on address changes.

Narrow residual: whether UniFi's v30iot zone policy permits port 80 as well as the 1883 that demonstrably works. Zone policies there are per-zone rather than per-port, so this is expected to work; it gets confirmed the moment real hardware polls.

🤖 Generated with Claude Code

Follow-up to #480, from watching the first reconcile. The deployment came up healthy — this fixes one thing that came up wrong and documents one that looks wrong but isn't. ## The device LoadBalancer got `10.2.99.0` Nothing asked for it. I deliberately left `lbipam.cilium.io/ips` off the Service in #480, reasoning that the device is provisioned with a hostname so the address could move freely. IPAM then handed out the first free address in the pool block (`10.2.99.0`–`10.2.99.255`) — the base address of the range. Each LB address is advertised to the UDM as its own BGP route (`advertisementType: Service`, `addresses: [LoadBalancerIP]`), and the UDM's prefix-list accepts `10.2.99.0/24 le 32`, so `10.2.99.0/32` is genuinely installed and routable. But it's a `.0` host address about to be handed to an ESP32-class IP stack, which is the one client that has to work. Not worth the risk for an address nobody chose on purpose. Confirmed live that external-dns faithfully published the accident: ``` trmnl-device.renner.uno A 10.2.99.0 trmnl-device.renner.uno AAAA fd99::89 ``` Now requests `10.2.99.89,fd99::89` explicitly: - `.89` continues the existing sequence (`.81`–`.88` in use) - matches the `fd99::89` IPAM had already picked for v6 - brings this in line with every other LB Service in the repo, all of which pin their address ## The sidekiq restart is expected On a fresh deploy the sidekiq container exits 1 with `PG::UndefinedTable: relation "account" does not exist`, then comes up healthy on the restart. It loses the race against the web container's `APP_SETUP` migration. Observed on first deploy, converged in ~1s. Left to Kubernetes' restart deliberately: there's no upstream ordering hook, and gating it behind an initContainer probing for a named table would couple us to Terminus's schema. Documented in place so the next person seeing `RESTARTS 1` can distinguish the benign case — the comment is explicit that repeated restarts, or restarts long after a rollout, are **not** this. ## Live state verified before committing ``` Kustomization trmnl Ready: Applied revision master@sha1:2d8324b3 tr-pg Cluster in healthy state (PostgreSQL 18) ExternalSecrets hetzner-s3-creds=SecretSynced terminus-secrets=SecretSynced Pods terminus Running terminus-valkey Running tr-pg-1 Running ``` The PostgreSQL 18 deviation works — `initdb` completed and the cluster reports healthy, which is the thing that would have failed outright on 16. Both image-automation policies also behaved as intended: Valkey auto-bumped `9.1-alpine` → `9.1.1-alpine` (commit 2d8324b3) while Terminus stayed pinned at `0.68.0`. Rebased onto that commit so this PR doesn't revert it. ## Device reachability — resolved, previously mis-flagged #480 listed "the device VLAN may not resolve `trmnl-device.renner.uno`" as an open risk. That was wrong and is withdrawn. `external-dns-unifi` runs `--source=service --label-filter=dns.renner.uno/public!=true`, so this Service is in scope and its records are published to the UDM — the resolver for every VLAN. Both records verified present above. Routing is a solved problem too, with a production precedent: `mosquitto-iot` (`10.2.99.81`) is reached by the Shelly/IoT fleet on `v30iot` today, and its manifest states plainly that "devices reach the broker via the UDM's BGP-learned routes — no on-link L2 announce on v30iot." Same VLAN, same pool, same mechanism. That file also records the same hostname-over-literal reasoning used here, to avoid reflash sweeps on address changes. Narrow residual: whether UniFi's `v30iot` zone policy permits port 80 as well as the 1883 that demonstrably works. Zone policies there are per-zone rather than per-port, so this is expected to work; it gets confirmed the moment real hardware polls. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-bot added 1 commit 2026-08-10 21:04:44 +00:00
fix(trmnl): request an explicit device LB address, document the sidekiq race
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
931810e28b
Two things the first reconcile surfaced.

Cilium IPAM allocated 10.2.99.0 for the device LoadBalancer. Nothing
requested it — the Service carried no lbipam.cilium.io/ips annotation, so
IPAM handed out the first free address in the pool block, which is the base
address of the 10.2.99.0-10.2.99.255 range. Each LB address is advertised
to the UDM as its own BGP route, so it is routable in principle, but a .0
host address is exactly the sort of thing constrained embedded IP stacks
get wrong, and the TRMNL firmware is the one client that has to work here.
Requesting .89 explicitly also brings this in line with every other LB
Service in the repo, all of which pin their address, and matches the
fd99::89 IPAM had already chosen for v6.

Omitting the annotation was my call on the original PR, on the reasoning
that the device is provisioned with a hostname so the address could move
freely. That reasoning still holds; it just is not worth spending it on an
address nobody picked.

Second, the sidekiq container exits 1 once on a fresh deploy with
`PG::UndefinedTable: relation "account" does not exist` — it loses the race
against the web container's migration, then comes up healthy on the
restart. Observed on the first deploy, converged in about a second. There
is no upstream ordering hook, and gating it behind an initContainer that
probes for a named table would couple us to Terminus's schema, so this is
left to Kubernetes' restart deliberately. Documented in place so the next
person reading `RESTARTS 1` can tell the benign case from a real one.

Verified on the live cluster before this commit: Kustomization Ready, tr-pg
"Cluster in healthy state" on PostgreSQL 18, both ExternalSecrets
SecretSynced, terminus and valkey Running.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
review-bot left a comment
Member

🤖 Automated review — model claude-haiku-4-5-20251001 (complexity: trivial), commit 931810e28bad3b931b6bf0530b3c9c68ebd361ee.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

This PR pins the Terminus device LoadBalancer to an explicit IPAM address (10.2.99.89/fd99::89) and documents an expected race condition in the sidekiq container on fresh deploys. Both changes are defensive improvements based on live deployment observation, with clear reasoning and thorough operational documentation.

✓ No Blocking Issues

🟡 Suggestions

kubernetes/apps/trmnl/device-service.yaml:30–35 — The comment references "every other LB Service in the repo" pinning addresses. A grep of the codebase shows 9 LoadBalancer services total, of which 4 currently have lbipam.cilium.io/ips annotations (mosquitto-iot, home-assistant-bgp, ldap-outpost-lb, and now this one). The pattern is consistent with what you're adding, but the phrasing might suggest all others are already pinned when they're not. Minor clarity issue, not a correctness problem.

🟢 Nits

kubernetes/apps/trmnl/terminus.yaml:127–136 — The comment is thorough and well-placed. One small note: the phrase "a single sidekiq restart shortly after a rollout is benign" could benefit from stating "due to the Recreate deployment strategy allowing the web container's migration to race ahead" to make the operational context even clearer for future readers unfamiliar with this design. However, the comment already captures the essential point.

Tests

No new tests are needed — this change is pure configuration and documentation. The PR body documents that the changes were verified on a live deployment (PostgreSQL 18, Valkey auto-bump, fresh Terminus deploy converged in ~1s with expected sidekiq restart).

Recommendation

Looks reasonable — no blocking issues; comments above are optional improvements to clarity.

🤖 **Automated review** — model `claude-haiku-4-5-20251001` (complexity: trivial), commit `931810e28bad3b931b6bf0530b3c9c68ebd361ee`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR pins the Terminus device LoadBalancer to an explicit IPAM address (`10.2.99.89`/`fd99::89`) and documents an expected race condition in the sidekiq container on fresh deploys. Both changes are defensive improvements based on live deployment observation, with clear reasoning and thorough operational documentation. ## ✓ No Blocking Issues ## 🟡 Suggestions **kubernetes/apps/trmnl/device-service.yaml:30–35** — The comment references "every other LB Service in the repo" pinning addresses. A grep of the codebase shows 9 LoadBalancer services total, of which 4 currently have `lbipam.cilium.io/ips` annotations (mosquitto-iot, home-assistant-bgp, ldap-outpost-lb, and now this one). The pattern is consistent with what you're adding, but the phrasing might suggest all others are already pinned when they're not. Minor clarity issue, not a correctness problem. ## 🟢 Nits **kubernetes/apps/trmnl/terminus.yaml:127–136** — The comment is thorough and well-placed. One small note: the phrase "a single sidekiq restart shortly after a rollout is benign" could benefit from stating "due to the Recreate deployment strategy allowing the web container's migration to race ahead" to make the operational context even clearer for future readers unfamiliar with this design. However, the comment already captures the essential point. ## Tests No new tests are needed — this change is pure configuration and documentation. The PR body documents that the changes were verified on a live deployment (PostgreSQL 18, Valkey auto-bump, fresh Terminus deploy converged in ~1s with expected sidekiq restart). ## Recommendation Looks reasonable — no blocking issues; comments above are optional improvements to clarity. <!-- pr-review:sha=931810e28bad3b931b6bf0530b3c9c68ebd361ee -->
k.renner merged commit 73c2674344 into master 2026-08-10 21:08:54 +00:00
Sign in to join this conversation.