From 2731adcea83069f45bed24e592718da80d250db1 Mon Sep 17 00:00:00 2001 From: yulia-ivashko Date: Fri, 7 Aug 2026 23:39:47 +0300 Subject: [PATCH] Delete the seed pod an interrupted create leaves holding both PVCs A completed create deletes its seed pod, so cleanup never looked for one. But an interrupted create leaves the pod behind - Completed, harmless-looking - still mounting both PVCs. Their protection finalizer then waits on it: each PVC delete below times out after ninety seconds, cleanup reports incomplete, and no retry can ever finish, because the one resource blocking everything is the one resource nobody deletes. Found on a real workspace whose deletion failed identically on every attempt. The pod stays out of expectedResources deliberately: verification of a healthy workspace must not demand a pod that a completed create correctly removed. Remove now deletes it explicitly, ownership-verified against the seed role labels, and before the PVCs so the finalizer releases. A foreign pod wearing the seed name is refused and reported, not deleted. --- src/providers/kubernetes.js | 13 +++++ src/providers/kubernetes.test.js | 87 +++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/providers/kubernetes.js b/src/providers/kubernetes.js index 81572b3..89139ce 100644 --- a/src/providers/kubernetes.js +++ b/src/providers/kubernetes.js @@ -260,6 +260,19 @@ export function createKubernetesProvider({ policy, sourceDirectory }) { stopPortForward(meta.providerResourceID); const result = await cleanupTransaction(meta.providerResourceID, async (cleanup) => { await verifyExistingResources(kubectl, meta, policy, { requireIssuer: false }); + // The seed pod is deleted by a completed create, so it is not a canonical + // resource and stays out of expectedResources — verification of a healthy + // workspace must not demand it. But an interrupted create leaves it behind, and + // while it exists the PVCs it mounts never finish terminating: their protection + // finalizer waits on the pod, both PVC deletes below time out, and cleanup + // reports incomplete forever. Removing it here, ownership-verified, is what + // makes remove idempotent for that leftover. + const seedPod = `${refs.deployment}-seed`; + await cleanup.remove(`pod:${seedPod}`, async () => { + if (await resourceExistsOwned(kubectl, 'pod', seedPod, refs.namespace, providerLabels(identityFromMetadata(meta), 'seed'))) { + await deleteResource(kubectl, 'pod', seedPod, refs.namespace); + } + }); for (const [kind, name] of expectedResources(refs).filter(([resourceKind]) => !['pvc'].includes(resourceKind))) { await cleanup.remove(`${kind}:${name}`, () => deleteResource(kubectl, kind, name, refs.namespace)); } diff --git a/src/providers/kubernetes.test.js b/src/providers/kubernetes.test.js index de9dab7..e53e245 100644 --- a/src/providers/kubernetes.test.js +++ b/src/providers/kubernetes.test.js @@ -1,7 +1,14 @@ -import { describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const processMocks = vi.hoisted(() => ({ commandExists: vi.fn(() => true), run: vi.fn(), runJson: vi.fn() })); +vi.mock('../process.js', async (importOriginal) => ({ ...await importOriginal(), ...processMocks })); + import { readPolicy } from '../policy.js'; import { buildManifests, createKubernetesProvider, KUBERNETES_SEED_EXTRACT_COMMAND, kubernetesCredentialRefreshCommands } from './kubernetes.js'; -import { canonicalResourceRefs, deriveWorkspaceIdentity } from '../metadata.js'; +import { canonicalResourceRefs, deriveWorkspaceIdentity, providerLabels } from '../metadata.js'; function policy() { return readPolicy({ @@ -118,3 +125,79 @@ describe('Kubernetes provider manifests', () => { })); }); }); + +describe('Kubernetes provider cleanup', () => { + let stateDirectory; + beforeEach(async () => { + stateDirectory = await mkdtemp(join(tmpdir(), 'workspace-kubernetes-test-')); + process.env.OPENCHAMBER_WORKSPACE_STATE_DIR = stateDirectory; + processMocks.commandExists.mockReturnValue(true); + processMocks.run.mockReset(); + processMocks.runJson.mockReset().mockRejectedValue(new Error('not found')); + }); + afterEach(async () => { + delete process.env.OPENCHAMBER_WORKSPACE_STATE_DIR; + await rm(stateDirectory, { recursive: true, force: true }); + }); + + function seedLabels(extra, role = 'seed') { + return providerLabels({ + provider: 'kubernetes', + providerResourceID: extra.providerResourceID, + projectID: extra.projectID, + controlPlaneWorkspaceID: extra.controlPlaneWorkspaceID, + originalControlPlaneWorkspaceID: extra.originalControlPlaneWorkspaceID ?? extra.controlPlaneWorkspaceID, + }, role); + } + + it('removes the seed pod an interrupted create left behind, before the PVCs it holds', async () => { + // A completed create deletes its seed pod, so cleanup never used to look for it. + // An interrupted create leaves it mounted on both PVCs, whose protection finalizer + // then waits on the pod: each PVC delete times out and cleanup reports incomplete + // forever. Remove must delete the leftover pod first, ownership-verified. + const currentPolicy = policy(); + const provider = createKubernetesProvider({ policy: currentPolicy, sourceDirectory: '/source' }); + const info = provider.configure({ id: 'control-id', projectID: 'project-id' }); + const refs = info.extra.resourceRefs; + const seedPod = `${refs.deployment}-seed`; + processMocks.run.mockImplementation(async (_binary, args) => { + if (args.includes('get') && args.includes('pod') && args.includes(seedPod)) { + return { stdout: JSON.stringify({ metadata: { labels: seedLabels(info.extra) } }), stderr: '' }; + } + if (args.includes('get')) throw new Error('Error from server (NotFound): resource not found'); + return { stdout: '', stderr: '' }; + }); + + const result = await provider.remove(info); + + const commands = processMocks.run.mock.calls.map(([, args]) => args); + const podDelete = commands.findIndex((args) => args.includes('delete') && args.includes('pod') && args.includes(seedPod)); + const pvcDelete = commands.findIndex((args) => args.includes('delete') && args.includes('pvc')); + expect(podDelete).toBeGreaterThanOrEqual(0); + expect(pvcDelete).toBeGreaterThan(podDelete); + expect(result.remainingResources ?? []).toEqual([]); + }); + + it('refuses a foreign pod wearing the seed name instead of deleting it', async () => { + const currentPolicy = policy(); + const provider = createKubernetesProvider({ policy: currentPolicy, sourceDirectory: '/source' }); + const info = provider.configure({ id: 'control-id', projectID: 'project-id' }); + const refs = info.extra.resourceRefs; + const seedPod = `${refs.deployment}-seed`; + processMocks.run.mockImplementation(async (_binary, args) => { + if (args.includes('get') && args.includes('pod') && args.includes(seedPod)) { + return { stdout: JSON.stringify({ metadata: { labels: { ...seedLabels(info.extra), 'openchamber.io/resource-id': 'ws-foreign' } } }), stderr: '' }; + } + if (args.includes('get')) throw new Error('Error from server (NotFound): resource not found'); + return { stdout: '', stderr: '' }; + }); + + await expect(provider.remove(info)).rejects.toMatchObject({ + remainingResources: expect.arrayContaining([`pod:${seedPod}`]), + }); + + const commands = processMocks.run.mock.calls.map(([, args]) => args); + expect(commands.some((args) => args.includes('delete') && args.includes('pod') && args.includes(seedPod))).toBe(false); + }); +}); +