feat: improve resource reconciliation and observability
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { afterEach, describe, expect, spyOn, test } from "bun:test";
|
||||
import { createHash } from "node:crypto";
|
||||
import { lstat, mkdtemp, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
@@ -12,7 +12,11 @@ import {
|
||||
} from "../../server/build-controller";
|
||||
import { createApp } from "../../server/app";
|
||||
import { hashToken, MemoryAuthStore } from "../../server/auth";
|
||||
import { MemoryBuildStore, type BuildStore } from "../../server/build-store";
|
||||
import {
|
||||
MemoryBuildStore,
|
||||
type BuildRecord,
|
||||
type BuildStore,
|
||||
} from "../../server/build-store";
|
||||
import { FilesystemCas } from "../../server/cas";
|
||||
import type { KubernetesJob } from "../../server/build-job";
|
||||
import {
|
||||
@@ -78,6 +82,30 @@ class SupersededBeforeJobStore extends MemoryBuildStore {
|
||||
}
|
||||
}
|
||||
|
||||
class TakeoverAtFencedWriteStore extends MemoryBuildStore {
|
||||
onTakeover?: () => void;
|
||||
private tookOver = false;
|
||||
|
||||
override async replaceBuild(
|
||||
record: BuildRecord,
|
||||
expectedResourceVersion: string,
|
||||
reconcileLeaseToken?: string,
|
||||
): Promise<void> {
|
||||
if (reconcileLeaseToken && !this.tookOver) {
|
||||
this.tookOver = true;
|
||||
const current = (await this.getBuild(record.metadata.name))!;
|
||||
current.status.reconcileLease!.expiresAt = new Date(0).toISOString();
|
||||
await super.replaceBuild(current, current.metadata.resourceVersion);
|
||||
this.onTakeover?.();
|
||||
}
|
||||
return super.replaceBuild(
|
||||
record,
|
||||
expectedResourceVersion,
|
||||
reconcileLeaseToken,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function fixture(
|
||||
maxLogBytes = 1024,
|
||||
store: BuildStore = new MemoryBuildStore(),
|
||||
@@ -277,6 +305,14 @@ describe("build controller", () => {
|
||||
).toMatchObject({ state: "queued" });
|
||||
expect(kubernetes.jobs).toHaveLength(1);
|
||||
const jobSpec = kubernetes.jobs[0]!.spec as any;
|
||||
expect(jobSpec.template.spec.tolerations).toEqual([
|
||||
{
|
||||
key: "arch",
|
||||
operator: "Equal",
|
||||
value: "amd64",
|
||||
effect: "NoExecute",
|
||||
},
|
||||
]);
|
||||
expect(jobSpec.template.spec.containers[0].volumeMounts).toContainEqual(
|
||||
expect.objectContaining({
|
||||
name: "workspace",
|
||||
@@ -449,6 +485,437 @@ describe("build controller", () => {
|
||||
).resolves.toMatchObject({ state: "queued" });
|
||||
});
|
||||
|
||||
test("reconciles only active records with created Jobs and isolates failures", async () => {
|
||||
const { controller, request, store } = await fixture();
|
||||
await controller.submitBuild(request);
|
||||
const candidate = (await store.getBuild(request.id))!;
|
||||
const terminal = structuredClone(candidate);
|
||||
terminal.metadata.name = "terminal";
|
||||
terminal.spec.request.id = "terminal";
|
||||
terminal.spec.imageKey = "terminal";
|
||||
terminal.status.state = "succeeded";
|
||||
await store.createBuild(terminal);
|
||||
const noJob = structuredClone(candidate);
|
||||
noJob.metadata.name = "no-job";
|
||||
noJob.spec.request.id = "no-job";
|
||||
noJob.spec.imageKey = "no-job";
|
||||
noJob.status.jobCreated = false;
|
||||
await store.createBuild(noJob);
|
||||
const secondCandidate = structuredClone(candidate);
|
||||
secondCandidate.metadata.name = "a-second-candidate";
|
||||
secondCandidate.spec.request.id = "a-second-candidate";
|
||||
secondCandidate.spec.imageKey = "a-second-candidate";
|
||||
await store.createBuild(secondCandidate);
|
||||
const reconciled: string[] = [];
|
||||
spyOn(controller, "reconcileBuild").mockImplementation(async (id) => {
|
||||
reconciled.push(id);
|
||||
if (id === "a-second-candidate") throw new Error("temporary failure");
|
||||
return controller.getBuildStatus(id);
|
||||
});
|
||||
|
||||
await controller.reconcilePendingBuilds();
|
||||
|
||||
expect(reconciled).toEqual(["a-second-candidate", request.id]);
|
||||
});
|
||||
|
||||
test("shares one reconciliation between an authenticated request and background scan", async () => {
|
||||
const { controller, kubernetes, request } = await fixture();
|
||||
await controller.submitBuild(request);
|
||||
kubernetes.logs = "build output\n";
|
||||
let startLogRead!: () => void;
|
||||
let releaseLogRead!: () => void;
|
||||
const logReadStarted = new Promise<void>((resolve) => {
|
||||
startLogRead = resolve;
|
||||
});
|
||||
const logReadReleased = new Promise<void>((resolve) => {
|
||||
releaseLogRead = resolve;
|
||||
});
|
||||
const getJobLogs = spyOn(kubernetes, "getJobLogs").mockImplementation(
|
||||
async () => {
|
||||
startLogRead();
|
||||
await logReadReleased;
|
||||
return kubernetes.logs;
|
||||
},
|
||||
);
|
||||
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 app = createApp({ store: auth, builds: controller });
|
||||
|
||||
const requestReconcile = app(
|
||||
new Request(`https://kuber.test/api/v2/builds/${request.id}/reconcile`, {
|
||||
method: "POST",
|
||||
headers: { authorization: "Bearer token" },
|
||||
}),
|
||||
);
|
||||
await logReadStarted;
|
||||
const backgroundReconcile = controller.reconcilePendingBuilds();
|
||||
releaseLogRead();
|
||||
|
||||
expect((await requestReconcile).status).toBe(200);
|
||||
await backgroundReconcile;
|
||||
expect(getJobLogs).toHaveBeenCalledTimes(1);
|
||||
expect(
|
||||
(await controller.getBuildEvents(request.id)).filter(
|
||||
(event) => event.type === "log",
|
||||
),
|
||||
).toHaveLength(1);
|
||||
});
|
||||
|
||||
test("does not overlap a hung aborted scan and resumes after it settles", async () => {
|
||||
const { cas, kubernetes, request, root, store } = await fixture();
|
||||
const submittingController = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
});
|
||||
await submittingController.submitBuild(request);
|
||||
let acquired!: () => void;
|
||||
const leaseAcquired = new Promise<void>((resolve) => {
|
||||
acquired = resolve;
|
||||
});
|
||||
let firstAcquire = true;
|
||||
let releaseFirstAcquire!: () => void;
|
||||
const firstAcquireReleased = new Promise<void>((resolve) => {
|
||||
releaseFirstAcquire = resolve;
|
||||
});
|
||||
const lease = {
|
||||
workspaceId: "",
|
||||
holder: "",
|
||||
expiresAt: "",
|
||||
renew: async () => true,
|
||||
release: async () => {},
|
||||
};
|
||||
const controller = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
reconcileLeases: {
|
||||
acquire: async () => {
|
||||
if (!firstAcquire) return lease;
|
||||
firstAcquire = false;
|
||||
acquired();
|
||||
await firstAcquireReleased;
|
||||
},
|
||||
},
|
||||
});
|
||||
const aborted = new AbortController();
|
||||
const first = controller.reconcilePendingBuilds({ signal: aborted.signal });
|
||||
await leaseAcquired;
|
||||
aborted.abort();
|
||||
|
||||
const getJob = spyOn(kubernetes, "getJob");
|
||||
const second = controller.reconcilePendingBuilds();
|
||||
await Promise.resolve();
|
||||
expect(getJob).not.toHaveBeenCalled();
|
||||
|
||||
releaseFirstAcquire();
|
||||
await expect(first).rejects.toThrow(
|
||||
"Build reconciliation scan was cancelled",
|
||||
);
|
||||
await second;
|
||||
await controller.reconcilePendingBuilds();
|
||||
expect(getJob).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test("skips reconciliation while another replica holds the shared lease", async () => {
|
||||
const { cas, kubernetes, request, root, store } = await fixture();
|
||||
const submittingController = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
});
|
||||
await submittingController.submitBuild(request);
|
||||
kubernetes.logs = "replica must not read this\n";
|
||||
const getJobLogs = spyOn(kubernetes, "getJobLogs");
|
||||
const controller = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
reconcileLeases: { acquire: async () => undefined },
|
||||
});
|
||||
|
||||
expect(await controller.reconcileBuild(request.id)).toMatchObject({
|
||||
state: "queued",
|
||||
});
|
||||
expect(getJobLogs).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("heartbeats a hung observation so its per-build lease cannot be taken over", async () => {
|
||||
const { cas, kubernetes, request, root, store } = await fixture();
|
||||
await new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
}).submitBuild(request);
|
||||
let releaseObservation!: () => void;
|
||||
let observationStarted!: () => void;
|
||||
const observationPending = new Promise<void>((resolve) => {
|
||||
releaseObservation = resolve;
|
||||
});
|
||||
const observationStartedPromise = new Promise<void>((resolve) => {
|
||||
observationStarted = resolve;
|
||||
});
|
||||
kubernetes.getJob = async () => {
|
||||
observationStarted();
|
||||
await observationPending;
|
||||
return { phase: "queued" };
|
||||
};
|
||||
const callbacks: Array<() => void> = [];
|
||||
const now = spyOn(Date, "now").mockReturnValue(0);
|
||||
const controller = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
reconcileLeases: {
|
||||
acquire: async () => ({
|
||||
workspaceId: `build-reconcile:${request.id}`,
|
||||
holder: "replica-one",
|
||||
expiresAt: "",
|
||||
renew: async () => true,
|
||||
release: async () => {},
|
||||
}),
|
||||
},
|
||||
reconcileSetTimeout: ((callback: () => void) => {
|
||||
callbacks.push(callback);
|
||||
return 0 as unknown as ReturnType<typeof setTimeout>;
|
||||
}) as typeof setTimeout,
|
||||
reconcileClearTimeout: (() => {}) as typeof clearTimeout,
|
||||
});
|
||||
|
||||
const reconciliation = controller.reconcileBuild(request.id);
|
||||
await observationStartedPromise;
|
||||
now.mockReturnValue(10_000);
|
||||
callbacks.shift()!();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
now.mockReturnValue(35_000);
|
||||
|
||||
expect(
|
||||
await store.acquireBuildReconciliationLease(
|
||||
request.id,
|
||||
"replica-two",
|
||||
30_000,
|
||||
),
|
||||
).toBeUndefined();
|
||||
expect(
|
||||
(await store.getBuild(request.id))?.status.reconcileLease?.expiresAt,
|
||||
).toBe(new Date(40_000).toISOString());
|
||||
|
||||
releaseObservation();
|
||||
await reconciliation;
|
||||
now.mockRestore();
|
||||
});
|
||||
|
||||
test("heartbeat loss fences persistence after a hung log read", async () => {
|
||||
const { cas, kubernetes, request, root, store } = await fixture();
|
||||
await new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
}).submitBuild(request);
|
||||
let releaseLogs!: () => void;
|
||||
let logsStarted!: () => void;
|
||||
const logsPending = new Promise<void>((resolve) => {
|
||||
releaseLogs = resolve;
|
||||
});
|
||||
const logsStartedPromise = new Promise<void>((resolve) => {
|
||||
logsStarted = resolve;
|
||||
});
|
||||
kubernetes.getJobLogs = async () => {
|
||||
logsStarted();
|
||||
await logsPending;
|
||||
return "stale output\n";
|
||||
};
|
||||
let renewals = 0;
|
||||
const callbacks: Array<() => void> = [];
|
||||
const controller = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
reconcileLeases: {
|
||||
acquire: async () => ({
|
||||
workspaceId: `build-reconcile:${request.id}`,
|
||||
holder: "replica-one",
|
||||
expiresAt: "",
|
||||
renew: async () => ++renewals === 1,
|
||||
release: async () => {},
|
||||
}),
|
||||
},
|
||||
reconcileSetTimeout: ((callback: () => void) => {
|
||||
callbacks.push(callback);
|
||||
return 0 as unknown as ReturnType<typeof setTimeout>;
|
||||
}) as typeof setTimeout,
|
||||
reconcileClearTimeout: (() => {}) as typeof clearTimeout,
|
||||
});
|
||||
|
||||
const reconciliation = controller.reconcileBuild(request.id);
|
||||
await logsStartedPromise;
|
||||
callbacks.shift()!();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
releaseLogs();
|
||||
|
||||
await expect(reconciliation).rejects.toThrow(
|
||||
"Build reconciliation lease ownership was lost",
|
||||
);
|
||||
expect((await store.getBuild(request.id))?.status).toMatchObject({
|
||||
logOffset: 0,
|
||||
logBytes: 0,
|
||||
events: [{ type: "status" }],
|
||||
});
|
||||
});
|
||||
|
||||
test("does not persist logs after reconciliation lease ownership is lost", async () => {
|
||||
const { cas, kubernetes, request, root, store } = await fixture();
|
||||
const submittingController = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
});
|
||||
await submittingController.submitBuild(request);
|
||||
kubernetes.logs = "unpersisted output\n";
|
||||
let renewals = 0;
|
||||
let released = false;
|
||||
const controller = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
reconcileLeases: {
|
||||
acquire: async () => ({
|
||||
workspaceId: `build-reconcile:${request.id}`,
|
||||
holder: "replica-one",
|
||||
expiresAt: "2030-01-01T00:00:00.000Z",
|
||||
renew: async () => ++renewals === 1,
|
||||
release: async () => {
|
||||
released = true;
|
||||
},
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
await expect(controller.reconcileBuild(request.id)).rejects.toThrow(
|
||||
"Build reconciliation lease ownership was lost",
|
||||
);
|
||||
expect(released).toBe(true);
|
||||
expect((await store.getBuild(request.id))?.status).toMatchObject({
|
||||
logOffset: 0,
|
||||
logBytes: 0,
|
||||
events: [{ type: "status" }],
|
||||
});
|
||||
});
|
||||
|
||||
test("fences a stale reconciler when its lease is taken over before persistence", async () => {
|
||||
const store = new TakeoverAtFencedWriteStore();
|
||||
const { cas, kubernetes, request, root } = await fixture(1024, store);
|
||||
const submitter = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
});
|
||||
await submitter.submitBuild(request);
|
||||
kubernetes.logs = "stale output\n";
|
||||
kubernetes.observation = { phase: "succeeded" };
|
||||
const stale = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
reconcileHolder: "stale-replica",
|
||||
resolveDigest: async () => `sha256:${"a".repeat(64)}`,
|
||||
});
|
||||
const current = new BuildController({
|
||||
cas,
|
||||
store,
|
||||
kubernetes,
|
||||
namespace: "builds",
|
||||
workspaceRoot: join(root, "workspaces"),
|
||||
workspaceClaimName: "workspaces",
|
||||
cacheImage: "registry.test/cache/app",
|
||||
reconcileHolder: "current-replica",
|
||||
resolveDigest: async () => `sha256:${"b".repeat(64)}`,
|
||||
});
|
||||
let currentReconciliation: Promise<unknown> | undefined;
|
||||
store.onTakeover = () => {
|
||||
kubernetes.logs = "current output\n";
|
||||
currentReconciliation = current.reconcileBuild(request.id);
|
||||
};
|
||||
|
||||
await expect(stale.reconcileBuild(request.id)).rejects.toThrow(
|
||||
"Build reconciliation lease ownership was lost",
|
||||
);
|
||||
await currentReconciliation;
|
||||
|
||||
const record = (await store.getBuild(request.id))!;
|
||||
expect(record.status).toMatchObject({
|
||||
state: "succeeded",
|
||||
digest: `sha256:${"b".repeat(64)}`,
|
||||
logOffset: Buffer.byteLength("current output\n"),
|
||||
});
|
||||
expect(
|
||||
record.status.events.filter((event) => event.type === "log"),
|
||||
).toEqual([
|
||||
expect.objectContaining({ message: "current output\n", sequence: 1 }),
|
||||
]);
|
||||
expect(await store.ownsBuild(record.spec.imageKey, request.id)).toBe(false);
|
||||
});
|
||||
|
||||
test("cancels idempotently and cleans up only terminal build resources", async () => {
|
||||
const { controller, kubernetes, request, root } = await fixture();
|
||||
await controller.submitBuild(request);
|
||||
|
||||
Reference in New Issue
Block a user