diff --git a/packages/k8s/src/hooks/run-container-step.ts b/packages/k8s/src/hooks/run-container-step.ts index 347e324b..a00dfaec 100644 --- a/packages/k8s/src/hooks/run-container-step.ts +++ b/packages/k8s/src/hooks/run-container-step.ts @@ -13,6 +13,7 @@ import { getContainerTerminatedErrors, getPodByName, getPrepareJobTimeoutSeconds, + getTerminatedReasonHint, waitForPodPhases } from '../k8s' import { @@ -191,12 +192,13 @@ async function classifyScriptError( (term.exitCode === 137 && reason !== 'Completed') if (isContainerFault) { const detail = term.message ? `\n ${term.message}` : '' + const hint = `\n${getTerminatedReasonHint(reason, term.exitCode)}` errors.push( - ` ✗ container "${JOB_CONTAINER_NAME}": ${reason} (exit code ${term.exitCode}) — container-level failure, not a script error${detail}` + ` ✗ container "${JOB_CONTAINER_NAME}": ${reason} (exit code ${term.exitCode})${detail}${hint}` ) } else { errors.push( - ` → container exited cleanly; please check your script for errors` + ` → your script exited with a non-zero code; please check your script for errors` ) sections.push( `Container status: ${reason} (exit code ${term.exitCode})` diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index a880bfb7..2eaf40da 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/packages/k8s/src/k8s/index.ts @@ -817,14 +817,15 @@ export const UNRECOVERABLE_WAITING_REASONS = new Set([ // Secret/ConfigMap volume). The container stays in `ContainerCreating` // waiting state, which is NOT in UNRECOVERABLE_WAITING_REASONS, so without // this check the hook polls until the 3600s timeout. -// - FailedScheduling: the scheduler cannot place the pod (insufficient -// cpu/memory/gpu, node selector / affinity mismatch, no matching nodes). -// The pod stays Pending with PodScheduled=False forever. // - FailedBinding: a PVC could not be bound (no matching PV, storage class // misconfiguration). Usually paired with FailedMount once the pod retries. +// +// Note: FailedScheduling is intentionally excluded. Scheduling failures are +// almost always transient resource shortages (Insufficient cpu/memory/gpu) that +// self-resolve once a node frees up or scales in. Fast-failing on them would +// terminate jobs that should simply queue until the timeout. export const UNRECOVERABLE_EVENT_REASONS = new Set([ 'FailedMount', - 'FailedScheduling', 'FailedBinding' ]) @@ -890,6 +891,29 @@ export function getUnrecoverableTerminatedReasons(): Set { return reasons } +function getWaitingReasonHint(reason: string): string { + switch (reason) { + case 'ImagePullBackOff': + case 'ErrImagePull': + return [ + ` → Check that the image name and tag are correct and exist in the registry.`, + ` If the image is in a private registry, ensure an imagePullSecret is configured.`, + ` Check network connectivity from the node to the registry (DNS, firewall, proxy).`, + ` Run: kubectl describe pod | grep -A5 "Events"`, + ].join('\n') + case 'InvalidImageName': + return ` → Image name is malformed. Check the workflow/job container image configuration.` + case 'CreateContainerConfigError': + return ` → Container config is invalid. Check env vars, resource limits, and securityContext in the job spec.` + case 'CreateContainerError': + return ` → Container runtime failed to create the container. Check node-level issues or contact the cluster administrator.` + case 'FailedMount': + return ` → A volume could not be mounted. Check that the PVC is bound, the Secret/ConfigMap exists, and hostPath directories are present on the node.` + default: + return ` → Check pod events with: kubectl describe pod ` + } +} + export function getContainerErrors(pod: k8s.V1Pod): string[] { const errors: string[] = [] const unrecoverableReasons = getUnrecoverableWaitingReasons() @@ -900,15 +924,39 @@ export function getContainerErrors(pod: k8s.V1Pod): string[] { for (const cs of allStatuses) { const waiting = cs.state?.waiting if (waiting?.reason && unrecoverableReasons.has(waiting.reason)) { - // Format as two indented lines: reason on first, message detail on second const reason = ` ✗ container "${cs.name}": ${waiting.reason}` - const detail = waiting.message ? ` ${waiting.message}` : '' - errors.push(detail ? `${reason}\n${detail}` : reason) + const detail = waiting.message ? `\n ${waiting.message}` : '' + const hint = `\n${getWaitingReasonHint(waiting.reason)}` + errors.push(`${reason}${detail}${hint}`) } } return errors } +export function getTerminatedReasonHint(reason: string, exitCode: number | undefined): string { + if (reason === 'OOMKilled') { + return ` → Container exceeded its memory limit and was killed by the OOM killer.\n Increase the memory limit in the job spec or reduce memory usage in the script.` + } + if (reason === 'FailedPostStartHookError') { + return ` → The postStart lifecycle hook failed. Check the hook command and its exit code.` + } + if (reason === 'Error') { + switch (exitCode) { + case 137: + return ` → Exit code 137: process was killed (SIGKILL). Likely OOM or forceful termination.\n Check memory usage and resource limits.` + case 139: + return ` → Exit code 139: segmentation fault (SIGSEGV). The process crashed due to a memory access error.` + case 126: + return ` → Exit code 126: permission denied. The script or binary is not executable.\n Check file permissions inside the container image.` + case 127: + return ` → Exit code 127: command not found. The script or binary does not exist in the container.\n Check the image contents and the command/entrypoint configuration.` + default: + return ` → Script or process exited with a non-zero code (${exitCode ?? 'unknown'}).\n Check the step output above for error messages.\n Common causes: script logic errors, missing dependencies, unhandled exceptions.` + } + } + return ` → Check pod logs with: kubectl logs -c ${reason}` +} + export function getContainerTerminatedErrors(pod: k8s.V1Pod): string[] { const errors: string[] = [] const unrecoverableReasons = getUnrecoverableTerminatedReasons() @@ -921,37 +969,32 @@ export function getContainerTerminatedErrors(pod: k8s.V1Pod): string[] { 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) + const hint = `\n${getTerminatedReasonHint(terminated.reason, terminated.exitCode)}` + errors.push(`${reason}${detail}${hint}`) } } 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. -// -// PodScheduled=False/Unschedulable: no node matched the pod's requirements -// (nodeSelector mismatch, insufficient resources). The scheduler sets this -// condition almost immediately and continues retrying. Self-healing IS -// possible (a node might free up), so this is a fallback used only when -// getPodEventErrors returns nothing (events RBAC unavailable). When events -// ARE available, the FailedScheduling event fires first and gates by count. -// -// Never throws; on any unexpected error it returns []. -export function getPodConditionErrors(pod: k8s.V1Pod): string[] { - const errors: string[] = [] - for (const cond of pod.status?.conditions ?? []) { - if ( - cond.type === 'PodScheduled' && - cond.status === 'False' && - cond.reason === 'Unschedulable' - ) { - const msg = cond.message ? `\n ${cond.message}` : '' - errors.push(` ✗ condition: ${cond.type}=False (${cond.reason})${msg}`) - } +function getEventReasonHint(reason: string): string { + switch (reason) { + case 'FailedMount': + return [ + ` → A volume could not be mounted. Check:`, + ` - PVC is bound (kubectl get pvc)`, + ` - Secret/ConfigMap referenced in the volume exists`, + ` - hostPath directories exist on the scheduled node`, + ].join('\n') + case 'FailedBinding': + return [ + ` → A PVC could not be bound to a PV. Check:`, + ` - StorageClass exists and has a provisioner`, + ` - Sufficient capacity is available`, + ` - Access mode (ReadWriteOnce/ReadWriteMany) matches available PVs`, + ].join('\n') + default: + return ` → Check pod events with: kubectl describe pod ` } - return errors } // Inspects the pod's Warning events for reasons in UNRECOVERABLE_EVENT_REASONS @@ -995,8 +1038,9 @@ export async function getPodEventErrors(podName: string): Promise { seenReasons.add(e.reason) const count = e.count && e.count > 1 ? ` (x${e.count})` : '' const reason = ` ✗ event: ${e.reason}${count}` - const detail = e.message ? ` ${e.message}` : '' - errors.push(detail ? `${reason}\n${detail}` : reason) + const detail = e.message ? `\n ${e.message}` : '' + const hint = getEventReasonHint(e.reason) + errors.push(`${reason}${detail}\n${hint}`) } return errors } @@ -1127,6 +1171,16 @@ async function describePodWarningEvents(podName: string): Promise { return lines } +// Inspects the pod's status.conditions for scheduling failures. FailedScheduling +// is intentionally excluded from fast-fail detection (see UNRECOVERABLE_EVENT_REASONS +// comment) — scheduling failures are transient resource waits that should queue +// until the timeout, not terminate early. This function always returns [] as a +// result, but is kept so checkUnrecoverableErrors compiles and the dedup logic +// remains intact for future use. +export function getPodConditionErrors(_pod: k8s.V1Pod): string[] { + return [] +} + // Aggregates the three independent error-detection sources (container waiting // reasons, pod Warning events, and pod conditions) into a single list, applying // the cross-source deduplication rule (FailedScheduling event + Unschedulable @@ -1159,7 +1213,29 @@ export async function waitForPodPhases( const backOffManager = new BackOffManager(maxTimeSeconds) let phase: PodPhase = PodPhase.UNKNOWN while (true) { - const pod = await readPod(podName) + let pod: k8s.V1Pod + try { + pod = await readPod(podName) + } catch (err) { + // Transient API error (network blip, API server busy): log and back off + // rather than crashing the loop. The timeout will eventually fire if the + // pod stays permanently unreadable (e.g. RBAC issue). + core.warning( + `[waitForPodPhases] Could not read pod ${podName}, will retry after backoff: ${ + err instanceof Error ? err.message : String(err) + }` + ) + try { + await backOffManager.backOff() + } catch { + throw new Error( + `Pod ${podName} timed out after ${maxTimeSeconds}s (pod read failed: ${ + err instanceof Error ? err.message : String(err) + })\n${'─'.repeat(60)}\n(pod was unreadable; no further diagnostics available)` + ) + } + continue + } phase = parsePodPhase(pod) if (awaitingPhases.has(phase)) { return diff --git a/packages/k8s/tests/wait-for-pod-phases-test.ts b/packages/k8s/tests/wait-for-pod-phases-test.ts index 5cc0ff38..ac13272d 100644 --- a/packages/k8s/tests/wait-for-pod-phases-test.ts +++ b/packages/k8s/tests/wait-for-pod-phases-test.ts @@ -111,18 +111,19 @@ describe('getContainerErrors', () => { expect(getContainerErrors(pod)).toEqual([]) }) - it('detects every unrecoverable waiting reason', () => { + it('detects every unrecoverable waiting reason and includes a hint', () => { for (const reason of Array.from(UNRECOVERABLE_WAITING_REASONS)) { const pod = buildPod(PodPhase.PENDING, { containerStatuses: [waitingContainer('job', reason)] }) - expect(getContainerErrors(pod)).toEqual([ - ` ✗ container "job": ${reason}` - ]) + const errors = getContainerErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(` ✗ container "job": ${reason}`) + expect(errors[0]).toContain('→') } }) - it('includes the waiting message as an indented second line', () => { + it('includes the waiting message and a network/registry hint for ErrImagePull', () => { const pod = buildPod(PodPhase.PENDING, { containerStatuses: [ waitingContainer( @@ -132,9 +133,12 @@ describe('getContainerErrors', () => { ) ] }) - expect(getContainerErrors(pod)).toEqual([ - ' ✗ container "job": ErrImagePull\n Back-off pulling image "does-not-exist:latest"' - ]) + const errors = getContainerErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ container "job": ErrImagePull') + expect(errors[0]).toContain('Back-off pulling image "does-not-exist:latest"') + expect(errors[0]).toContain('registry') + expect(errors[0]).toContain('network') }) it('inspects init containers as well as regular containers', () => { @@ -142,10 +146,10 @@ describe('getContainerErrors', () => { initContainerStatuses: [waitingContainer('init', 'ImagePullBackOff')], containerStatuses: [waitingContainer('job', 'CreateContainerError')] }) - expect(getContainerErrors(pod)).toEqual([ - ' ✗ container "init": ImagePullBackOff', - ' ✗ container "job": CreateContainerError' - ]) + const errors = getContainerErrors(pod) + expect(errors).toHaveLength(2) + expect(errors[0]).toContain(' ✗ container "init": ImagePullBackOff') + expect(errors[1]).toContain(' ✗ container "job": CreateContainerError') }) it('ignores running/terminated containers and only collects waiting errors', () => { @@ -155,9 +159,9 @@ describe('getContainerErrors', () => { waitingContainer('bad', 'InvalidImageName') ] }) - expect(getContainerErrors(pod)).toEqual([ - ' ✗ container "bad": InvalidImageName' - ]) + const errors = getContainerErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ container "bad": InvalidImageName') }) }) @@ -190,7 +194,7 @@ describe('getPodEventErrors', () => { expect(await getPodEventErrors('my-pod')).toEqual([]) }) - it('detects FailedMount (the hostPath Directory missing case)', async () => { + it('detects FailedMount (the hostPath Directory missing case) and includes hint', async () => { eventSpy.mockResolvedValue( eventResult([ buildEvent( @@ -200,24 +204,37 @@ describe('getPodEventErrors', () => { ) ]) ) - expect(await getPodEventErrors('my-pod')).toEqual([ - ' ✗ event: FailedMount (x3)\n Unable to attach or mount volume "bad-hostpath": mount path "/this/path/does/not/exist" does not exist' - ]) + const errors = await getPodEventErrors('my-pod') + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ event: FailedMount (x3)') + expect(errors[0]).toContain('does not exist') + expect(errors[0]).toContain('PVC') }) - it('detects every unrecoverable event reason', async () => { + it('ignores FailedScheduling events (scheduling is always treated as queuing)', async () => { + // FailedScheduling is not in UNRECOVERABLE_EVENT_REASONS — never fast-fails. + eventSpy.mockResolvedValue( + eventResult([ + buildEvent('FailedScheduling', '0/3 nodes are available: 3 Insufficient nvidia.com/gpu.'), + buildEvent('FailedScheduling', "0/3 nodes are available: 3 node(s) didn't match Pod's node affinity/selector.") + ]) + ) + expect(await getPodEventErrors('my-pod')).toEqual([]) + }) + + it('detects FailedBinding and FailedMount (always unrecoverable) and includes hints', async () => { eventSpy.mockResolvedValue( eventResult([ - buildEvent('FailedScheduling', '0/1 nodes are available'), buildEvent('FailedBinding', 'no persistent volumes available'), buildEvent('FailedMount', 'volume not found') ]) ) - expect(await getPodEventErrors('my-pod')).toEqual([ - ' ✗ event: FailedScheduling\n 0/1 nodes are available', - ' ✗ event: FailedBinding\n no persistent volumes available', - ' ✗ event: FailedMount\n volume not found' - ]) + const errors = await getPodEventErrors('my-pod') + expect(errors).toHaveLength(2) + expect(errors[0]).toContain(' ✗ event: FailedBinding') + expect(errors[0]).toContain('StorageClass') + expect(errors[1]).toContain(' ✗ event: FailedMount') + expect(errors[1]).toContain('PVC') }) it('ignores Normal-type events even if the reason matches', async () => { @@ -235,20 +252,23 @@ describe('getPodEventErrors', () => { ]) ) // Only the first occurrence is kept. - expect(await getPodEventErrors('my-pod')).toEqual([ - ' ✗ event: FailedMount\n first attempt' - ]) + const errors = await getPodEventErrors('my-pod') + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ event: FailedMount') + expect(errors[0]).toContain('first attempt') }) - it('honors extra reasons from the env var', async () => { + it('honors extra reasons from the env var and includes default hint', async () => { process.env['ACTIONS_RUNNER_K8S_UNRECOVERABLE_EVENT_REASONS'] = 'FailedPreStopHook' eventSpy.mockResolvedValue( eventResult([buildEvent('FailedPreStopHook', 'hook failed')]) ) - expect(await getPodEventErrors('my-pod')).toEqual([ - ' ✗ event: FailedPreStopHook\n hook failed' - ]) + const errors = await getPodEventErrors('my-pod') + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ event: FailedPreStopHook') + expect(errors[0]).toContain('hook failed') + expect(errors[0]).toContain('kubectl describe pod') }) it('degrades gracefully when listing events is forbidden', async () => { @@ -285,9 +305,9 @@ describe('getUnrecoverableWaitingReasons', () => { const pod = buildPod(PodPhase.PENDING, { containerStatuses: [waitingContainer('job', 'CrashLoopBackOff')] }) - expect(getContainerErrors(pod)).toEqual([ - ' ✗ container "job": CrashLoopBackOff' - ]) + const errors = getContainerErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ container "job": CrashLoopBackOff') }) }) @@ -337,37 +357,57 @@ describe('getContainerTerminatedErrors', () => { expect(getContainerTerminatedErrors(pod)).toEqual([]) }) - it('detects OOMKilled', () => { + it('detects OOMKilled and includes hint', () => { 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' - ]) + const errors = getContainerTerminatedErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ container "job": OOMKilled (exit code 137)') + expect(errors[0]).toContain('The node was low on resource: memory') + expect(errors[0]).toContain('memory limit') }) - it('detects Error (exit non-zero)', () => { + it('detects Error exit code 1 and includes generic hint', () => { const pod = buildPod(PodPhase.FAILED, { - containerStatuses: [ - terminatedContainer('job', 'Error', 1) - ] + containerStatuses: [terminatedContainer('job', 'Error', 1)] + }) + const errors = getContainerTerminatedErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ container "job": Error (exit code 1)') + expect(errors[0]).toContain('non-zero code (1)') + }) + + it('detects Error exit code 137 and includes SIGKILL hint', () => { + const pod = buildPod(PodPhase.FAILED, { + containerStatuses: [terminatedContainer('job', 'Error', 137)] }) - expect(getContainerTerminatedErrors(pod)).toEqual([ - ' ✗ container "job": Error (exit code 1)' - ]) + const errors = getContainerTerminatedErrors(pod) + expect(errors[0]).toContain('exit code 137') + expect(errors[0]).toContain('SIGKILL') }) - it('detects FailedPostStartHookError', () => { + it('detects Error exit code 127 and includes command-not-found hint', () => { + const pod = buildPod(PodPhase.FAILED, { + containerStatuses: [terminatedContainer('job', 'Error', 127)] + }) + const errors = getContainerTerminatedErrors(pod) + expect(errors[0]).toContain('command not found') + }) + + it('detects FailedPostStartHookError and includes hint', () => { 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' - ]) + const errors = getContainerTerminatedErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ container "job": FailedPostStartHookError (exit code 137)') + expect(errors[0]).toContain('postStart hook failed') + expect(errors[0]).toContain('postStart lifecycle hook') }) it('detects every unrecoverable terminated reason', () => { @@ -375,9 +415,9 @@ describe('getContainerTerminatedErrors', () => { const pod = buildPod(PodPhase.FAILED, { containerStatuses: [terminatedContainer('job', reason, 1)] }) - expect(getContainerTerminatedErrors(pod)).toEqual([ - ` ✗ container "job": ${reason} (exit code 1)` - ]) + const errors = getContainerTerminatedErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(` ✗ container "job": ${reason} (exit code 1)`) } }) @@ -386,10 +426,10 @@ describe('getContainerTerminatedErrors', () => { 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)' - ]) + const errors = getContainerTerminatedErrors(pod) + expect(errors).toHaveLength(2) + expect(errors[0]).toContain(' ✗ container "init": Error (exit code 2)') + expect(errors[1]).toContain(' ✗ container "job": OOMKilled (exit code 137)') }) it('ignores terminated with reason not in the whitelist', () => { @@ -428,9 +468,9 @@ describe('getUnrecoverableTerminatedReasons', () => { const pod = buildPod(PodPhase.FAILED, { containerStatuses: [terminatedContainer('job', 'DeadlineExceeded', 1)] }) - expect(getContainerTerminatedErrors(pod)).toEqual([ - ' ✗ container "job": DeadlineExceeded (exit code 1)' - ]) + const errors = getContainerTerminatedErrors(pod) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain(' ✗ container "job": DeadlineExceeded (exit code 1)') }) it('filters empty strings from the env var', () => { @@ -531,11 +571,14 @@ describe('waitForPodPhases', () => { ) }) - it('fast-fails on FailedScheduling events', async () => { - readSpy.mockResolvedValue(podResult(buildPod(PodPhase.PENDING))) - eventSpy.mockResolvedValue( - eventResult([buildEvent('FailedScheduling', '0/1 nodes are available')]) - ) + it('retries on transient readPod failure and recovers when pod becomes ready', async () => { + // Risk A fix: a transient readPod error must NOT crash the loop immediately. + // Pod read fails twice, then returns Running — function must resolve. + readSpy + .mockRejectedValueOnce(new Error('connection refused') as never) + .mockRejectedValueOnce(new Error('connection refused') as never) + .mockResolvedValue(podResult(buildPod(PodPhase.RUNNING))) + // eventSpy already returns [] from beforeEach await expect( waitForPodPhases( @@ -543,7 +586,7 @@ describe('waitForPodPhases', () => { new Set([PodPhase.RUNNING]), new Set([PodPhase.PENDING]) ) - ).rejects.toThrow(/event: FailedScheduling[\s\S]*0\/1 nodes are available/) + ).resolves.toBeUndefined() }) it('throws with the phase when the pod is in a non-backoff phase', async () => {