Hosted runners on a replaced image are turned away while a deploy rolls out (#149)
runnerSpec refuses a hosted runner that reports an image other than the deployed one (RUNNER_IMAGE) and places the job again, up to STALE_IMAGE_TRIES (4) times. After that, the job runs on the image it got. Self-hosted runners are unaffected. The e2e suite reads the admin error list and checks that an unknown request id gets a 404. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ruyx3Qr4jYrZEpPi466deb
Canberk Demirci authored · GitHub committed signed1 parent cd4c05ccommit 01989b4b078e109ede2cc20e6370997402654019Browse files
5 files changed+72−3
Files (5)
modifiedapps/diffinery/docs/actions/runners.md+1−1View file
| @@ -146,7 +146,7 @@ | ||
| 146 | 146 | |
| 147 | 147 | If the container stops unexpectedly (for example it runs out of memory), the job fails with `The runner stopped unexpectedly (exit code N).` A container that stops before its runner fetched the job is replaced instead, since no step ran. |
| 148 | 148 | |
| 149 | -The log's first lines name where the job ran, for example `Runner: image diffinery-runner:96dbbe31790f, in fra06, EEUR, container 6f1c…`. The jobs API returns the same as `runner` (`image`, `location`, `instance`), next to `runner_name`. `GET /meta` names the image the last deploy set as `runner_image`; while a deploy's rollout is updating instances, a job can still start on the previous one. | |
| 149 | +The log's first lines name where the job ran, for example `Runner: image diffinery-runner:96dbbe31790f, in fra06, EEUR, container 6f1c…`. The jobs API returns the same as `runner` (`image`, `location`, `instance`), next to `runner_name`. `GET /meta` names the image the last deploy set as `runner_image`. While a deploy's rollout is updating instances, a new container can still start on the previous image. Diffinery turns that runner away before it runs anything and starts the job again on another container (after 30 s, then 1, 2 and 4 minutes). After four tries, the job runs on whatever image it got rather than wait for a rollout that has stalled. | |
| 150 | 150 | |
| 151 | 151 | ### Windows and macOS |
| 152 | 152 |
modifiedapps/diffinery/docs/self-hosting/deploy.md+1−1View file
| @@ -28,7 +28,7 @@ | ||
| 28 | 28 | 1. Install dependencies, build the runner agent, the web UI's client bundles and the documentation (the pages under `apps/diffinery/docs`, the CLI reference and the CLI file served at `/-/cli/diffinery.mjs`), typecheck and run the unit tests. |
| 29 | 29 | 2. `scripts/bootstrap.sh` creates whatever is missing in your account (D1 database, R2 bucket, queues, the Cloudflare Access application for `/auth/access`), sets the R2 bucket's lifecycle rules, checks Email Sending, writes `apps/diffinery/wrangler.deploy.jsonc` with the real IDs and your hostname's route, `PUBLIC_URL` and email sender, and applies D1 migrations. |
| 30 | 30 | 3. `wrangler deploy -c wrangler.deploy.jsonc` uploads the Worker, the Durable Object classes, the two Workflows (workflow runs and GitHub imports) and the cron trigger, and builds and pushes the runner image for Containers. |
| 31 | -4. `scripts/wait-container-rollout.sh` waits until the container rollouts of the new runner image reach their last step. Even then, an instance the rollout hasn't updated yet can start a job on the old image, so the live test waits until jobs report the deployed image (`runner_image` on `GET /api/v3/meta`). Running containers keep the old image until they finish. It gives up with a warning, not a failure, after `ROLLOUT_WAIT_MINUTES` (10 by default). | |
| 31 | +4. `scripts/wait-container-rollout.sh` waits until the container rollouts of the new runner image reach their last step. Even then, an instance the rollout hasn't updated yet can start a container on the old image. Diffinery turns away a runner that reports any image other than `RUNNER_IMAGE` and starts the job again, up to four times. The live test waits until jobs report the deployed image (`runner_image` on `GET /api/v3/meta`) and then checks that every job ran on it. Running containers keep the old image until they finish. It gives up with a warning, not a failure, after `ROLLOUT_WAIT_MINUTES` (10 by default). | |
| 32 | 32 | 5. `scripts/ensure-secrets.sh` creates the Worker's two secrets, `MASTER_KEY` and `OIDC_PRIVATE_KEY`, the first time only. |
| 33 | 33 | 6. On the very first deploy, bootstrap runs again to finish the Access setup, and the Worker is redeployed. |
| 34 | 34 | 7. Smoke tests check `/healthz`, the sign-in pages, static assets and D1 sessions. |
modifiedapps/diffinery/src/repo/actions.ts+16−0View file
| @@ -59,6 +59,13 @@ | ||
| 59 | 59 | export const MAX_PLACEMENT_TRIES = 8; |
| 60 | 60 | /** Seconds before the next start of a job whose container got no machine: 30 s, doubling to 5 minutes. */ |
| 61 | 61 | export const placementDelay = (tries: number): number => Math.min(30 * 2 ** (tries - 1), 300); |
| 62 | +/** | |
| 63 | + * While a deploy's container rollout updates instances, a new container can still start on the | |
| 64 | + * previous runner image. A hosted runner that reports another image than the deployed one | |
| 65 | + * (RUNNER_IMAGE) is turned away before it runs anything, and the job is placed again, up to this | |
| 66 | + * many times; after that it runs on whatever image it got rather than wait on a stuck rollout. | |
| 67 | + */ | |
| 68 | +export const STALE_IMAGE_TRIES = 4; | |
| 62 | 69 | const NO_INSTANCE = "no container instance"; |
| 63 | 70 | /** Runs one repository may have unfinished at once; past it, new ones aren't started. */ |
| 64 | 71 | export const MAX_ACTIVE_RUNS = 500; |
| @@ -1660,8 +1667,17 @@ | ||
| 1660 | 1667 | return run.event === "pull_request" && this.repo.policy.isForkPull(run.pr_number); |
| 1661 | 1668 | } |
| 1662 | 1669 | |
| 1670 | + /** Turns away a hosted runner on an image other than the deployed one (see STALE_IMAGE_TRIES). */ | |
| 1671 | + private async refuseStaleRunner(job: JobRow, runner: JobRunnerInfo | null): Promise<void> { | |
| 1672 | + const want = this.env.RUNNER_IMAGE; | |
| 1673 | + if (!want || job.labels || runner?.image === want || job.placement_tries >= STALE_IMAGE_TRIES) return; | |
| 1674 | + await this.placementFailed(job.id, `a runner started on ${runner?.image ?? "an older image"} while the deploy of ${want} was rolling out`); | |
| 1675 | + throw new HttpError(409, `this runner's image (${runner?.image ?? "unknown"}) was replaced by ${want}; the job starts again on a new runner`); | |
| 1676 | + } | |
| 1677 | + | |
| 1663 | 1678 | async runnerSpec(jobId: string, token: string, runner: JobRunnerInfo | null = null): Promise<JobSpec> { |
| 1664 | 1679 | const job = await this.authJob(jobId, token); |
| 1680 | + await this.refuseStaleRunner(job, runner); | |
| 1665 | 1681 | const run = this.runRow(job.run_id); |
| 1666 | 1682 | const exp = await this.expanded(run); |
| 1667 | 1683 | const def: Job = exp.workflow.jobs[job.job_key]!; |
modifiedapps/diffinery/test/e2e/e2e.test.mjs+4−0View file
| @@ -2653,6 +2653,10 @@ | ||
| 2653 | 2653 | assert.equal(state.tick_minutes, 1); |
| 2654 | 2654 | assert.ok(state.last_tick_at && state.covered_until, JSON.stringify(state)); |
| 2655 | 2655 | assert.ok(Date.parse(state.next_tick_at) > Date.parse(state.last_tick_at), JSON.stringify(state)); |
| 2656 | + // And the newest unexpected server errors; a request id that isn't kept is a 404. | |
| 2657 | + const errors = await ok("GET", "/diffinery/admin/errors?per_page=5"); | |
| 2658 | + assert.ok(Array.isArray(errors) && errors.length <= 5, JSON.stringify(errors)); | |
| 2659 | + assert.equal((await api("GET", "/diffinery/admin/errors/not-a-request-id")).status, 404); | |
| 2656 | 2660 | // A push without the schedule removes it. |
| 2657 | 2661 | const src = join(work, "schedules"); |
| 2658 | 2662 | rmSync(join(src, ".github/workflows/tick.yml")); |
modifiedapps/diffinery/test/unit/actions-runtime.test.ts+50−1View file
| @@ -1,7 +1,7 @@ | ||
| 1 | 1 | import { describe, expect, it } from "vitest"; |
| 2 | 2 | import { BodyTooLarge, STEP_OPS, batchParts, blockKey, chunkKey, compose, finalKey, listStaged, parseBlockList, parseRange, planCompose, putStream, runComposeStep, stagingPrefix, type BlobBucket, type ComposeLimits, type ComposeStep } from "../../src/lib/blob-store.js"; |
| 3 | 3 | import { DatabaseSync } from "node:sqlite"; |
| 4 | -import { Actions, MAX_ACTIVE_RUNS, MAX_HOSTED_JOBS, MAX_PLACEMENT_TRIES, parseRunnerInfo, placementDelay } from "../../src/repo/actions.js"; | |
| 4 | +import { Actions, MAX_ACTIVE_RUNS, MAX_HOSTED_JOBS, MAX_PLACEMENT_TRIES, parseRunnerInfo, placementDelay, STALE_IMAGE_TRIES } from "../../src/repo/actions.js"; | |
| 5 | 5 | import { testD1 } from "./stubs/d1.js"; |
| 6 | 6 | import { DEFAULT_ARTIFACT_QUOTA, Runtime, artifactFileName, artifactObjectKeys } from "../../src/repo/runtime.js"; |
| 7 | 7 | import { REPO_SCHEMA, migrateRepoSchema } from "../../src/repo/schema.js"; |
| @@ -757,6 +757,55 @@ | ||
| 757 | 757 | expect(sql.exec("SELECT failure FROM jobs WHERE id = 'j1'").one().failure).toMatch(/No hosted runner could be started/); |
| 758 | 758 | expect([1, 2, 3, 4, 5, 6].map(placementDelay)).toEqual([30, 60, 120, 240, 300, 300]); |
| 759 | 759 | }); |
| 760 | + | |
| 761 | + it("a hosted runner on an image other than the deployed one is turned away and the job placed again, up to STALE_IMAGE_TRIES", async () => { | |
| 762 | + const sql = sqlStorage(); | |
| 763 | + sql.exec(REPO_SCHEMA); | |
| 764 | + migrateRepoSchema(sql as unknown as SqlStorage); | |
| 765 | + const events: { type: string; payload: Record<string, unknown> }[] = []; | |
| 766 | + const repo = { | |
| 767 | + sql, | |
| 768 | + meta: { repoId: "r", owner: "acme", ownerId: "acme", name: "app", defaultBranch: "main" }, | |
| 769 | + upsertCheck: () => {}, | |
| 770 | + broadcast: () => {}, | |
| 771 | + deploys: { jobCompleted: () => {} }, | |
| 772 | + runtime: { sweepJob: async () => {} }, | |
| 773 | + } as unknown as RepoDO; | |
| 774 | + const DB = testD1(); | |
| 775 | + DB.sqlite.prepare("INSERT INTO accounts (id, login, kind, status, created_at) VALUES ('acme', 'acme', 'org', 'active', ?)").run(new Date().toISOString()); | |
| 776 | + const env = { | |
| 777 | + DB, | |
| 778 | + PUBLIC_URL: "https://d.test", | |
| 779 | + RUNNER_IMAGE: "diffinery-runner:new", | |
| 780 | + RUNNER: { idFromName: (n: string) => n, get: () => ({ startJob: async () => {}, stopJob: async () => {} }) }, | |
| 781 | + RUN_WORKFLOW: { get: async () => ({ sendEvent: async (e: { type: string; payload: Record<string, unknown> }) => void events.push(e) }) }, | |
| 782 | + } as unknown as Env; | |
| 783 | + const actions = new Actions(repo, env); | |
| 784 | + const t = new Date().toISOString(); | |
| 785 | + sql.exec("INSERT INTO runs (id, run_number, workflow_path, workflow_name, event, ref, sha, actor, status, created_at, updated_at, event_payload) VALUES ('run1', 1, 'w.yml', 'w', 'push', 'refs/heads/main', 'abc', '{}', 'in_progress', ?, ?, '{}')", t, t); | |
| 786 | + sql.exec("INSERT INTO jobs (id, run_id, job_key, name, status) VALUES ('j1', 'run1', 'k', 'k', 'queued'), ('s1', 'run1', 's', 's', 'waiting')"); | |
| 787 | + sql.exec("UPDATE jobs SET labels = '[\"self-hosted\"]' WHERE id = 's1'"); | |
| 788 | + const a = actions as unknown as { launch(id: string): Promise<void>; refuseStaleRunner(job: unknown, runner: unknown): Promise<void>; jobRow(id: string): unknown }; | |
| 789 | + const refuse = (id: string, image?: string) => a.refuseStaleRunner(a.jobRow(id), image ? { image } : null); | |
| 790 | + const job = () => sql.exec("SELECT status, placement_tries FROM jobs WHERE id = 'j1'").toArray()[0]; | |
| 791 | + | |
| 792 | + await a.launch("j1"); | |
| 793 | + // The deployed image runs the job. | |
| 794 | + await expect(refuse("j1", "diffinery-runner:new")).resolves.toBeUndefined(); | |
| 795 | + expect(job()).toMatchObject({ status: "waiting", placement_tries: 0 }); | |
| 796 | + // The previous image (or one that doesn't say) is turned away, and the job waits to start again. | |
| 797 | + for (const [i, image] of ["diffinery-runner:old", undefined, "diffinery-runner:old", "diffinery-runner:old"].entries()) { | |
| 798 | + await expect(refuse("j1", image)).rejects.toMatchObject({ status: 409 }); | |
| 799 | + expect(job()).toMatchObject({ status: "queued", placement_tries: i + 1 }); | |
| 800 | + expect(events.at(-1)).toEqual({ type: "job-j1", payload: { retryInSeconds: placementDelay(i + 1) } }); | |
| 801 | + await actions.relaunchJob("j1"); | |
| 802 | + } | |
| 803 | + // A rollout that doesn't finish doesn't hold the job up past STALE_IMAGE_TRIES. | |
| 804 | + expect(job()).toMatchObject({ status: "waiting", placement_tries: STALE_IMAGE_TRIES }); | |
| 805 | + await expect(refuse("j1", "diffinery-runner:old")).resolves.toBeUndefined(); | |
| 806 | + // Self-hosted runners run any image. | |
| 807 | + await expect(refuse("s1", "my-image")).resolves.toBeUndefined(); | |
| 808 | + }); | |
| 760 | 809 | }); |
| 761 | 810 | |
| 762 | 811 | describe("runner info", () => { |