diff --git a/.github/workflows/test-cloud-hypervisor.yml b/.github/workflows/test-cloud-hypervisor.yml index 76db2948f..1ac2e5c4e 100644 --- a/.github/workflows/test-cloud-hypervisor.yml +++ b/.github/workflows/test-cloud-hypervisor.yml @@ -279,7 +279,7 @@ jobs: echo "::error::Cloud Hypervisor cgroup residue remains after cleanup" exit 1 fi - if pgrep -f 'cloud-hypervisor --api-socket' >/dev/null 2>&1; then + if sudo pgrep -f 'cloud-hypervisor --api-socket' >/dev/null 2>&1; then echo "::error::Cloud Hypervisor process residue remains after cleanup" exit 1 fi diff --git a/scripts/ci/cloud-hypervisor-live-smoke.sh b/scripts/ci/cloud-hypervisor-live-smoke.sh index bf784c959..0d0d7c01d 100755 --- a/scripts/ci/cloud-hypervisor-live-smoke.sh +++ b/scripts/ci/cloud-hypervisor-live-smoke.sh @@ -120,13 +120,13 @@ assert_no_residue() { echo "Cloud Hypervisor cgroup residue detected" >&2 return 1 fi - if pgrep -f 'cloud-hypervisor --api-socket' >/dev/null 2>&1; then - pgrep -af 'cloud-hypervisor --api-socket' >&2 + if sudo pgrep -f 'cloud-hypervisor --api-socket' >/dev/null 2>&1; then + sudo pgrep -af 'cloud-hypervisor --api-socket' >&2 echo "Cloud Hypervisor process residue detected" >&2 return 1 fi - if pgrep -f "$ARTIFACT_DIR/virtiofsd.*--shared-dir=" >/dev/null 2>&1; then - pgrep -af "$ARTIFACT_DIR/virtiofsd.*--shared-dir=" >&2 + if sudo pgrep -f "$ARTIFACT_DIR/virtiofsd.*--shared-dir=" >/dev/null 2>&1; then + sudo pgrep -af "$ARTIFACT_DIR/virtiofsd.*--shared-dir=" >&2 echo "Cloud Hypervisor virtiofsd process residue detected" >&2 return 1 fi @@ -569,7 +569,7 @@ for _ in $(seq 1 90); do sudo ip netns list | grep -q '^awfvm-' && sudo find /run/awf-cloud-hypervisor/pending-cleanup -maxdepth 1 -name '*.json' | grep -q . && sudo find "$CGROUP_ROOT" -mindepth 1 -maxdepth 1 -type d | grep -q . && - pgrep -f "$ARTIFACT_DIR/cloud-hypervisor --api-socket" >/dev/null; then + sudo pgrep -f "$ARTIFACT_DIR/cloud-hypervisor --api-socket" >/dev/null; then break fi sleep 1 @@ -591,7 +591,7 @@ sudo ip netns list | grep -q '^awfvm-' || { echo "process-death: abrupt exit did not leave the expected recovery fixture" >&2 exit 1 } -pgrep -f "$ARTIFACT_DIR/cloud-hypervisor --api-socket" >/dev/null || { +sudo pgrep -f "$ARTIFACT_DIR/cloud-hypervisor --api-socket" >/dev/null || { echo "process-death: VMM did not survive abrupt owner death" >&2 exit 1 } diff --git a/src/cloud-hypervisor/manager.test.ts b/src/cloud-hypervisor/manager.test.ts index 58864493f..d8630b797 100644 --- a/src/cloud-hypervisor/manager.test.ts +++ b/src/cloud-hypervisor/manager.test.ts @@ -1075,7 +1075,7 @@ describe('CloudHypervisorManager', () => { createVsockClient: jest.fn().mockReturnValue(guestClient), }); const manager = new CloudHypervisorManager( - config(), + config({ apiTimeoutMs: 1_000 }), '/tmp/awf', deps, 'ready-guest', diff --git a/src/cloud-hypervisor/virtiofsd-sandbox.ts b/src/cloud-hypervisor/virtiofsd-sandbox.ts index ab86bf02c..6fa6c4a37 100644 --- a/src/cloud-hypervisor/virtiofsd-sandbox.ts +++ b/src/cloud-hypervisor/virtiofsd-sandbox.ts @@ -113,12 +113,14 @@ export async function verifyVirtiofsdSandbox( assertSameIdentity(options.parentIdentity, currentParent, 'parent'); assertLaunchCommand(currentParent, options); - const workerPid = await waitForWorker(options.parentIdentity.pid, dependencies); - const workerIdentity = await captureVirtiofsdProcessIdentity(workerPid, dependencies); - assertExecutable(workerIdentity.executable, options.expectedExecutable, 'worker'); - await options.assignToCgroup(workerPid); - const currentWorker = await captureVirtiofsdProcessIdentity(workerPid, dependencies); - assertSameIdentity(workerIdentity, currentWorker, 'worker'); + const workerIdentity = await waitForWorker( + options.parentIdentity.pid, + options.expectedExecutable, + options.assignToCgroup, + dependencies, + ); + const workerPid = workerIdentity.pid; + const currentWorker = workerIdentity; const parent = await collectProcessEvidence(currentParent, options.cgroupPath, dependencies); const worker = await collectProcessEvidence(currentWorker, options.cgroupPath, dependencies); @@ -219,8 +221,10 @@ export async function verifyVirtiofsdSandbox( async function waitForWorker( parentPid: number, + expectedExecutable: string, + assignToCgroup: (pid: number) => Promise, dependencies: VirtiofsdSandboxDependencies, -): Promise { +): Promise { const deadline = Date.now() + WORKER_READY_TIMEOUT_MS; do { const children = ( @@ -229,11 +233,41 @@ async function waitForWorker( for (const value of children) { const candidate = Number(value); if (!Number.isSafeInteger(candidate) || candidate <= 1) continue; + let identity: VirtiofsdProcessIdentity; try { const comm = await dependencies.readFile(`/proc/${candidate}/comm`, 'utf8'); - if (comm.trim() === 'virtiofsd') return candidate; + if (comm.trim() !== 'virtiofsd') continue; + identity = await captureVirtiofsdProcessIdentity(candidate, dependencies); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ENOENT' || code === 'ESRCH') continue; + throw error; + } + assertExecutable(identity.executable, expectedExecutable, 'worker'); + try { + await assignToCgroup(candidate); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ESRCH') continue; + if (code === 'ENOENT') { + try { + await captureVirtiofsdProcessIdentity(candidate, dependencies); + } catch (identityError) { + const identityCode = (identityError as NodeJS.ErrnoException).code; + if (identityCode === 'ENOENT' || identityCode === 'ESRCH') continue; + throw identityError; + } + } + throw error; + } + try { + const current = await captureVirtiofsdProcessIdentity(candidate, dependencies); + assertSameIdentity(identity, current, 'worker'); + return current; } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ENOENT' || code === 'ESRCH') continue; + throw error; } } await dependencies.sleep(WORKER_READY_INTERVAL_MS); diff --git a/src/cloud-hypervisor/virtiofsd.test.ts b/src/cloud-hypervisor/virtiofsd.test.ts index 1d3931e7d..30ad2a472 100644 --- a/src/cloud-hypervisor/virtiofsd.test.ts +++ b/src/cloud-hypervisor/virtiofsd.test.ts @@ -485,6 +485,184 @@ describe('VirtiofsdManager', () => { await expect(manager(deps).start([workspace])).resolves.toHaveLength(1); }); + it('ignores a child candidate that is not virtiofsd', async () => { + const deps = dependencies(); + const readFile = deps.readFile; + deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { + if (filePath.endsWith('/children')) return '1099 1100'; + if (filePath === '/proc/1099/comm') return 'other-process\n'; + return readFile(filePath, encoding); + }); + + await expect(manager(deps).start([workspace])).resolves.toHaveLength(1); + }); + + it.each(['ENOENT', 'ESRCH'])( + 'retries when a discovered sandbox worker exits before identity capture (%s)', + async (code) => { + const deps = dependencies(); + const readFile = deps.readFile; + let discovery = 0; + deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { + if (filePath.endsWith('/children')) { + discovery += 1; + return discovery === 1 ? '1099' : '1100'; + } + if (filePath === '/proc/1099/comm') return 'virtiofsd\n'; + if (filePath === '/proc/1099/stat') { + throw Object.assign(new Error('exited'), { code }); + } + return readFile(filePath, encoding); + }); + + await expect(manager(deps).start([workspace])).resolves.toHaveLength(1); + expect(deps.sleep).toHaveBeenCalled(); + }, + ); + + it('retries with a replacement worker when cgroup assignment reports ESRCH', async () => { + const deps = dependencies(); + const readFile = deps.readFile; + let discovery = 0; + deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { + if (filePath.endsWith('/children')) { + discovery += 1; + return discovery === 1 ? '1099' : '1100'; + } + return readFile(filePath, encoding); + }); + const cgroup = { + cgroupPath: '/sys/fs/cgroup/awf-cloud-hypervisor/test', + assign: jest.fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(Object.assign(new Error('gone'), { code: 'ESRCH' })) + .mockResolvedValue(undefined), + }; + + await expect(manager(deps, cgroup).start([workspace])).resolves.toHaveLength(1); + expect(cgroup.assign).toHaveBeenNthCalledWith(2, 1099); + expect(cgroup.assign).toHaveBeenNthCalledWith(3, 1100); + }); + + it('does not retry cgroup ENOENT while the candidate worker still exists', async () => { + const deps = dependencies(); + const cgroup = { + cgroupPath: '/sys/fs/cgroup/awf-cloud-hypervisor/test', + assign: jest.fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValue(Object.assign( + new Error('cgroup disappeared'), + { code: 'ENOENT' }, + )), + }; + + await expect(manager(deps, cgroup).start([workspace])) + .rejects.toThrow(/cgroup disappeared/); + expect(deps.sleep).not.toHaveBeenCalled(); + }); + + it('surfaces unexpected cgroup assignment errors immediately', async () => { + const deps = dependencies(); + const cgroup = { + cgroupPath: '/sys/fs/cgroup/awf-cloud-hypervisor/test', + assign: jest.fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(Object.assign(new Error('assignment denied'), { code: 'EACCES' })), + }; + + await expect(manager(deps, cgroup).start([workspace])) + .rejects.toThrow(/assignment denied/); + expect(deps.sleep).not.toHaveBeenCalled(); + }); + + it.each(['ENOENT', 'ESRCH'])( + 'retries cgroup ENOENT when procfs confirms the candidate exited (%s)', + async (code) => { + const deps = dependencies(); + const readFile = deps.readFile; + let discovery = 0; + let candidateStatReads = 0; + deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { + if (filePath.endsWith('/children')) { + discovery += 1; + return discovery === 1 ? '1099' : '1100'; + } + if (filePath === '/proc/1099/stat' && ++candidateStatReads === 2) { + throw Object.assign(new Error('exited'), { code }); + } + return readFile(filePath, encoding); + }); + const cgroup = { + cgroupPath: '/sys/fs/cgroup/awf-cloud-hypervisor/test', + assign: jest.fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(Object.assign(new Error('missing target'), { code: 'ENOENT' })) + .mockResolvedValue(undefined), + }; + + await expect(manager(deps, cgroup).start([workspace])).resolves.toHaveLength(1); + expect(cgroup.assign).toHaveBeenNthCalledWith(2, 1099); + expect(cgroup.assign).toHaveBeenNthCalledWith(3, 1100); + }, + ); + + it('surfaces unexpected procfs errors while confirming cgroup ENOENT', async () => { + const deps = dependencies(); + const readFile = deps.readFile; + let candidateStatReads = 0; + deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { + if (filePath === '/proc/1100/stat' && ++candidateStatReads === 2) { + throw Object.assign(new Error('proc denied'), { code: 'EACCES' }); + } + return readFile(filePath, encoding); + }); + const cgroup = { + cgroupPath: '/sys/fs/cgroup/awf-cloud-hypervisor/test', + assign: jest.fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(Object.assign(new Error('missing target'), { code: 'ENOENT' })), + }; + + await expect(manager(deps, cgroup).start([workspace])).rejects.toThrow(/proc denied/); + }); + + it.each(['ENOENT', 'ESRCH'])( + 'retries when the assigned worker exits before identity revalidation (%s)', + async (code) => { + const deps = dependencies(); + const readFile = deps.readFile; + let discovery = 0; + let candidateStatReads = 0; + deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { + if (filePath.endsWith('/children')) { + discovery += 1; + return discovery === 1 ? '1099' : '1100'; + } + if (filePath === '/proc/1099/stat' && ++candidateStatReads === 2) { + throw Object.assign(new Error('exited'), { code }); + } + return readFile(filePath, encoding); + }); + + await expect(manager(deps).start([workspace])).resolves.toHaveLength(1); + expect(deps.sleep).toHaveBeenCalled(); + }, + ); + + it('surfaces unexpected procfs errors during worker identity revalidation', async () => { + const deps = dependencies(); + const readFile = deps.readFile; + let candidateStatReads = 0; + deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { + if (filePath === '/proc/1100/stat' && ++candidateStatReads === 2) { + throw Object.assign(new Error('proc denied'), { code: 'EACCES' }); + } + return readFile(filePath, encoding); + }); + + await expect(manager(deps).start([workspace])).rejects.toThrow(/proc denied/); + }); + it('surfaces an unexpected procfs error while discovering the sandbox worker', async () => { const deps = dependencies(); const readFile = deps.readFile;