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); + }); +}); +