482f64e343
## Thinking Path Production e2e on the merged #5790 plugin failed on every fresh lease with "Failed to install the adapter runtime command" for a harness that was present in the runtime image. Tracing the lease showed the first exec resolved no pod: the exact-label fallback added during the #5790 review queries `agents.x-k8s.io/sandbox-name=<name>`, but the kubernetes-sigs agent-sandbox controller labels pods only with `agents.x-k8s.io/sandbox-name-hash` (see `sandboxLabel` in its `controllers/sandbox_controller.go`) and NAMES the backing pod exactly after the Sandbox CR. The selector matches nothing, `findPodForSandbox` returns null, execute returns "podName could not be resolved", and adapter-utils misreports it as a missing runtime command. ## What Changed Between the `status.podName` read and the label fallback, try an exact-name pod GET (`readNamespacedPod({namespace, name})`). This is collision-free, so the original review concern (name-prefix matching execing into a concurrent sandbox's pod) stays honored. A 404 falls through to the existing full-name label selector for controller versions that do set such a label. Non-404 errors propagate unchanged. ## Verification - New unit test pins the controller reality: pod named exactly like the sandbox, only a `sandbox-name-hash` label, no full-name label; fails before the fix, passes after. - Review-feedback round: the primary-path test now asserts the exact-name GET is never called, and a new test covers non-404 error propagation (403 rejects, no fallback). 153/153 plugin tests green, tsc clean. - Production-verified on our deployment: agent runs were broken on every fresh lease before this patch and complete end-to-end after it (gVisor sandbox pool, agent-sandbox controller v0.4.6; verified run with cost event and agent reply on a fresh tenant). ## Risks Low: one additional pod GET per first-exec on a fresh lease, only when `status.podName` is unset. Non-404 errors from the GET propagate unchanged (now test-pinned). ## Issue No existing issue; the defect is described in full under Thinking Path (introduced by the review-round fallback change in #5790, first hit in production e2e on 2026-06-11). ## Model Used Claude Fable 5 (claude-fable-5, Claude Code CLI, extended reasoning, tool use) ## Duplicate search Searched open and closed PRs for `findPodForSandbox`, `sandbox-name-hash`, and pod-resolution fixes; no duplicate found. Related parent: #5790 (introduced the fallback this PR repairs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots (no UI change) - [x] I have updated relevant documentation to reflect my changes (code comments; no doc surface affected) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code)
271 lines
10 KiB
TypeScript
271 lines
10 KiB
TypeScript
import { describe, it, expect, vi } from "vitest";
|
|
import {
|
|
createSandboxCr,
|
|
deleteSandboxCr,
|
|
getSandboxCrStatus,
|
|
findPodForSandbox,
|
|
SandboxCrTimeoutError,
|
|
waitForSandboxReady,
|
|
} from "../../src/sandbox-cr-orchestrator.js";
|
|
|
|
const SANDBOX_GROUP = "agents.x-k8s.io";
|
|
const SANDBOX_VERSION = "v1alpha1";
|
|
const SANDBOX_PLURAL = "sandboxes";
|
|
|
|
// Helpers to build mock CR objects with given phase
|
|
function makeCr(phase: string, podName?: string): Record<string, unknown> {
|
|
return {
|
|
metadata: { uid: "sandbox-uid-123" },
|
|
status: {
|
|
phase,
|
|
...(podName ? { podName } : {}),
|
|
},
|
|
};
|
|
}
|
|
|
|
describe("createSandboxCr", () => {
|
|
it("calls custom.createNamespacedCustomObject with the correct params", async () => {
|
|
const create = vi.fn().mockResolvedValue({ metadata: { uid: "test-uid" } });
|
|
const clients = { custom: { createNamespacedCustomObject: create } };
|
|
const manifest = {
|
|
apiVersion: "agents.x-k8s.io/v1alpha1",
|
|
kind: "Sandbox",
|
|
metadata: { name: "pc-abc", namespace: "paperclip-acme" },
|
|
};
|
|
const result = await createSandboxCr(clients as never, "paperclip-acme", manifest);
|
|
expect(create).toHaveBeenCalledWith({
|
|
group: SANDBOX_GROUP,
|
|
version: SANDBOX_VERSION,
|
|
namespace: "paperclip-acme",
|
|
plural: SANDBOX_PLURAL,
|
|
body: manifest,
|
|
});
|
|
expect(result.uid).toBe("test-uid");
|
|
});
|
|
|
|
it("throws if the API response has no UID", async () => {
|
|
const create = vi.fn().mockResolvedValue({ metadata: {} });
|
|
const clients = { custom: { createNamespacedCustomObject: create } };
|
|
await expect(
|
|
createSandboxCr(clients as never, "ns", {}),
|
|
).rejects.toThrow("Sandbox CR created without a UID");
|
|
});
|
|
});
|
|
|
|
describe("getSandboxCrStatus", () => {
|
|
it("maps phase=Ready to SandboxStatus.phase=Running with active=1", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Ready"));
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
const status = await getSandboxCrStatus(clients as never, "ns", "pc-abc");
|
|
expect(status.phase).toBe("Running");
|
|
expect(status.active).toBe(1);
|
|
expect(status.complete).toBe(false);
|
|
});
|
|
|
|
it("maps phase=Pending to SandboxStatus.phase=Pending", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Pending"));
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
const status = await getSandboxCrStatus(clients as never, "ns", "pc-abc");
|
|
expect(status.phase).toBe("Pending");
|
|
expect(status.active).toBe(0);
|
|
});
|
|
|
|
it("maps phase=Failed to SandboxStatus.phase=Failed with failed=1", async () => {
|
|
const get = vi.fn().mockResolvedValue({
|
|
metadata: { uid: "uid-1" },
|
|
status: {
|
|
phase: "Failed",
|
|
conditions: [
|
|
{ type: "Failed", reason: "ImagePullFailed", message: "no image" },
|
|
],
|
|
},
|
|
});
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
const status = await getSandboxCrStatus(clients as never, "ns", "pc-abc");
|
|
expect(status.phase).toBe("Failed");
|
|
expect(status.failed).toBe(1);
|
|
expect(status.reason).toBe("ImagePullFailed");
|
|
});
|
|
|
|
it("maps phase=Terminating to SandboxStatus.phase=Running with reason=Terminating", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Terminating"));
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
const status = await getSandboxCrStatus(clients as never, "ns", "pc-abc");
|
|
expect(status.phase).toBe("Running");
|
|
expect(status.reason).toBe("Terminating");
|
|
});
|
|
});
|
|
|
|
describe("findPodForSandbox", () => {
|
|
it("returns status.podName from the Sandbox CR when set", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Ready", "pc-abc-pod-xyz"));
|
|
const clients = {
|
|
custom: { getNamespacedCustomObject: get },
|
|
core: { readNamespacedPod: vi.fn(), listNamespacedPod: vi.fn() },
|
|
};
|
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
|
expect(podName).toBe("pc-abc-pod-xyz");
|
|
// Primary path succeeded: neither the exact-name GET nor the label list runs.
|
|
expect(clients.core.readNamespacedPod).not.toHaveBeenCalled();
|
|
expect(clients.core.listNamespacedPod).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("resolves the pod by EXACT NAME when the controller names it after the sandbox (v0.4.x: pods carry only agents.x-k8s.io/sandbox-name-hash, never the full-name label)", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Ready")); // no podName in status
|
|
const read = vi.fn().mockResolvedValue({
|
|
metadata: { name: "pc-abc", labels: { "agents.x-k8s.io/sandbox-name-hash": "1a2b3c" } },
|
|
status: { phase: "Running" },
|
|
});
|
|
const list = vi.fn().mockResolvedValue({ items: [] }); // full-name label selector matches nothing on v0.4.x
|
|
const clients = {
|
|
custom: { getNamespacedCustomObject: get },
|
|
core: { readNamespacedPod: read, listNamespacedPod: list },
|
|
};
|
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
|
expect(read).toHaveBeenCalledWith({ namespace: "ns", name: "pc-abc" });
|
|
expect(podName).toBe("pc-abc");
|
|
expect(list).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("falls back to pod listing scoped by the unique sandbox-name label", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Pending")); // no podName
|
|
const read = vi.fn().mockRejectedValue({ code: 404 }); // no exact-name pod
|
|
const list = vi.fn().mockResolvedValue({
|
|
items: [
|
|
{
|
|
metadata: { name: "pc-abc-001", labels: { "agents.x-k8s.io/sandbox-name": "pc-abc" } },
|
|
status: { phase: "Running" },
|
|
},
|
|
],
|
|
});
|
|
const clients = {
|
|
custom: { getNamespacedCustomObject: get },
|
|
core: { readNamespacedPod: read, listNamespacedPod: list },
|
|
};
|
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
|
expect(list).toHaveBeenCalledWith(
|
|
expect.objectContaining({ labelSelector: "agents.x-k8s.io/sandbox-name=pc-abc" }),
|
|
);
|
|
expect(podName).toBe("pc-abc-001");
|
|
});
|
|
|
|
it("never matches another sandbox's pod by name prefix", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Pending"));
|
|
const list = vi.fn().mockResolvedValue({
|
|
items: [
|
|
{
|
|
// Same name prefix, different sandbox label: must NOT match.
|
|
metadata: { name: "pc-abc-zzz", labels: { "agents.x-k8s.io/sandbox-name": "pc-abc-zzz" } },
|
|
status: { phase: "Running" },
|
|
},
|
|
],
|
|
});
|
|
const clients = {
|
|
custom: { getNamespacedCustomObject: get },
|
|
core: { readNamespacedPod: vi.fn().mockRejectedValue({ code: 404 }), listNamespacedPod: list },
|
|
};
|
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
|
expect(podName).toBeNull();
|
|
});
|
|
|
|
it("propagates non-404 errors from the exact-name pod GET instead of falling through", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Pending"));
|
|
const read = vi.fn().mockRejectedValue({ code: 403, message: "forbidden" });
|
|
const list = vi.fn();
|
|
const clients = {
|
|
custom: { getNamespacedCustomObject: get },
|
|
core: { readNamespacedPod: read, listNamespacedPod: list },
|
|
};
|
|
await expect(findPodForSandbox(clients as never, "ns", "pc-abc")).rejects.toMatchObject({
|
|
code: 403,
|
|
});
|
|
expect(list).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("returns null when no pod is found in fallback", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Pending"));
|
|
const list = vi.fn().mockResolvedValue({ items: [] });
|
|
const clients = {
|
|
custom: { getNamespacedCustomObject: get },
|
|
core: { readNamespacedPod: vi.fn().mockRejectedValue({ code: 404 }), listNamespacedPod: list },
|
|
};
|
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
|
expect(podName).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe("deleteSandboxCr", () => {
|
|
it("calls custom.deleteNamespacedCustomObject with Foreground propagation", async () => {
|
|
const del = vi.fn().mockResolvedValue({});
|
|
const clients = { custom: { deleteNamespacedCustomObject: del } };
|
|
await deleteSandboxCr(clients as never, "ns", "pc-abc");
|
|
expect(del).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
group: SANDBOX_GROUP,
|
|
version: SANDBOX_VERSION,
|
|
namespace: "ns",
|
|
plural: SANDBOX_PLURAL,
|
|
name: "pc-abc",
|
|
propagationPolicy: "Foreground",
|
|
}),
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("waitForSandboxReady", () => {
|
|
it("resolves immediately when Sandbox is already Ready", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Ready"));
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
const status = await waitForSandboxReady(
|
|
clients as never,
|
|
"ns",
|
|
"pc-abc",
|
|
{ timeoutMs: 5000, pollMs: 10 },
|
|
);
|
|
expect(status.phase).toBe("Running"); // Ready maps to Running
|
|
expect(get).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("polls until Ready", async () => {
|
|
const get = vi
|
|
.fn()
|
|
.mockResolvedValueOnce(makeCr("Pending"))
|
|
.mockResolvedValueOnce(makeCr("Pending"))
|
|
.mockResolvedValueOnce(makeCr("Ready"));
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
const status = await waitForSandboxReady(
|
|
clients as never,
|
|
"ns",
|
|
"pc-abc",
|
|
{ timeoutMs: 5000, pollMs: 10 },
|
|
);
|
|
expect(status.phase).toBe("Running");
|
|
expect(get).toHaveBeenCalledTimes(3);
|
|
});
|
|
|
|
it("throws SandboxCrTimeoutError when deadline is exceeded", async () => {
|
|
const get = vi.fn().mockResolvedValue(makeCr("Pending"));
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
await expect(
|
|
waitForSandboxReady(clients as never, "ns", "pc-abc", {
|
|
timeoutMs: 50,
|
|
pollMs: 10,
|
|
}),
|
|
).rejects.toBeInstanceOf(SandboxCrTimeoutError);
|
|
});
|
|
|
|
it("throws an error describing the failure when Sandbox fails", async () => {
|
|
const get = vi.fn().mockResolvedValue({
|
|
metadata: { uid: "u1" },
|
|
status: { phase: "Failed", conditions: [{ type: "Failed", reason: "OOMKilled" }] },
|
|
});
|
|
const clients = { custom: { getNamespacedCustomObject: get } };
|
|
await expect(
|
|
waitForSandboxReady(clients as never, "ns", "pc-abc", {
|
|
timeoutMs: 5000,
|
|
pollMs: 10,
|
|
}),
|
|
).rejects.toThrow(/failed.*OOMKilled/i);
|
|
});
|
|
});
|