feat: harden self-managed reconciliation
This commit is contained in:
@@ -10,7 +10,9 @@ import {
|
||||
type BuildJobObservation,
|
||||
type BuildKubernetesOperations,
|
||||
} from "../../server/build-controller";
|
||||
import { MemoryBuildStore } from "../../server/build-store";
|
||||
import { createApp } from "../../server/app";
|
||||
import { hashToken, MemoryAuthStore } from "../../server/auth";
|
||||
import { MemoryBuildStore, type BuildStore } from "../../server/build-store";
|
||||
import { FilesystemCas } from "../../server/cas";
|
||||
import type { KubernetesJob } from "../../server/build-job";
|
||||
import {
|
||||
@@ -36,7 +38,10 @@ class FakeKubernetes implements BuildKubernetesOperations {
|
||||
deleted: string[] = [];
|
||||
observation: BuildJobObservation | undefined = { phase: "queued" };
|
||||
logs = "";
|
||||
createError?: Error;
|
||||
preserveAfterDelete = false;
|
||||
async createJob(job: KubernetesJob) {
|
||||
if (this.createError) throw this.createError;
|
||||
this.jobs.push(job);
|
||||
}
|
||||
async getJob() {
|
||||
@@ -47,14 +52,39 @@ class FakeKubernetes implements BuildKubernetesOperations {
|
||||
}
|
||||
async deleteJob(_namespace: string, name: string) {
|
||||
this.deleted.push(name);
|
||||
if (!this.preserveAfterDelete) this.observation = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
async function fixture(maxLogBytes = 1024) {
|
||||
class SupersededBeforeJobStore extends MemoryBuildStore {
|
||||
private superseded = false;
|
||||
|
||||
override async ownsBuild(
|
||||
imageKey: string,
|
||||
buildId: string,
|
||||
): Promise<boolean> {
|
||||
if (!this.superseded) {
|
||||
this.superseded = true;
|
||||
const current = await this.getBuild(buildId);
|
||||
if (current) {
|
||||
const newer = structuredClone(current);
|
||||
newer.metadata.name = "newer-build";
|
||||
newer.metadata.creationTimestamp = "2026-09-02T00:00:01.000Z";
|
||||
newer.spec.request.id = "newer-build";
|
||||
await super.createBuild(newer);
|
||||
}
|
||||
}
|
||||
return super.ownsBuild(imageKey, buildId);
|
||||
}
|
||||
}
|
||||
|
||||
async function fixture(
|
||||
maxLogBytes = 1024,
|
||||
store: BuildStore = new MemoryBuildStore(),
|
||||
) {
|
||||
const root = await mkdtemp(join(tmpdir(), "kuber-controller-"));
|
||||
roots.push(root);
|
||||
const cas = new FilesystemCas(join(root, "cas"));
|
||||
const store = new MemoryBuildStore();
|
||||
const kubernetes = new FakeKubernetes();
|
||||
const source = Buffer.from("FROM scratch\n");
|
||||
const sourceDigest = await cas.put(source);
|
||||
@@ -237,7 +267,7 @@ describe("build controller", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("submits once, blocks competing image builds, captures bounded logs, and resolves immutable results", async () => {
|
||||
test("submits once, supersedes competing image builds, captures bounded logs, and resolves immutable results", async () => {
|
||||
const { controller, kubernetes, request } = await fixture(8);
|
||||
expect(await controller.submitBuild(request)).toMatchObject({
|
||||
state: "queued",
|
||||
@@ -247,18 +277,18 @@ describe("build controller", () => {
|
||||
).toMatchObject({ state: "queued" });
|
||||
expect(kubernetes.jobs).toHaveLength(1);
|
||||
const jobSpec = kubernetes.jobs[0]!.spec as any;
|
||||
expect(
|
||||
jobSpec.template.spec.containers[0].volumeMounts,
|
||||
).toContainEqual(
|
||||
expect(jobSpec.template.spec.containers[0].volumeMounts).toContainEqual(
|
||||
expect.objectContaining({
|
||||
name: "workspace",
|
||||
mountPath: "/workspace",
|
||||
subPath: `workspaces/${kubernetes.jobs[0]!.metadata.name}`,
|
||||
}),
|
||||
);
|
||||
await expect(
|
||||
controller.submitBuild({ ...request, id: "request-two" }),
|
||||
).rejects.toBeInstanceOf(BuildConflictError);
|
||||
const replacement = { ...request, id: "request-two" };
|
||||
expect(await controller.submitBuild(replacement)).toMatchObject({
|
||||
state: "queued",
|
||||
});
|
||||
expect(kubernetes.jobs).toHaveLength(2);
|
||||
await expect(
|
||||
controller.submitBuild({ ...request, project: "changed" }),
|
||||
).rejects.toBeInstanceOf(BuildConflictError);
|
||||
@@ -268,10 +298,10 @@ describe("build controller", () => {
|
||||
phase: "running",
|
||||
startedAt: "2026-09-02T00:00:03.000Z",
|
||||
};
|
||||
expect(await controller.reconcileBuild(request.id)).toMatchObject({
|
||||
expect(await controller.reconcileBuild(replacement.id)).toMatchObject({
|
||||
state: "running",
|
||||
});
|
||||
const logs = (await controller.getBuildEvents(request.id)).filter(
|
||||
const logs = (await controller.getBuildEvents(replacement.id)).filter(
|
||||
(event) => event.type === "log",
|
||||
);
|
||||
expect(
|
||||
@@ -286,17 +316,137 @@ describe("build controller", () => {
|
||||
phase: "succeeded",
|
||||
finishedAt: "2026-09-02T00:00:04.000Z",
|
||||
};
|
||||
const status = await controller.reconcileBuild(request.id);
|
||||
const status = await controller.reconcileBuild(replacement.id);
|
||||
expect(status).toMatchObject({
|
||||
state: "succeeded",
|
||||
digest: `sha256:${"f".repeat(64)}`,
|
||||
});
|
||||
expect(await controller.getBuildResult(request.id)).toEqual({
|
||||
expect(await controller.getBuildResult(replacement.id)).toEqual({
|
||||
image: "registry.test/demo/web",
|
||||
digest: `sha256:${"f".repeat(64)}`,
|
||||
reference: `registry.test/demo/web@sha256:${"f".repeat(64)}`,
|
||||
});
|
||||
expect(await controller.reconcileBuild(request.id)).toEqual(status);
|
||||
expect(await controller.reconcileBuild(replacement.id)).toEqual(status);
|
||||
});
|
||||
|
||||
test("concurrent controllers converge on one same-ID record and Job", async () => {
|
||||
const first = await fixture();
|
||||
const second = new BuildController({
|
||||
cas: first.cas,
|
||||
store: first.store,
|
||||
kubernetes: first.kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(first.root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
});
|
||||
await Promise.all([
|
||||
first.controller.submitBuild(first.request),
|
||||
second.submitBuild(structuredClone(first.request)),
|
||||
]);
|
||||
expect(first.kubernetes.jobs).toHaveLength(1);
|
||||
expect(await first.store.getBuild(first.request.id)).toMatchObject({
|
||||
spec: { request: { id: first.request.id } },
|
||||
});
|
||||
});
|
||||
|
||||
test("starts a replacement without waiting for superseded Job deletion", async () => {
|
||||
const { controller, kubernetes, request, root, store } = await fixture();
|
||||
await controller.submitBuild(request);
|
||||
const oldJobName = kubernetes.jobs[0]!.metadata.name;
|
||||
kubernetes.preserveAfterDelete = true;
|
||||
await expect(
|
||||
controller.submitBuild({ ...request, id: "replacement" }),
|
||||
).resolves.toMatchObject({
|
||||
state: "queued",
|
||||
});
|
||||
expect(kubernetes.deleted).toEqual([oldJobName]);
|
||||
expect(kubernetes.jobs).toHaveLength(2);
|
||||
await expect(
|
||||
lstat(join(root, "workspaces", oldJobName)),
|
||||
).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect((await store.getBuild(request.id))?.status).toMatchObject({
|
||||
state: "failed",
|
||||
error: "Superseded by newer build",
|
||||
cancelled: true,
|
||||
});
|
||||
});
|
||||
|
||||
test("deletes an ambiguous superseded Job even when status was not persisted", async () => {
|
||||
const { controller, kubernetes, request, store } = await fixture();
|
||||
await controller.submitBuild(request);
|
||||
const oldJobName = kubernetes.jobs[0]!.metadata.name;
|
||||
const old = (await store.getBuild(request.id))!;
|
||||
old.status.jobCreated = false;
|
||||
await store.replaceBuild(old, old.metadata.resourceVersion);
|
||||
|
||||
await expect(
|
||||
controller.submitBuild({ ...request, id: "replacement" }),
|
||||
).resolves.toMatchObject({ state: "queued" });
|
||||
expect(kubernetes.deleted).toEqual([oldJobName]);
|
||||
});
|
||||
|
||||
test("does not create a Job when the candidate is superseded before Job creation", async () => {
|
||||
const store = new SupersededBeforeJobStore();
|
||||
const { controller, kubernetes, request } = await fixture(1024, store);
|
||||
|
||||
await expect(controller.submitBuild(request)).rejects.toBeInstanceOf(
|
||||
BuildConflictError,
|
||||
);
|
||||
expect(kubernetes.jobs).toHaveLength(0);
|
||||
});
|
||||
|
||||
test("maps supersession before Job creation to HTTP 409", async () => {
|
||||
const auth = new MemoryAuthStore();
|
||||
await auth.putUser({
|
||||
username: "operator",
|
||||
passwordHash: "hash",
|
||||
roles: ["operator"],
|
||||
});
|
||||
await auth.putSession({
|
||||
tokenHash: hashToken("token"),
|
||||
username: "operator",
|
||||
authVersion: 1,
|
||||
expiresAt: "2030-01-01T00:00:00.000Z",
|
||||
});
|
||||
const store = new SupersededBeforeJobStore();
|
||||
const {
|
||||
controller,
|
||||
kubernetes,
|
||||
request: buildRequest,
|
||||
} = await fixture(1024, store);
|
||||
const app = createApp({ store: auth, builds: controller });
|
||||
|
||||
const response = await app(
|
||||
new Request("https://kuber.test/api/v2/builds", {
|
||||
method: "POST",
|
||||
headers: {
|
||||
authorization: "Bearer token",
|
||||
"content-type": "application/json",
|
||||
},
|
||||
body: JSON.stringify(buildRequest),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(response.status).toBe(409);
|
||||
expect(await response.json()).toMatchObject({ code: "BUILD_CONFLICT" });
|
||||
expect(kubernetes.jobs).toHaveLength(0);
|
||||
});
|
||||
|
||||
test("releases the image lock after an ambiguous Job creation failure", async () => {
|
||||
const { controller, kubernetes, request, store } = await fixture();
|
||||
kubernetes.createError = new Error("create response lost");
|
||||
kubernetes.preserveAfterDelete = true;
|
||||
await expect(controller.submitBuild(request)).rejects.toThrow(
|
||||
"create response lost",
|
||||
);
|
||||
expect((await store.getBuild(request.id))?.status).toMatchObject({
|
||||
state: "failed",
|
||||
});
|
||||
kubernetes.createError = undefined;
|
||||
await expect(
|
||||
controller.submitBuild({ ...request, id: "replacement" }),
|
||||
).resolves.toMatchObject({ state: "queued" });
|
||||
});
|
||||
|
||||
test("cancels idempotently and cleans up only terminal build resources", async () => {
|
||||
|
||||
Reference in New Issue
Block a user