fix(ci): the broker reported healthy while unable to reach docker at all #5603
4 changed files+97−1
Modified.env.example+7−0View fileUnifiedSplit
@@ -557,3 +557,10 @@ JOB_NETWORK=bridge
557557# Path to the docker socket inside the broker container. Only change this if
558558# the socket is mounted somewhere else.
559559DOCKER_SOCKET=/var/run/docker.sock
560
561# GID of the docker group on the HOST, so the broker can read the socket.
562# The socket is root:docker 0660 and the broker runs as uid 1000, so without
563# this every Docker call fails with EACCES — which is how the broker's first
564# deploy came up "healthy" and completely unable to work.
565# Find it with: stat -c %g /var/run/docker.sock
566DOCKER_GID=999
Modifieddocker-compose.standalone.yml+11−0View fileUnifiedSplit
@@ -270,6 +270,17 @@ services:
270270 - JOB_NETWORK=${JOB_NETWORK:-bridge}
271271 volumes:
272272 - /var/run/docker.sock:/var/run/docker.sock
273 # The socket is root:docker 0660 and this container runs as uid 1000, so
274 # without the socket's GROUP every Docker call fails with EACCES — which
275 # is how the first deploy came up "healthy" and completely unable to work.
276 # 999 is the docker gid on this host (`stat -c %g /var/run/docker.sock`);
277 # override DOCKER_GID in .env on a host that numbers it differently.
278 #
279 # This is a supplementary group, NOT `user: root`. The broker keeps uid
280 # 1000, so a compromise of the broker process still has to go through the
281 # socket rather than starting as root inside the container.
282 group_add:
283 - "${DOCKER_GID:-999}"
273284 expose:
274285 - "3002"
275286 restart: unless-stopped
Modifiedsrc/__tests__/ci-job-broker.test.ts+44−0View fileUnifiedSplit
@@ -154,6 +154,50 @@ describe("only the broker holds the docker socket", () => {
154154 });
155155});
156156
157describe("the broker cannot report healthy while unable to work", () => {
158 /**
159 * The first deploy of this service came up "healthy" and could not reach the
160 * Docker API at all: it runs as uid 1000, the socket is root:docker 0660,
161 * and every call failed with EACCES. `/healthz` returned a constant "ok".
162 *
163 * A container that reports healthy while unable to do the one thing it
164 * exists for is precisely the defect this codebase spent a day removing
165 * from everything else. Shipping it here, in the same day, is the argument
166 * for asserting it rather than remembering it.
167 */
168 test("healthz answers the real question, not a constant", () => {
169 const health = brokerSrc.slice(
170 brokerSrc.indexOf('url.pathname === "/healthz"'),
171 brokerSrc.indexOf('url.pathname === "/status"')
172 );
173 expect(health).toContain("jobImage && workHostDir");
174 expect(health).toContain("503");
175 });
176
177 test("an unresolved broker keeps retrying rather than giving up", () => {
178 // The daemon may not be answering yet on a cold boot. Giving up after one
179 // attempt leaves a broker useless until someone restarts it by hand.
180 expect(brokerSrc).toContain("setInterval");
181 expect(brokerSrc).toContain("clearInterval");
182 });
183
184 test("a job is refused with a legible reason when the broker is unconfigured", () => {
185 // Not a crash and not a silent success: the operator who enabled the
186 // broker needs to see WHY every job is failing.
187 expect(brokerSrc).toContain("broker is not configured");
188 });
189
190 test("compose gives the broker the socket's group, not root", () => {
191 const block = serviceBlock("broker");
192 expect(block).toMatch(/group_add/);
193 // `user: root` would also fix the EACCES and is the tempting shortcut.
194 // Holding the socket is already root-equivalent on the host, but a
195 // supplementary group keeps a broker compromise from STARTING as root
196 // inside the container.
197 expect(block).not.toMatch(/user:\s*root/);
198 });
199});
200
157201describe("a workspace name cannot become a path", () => {
158202 test("traversal is rejected outright", () => {
159203 // Unchecked, this mounts an arbitrary host directory into a container the
Modifiedsrc/broker-agent.ts+35−1View fileUnifiedSplit
@@ -334,7 +334,25 @@ const server = Bun.serve({
334334 idleTimeout: 255,
335335 async fetch(req) {
336336 const url = new URL(req.url);
337 if (url.pathname === "/healthz") return new Response("ok");
337 if (url.pathname === "/healthz") {
338 // READINESS, not liveness, and the difference is the whole point.
339 //
340 // The first deploy of this service came up "healthy" while unable to
341 // reach the Docker API at all: it runs as uid 1000 and the socket is
342 // root:docker 0660, so every call failed with EACCES. A constant "ok"
343 // reported a container that could not do the one thing it exists for —
344 // the exact defect this codebase spent a day removing from everything
345 // else, shipped here by the person removing it.
346 //
347 // Now the check answers the real question, and its message names the
348 // likeliest cause so nobody has to go and find this comment.
349 if (jobImage && workHostDir) return new Response("ok");
350 return new Response(
351 `not ready: image=${jobImage || "unresolved"} workdir=${workHostDir || "unresolved"} — ` +
352 `can this container read ${DOCKER_SOCKET}? it needs the socket's group (see group_add in compose)`,
353 { status: 503 }
354 );
355 }
338356 if (!authorized(req)) return json({ error: "unauthorized" }, 401);
339357
340358 if (url.pathname === "/status" && req.method === "GET") {
@@ -376,4 +394,20 @@ if (!TOKEN) {
376394 );
377395}
378396void resolveRuntime();
397/**
398 * Keep trying while unresolved.
399 *
400 * The daemon may simply not be answering yet on a cold boot, and a broker that
401 * gave up after one attempt would stay useless until someone restarted it.
402 * Stops as soon as it succeeds, so a working broker makes no periodic calls.
403 */
404const RESOLVE_RETRY_MS = 30_000;
405const retry = setInterval(() => {
406 if (jobImage && workHostDir) {
407 clearInterval(retry);
408 return;
409 }
410 void resolveRuntime();
411}, RESOLVE_RETRY_MS);
412
379413console.log(`[broker] listening on :${server.port}`);
380414
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts