fix(plugin-kubernetes): resolve sandbox pod by exact name (controller labels pods with sandbox-name-hash, not sandbox-name) (#7982)
## 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)
This commit is contained in:
committed by
GitHub
parent
bb7978327e
commit
482f64e343
@@ -152,8 +152,29 @@ export async function findPodForSandbox(
|
|||||||
return podName;
|
return podName;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fallback: list pods by the controller's sandbox-name label, which uniquely
|
// Secondary: the agent-sandbox controller (v0.4.x) names the backing pod
|
||||||
// identifies the pod for THIS sandbox. A broader managed-by selector plus
|
// EXACTLY after the Sandbox CR and labels it only with
|
||||||
|
// agents.x-k8s.io/sandbox-name-hash (a hash, not the full name), so the
|
||||||
|
// full-name label selector below matches nothing on that controller. An
|
||||||
|
// exact-name GET is collision-free (unlike name-prefix matching, which
|
||||||
|
// could hit a concurrent sandbox sharing a prefix) and resolves the pod on
|
||||||
|
// every controller version that keeps pod name == sandbox name.
|
||||||
|
try {
|
||||||
|
const pod = (await clients.core.readNamespacedPod({ namespace, name })) as {
|
||||||
|
metadata?: { name?: string };
|
||||||
|
};
|
||||||
|
if (pod?.metadata?.name) {
|
||||||
|
return pod.metadata.name;
|
||||||
|
}
|
||||||
|
} catch (err) {
|
||||||
|
const code =
|
||||||
|
(err as { code?: number; statusCode?: number }).code ??
|
||||||
|
(err as { code?: number; statusCode?: number }).statusCode;
|
||||||
|
if (code !== 404) throw err;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fallback: list pods by a full-name sandbox label, for controller versions
|
||||||
|
// that label pods with the sandbox name. A broader managed-by selector plus
|
||||||
// name-prefix narrowing could match a concurrent sandbox whose generated
|
// name-prefix narrowing could match a concurrent sandbox whose generated
|
||||||
// name shares a prefix, and exec would target the wrong lease's pod.
|
// name shares a prefix, and exec would target the wrong lease's pod.
|
||||||
const result = await clients.core.listNamespacedPod({
|
const result = await clients.core.listNamespacedPod({
|
||||||
|
|||||||
+38
-5
@@ -101,16 +101,35 @@ describe("findPodForSandbox", () => {
|
|||||||
const get = vi.fn().mockResolvedValue(makeCr("Ready", "pc-abc-pod-xyz"));
|
const get = vi.fn().mockResolvedValue(makeCr("Ready", "pc-abc-pod-xyz"));
|
||||||
const clients = {
|
const clients = {
|
||||||
custom: { getNamespacedCustomObject: get },
|
custom: { getNamespacedCustomObject: get },
|
||||||
core: { listNamespacedPod: vi.fn() },
|
core: { readNamespacedPod: vi.fn(), listNamespacedPod: vi.fn() },
|
||||||
};
|
};
|
||||||
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
||||||
expect(podName).toBe("pc-abc-pod-xyz");
|
expect(podName).toBe("pc-abc-pod-xyz");
|
||||||
// Should NOT have called listNamespacedPod (primary path succeeded)
|
// 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();
|
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 () => {
|
it("falls back to pod listing scoped by the unique sandbox-name label", async () => {
|
||||||
const get = vi.fn().mockResolvedValue(makeCr("Pending")); // no podName
|
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({
|
const list = vi.fn().mockResolvedValue({
|
||||||
items: [
|
items: [
|
||||||
{
|
{
|
||||||
@@ -121,7 +140,7 @@ describe("findPodForSandbox", () => {
|
|||||||
});
|
});
|
||||||
const clients = {
|
const clients = {
|
||||||
custom: { getNamespacedCustomObject: get },
|
custom: { getNamespacedCustomObject: get },
|
||||||
core: { listNamespacedPod: list },
|
core: { readNamespacedPod: read, listNamespacedPod: list },
|
||||||
};
|
};
|
||||||
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
||||||
expect(list).toHaveBeenCalledWith(
|
expect(list).toHaveBeenCalledWith(
|
||||||
@@ -143,18 +162,32 @@ describe("findPodForSandbox", () => {
|
|||||||
});
|
});
|
||||||
const clients = {
|
const clients = {
|
||||||
custom: { getNamespacedCustomObject: get },
|
custom: { getNamespacedCustomObject: get },
|
||||||
core: { listNamespacedPod: list },
|
core: { readNamespacedPod: vi.fn().mockRejectedValue({ code: 404 }), listNamespacedPod: list },
|
||||||
};
|
};
|
||||||
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
||||||
expect(podName).toBeNull();
|
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 () => {
|
it("returns null when no pod is found in fallback", async () => {
|
||||||
const get = vi.fn().mockResolvedValue(makeCr("Pending"));
|
const get = vi.fn().mockResolvedValue(makeCr("Pending"));
|
||||||
const list = vi.fn().mockResolvedValue({ items: [] });
|
const list = vi.fn().mockResolvedValue({ items: [] });
|
||||||
const clients = {
|
const clients = {
|
||||||
custom: { getNamespacedCustomObject: get },
|
custom: { getNamespacedCustomObject: get },
|
||||||
core: { listNamespacedPod: list },
|
core: { readNamespacedPod: vi.fn().mockRejectedValue({ code: 404 }), listNamespacedPod: list },
|
||||||
};
|
};
|
||||||
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
const podName = await findPodForSandbox(clients as never, "ns", "pc-abc");
|
||||||
expect(podName).toBeNull();
|
expect(podName).toBeNull();
|
||||||
|
|||||||
Reference in New Issue
Block a user