From 6c16e5ee07e9cfa2a7c0c198d7f05f9784e303a1 Mon Sep 17 00:00:00 2001 From: Longwt123 <1075015495@qq.com> Date: Wed, 1 Jul 2026 09:30:05 +0000 Subject: [PATCH] feat(runner-container-hooks): expand k8s error detection from Pending-only to 11 types - Add FailedMount to UNRECOVERABLE_WAITING_REASONS (covers waiting-state volume mount failures) - Add UNRECOVERABLE_TERMINATED_REASONS (OOMKilled, Error, FailedPostStartHookError) for Running-phase errors - Add getUnrecoverableTerminatedReasons() with env var extension support - Add getContainerTerminatedErrors() for detecting terminated containers with unrecoverable reasons - Add terminated error detection in runContainerStep catch block with describePodFailure diagnostics - Branch based on release/no_volumes (not main) per issue #1203 request UT coverage: - wait-for-pod-phases-test.ts: 10 new tests for getContainerTerminatedErrors (8) and getUnrecoverableTerminatedReasons (4) - run-container-step-terminated-test.ts: 5 new tests for runContainerStep terminated error detection (OOMKilled, Error, FailedPostStartHookError, no errors, getPodByName failure) --- packages/k8s/src/hooks/run-container-step.ts | 15 ++ packages/k8s/src/k8s/index.ts | 42 ++++- .../run-container-step-terminated-test.ts | 177 ++++++++++++++++++ .../k8s/tests/wait-for-pod-phases-test.ts | 148 +++++++++++++++ 4 files changed, 381 insertions(+), 1 deletion(-) create mode 100644 packages/k8s/tests/run-container-step-terminated-test.ts diff --git a/packages/k8s/src/hooks/run-container-step.ts b/packages/k8s/src/hooks/run-container-step.ts index 1786a38a..9b295cd3 100644 --- a/packages/k8s/src/hooks/run-container-step.ts +++ b/packages/k8s/src/hooks/run-container-step.ts @@ -6,9 +6,12 @@ import { dirname } from 'path' import { createContainerStepPod, deletePod, + describePodFailure, execCpFromPod, execCpToPod, execPodStep, + getContainerTerminatedErrors, + getPodByName, getPrepareJobTimeoutSeconds, waitForPodPhases } from '../k8s' @@ -116,6 +119,18 @@ export async function runContainerStep( fs.rmSync(runnerPath, { force: true }) } } catch (error) { + try { + const pod = await getPodByName(podName) + const terminatedErrors = getContainerTerminatedErrors(pod) + if (terminatedErrors.length > 0) { + const details = await describePodFailure(podName) + core.error( + `Pod ${podName} has unrecoverable container errors:\n${terminatedErrors.join('\n')}\n${details}` + ) + } + } catch { + // Best-effort: pod may already be deleted or unreachable + } core.error(`Failed to run container step: ${error}`) throw error } finally { diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index 4339d3c2..b23c6ce6 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/packages/k8s/src/k8s/index.ts @@ -679,7 +679,8 @@ export const UNRECOVERABLE_WAITING_REASONS = new Set([ 'ErrImagePull', 'InvalidImageName', 'CreateContainerConfigError', - 'CreateContainerError' + 'CreateContainerError', + 'FailedMount' ]) // Pod *event* reasons (from the event stream, not container status) that @@ -704,6 +705,12 @@ export const UNRECOVERABLE_EVENT_REASONS = new Set([ 'FailedBinding' ]) +export const UNRECOVERABLE_TERMINATED_REASONS = new Set([ + 'OOMKilled', + 'Error', + 'FailedPostStartHookError' +]) + // Maximum number of Warning events to include in a pod's failure description so // that the GitHub Actions log is not flooded. const MAX_DIAGNOSTIC_EVENTS = 10 @@ -745,6 +752,21 @@ export function getUnrecoverableEventReasons(): Set { return reasons } +export function getUnrecoverableTerminatedReasons(): Set { + const extra = process.env['ACTIONS_RUNNER_K8S_UNRECOVERABLE_TERMINATED_REASONS'] + if (!extra) { + return UNRECOVERABLE_TERMINATED_REASONS + } + const reasons = new Set(UNRECOVERABLE_TERMINATED_REASONS) + for (const reason of extra.split(',')) { + const trimmed = reason.trim() + if (trimmed) { + reasons.add(trimmed) + } + } + return reasons +} + export function getContainerErrors(pod: k8s.V1Pod): string[] { const errors: string[] = [] const unrecoverableReasons = getUnrecoverableWaitingReasons() @@ -764,6 +786,24 @@ export function getContainerErrors(pod: k8s.V1Pod): string[] { return errors } +export function getContainerTerminatedErrors(pod: k8s.V1Pod): string[] { + const errors: string[] = [] + const unrecoverableReasons = getUnrecoverableTerminatedReasons() + const allStatuses = [ + ...(pod.status?.initContainerStatuses ?? []), + ...(pod.status?.containerStatuses ?? []) + ] + for (const cs of allStatuses) { + const terminated = cs.state?.terminated + if (terminated?.reason && unrecoverableReasons.has(terminated.reason)) { + const reason = ` ✗ container "${cs.name}": ${terminated.reason} (exit code ${terminated.exitCode})` + const detail = terminated.message ? `\n ${terminated.message}` : '' + errors.push(detail ? `${reason}${detail}` : reason) + } + } + return errors +} + // Inspects the pod's status.conditions for deterministic scheduling failures. // This is the RBAC-free companion to getPodEventErrors: pod conditions are // always readable without the optional 'events' list permission. diff --git a/packages/k8s/tests/run-container-step-terminated-test.ts b/packages/k8s/tests/run-container-step-terminated-test.ts new file mode 100644 index 00000000..8111a895 --- /dev/null +++ b/packages/k8s/tests/run-container-step-terminated-test.ts @@ -0,0 +1,177 @@ +import * as k8s from '@kubernetes/client-node' +import * as core from '@actions/core' +import { runContainerStep } from '../src/hooks' +import * as k8sModule from '../src/k8s' +import { RunContainerStepArgs } from 'hooklib' + +jest.mock('@actions/core', () => ({ + debug: jest.fn(), + error: jest.fn(), + warning: jest.fn(), + info: jest.fn() +})) + +function terminatedContainer( + name: string, + reason: string, + exitCode: number, + message?: string +): k8s.V1ContainerStatus { + return { + name, + state: { terminated: { reason, exitCode, message } } + } as k8s.V1ContainerStatus +} + +function buildPodWithTerminated( + containerStatuses: k8s.V1ContainerStatus[] +): k8s.V1Pod { + return { + metadata: { name: 'test-step-pod' }, + status: { + phase: 'Failed', + containerStatuses + } + } as k8s.V1Pod +} + +function makeMinimalArgs(): RunContainerStepArgs { + return { + image: 'ubuntu:latest', + entryPoint: 'sh', + entryPointArgs: ['-c', 'echo test'] + } as RunContainerStepArgs +} + +describe('runContainerStep terminated error detection', () => { + let createStepPodSpy: jest.SpyInstance + let waitForPodPhasesSpy: jest.SpyInstance + let getPodByNameSpy: jest.SpyInstance + let getContainerTerminatedErrorsSpy: jest.SpyInstance + let describePodFailureSpy: jest.SpyInstance + let deletePodSpy: jest.SpyInstance + let coreErrorSpy: jest.SpyInstance + + beforeEach(() => { + process.env['ACTIONS_RUNNER_KUBERNETES_NAMESPACE'] = 'default' + process.env['RUNNER_WORKSPACE'] = '/tmp/runner/_work/repo' + process.env['GITHUB_WORKSPACE'] = '/tmp/runner/_work/repo/repo' + process.env['ACTIONS_RUNNER_POD_NAME'] = 'test-runner-pod' + + createStepPodSpy = jest + .spyOn(k8sModule, 'createContainerStepPod') + .mockResolvedValue({ + metadata: { name: 'test-step-pod' } + } as k8s.V1Pod) + + waitForPodPhasesSpy = jest + .spyOn(k8sModule, 'waitForPodPhases') + .mockRejectedValue(new Error('Pod test-step-pod has unrecoverable errors')) + + getPodByNameSpy = jest + .spyOn(k8sModule, 'getPodByName') + + getContainerTerminatedErrorsSpy = jest + .spyOn(k8sModule, 'getContainerTerminatedErrors') + + describePodFailureSpy = jest + .spyOn(k8sModule, 'describePodFailure') + .mockResolvedValue('Pod status: Failed\nContainer details:\n ✗ container "job" terminated: OOMKilled (exit code 137)') + + deletePodSpy = jest + .spyOn(k8sModule, 'deletePod') + .mockResolvedValue(undefined) + + coreErrorSpy = jest.spyOn(core, 'error') + }) + + afterEach(() => { + jest.restoreAllMocks() + delete process.env['ACTIONS_RUNNER_KUBERNETES_NAMESPACE'] + delete process.env['RUNNER_WORKSPACE'] + delete process.env['GITHUB_WORKSPACE'] + delete process.env['ACTIONS_RUNNER_POD_NAME'] + }) + + it('detects OOMKilled and logs describePodFailure output', async () => { + getPodByNameSpy.mockResolvedValue( + buildPodWithTerminated([ + terminatedContainer('job', 'OOMKilled', 137, 'The node was low on resource: memory') + ]) + ) + getContainerTerminatedErrorsSpy.mockReturnValue([ + ' ✗ container "job": OOMKilled (exit code 137)\n The node was low on resource: memory' + ]) + + await expect(runContainerStep(makeMinimalArgs())).rejects.toThrow() + + expect(getPodByNameSpy).toHaveBeenCalledWith('test-step-pod') + expect(getContainerTerminatedErrorsSpy).toHaveBeenCalled() + expect(describePodFailureSpy).toHaveBeenCalledWith('test-step-pod') + expect(coreErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('OOMKilled') + ) + }) + + it('detects Error (exit non-zero) and logs describePodFailure output', async () => { + getPodByNameSpy.mockResolvedValue( + buildPodWithTerminated([ + terminatedContainer('job', 'Error', 1) + ]) + ) + getContainerTerminatedErrorsSpy.mockReturnValue([ + ' ✗ container "job": Error (exit code 1)' + ]) + + await expect(runContainerStep(makeMinimalArgs())).rejects.toThrow() + + expect(getPodByNameSpy).toHaveBeenCalledWith('test-step-pod') + expect(getContainerTerminatedErrorsSpy).toHaveBeenCalled() + expect(describePodFailureSpy).toHaveBeenCalledWith('test-step-pod') + expect(coreErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Error (exit code 1)') + ) + }) + + it('detects FailedPostStartHookError and logs describePodFailure output', async () => { + getPodByNameSpy.mockResolvedValue( + buildPodWithTerminated([ + terminatedContainer('job', 'FailedPostStartHookError', 137, 'postStart hook failed') + ]) + ) + getContainerTerminatedErrorsSpy.mockReturnValue([ + ' ✗ container "job": FailedPostStartHookError (exit code 137)\n postStart hook failed' + ]) + + await expect(runContainerStep(makeMinimalArgs())).rejects.toThrow() + + expect(getPodByNameSpy).toHaveBeenCalledWith('test-step-pod') + expect(getContainerTerminatedErrorsSpy).toHaveBeenCalled() + expect(describePodFailureSpy).toHaveBeenCalledWith('test-step-pod') + expect(coreErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('FailedPostStartHookError') + ) + }) + + it('does not log terminated errors when pod has none', async () => { + getPodByNameSpy.mockResolvedValue(buildPodWithTerminated([])) + getContainerTerminatedErrorsSpy.mockReturnValue([]) + + await expect(runContainerStep(makeMinimalArgs())).rejects.toThrow() + + expect(getPodByNameSpy).toHaveBeenCalledWith('test-step-pod') + expect(getContainerTerminatedErrorsSpy).toHaveBeenCalled() + expect(describePodFailureSpy).not.toHaveBeenCalled() + }) + + it('gracefully handles getPodByName failure', async () => { + getPodByNameSpy.mockRejectedValue(new Error('pod not found')) + + await expect(runContainerStep(makeMinimalArgs())).rejects.toThrow() + + expect(getPodByNameSpy).toHaveBeenCalledWith('test-step-pod') + expect(coreErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Failed to run container step') + ) + }) +}) diff --git a/packages/k8s/tests/wait-for-pod-phases-test.ts b/packages/k8s/tests/wait-for-pod-phases-test.ts index 60758615..5cc0ff38 100644 --- a/packages/k8s/tests/wait-for-pod-phases-test.ts +++ b/packages/k8s/tests/wait-for-pod-phases-test.ts @@ -2,11 +2,14 @@ import * as k8s from '@kubernetes/client-node' import { describePodFailure, getContainerErrors, + getContainerTerminatedErrors, getPodEventErrors, getUnrecoverableEventReasons, + getUnrecoverableTerminatedReasons, getUnrecoverableWaitingReasons, parsePodPhase, UNRECOVERABLE_EVENT_REASONS, + UNRECOVERABLE_TERMINATED_REASONS, UNRECOVERABLE_WAITING_REASONS, waitForPodPhases } from '../src/k8s' @@ -41,6 +44,18 @@ function waitingContainer( } as k8s.V1ContainerStatus } +function terminatedContainer( + name: string, + reason?: string, + exitCode?: number, + message?: string +): k8s.V1ContainerStatus { + return { + name, + state: { terminated: { reason, exitCode, message } } + } as k8s.V1ContainerStatus +} + // Build a Warning event with the fields getPodEventErrors() inspects. function buildEvent( reason: string, @@ -297,6 +312,139 @@ describe('getUnrecoverableEventReasons', () => { }) }) +describe('getContainerTerminatedErrors', () => { + it('returns no errors when there are no container statuses', () => { + expect(getContainerTerminatedErrors(buildPod(PodPhase.FAILED))).toEqual([]) + expect(getContainerTerminatedErrors({} as k8s.V1Pod)).toEqual([]) + }) + + it('returns no errors when containers have no terminated state', () => { + const pod = buildPod(PodPhase.RUNNING, { + containerStatuses: [ + { name: 'job', state: { running: {} } } as k8s.V1ContainerStatus, + waitingContainer('sidecar', 'ContainerCreating') + ] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([]) + }) + + it('returns no errors for terminated containers with recoverable reasons', () => { + const pod = buildPod(PodPhase.SUCCEEDED, { + containerStatuses: [ + terminatedContainer('fs-init', 'Completed', 0) + ] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([]) + }) + + it('detects OOMKilled', () => { + const pod = buildPod(PodPhase.FAILED, { + containerStatuses: [ + terminatedContainer('job', 'OOMKilled', 137, 'The node was low on resource: memory') + ] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([ + ' ✗ container "job": OOMKilled (exit code 137)\n The node was low on resource: memory' + ]) + }) + + it('detects Error (exit non-zero)', () => { + const pod = buildPod(PodPhase.FAILED, { + containerStatuses: [ + terminatedContainer('job', 'Error', 1) + ] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([ + ' ✗ container "job": Error (exit code 1)' + ]) + }) + + it('detects FailedPostStartHookError', () => { + const pod = buildPod(PodPhase.FAILED, { + containerStatuses: [ + terminatedContainer('job', 'FailedPostStartHookError', 137, 'postStart hook failed') + ] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([ + ' ✗ container "job": FailedPostStartHookError (exit code 137)\n postStart hook failed' + ]) + }) + + it('detects every unrecoverable terminated reason', () => { + for (const reason of Array.from(UNRECOVERABLE_TERMINATED_REASONS)) { + const pod = buildPod(PodPhase.FAILED, { + containerStatuses: [terminatedContainer('job', reason, 1)] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([ + ` ✗ container "job": ${reason} (exit code 1)` + ]) + } + }) + + it('detects terminated errors in init containers as well', () => { + const pod = buildPod(PodPhase.FAILED, { + initContainerStatuses: [terminatedContainer('init', 'Error', 2)], + containerStatuses: [terminatedContainer('job', 'OOMKilled', 137)] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([ + ' ✗ container "init": Error (exit code 2)', + ' ✗ container "job": OOMKilled (exit code 137)' + ]) + }) + + it('ignores terminated with reason not in the whitelist', () => { + const pod = buildPod(PodPhase.SUCCEEDED, { + containerStatuses: [ + terminatedContainer('job', 'Completed', 0) + ] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([]) + }) +}) + +describe('getUnrecoverableTerminatedReasons', () => { + afterEach(() => { + delete process.env['ACTIONS_RUNNER_K8S_UNRECOVERABLE_TERMINATED_REASONS'] + }) + + it('returns the built-in defaults when the env var is unset', () => { + expect(getUnrecoverableTerminatedReasons()).toEqual(UNRECOVERABLE_TERMINATED_REASONS) + }) + + it('adds extra reasons from the env var without dropping the defaults', () => { + process.env['ACTIONS_RUNNER_K8S_UNRECOVERABLE_TERMINATED_REASONS'] = + 'DeadlineExceeded, CustomReason' + const reasons = getUnrecoverableTerminatedReasons() + for (const builtin of Array.from(UNRECOVERABLE_TERMINATED_REASONS)) { + expect(reasons.has(builtin)).toBe(true) + } + expect(reasons.has('DeadlineExceeded')).toBe(true) + expect(reasons.has('CustomReason')).toBe(true) + }) + + it('makes getContainerTerminatedErrors honor the extended whitelist', () => { + process.env['ACTIONS_RUNNER_K8S_UNRECOVERABLE_TERMINATED_REASONS'] = + 'DeadlineExceeded' + const pod = buildPod(PodPhase.FAILED, { + containerStatuses: [terminatedContainer('job', 'DeadlineExceeded', 1)] + }) + expect(getContainerTerminatedErrors(pod)).toEqual([ + ' ✗ container "job": DeadlineExceeded (exit code 1)' + ]) + }) + + it('filters empty strings from the env var', () => { + process.env['ACTIONS_RUNNER_K8S_UNRECOVERABLE_TERMINATED_REASONS'] = + ', , CustomReason, ' + const reasons = getUnrecoverableTerminatedReasons() + expect(reasons.has('')).toBe(false) + expect(reasons.has('CustomReason')).toBe(true) + for (const builtin of Array.from(UNRECOVERABLE_TERMINATED_REASONS)) { + expect(reasons.has(builtin)).toBe(true) + } + }) +}) + describe('waitForPodPhases', () => { let readSpy: jest.SpyInstance let eventSpy: jest.SpyInstance