CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

fix(ci): the broker reported healthy while unable to reach docker at all #5603

MergedXSccantynz wants to mergefix/broker-socket-accessmainopened 1d ago
ccantynzcommented 1d ago

The broker's first deploy came up healthy and could not make a single Docker API call.

The container runs as uid 1000; /var/run/docker.sock is root:docker 0660; every call failed with EACCES. The startup line said so plainly —

[broker] image=UNRESOLVED workdir=UNRESOLVED network=bridge

— and nothing else did, because /healthz returned a constant "ok".

A container reporting healthy while unable to do the one thing it exists for is precisely the defect this codebase spent today removing from everything else — a false green, shipped by the person removing them, hours after writing a document about it. Same family as a job that executed nothing reporting success, and a gate that stopped existing reporting a pass.

Two fixes; the second generalises

  1. group_add: ${DOCKER_GID:-999} gives the broker the socket's group.

    Not user: root, which also fixes the EACCES and is the tempting shortcut. Holding the socket is already root-equivalent on the host — but a supplementary group keeps a broker compromise from starting as root inside the container.

  2. /healthz answers the real question — 200 only when the image and work directory are resolved, 503 otherwise with a message naming the likeliest cause. It re-resolves every 30s while unresolved and stops once it succeeds, so a daemon that's merely slow on a cold boot heals itself.

Both mutation-verified: restoring the constant "ok", or swapping group_add for user: root, each fails a specific test.

The general lesson, which argues against my own work

The broker's only external dependency is the docker socket, and its health check did not check it. When a service has one dependency, "is it up" and "can it reach that dependency" are the same question — and answering the easier one is how a deploy passes green while the feature is dead.

ccantynzAI Reviewcommented 1d ago

AI Triage

(no summary)

Priority: medium Risk area: mixed

Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)

Suggestions only — nothing has been applied. The PR author stays in control.

gluecron[bot]🤖 botAI Reviewcommented 1d ago

AI review unavailable

The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.

Cross-repo impact

See what breaks downstream if this PR merges.

Analyze →
⮌ Merged

This pull request was merged into main.

c comment · e edit title · m merge · a approve · r request changes · ? shortcuts