Skip to content

Commit 31d39da

Browse files
myftijaTrigger.dev RepoOps
authored andcommitted
feat(supervisor): release a run whose Runner create the API refused
With `RELEASE_UNSTARTED_RUNS_ENABLED=true`, the supervisor now also gives back a run whose Runner the cluster refused to create, when the run-crd workload manager is on, so the run is requeued at once instead of waiting out the pending execution timeout. Off by default, nothing changes. Mono-RevId: ecf6db0d6700593ba3d2b644f74ae8c19c15c37d
1 parent c370fee commit 31d39da

4 files changed

Lines changed: 145 additions & 9 deletions

File tree

‎apps/supervisor/src/env.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,8 @@ export const Env = z
363363
// Report a run whose runner the runtime killed, so it fails as out of memory only
364364
// when the runtime says so. Without it the run waits out the heartbeat timeout.
365365
RUNNER_KILL_REPORTS_ENABLED: BoolEnv.default(true),
366-
// Release a run whose container or pod could not be created, so it is requeued at once instead
367-
// of waiting out the pending execution timeout.
366+
// Release a run whose container, pod or Runner could not be created, so it is requeued at once
367+
// instead of waiting out the pending execution timeout.
368368
RELEASE_UNSTARTED_RUNS_ENABLED: BoolEnv.default(false),
369369
// Release a run whose pod failed before the run started, likewise. Independent of the flag above.
370370
RELEASE_FAILED_POD_RUNS_ENABLED: BoolEnv.default(false),

‎apps/supervisor/src/workloadManager/createErrors.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ function socketError(code: string, message: string) {
2828
}
2929

3030
describe("k8sCreateRefused", () => {
31-
it.each([401, 403, 422])("is true for a %i", (code) => {
31+
it.each([400, 401, 403, 404, 422])("is true for a %i", (code) => {
3232
expect(k8sCreateRefused(apiError(code, "refused"))).toBe(true);
3333
});
3434

35-
it.each([408, 409, 500, 504])("is false for a %i, after which a pod may exist", (code) => {
35+
it.each([408, 409, 500, 504])("is false for a %i, after which the object may exist", (code) => {
3636
expect(k8sCreateRefused(apiError(code, "maybe created"))).toBe(false);
3737
});
3838

‎apps/supervisor/src/workloadManager/runCrd.test.ts‎

Lines changed: 131 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
type Watch,
3131
} from "@kubernetes/client-node";
3232
import type { K8sApi } from "../clients/kubernetes.js";
33+
import { WorkloadNotCreatedError } from "./createErrors.js";
3334
import { getRestoreRunnerId, getRunnerId } from "../util.js";
3435
import type { WorkloadManagerCreateOptions } from "./types.js";
3536

@@ -574,12 +575,13 @@ describe("run-crd carries every shared create-option or excludes it on purpose",
574575
});
575576

576577
describe("RunCrdWorkloadManager.create", () => {
577-
const manager = () =>
578+
const manager = (releaseUnstartedRuns?: boolean) =>
578579
new RunCrdWorkloadManager({
579580
workloadApiProtocol: "http",
580581
workloadApiPort: 8020,
581582
namespace: "v4-runs",
582583
runtime: "microvm",
584+
releaseUnstartedRuns,
583585
});
584586

585587
beforeEach(() => {
@@ -588,6 +590,12 @@ describe("RunCrdWorkloadManager.create", () => {
588590
getRunner.mockReset();
589591
deleteRunner.mockReset();
590592
deleteRunner.mockResolvedValue({});
593+
createSecret.mockReset();
594+
createSecret.mockResolvedValue({ metadata: { uid: "uid-secret" } });
595+
deleteSecret.mockReset();
596+
deleteSecret.mockResolvedValue({});
597+
patchObject.mockReset();
598+
patchObject.mockResolvedValue({});
591599
});
592600

593601
// keyedOptions() is dequeued at 03:00:00, so the default is an earlier delivery.
@@ -641,9 +649,6 @@ describe("RunCrdWorkloadManager.create", () => {
641649
.mockRejectedValueOnce({ code: 409 })
642650
.mockResolvedValueOnce({ metadata: { uid: "uid-recreated" } });
643651
getRunner.mockResolvedValue(inTheWay("snapshot_earlier"));
644-
createSecret.mockReset();
645-
createSecret.mockResolvedValue({ metadata: { uid: "uid-secret" } });
646-
patchObject.mockResolvedValue({});
647652

648653
await manager().create(keyedOptions({ deploymentToken: "tok" }));
649654

@@ -706,17 +711,130 @@ describe("RunCrdWorkloadManager.create", () => {
706711
expect(deleteRunner).not.toHaveBeenCalled();
707712
expect(createRunner).toHaveBeenCalledTimes(2);
708713
});
714+
715+
const apiError = (code: number, message = "refused") => ({
716+
code,
717+
body: JSON.stringify({ kind: "Status", code, message }),
718+
});
719+
const withToken = keyedOptions({ deploymentToken: "tok" });
720+
721+
it.each([
722+
["off", false],
723+
["unset", undefined],
724+
])("rethrows a refused Runner create as it is with the option %s", async (_, flag) => {
725+
const failure = apiError(422);
726+
createRunner.mockRejectedValueOnce(failure);
727+
728+
await expect(manager(flag).create(keyedOptions())).rejects.toBe(failure);
729+
expect(createRunner).toHaveBeenCalledTimes(1);
730+
});
731+
732+
it("rethrows a refused token Secret create as it is with the option off", async () => {
733+
const failure = apiError(403);
734+
createSecret.mockRejectedValueOnce(failure);
735+
736+
await expect(manager(false).create(withToken)).rejects.toBe(failure);
737+
expect(createRunner).not.toHaveBeenCalled();
738+
});
739+
740+
describe("with releases on", () => {
741+
it("reports a Runner create the API refused as not created", async () => {
742+
createRunner.mockRejectedValueOnce(apiError(422, "spec.machine: Unsupported value"));
743+
744+
const failed = manager(true).create(keyedOptions());
745+
746+
await expect(failed).rejects.toBeInstanceOf(WorkloadNotCreatedError);
747+
await expect(failed).rejects.toMatchObject({ message: "spec.machine: Unsupported value" });
748+
expect(createRunner).toHaveBeenCalledTimes(1);
749+
expect(getRunner).not.toHaveBeenCalled();
750+
});
751+
752+
it("releases the token Secret of a refused create", async () => {
753+
createRunner.mockRejectedValueOnce(apiError(422));
754+
755+
await expect(manager(true).create(withToken)).rejects.toThrow(WorkloadNotCreatedError);
756+
757+
expect(createSecret).toHaveBeenCalledTimes(1);
758+
expect(deleteSecret).toHaveBeenCalledTimes(1);
759+
});
760+
761+
it("reports a refused token Secret create as not created, with nothing to delete", async () => {
762+
createSecret.mockRejectedValueOnce(apiError(403, "secrets is forbidden"));
763+
764+
const failed = manager(true).create(withToken);
765+
766+
await expect(failed).rejects.toBeInstanceOf(WorkloadNotCreatedError);
767+
await expect(failed).rejects.toMatchObject({ message: "secrets is forbidden" });
768+
expect(createRunner).not.toHaveBeenCalled();
769+
expect(deleteSecret).not.toHaveBeenCalled();
770+
});
771+
772+
it("keeps the token Secret a redrive found, which it did not create", async () => {
773+
createSecret.mockRejectedValueOnce({ code: 409 });
774+
createRunner.mockRejectedValueOnce(apiError(422));
775+
776+
await expect(manager(true).create(withToken)).rejects.toThrow(WorkloadNotCreatedError);
777+
778+
expect(deleteSecret).not.toHaveBeenCalled();
779+
});
780+
781+
it("rethrows a Runner create that failed with 500 as it is", async () => {
782+
const failure = apiError(500);
783+
createRunner.mockRejectedValueOnce(failure);
784+
785+
await expect(manager(true).create(keyedOptions())).rejects.toBe(failure);
786+
expect(createRunner).toHaveBeenCalledTimes(1);
787+
});
788+
789+
it("rethrows a token Secret create that failed with 500 as it is", async () => {
790+
const failure = apiError(500);
791+
createSecret.mockRejectedValueOnce(failure);
792+
793+
await expect(manager(true).create(withToken)).rejects.toBe(failure);
794+
expect(createRunner).not.toHaveBeenCalled();
795+
});
796+
797+
it("reports a refused replacement as not created, with both attempts' token Secrets deleted", async () => {
798+
createRunner.mockRejectedValueOnce({ code: 409 }).mockRejectedValueOnce(apiError(403));
799+
createSecret
800+
.mockResolvedValueOnce({ metadata: { uid: "uid-secret-1" } })
801+
.mockResolvedValueOnce({ metadata: { uid: "uid-secret-2" } });
802+
getRunner.mockResolvedValue(inTheWay("snapshot_earlier"));
803+
804+
await expect(manager(true).create(withToken)).rejects.toThrow(WorkloadNotCreatedError);
805+
806+
expect(deleteRunner).toHaveBeenCalledTimes(1);
807+
expect(createRunner).toHaveBeenCalledTimes(2);
808+
const [first, second] = createSecret.mock.calls.map(([{ body }]) => body.metadata.name);
809+
const deleted = deleteSecret.mock.calls.map(([{ name, body }]) => [
810+
name,
811+
body.preconditions.uid,
812+
]);
813+
expect(deleted).toEqual([
814+
[first, "uid-secret-1"],
815+
[second, "uid-secret-2"],
816+
]);
817+
});
818+
819+
it("does not wrap a template key refusal", async () => {
820+
const failed = manager(true).create(createOptions({ templateKey: undefined }));
821+
822+
await expect(failed).rejects.toThrow("TemplateKeyMissing");
823+
await expect(failed).rejects.not.toBeInstanceOf(WorkloadNotCreatedError);
824+
});
825+
});
709826
});
710827

711828
describe("RunCrdWorkloadManager.restore", () => {
712829
const checkpoint = { id: "checkpoint_abc", location: "node-a/6f1c2a9e-snap" };
713830

714-
function manager(runtime: "container" | "microvm" = "microvm") {
831+
function manager(runtime: "container" | "microvm" = "microvm", releaseUnstartedRuns?: boolean) {
715832
return new RunCrdWorkloadManager({
716833
workloadApiProtocol: "http",
717834
workloadApiPort: 8020,
718835
namespace: "v4-runs",
719836
runtime,
837+
releaseUnstartedRuns,
720838
});
721839
}
722840

@@ -753,6 +871,14 @@ describe("RunCrdWorkloadManager.restore", () => {
753871
expect(body.spec.restore).toEqual({ snapshotID: "6f1c2a9e-snap", node: "node-a" });
754872
});
755873

874+
it("reports a refused resume create as not created, with releases on", async () => {
875+
createRunner.mockRejectedValueOnce({ code: 422 });
876+
877+
await expect(
878+
manager("microvm", true).restore(keyedOptions(), checkpoint)
879+
).rejects.toBeInstanceOf(WorkloadNotCreatedError);
880+
});
881+
756882
it("creates no resume without a template key", async () => {
757883
await expect(manager().restore(createOptions(), checkpoint)).rejects.toThrow(
758884
"TemplateKeyMissing"

‎apps/supervisor/src/workloadManager/runCrd.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type {
2525
WorkloadManagerOptions,
2626
} from "./types.js";
2727
import { killCode, type RunnerKill } from "../services/runnerKills.js";
28+
import { k8sCreateRefused, k8sErrorMessage, WorkloadNotCreatedError } from "./createErrors.js";
2829

2930
const GROUP = "compute.trigger.dev";
3031
const VERSION = "v1alpha1";
@@ -115,11 +116,13 @@ export class RunCrdWorkloadManager implements WorkloadManager, RunnerSnapshotter
115116
private readonly suspendTimeoutMs: number;
116117
private readonly restoreTimeoutMs: number;
117118
private readonly restoreInformer?: RunnerRestoreInformer;
119+
private readonly releaseUnstartedRuns: boolean;
118120

119121
constructor(opts: RunCrdWorkloadManagerOptions) {
120122
this.k8s = createK8sApi();
121123
this.namespace = opts.namespace;
122124
this.runtime = opts.runtime;
125+
this.releaseUnstartedRuns = opts.releaseUnstartedRuns ?? false;
123126
this.snapshots = opts.snapshots;
124127
this.suspendPollMs = opts.suspendPollMs ?? 5_000;
125128
this.suspendTimeoutMs = opts.suspendTimeoutMs ?? 6 * 60_000;
@@ -557,6 +560,10 @@ export class RunCrdWorkloadManager implements WorkloadManager, RunnerSnapshotter
557560
if (statusCodeOf(err) === 409) {
558561
return false;
559562
}
563+
if (this.releaseUnstartedRuns && k8sCreateRefused(err)) {
564+
this.logger.error("[RunCrdWorkloadManager] Create refused", { runnerId, rawError: err });
565+
throw new WorkloadNotCreatedError(k8sErrorMessage(err), { cause: err });
566+
}
560567
this.logger.error("[RunCrdWorkloadManager] Create failed", { runnerId, rawError: err });
561568
throw err;
562569
}
@@ -698,6 +705,9 @@ export class RunCrdWorkloadManager implements WorkloadManager, RunnerSnapshotter
698705
name,
699706
rawError: err,
700707
});
708+
if (this.releaseUnstartedRuns && k8sCreateRefused(err)) {
709+
throw new WorkloadNotCreatedError(k8sErrorMessage(err), { cause: err });
710+
}
701711
throw err;
702712
}
703713
}

0 commit comments

Comments
 (0)