Skip to content

Commit fd98984

Browse files
1stvampTrigger.dev RepoOps
authored andcommitted
feat(supervisor): carry the deployment's template key to the Runner
Carry a deployment's prebuilt template key on the dequeued run message, so the supervisor can hand it to the runner it creates for both new runs and restores. On a worker group that starts runs from templates, a run that cannot start from one goes back to the region's container worker group instead. Mono-RevId: 866804b09fe4724fc98af488aded1b85364f0098
1 parent 9e9bae0 commit fd98984

21 files changed

Lines changed: 1571 additions & 59 deletions

‎apps/supervisor/src/index.ts‎

Lines changed: 136 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ import {
2222
RunCrdWorkloadManager,
2323
RunnerKillInformer,
2424
RunnerRestoreInformer,
25+
reportTemplateKeyRefusal,
2526
settleRestoreFailure,
27+
type RunnerStartFailure,
2628
} from "./workloadManager/runCrd.js";
2729
import { workerLaneFor, type WorkloadManagerBackend } from "./workerLane.js";
2830
import { RestoreWatcher, type RestoreFailure } from "./services/restoreWatcher.js";
@@ -89,6 +91,20 @@ const outboundRequestDuration = new Histogram({
8991
registers: [register],
9092
});
9193

94+
const startReportsTotal = new Counter({
95+
name: "supervisor_start_reports_total",
96+
help: "Cold starts the operator failed before they began, moved to the fallback lane through the platform, by result: ok, conflict when the run had already moved on, or error.",
97+
labelNames: ["result"],
98+
registers: [register],
99+
});
100+
101+
const templateKeyRefusalsTotal = new Counter({
102+
name: "supervisor_template_key_refusals_total",
103+
help: "Dequeued runs this lane created no Runner for, as they had no valid template key, by outcome (fallback or fail) and result: ok, conflict when the run had already moved on, or error.",
104+
labelNames: ["outcome", "result"],
105+
registers: [register],
106+
});
107+
92108
const restoreReportsTotal = new Counter({
93109
name: "supervisor_restore_reports_total",
94110
help: "Failed restores reported to the platform, by outcome (requeue or fail) and result: ok, conflict when the run had already moved on, or error.",
@@ -183,6 +199,9 @@ class ManagedSupervisor {
183199
this.runnerKillInformer = new RunnerKillInformer({
184200
namespace: env.KUBERNETES_NAMESPACE,
185201
onRunnerKilled: (runnerId, kill) => void killReporter.onRunnerKilled(runnerId, kill),
202+
...(env.KUBERNETES_RUN_CRD_ENABLED
203+
? { onRunnerStartFailed: (failure) => void this.reportStartFailure(failure) }
204+
: {}),
186205
});
187206
}
188207
}
@@ -197,10 +216,16 @@ class ManagedSupervisor {
197216
this.restoreInformer = new RunnerRestoreInformer({
198217
namespace: env.KUBERNETES_NAMESPACE,
199218
onUnwatchedFailure: (failure) => void this.restoreWatcher?.report(failure),
200-
onUnwatchedRestore: ({ runnerId, uid, runFriendlyId, snapshotFriendlyId }) => {
219+
onUnwatchedRestore: ({
220+
runnerId,
221+
uid,
222+
runFriendlyId,
223+
snapshotFriendlyId,
224+
environmentId,
225+
}) => {
201226
if (runFriendlyId && snapshotFriendlyId) {
202227
void this.restoreWatcher?.watch(
203-
{ runFriendlyId, snapshotFriendlyId },
228+
{ runFriendlyId, snapshotFriendlyId, environmentId },
204229
{ runnerId, uid }
205230
);
206231
}
@@ -505,6 +530,11 @@ class ManagedSupervisor {
505530

506531
const { checkpoint, ...rest } = message;
507532

533+
if (this.runCrdManager && (await this.refuseWithoutTemplateKey(message))) {
534+
setExtra(fromContext(), "path_taken", "no_template_key");
535+
return;
536+
}
537+
508538
if (checkpoint) {
509539
setExtra(fromContext(), "path_taken", "restore");
510540
this.logger.debug("Restoring run", { runId: message.run.id });
@@ -655,6 +685,7 @@ class ManagedSupervisor {
655685
envId: message.environment.id,
656686
envType: message.environment.type,
657687
image: message.image,
688+
templateKey: message.templateKey,
658689
machine: message.run.machine,
659690
orgId: message.organization.id,
660691
projectId: message.project.id,
@@ -691,6 +722,7 @@ class ManagedSupervisor {
691722
{
692723
runFriendlyId: message.run.friendlyId,
693724
snapshotFriendlyId: message.snapshot.friendlyId,
725+
environmentId: message.environment.id,
694726
},
695727
restore
696728
);
@@ -734,6 +766,7 @@ class ManagedSupervisor {
734766
private async reportRestoreFailure({
735767
runFriendlyId,
736768
snapshotFriendlyId,
769+
environmentId,
737770
runnerId,
738771
outcome,
739772
}: RestoreFailure) {
@@ -760,7 +793,8 @@ class ManagedSupervisor {
760793
runFriendlyId,
761794
snapshotFriendlyId,
762795
body,
763-
runnerId
796+
runnerId,
797+
environmentId
764798
),
765799
}
766800
);
@@ -806,6 +840,105 @@ class ManagedSupervisor {
806840
}
807841
}
808842

843+
/** Reports a run this lane needs a template key for and has none, rather than starting it. */
844+
private async refuseWithoutTemplateKey(message: DequeuedMessage): Promise<boolean> {
845+
const refused = await reportTemplateKeyRefusal(
846+
{
847+
runtime: env.KUBERNETES_RUNNER_RUNTIME,
848+
templateKey: message.templateKey,
849+
checkpoint: message.checkpoint,
850+
},
851+
(body) =>
852+
this.workerSession.httpClient.reportRestoreOutcome(
853+
message.run.friendlyId,
854+
message.snapshot.friendlyId,
855+
body,
856+
undefined,
857+
message.environment.id
858+
)
859+
);
860+
if (!refused) {
861+
return false;
862+
}
863+
templateKeyRefusalsTotal.inc({ outcome: refused.outcome, result: refused.result });
864+
const fields = {
865+
runId: message.run.friendlyId,
866+
snapshotId: message.snapshot.friendlyId,
867+
outcome: refused.outcome,
868+
reason: refused.reason,
869+
result: refused.result,
870+
};
871+
if (refused.result === "error") {
872+
this.logger.error("No Runner created without a template key, and the report failed", {
873+
...fields,
874+
error: refused.error,
875+
});
876+
} else {
877+
this.logger.warn("No Runner created without a template key", fields);
878+
}
879+
return true;
880+
}
881+
882+
/**
883+
* Moves a cold start the operator failed before its attempt began to the platform's fallback
884+
* lane, rather than leaving it to the stall timeout. Without one it is requeued here, and the
885+
* queue's redelivery budget bounds how often a run comes back this way.
886+
*/
887+
private async reportStartFailure(failure: RunnerStartFailure) {
888+
const { runFriendlyId, snapshotFriendlyId, runnerId, environmentId, outcome } = failure;
889+
const fields = { runFriendlyId, snapshotFriendlyId, runnerId, reason: outcome.reason };
890+
this.logger.warn("Runner start failed (run-crd)", { ...fields, error: outcome.error });
891+
const manager = this.runCrdManager;
892+
if (!manager) {
893+
this.logger.warn(
894+
"Runner start failure left to the stall timeout, no run-crd manager",
895+
fields
896+
);
897+
return;
898+
}
899+
await this.workerSession.httpClient.sendDebugLog(runFriendlyId, {
900+
time: new Date(),
901+
message: "runner failed to start on the node",
902+
properties: { runnerId, error: outcome.error },
903+
});
904+
const settled = await settleRestoreFailure(
905+
{ runnerId, outcome, action: { outcome: "fallback", deleteRunner: true } },
906+
{
907+
deleteRunner: (name, uid) => manager.deleteFailedRunner(name, uid),
908+
report: (body) =>
909+
this.workerSession.httpClient.reportRestoreOutcome(
910+
runFriendlyId,
911+
snapshotFriendlyId,
912+
body,
913+
runnerId,
914+
environmentId
915+
),
916+
}
917+
);
918+
switch (settled.action) {
919+
case "none":
920+
return;
921+
case "kept":
922+
this.logger.warn("Failed start Runner not deleted, so the run was not moved", {
923+
...fields,
924+
error: settled.error,
925+
});
926+
return;
927+
case "reported":
928+
startReportsTotal.inc({ result: settled.result });
929+
if (settled.result === "error") {
930+
this.logger.warn("Start failure report failed", { ...fields, error: settled.error });
931+
} else {
932+
this.logger.info(
933+
settled.result === "conflict"
934+
? "Start failure not applied, the run has moved on"
935+
: "Start failure reported, the run was moved",
936+
fields
937+
);
938+
}
939+
}
940+
}
941+
809942
private async createWorkload(message: DequeuedMessage, timings: WarmStartTimings) {
810943
const createStart = performance.now();
811944
try {

‎apps/supervisor/src/services/restoreWatcher.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,28 @@ describe("RestoreWatcher", () => {
5050
});
5151
});
5252

53+
it("reports a failed restore with the environment it was dequeued for", async () => {
54+
const reported: RestoreFailure[] = [];
55+
const { watcher, outcomes } = setup({ reportFailure: async (f) => void reported.push(f) });
56+
57+
const watched = watcher.watch(
58+
{ ...target, environmentId: "env_a" },
59+
{ runnerId: "runner-a", uid: "uid-a" }
60+
);
61+
outcomes[0]!.resolve(failed);
62+
await watched;
63+
64+
expect(reported).toEqual([
65+
{
66+
runFriendlyId: "run_a",
67+
snapshotFriendlyId: "snapshot_1",
68+
environmentId: "env_a",
69+
runnerId: "runner-a",
70+
outcome: failed,
71+
},
72+
]);
73+
});
74+
5375
it("names the snapshot dequeued last when the restore is redelivered", async () => {
5476
const { watcher, outcomes, awaitRestore, reportFailure } = setup();
5577

‎apps/supervisor/src/services/restoreWatcher.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ import {
1111
type RestoreWatchTarget = {
1212
runFriendlyId: string;
1313
snapshotFriendlyId: string;
14+
environmentId?: string;
1415
};
1516

1617
/** A failed restore, with what the watch or the Runner knew of its run. */
1718
export type RestoreFailure = {
1819
runFriendlyId?: string;
1920
snapshotFriendlyId?: string;
21+
environmentId?: string;
2022
runnerId: string;
2123
outcome: RestoreWatchResult & { ok: false };
2224
};
@@ -136,6 +138,7 @@ export class RestoreWatcher {
136138
await this.report({
137139
runFriendlyId,
138140
snapshotFriendlyId: entry.snapshotFriendlyId,
141+
environmentId: entry.environmentId,
139142
runnerId,
140143
outcome,
141144
});

‎apps/supervisor/src/services/runnerKills.test.ts‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,13 @@ import type { AddressInfo } from "node:net";
1111
import { Registry } from "prom-client";
1212
import { afterEach, describe, expect, it, vi } from "vitest";
1313
import type { K8sApi } from "../clients/kubernetes.js";
14-
import { RunnerKillInformer, runnerKill, SUSPEND_ANNOTATION } from "../workloadManager/runCrd.js";
14+
import {
15+
RESTORE_LABEL,
16+
RunnerKillInformer,
17+
runnerKill,
18+
SUSPEND_ANNOTATION,
19+
type RunnerStartFailure,
20+
} from "../workloadManager/runCrd.js";
1521
import { FailedPodHandler } from "./failedPodHandler.js";
1622
import { podKill, RunnerKillReporter, type RunnerKill } from "./runnerKills.js";
1723

@@ -497,3 +503,66 @@ describe("RunnerKillInformer", () => {
497503
]);
498504
});
499505
});
506+
507+
describe("RunnerKillInformer start failures", () => {
508+
function templateMissing(name: string, labels?: Record<string, string>) {
509+
const runner = failedRunner(
510+
name,
511+
"TemplateNotFound",
512+
"template abc is not available on node node-a: not in the store",
513+
"run_a"
514+
);
515+
return {
516+
...runner,
517+
metadata: { ...runner.metadata, ...(labels ? { labels } : {}) },
518+
spec: { ...runner.spec, bootstrap: { runFriendlyID: "run_a", snapshotFriendlyID: "snap_a" } },
519+
} as KubernetesObject;
520+
}
521+
522+
it("passes on a cold start whose template is missing once, and not as a kill", async () => {
523+
const cold = templateMissing("runner-cold");
524+
const { k8s, watchCallbacks } = fakeK8s([
525+
cold,
526+
templateMissing("runner-resume", { [RESTORE_LABEL]: "true" }),
527+
]);
528+
const killed: string[] = [];
529+
const failed: RunnerStartFailure[] = [];
530+
const informer = new RunnerKillInformer({
531+
namespace: "runs",
532+
k8s,
533+
onRunnerKilled: (runnerId) => killed.push(runnerId),
534+
onRunnerStartFailed: (failure) => failed.push(failure),
535+
});
536+
537+
await informer.start();
538+
await vi.waitFor(() => expect(failed).toHaveLength(1));
539+
await vi.waitFor(() => expect(watchCallbacks).toHaveLength(1));
540+
watchCallbacks[0]!("MODIFIED", {
541+
...cold,
542+
metadata: { ...cold.metadata, resourceVersion: "2" },
543+
});
544+
await informer.stop();
545+
546+
expect(killed).toEqual([]);
547+
expect(
548+
failed.map((f) => [f.runnerId, f.runFriendlyId, f.snapshotFriendlyId, f.outcome.uid])
549+
).toEqual([["runner-cold", "run_a", "snap_a", "runner-cold-uid"]]);
550+
});
551+
552+
it("passes on nothing for it without a start failure handler", async () => {
553+
const { k8s, watchCallbacks } = fakeK8s([templateMissing("runner-cold")]);
554+
const killed: string[] = [];
555+
const informer = new RunnerKillInformer({
556+
namespace: "runs",
557+
k8s,
558+
onRunnerKilled: (runnerId) => killed.push(runnerId),
559+
});
560+
561+
await informer.start();
562+
// The watch starts once the list is handled.
563+
await vi.waitFor(() => expect(watchCallbacks).toHaveLength(1));
564+
await informer.stop();
565+
566+
expect(killed).toEqual([]);
567+
});
568+
});

0 commit comments

Comments
 (0)