Skip to content

Commit c202e95

Browse files
committed
Harden managed service timeout cleanup
1 parent 21cdaff commit c202e95

5 files changed

Lines changed: 127 additions & 10 deletions

File tree

packages/cli/src/commands/recipe-run-finalizer.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ function classifyRunResourceFailure(status: RuntimeRunRecord["status"], failure:
353353

354354
function classifyRecipePhaseFailure(phase: string): string {
355355
switch (phase) {
356+
case "provision_runtime_services":
356357
case "runtime_startup":
357358
case "run_blueprint_steps":
358359
return "startup"

packages/cli/src/commands/recipe-run.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { RecipePhaseError } from "./recipe-run-phases.js"
2929
import { markPreviewLeaseAvailable, markPreviewLeaseFailed, markPreviewLeaseReleased, startPreviewLeaseRecipeRun } from "./preview-lease.js"
3030
import { importRecipeSiteSeeds } from "./recipe-site-seeds.js"
3131
import { applyRecipeRuntimeSetup, cleanupInputMountBaselines, prepareRecipeRuntimeSetup, recipeRunDependencyOverlay, recipeRunExtraPlugin, recipeRunStagedFile, rewriteInputMountPathArgs } from "./recipe-runtime-setup.js"
32-
import { provisionRuntimeServices, runtimeServiceEvidenceFromError, type RuntimeServiceEvidence } from "../runtime-services.js"
32+
import { provisionRuntimeServices, provisionRuntimeServicesForRecipe, runtimeServiceEvidenceFromError, type RuntimeServiceEvidence } from "../runtime-services.js"
3333
import { distributionStartupProbeFailure, executeRecipeCollectWorkloadResult, executeRecipeWorkflowStep, recipeAdvisoryFailure, recipeBrowserEvidence, recipeStepFailure, recipeWorkflowArgsEvidence, recipeWorkflowStepIsAdvisory, runDistributionSetupArtifacts, runDistributionStartupProbes, runRecipeProbes, withRecipeExecutionPhase } from "./recipe-run-workflow-evidence.js"
3434
import type { RecipeAdvisoryFailure, RecipeBrowserEvidence, RecipeDiagnosticArtifactRef, RecipeEffectiveRecipeArtifact, RecipeExecutionResult, RecipeFuzzCaseCommandRef, RecipeFuzzCaseResult, RecipeFuzzCaseStatus, RecipeFuzzRunResult, RecipeInterruptionController, RecipePhaseEvidence, RecipePhaseName, RecipePhpWasmRuntimeDiagnostic, RecipeRunCommandOutput, RecipeRunComponentContract, RecipeRunDeclaredArtifact, RecipeRunDistributionSetupArtifact, RecipeRunDistributionStartupProbe, RecipeRunFixtureDatabase, RecipeRunOptions, RecipeRunOutput, RecipeRunProbe, RecipeRunProvenance, RecipeRunStagedFile, RecipeRuntimeDiagnostic, RecipeStepFailure, RecipeValidateOptions, RecipeValidateOutput } from "./recipe-run-types.js"
3535

@@ -162,7 +162,13 @@ export async function runRecipe(options: RecipeRunOptions, interruption?: Recipe
162162
let startupDurationMs: number | undefined
163163
let cleanupEvidence: RunResourceCleanupEvidence | undefined
164164
let managedServices: Awaited<ReturnType<typeof provisionRuntimeServices>> | undefined
165+
let managedServicesReleased = false
165166
let serviceEvidence: RuntimeServiceEvidence[] = []
167+
const releaseManagedServices = async (): Promise<void> => {
168+
if (!managedServices || managedServicesReleased) return
169+
await managedServices.release()
170+
managedServicesReleased = true
171+
}
166172
let runtimeDestroyed = false
167173
const destroyActiveRuntime = async (): Promise<void> => {
168174
if (!runtime || runtimeDestroyed) {
@@ -183,7 +189,11 @@ export async function runRecipe(options: RecipeRunOptions, interruption?: Recipe
183189
interruption?.throwIfInterrupted()
184190

185191
runRecord = await runRegistry.update(runRecord.runId, { status: "booting" })
186-
managedServices = await phaseTracker.run("provision_runtime_services", { services: recipe.inputs?.services?.map(({ id, kind }) => ({ id, kind })) ?? [] }, async () => await awaitRecipe("runtime-services.provision", () => provisionRuntimeServices(recipe.inputs?.services ?? [], { signal: interruption?.signal })))
192+
managedServices = await phaseTracker.run("provision_runtime_services", { services: recipe.inputs?.services?.map(({ id, kind }) => ({ id, kind })) ?? [] }, async () => await provisionRuntimeServicesForRecipe(
193+
recipe.inputs?.services ?? [],
194+
async (provisioning) => await awaitRecipe("runtime-services.provision", provisioning),
195+
{ signal: interruption?.signal, onEvidence: (evidence) => { serviceEvidence = evidence } },
196+
))
187197
serviceEvidence = managedServices.evidence
188198
Object.assign(runtimeEnv, managedServices.env)
189199
const runtimeEnvironment = {
@@ -367,7 +377,7 @@ export async function runRecipe(options: RecipeRunOptions, interruption?: Recipe
367377
interruption.clear()
368378
}
369379
})
370-
await managedServices?.release()
380+
await releaseManagedServices()
371381
await cleanupRecipePreparedSources(workspaceMounts, extraPlugins, stagedFiles, overlays, dependencyOverlays)
372382
await cleanupInputMountBaselines(inputMountBaselinePaths)
373383
})
@@ -507,7 +517,7 @@ export async function runRecipe(options: RecipeRunOptions, interruption?: Recipe
507517
}
508518

509519
cleanupEvidence = await runManagedServiceCleanup(runRegistry, runRecord, serviceEvidence, true, async () => {
510-
await managedServices?.release()
520+
await releaseManagedServices()
511521
await cleanupRecipePreparedSources(workspaceMounts, extraPlugins, stagedFiles, overlays, dependencyOverlays)
512522
await cleanupInputMountBaselines(inputMountBaselinePaths)
513523
})
@@ -548,6 +558,10 @@ export async function runRecipe(options: RecipeRunOptions, interruption?: Recipe
548558
},
549559
})
550560
} finally {
561+
if (managedServices && !managedServicesReleased) {
562+
await managedServices.release().catch(() => undefined)
563+
await runRegistry.update(runRecord.runId, { metadata: { managedRuntimeServices: managedServices.evidence } }).catch(() => undefined)
564+
}
551565
cancellationWatcher?.dispose()
552566
}
553567
}

packages/cli/src/recipe-validation.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,11 @@ export async function validateWorkspaceRecipeSemantics(recipe: WorkspaceRecipe,
702702

703703
function validateRecipeRuntimeServices(recipe: WorkspaceRecipe, addIssue: (code: string, path: string, message: string) => void): void {
704704
const ids = new Set<string>()
705-
const environment = new Set<string>(Object.keys(recipe.inputs?.runtimeEnv ?? {}))
705+
const environment = new Set<string>([
706+
...Object.keys(recipe.distribution?.env ?? {}),
707+
...Object.keys(recipe.inputs?.runtimeEnv ?? {}),
708+
...(recipe.inputs?.secretEnv ?? []),
709+
])
706710
for (const [index, service] of (recipe.inputs?.services ?? []).entries()) {
707711
const path = `$.inputs.services[${index}]`
708712
if (!/^[A-Za-z0-9][A-Za-z0-9_.-]*$/.test(service.id)) addIssue("invalid-runtime-service-id", `${path}.id`, "Runtime service ids must be stable identifiers.")

packages/cli/src/runtime-services.ts

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export async function provisionRuntimeServices(services: WorkspaceRecipeRuntimeS
7878
provisioned.push(managed)
7979
}
8080
} catch (error) {
81-
await releaseServices(provisioned)
81+
await releaseServices(provisioned).catch(() => undefined)
8282
if (error instanceof RuntimeServiceProvisionError) throw error
8383
throw new RuntimeServiceProvisionError("Managed runtime service provisioning failed", evidence)
8484
}
@@ -101,6 +101,37 @@ export async function provisionRuntimeServices(services: WorkspaceRecipeRuntimeS
101101
}
102102
}
103103

104+
export async function provisionRuntimeServicesForRecipe(
105+
services: WorkspaceRecipeRuntimeService[],
106+
guard: <T>(promise: Promise<T>) => Promise<T>,
107+
options: { signal?: AbortSignal; dependencies?: RuntimeServiceDependencies; onEvidence?: (evidence: RuntimeServiceEvidence[]) => void } = {},
108+
): Promise<Awaited<ReturnType<typeof provisionRuntimeServices>>> {
109+
const controller = new AbortController()
110+
const abort = () => controller.abort()
111+
options.signal?.addEventListener("abort", abort, { once: true })
112+
if (options.signal?.aborted) controller.abort()
113+
const provisioning = provisionRuntimeServices(services, { signal: controller.signal, dependencies: options.dependencies })
114+
try {
115+
return await guard(provisioning)
116+
} catch (error) {
117+
controller.abort()
118+
try {
119+
const provisioned = await provisioning
120+
try {
121+
await provisioned.release()
122+
} finally {
123+
options.onEvidence?.(provisioned.evidence)
124+
}
125+
} catch (provisionError) {
126+
const evidence = runtimeServiceEvidenceFromError(provisionError)
127+
if (evidence) options.onEvidence?.(evidence)
128+
}
129+
throw error
130+
} finally {
131+
options.signal?.removeEventListener("abort", abort)
132+
}
133+
}
134+
104135
const mysqlDockerProvider: RuntimeServiceProvider = {
105136
name: "docker",
106137
kind: "mysql",
@@ -118,7 +149,7 @@ async function provisionMysqlDockerService(service: WorkspaceRecipeRuntimeServic
118149
evidenceList.push(evidence)
119150
const container = `wp-codebox-${service.id}-${dependencies.randomBytes(6).toString("hex")}`
120151
const password = dependencies.randomBytes(24).toString("base64url")
121-
const childEnvironment = { PATH: process.env.PATH, MYSQL_DATABASE: "runtime", MYSQL_USER: "runtime", MYSQL_PASSWORD: password, MYSQL_ROOT_PASSWORD: password }
152+
const childEnvironment = { ...process.env, MYSQL_DATABASE: "runtime", MYSQL_USER: "runtime", MYSQL_PASSWORD: password, MYSQL_ROOT_PASSWORD: password }
122153
const runArgs = ["run", "--detach", "--rm", "--name", container, "--publish", "127.0.0.1::3306", "--tmpfs", "/var/lib/mysql", "--env", "MYSQL_DATABASE", "--env", "MYSQL_USER", "--env", "MYSQL_PASSWORD", "--env", "MYSQL_ROOT_PASSWORD", MYSQL_IMAGE]
123154
let started = false
124155
try {
@@ -164,14 +195,25 @@ async function releaseService(container: string, evidence: RuntimeServiceEvidenc
164195
await dependencies.execute("docker", ["rm", "--force", container], { signal, timeout: 30_000 })
165196
evidence.lifecycle = "released"
166197
evidence.teardown = "completed"
167-
} catch {
198+
} catch (error) {
199+
if (dockerContainerIsAbsent(error)) {
200+
evidence.lifecycle = "released"
201+
evidence.teardown = "completed"
202+
return
203+
}
168204
evidence.lifecycle = "failed"
169205
evidence.teardown = "failed"
170206
evidence.diagnostic = { code: "teardown-failed" }
171207
throw new Error(`Managed runtime service teardown failed: ${evidence.id}`)
172208
}
173209
}
174210

211+
function dockerContainerIsAbsent(error: unknown): boolean {
212+
if (!(error instanceof Error)) return false
213+
const stderr = "stderr" in error && typeof error.stderr === "string" ? error.stderr : ""
214+
return /No such container/i.test(`${error.message}\n${stderr}`)
215+
}
216+
175217
export function parseLoopbackPort(output: string): number {
176218
const match = output.trim().match(/^127\.0\.0\.1:(\d+)$/m)
177219
const port = match ? Number(match[1]) : NaN

tests/runtime-services.test.ts

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import assert from "node:assert/strict"
22
import { createServer } from "node:net"
3-
import { parseLoopbackPort, provisionRuntimeServices, RuntimeServiceProvisionError, runtimeServiceEvidenceFromError, runtimeServicePlan, waitForMysqlProtocol, type RuntimeServiceDependencies } from "../packages/cli/src/runtime-services.ts"
4-
import { validateWorkspaceRecipeJsonSchema } from "../packages/runtime-core/src/recipe-schema.ts"
3+
import { parseLoopbackPort, provisionRuntimeServices, provisionRuntimeServicesForRecipe, RuntimeServiceProvisionError, runtimeServiceEvidenceFromError, runtimeServicePlan, waitForMysqlProtocol, type RuntimeServiceDependencies } from "../packages/cli/src/runtime-services.ts"
4+
import { planWorkspaceRecipe } from "../packages/cli/src/recipe-dry-run.ts"
5+
import { validateWorkspaceRecipeSemantics } from "../packages/cli/src/recipe-validation.ts"
6+
import { validateWorkspaceRecipeJsonSchema, type WorkspaceRecipe } from "../packages/runtime-core/src/index.ts"
57

68
const service = { id: "test-db", kind: "mysql", outputs: { host: "DB_HOST", port: "DB_PORT", password: "DB_PASSWORD" } } as const
79
const plan = runtimeServicePlan([service])
@@ -13,6 +15,22 @@ const valid = validateWorkspaceRecipeJsonSchema({ schema: "wp-codebox/workspace-
1315
assert.equal(valid.valid, true)
1416
const unsafe = validateWorkspaceRecipeJsonSchema({ schema: "wp-codebox/workspace-recipe/v1", inputs: { services: [{ ...service, outputs: { port: "bad-name" } }] }, workflow: { steps: [{ command: "wordpress.run-php" }] } })
1517
assert.equal(unsafe.valid, false)
18+
const recipe: WorkspaceRecipe = { schema: "wp-codebox/workspace-recipe/v1", inputs: { services: [service] }, workflow: { steps: [{ command: "wordpress.run-php", args: ["code=echo 'ok';"] }] } }
19+
assert.deepEqual(await validateWorkspaceRecipeSemantics(recipe, "recipe.json"), [])
20+
const dryRun = await planWorkspaceRecipe(recipe, process.cwd(), { recipePath: "recipe.json" }, {
21+
defaultWordPressVersion: "latest",
22+
resolveExecutionSpec: async (step) => ({ command: step.command, args: step.args ?? [] }),
23+
})
24+
assert.deepEqual(dryRun.services, plan)
25+
const collisions: WorkspaceRecipe = {
26+
...recipe,
27+
distribution: { name: "fixture", wordpress: { root: "/wordpress" }, env: { DB_HOST: "distribution" } },
28+
inputs: { runtimeEnv: { DB_PORT: "3306" }, secretEnv: ["DB_PASSWORD"], services: [service] },
29+
}
30+
assert.deepEqual(
31+
(await validateWorkspaceRecipeSemantics(collisions, "recipe.json")).map((issue) => issue.code),
32+
["duplicate-runtime-service-env", "duplicate-runtime-service-env", "duplicate-runtime-service-env"],
33+
)
1634

1735
const server = createServer((socket) => socket.end(Buffer.from([1, 0, 0, 0, 10])))
1836
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve))
@@ -44,8 +62,12 @@ assert.equal(provisioned.env.DB_PORT, "41001")
4462
assert.equal(provisioned.env.DB_PASSWORD, Buffer.alloc(24, 7).toString("base64url"))
4563
const runCall = calls.find((call) => call.args[0] === "run")
4664
assert.ok(runCall?.args.includes("MYSQL_PASSWORD"))
65+
assert.ok(runCall?.args.includes("127.0.0.1::3306"), "Docker publishes MySQL on a loopback ephemeral port")
66+
assert.deepEqual(runCall?.args.slice(runCall.args.indexOf("--tmpfs"), runCall.args.indexOf("--tmpfs") + 2), ["--tmpfs", "/var/lib/mysql"])
67+
assert.equal(runCall?.args.includes("--volume") || runCall?.args.includes("--mount"), false, "Docker uses no persistent volume")
4768
assert.equal(runCall?.args.some((arg) => arg.includes(provisioned.env.DB_PASSWORD)), false, "credentials never enter Docker argv")
4869
assert.equal(JSON.stringify(provisioned.evidence).includes(provisioned.env.DB_PASSWORD), false, "credentials never enter evidence")
70+
assert.equal(runCall?.env?.DOCKER_HOST, process.env.DOCKER_HOST, "Docker provider context is preserved")
4971
assert.equal(calls[0]?.args[0], "image", "the provider checks the image before starting the service")
5072
await provisioned.release()
5173
await provisioned.release()
@@ -58,6 +80,40 @@ await provisionedBeforeAbort.release()
5880
const cleanupCall = calls.filter((call) => call.args[0] === "rm").at(-1)
5981
assert.equal(cleanupCall?.signal, undefined, "teardown has an independent cleanup context after interruption")
6082

83+
let finishLateProvisioning: (() => void) | undefined
84+
let lateRemoval = false
85+
const lateDependencies: RuntimeServiceDependencies = {
86+
...dependencies,
87+
async execute(command, args, options) {
88+
if (args[0] === "run") await new Promise<void>((resolve) => { finishLateProvisioning = resolve })
89+
if (args[0] === "rm") lateRemoval = true
90+
return dependencies.execute(command, args, options)
91+
},
92+
}
93+
const guardedProvisioning = provisionRuntimeServicesForRecipe([service], async () => {
94+
await new Promise<void>((resolve) => setTimeout(resolve, 5))
95+
throw new Error("recipe timeout")
96+
}, { dependencies: lateDependencies })
97+
while (!finishLateProvisioning) await new Promise<void>((resolve) => setTimeout(resolve, 1))
98+
finishLateProvisioning()
99+
await assert.rejects(guardedProvisioning, /recipe timeout/)
100+
assert.equal(lateRemoval, true, "a timeout waits for and tears down late provisioning")
101+
102+
const absentDependencies: RuntimeServiceDependencies = {
103+
...dependencies,
104+
async execute(command, args, options) {
105+
if (args[0] === "rm") {
106+
const error = new Error("docker rm failed") as Error & { stderr: string }
107+
error.stderr = `Error response from daemon: No such container: fixture`
108+
throw error
109+
}
110+
return dependencies.execute(command, args, options)
111+
},
112+
}
113+
const alreadyAbsent = await provisionRuntimeServices([service], { dependencies: absentDependencies })
114+
await alreadyAbsent.release()
115+
assert.equal(alreadyAbsent.evidence[0]?.teardown, "completed", "an already absent container is idempotently released")
116+
61117
let failedCleanup = false
62118
const failingDependencies: RuntimeServiceDependencies = {
63119
...dependencies,

0 commit comments

Comments
 (0)