feat(k8s): surface container errors and fast-fail on unrecoverable pod errors (runner 2.337.0) - #51
Open
Longwt123 wants to merge 66 commits into
Open
feat(k8s): surface container errors and fast-fail on unrecoverable pod errors (runner 2.337.0)#51Longwt123 wants to merge 66 commits into
Longwt123 wants to merge 66 commits into
Conversation
When a CI job references a non-existent image or one it lacks permission to pull, the pod stays in Pending and waitForPodPhases previously timed out with only a generic phase-status message. GitHub Actions users had no indication of the real cause. Detect unrecoverable container waiting reasons (ImagePullBackOff, ErrImagePull, InvalidImageName, CreateContainerConfigError, CreateContainerError) on both init and regular containers, and fail fast with the container name, reason, and Kubernetes message so the error is visible in the Actions log. Refactor getPodPhase into readPod + parsePodPhase so the pod object can be inspected for container errors, and add unit tests covering parsePodPhase, getContainerErrors, and waitForPodPhases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r reasons
Extend the error feedback mechanism introduced in the previous commit with:
- describePodFailure(): aggregates pod phase, conditions, container statuses
and Warning events into a single human-readable diagnostic string.
Never throws — safe to call from any error path.
- describePodWarningEvents(): best-effort retrieval of recent Warning K8s
events (requires optional "events" RBAC permission). Degrades gracefully
when the permission is missing.
- getUnrecoverableWaitingReasons(): allows operators to extend the built-in
fast-fail whitelist via the ACTIONS_RUNNER_K8S_UNRECOVERABLE_WAITING_REASONS
environment variable without a code change. Built-in defaults cannot be
removed.
- All three failure paths in waitForPodPhases now attach full diagnostics:
1. Non-backoff phase (e.g. Failed) — includes pod details + events
2. Unrecoverable container error (e.g. ImagePullBackOff) — fail-fast with
diagnostics instead of waiting for timeout
3. Timeout — includes pod details so the user can see WHY the pod never
became ready
- README: document the optional "events" permission and the new env var.
- Tests: 8 new test cases (19 total) covering describePodFailure,
describePodWarningEvents, getUnrecoverableWaitingReasons, and edge cases
such as forbidden events API and unreadable pods.
The project pins prettier@2.6.2 in package-lock.json, but the previous commit was formatted with prettier 3.x which has different line-wrapping rules for template literals. Re-format with prettier 2.6.2 to pass CI.
v2.329.0 was deprecated by GitHub and rejected at the broker level with "Runner version v2.329.0 is deprecated and cannot receive messages.", causing runner pods to crash-loop immediately after connecting. Also fix Dockerfile layer ordering: switch to root before COPY so that the subsequent chown is not run as the unprivileged runner user. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- namespace() now reads /var/run/secrets/kubernetes.io/serviceaccount/namespace as fallback when kubeconfig context has no namespace (in-cluster ARC setup) - Dockerfile: add --platform=linux/amd64 to runner base image stage Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The release/no_volumes base uses @kubernetes/client-node v0.22+ where API methods take an options object and return the resource directly (no .body wrapper). Fix describePodWarningEvents() accordingly. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Remove --platform=linux/amd64: Jenkins build host is aarch64, without the flag the final image is ARM64, matching the working release/no_volumes build - Revert to ghcr.io/actions/actions-runner:2.334.0 (same as release/no_volumes); the nju.edu.cn mirror at 2.335.1 caused silent 0-second exit with no logs (likely corrupted/wrong image) - Remove USER root + chown (not needed, matches release/no_volumes) - Update nodeSelector to arm64 to match ARM64 image architecture Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- describePodFailure(): skip container waiting reasons already in UNRECOVERABLE_WAITING_REASONS (they appear in the caller's first line via getContainerErrors, printing them again is redundant) - describePodFailure(): suppress terminated containers with exitCode=0 (e.g. fs-init Completed) -- successful init containers are noise - Add tao/ to .gitignore; remove accidentally committed values file Result: InvalidImageName/ErrImagePull now shows as one clean block: Pod <name> has unrecoverable container errors: container "job": ErrImagePull - <message> Phase: Pending Condition Ready=False ... Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Layer the error output into clearly separated sections:
Pod <name> has unrecoverable container errors:
✗ container "job": ErrImagePull
Error response from daemon: manifest for ... not found
────────────────────────────────────────────────────────────
Pod status: Pending
✗ Ready=False (ContainersNotReady): containers with ...
✗ ContainersReady=False (ContainersNotReady): ...
Recent warning events:
[Failed] (x3) Error response from daemon: ...
Changes:
- getContainerErrors(): format each entry as indented ✗ lines with
message detail on a separate indented line
- describePodFailure(): group output into sections (pod status,
container details, warning events) separated by blank lines
- waitForPodPhases(): add ─── separator between summary and details
- describePodWarningEvents(): indent event lines consistently
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- prepare-job.ts: use err.message instead of String(err) to avoid
double "Error: Error:" prefix; change separator to newline so the
multi-section detail block renders on its own lines:
pod failed to come online:
Pod <name> has unrecoverable container errors:
✗ container "job": ErrImagePull
<message>
────────────────────────────────────
Pod status: Pending
✗ Ready=False ...
- .gitignore: fix corrupted line (tao/ was appended without newline
to test-kind.yaml entry); restore original content, drop tao/
(the values file should not be tracked in this repo)
- Dockerfile: restore to byte-identical match with release/no_volumes
(only had trailing-newline diff, no content change)
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Add event-based fast-fail detection alongside the existing container waiting-reason check in waitForPodPhases(). Problem: some failures never surface in container.status.state.waiting .reason — the container stays stuck in ContainerCreating — so the hook polled until the 3600s timeout. Examples: - FailedMount: hostPath directory missing, missing Secret/ConfigMap volume - FailedScheduling: no schedulable node (resource/nodeSelector/affinity) - FailedBinding: PVC cannot bind (no PV, wrong storageClass) Changes: - UNRECOVERABLE_EVENT_REASONS: new exported Set with the three reasons above - getUnrecoverableEventReasons(): mirrors getUnrecoverableWaitingReasons(), extended at runtime via ACTIONS_RUNNER_K8S_UNRECOVERABLE_EVENT_REASONS env var (comma-separated, additive) - getPodEventErrors(): queries the event API for Warning events whose reason is in the unrecoverable set; deduplicates by reason; gracefully degrades to [] when the optional events RBAC permission is missing - waitForPodPhases(): call getPodEventErrors() alongside getContainerErrors() each poll; fail fast when either returns results - Error message updated: "unrecoverable container errors" → "unrecoverable errors" to cover both detection paths - Tests: add coverage for FailedMount/FailedScheduling/FailedBinding event detection, env-var extension, RBAC degradation, and deduplication Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…rors
The k8s client throws HttpException with a multi-line message:
HTTP-Code: 422
Message: Unknown API Status Code!
Body: "{\"message\":\"Pod is invalid: ...\"}"
Parse the "message" field from the embedded JSON body so the log shows:
failed to create job pod:
Pod "xxx" is invalid: spec.volumes[5].name: Duplicate value: "bad-hostpath"
Falls back to the raw string if parsing fails (e.g. non-422 errors or
network errors that don't have a JSON body).
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The dotAll flag (s) requires ES2018+. The JSON body is single-line so the flag is unnecessary — remove it to restore build compatibility. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Replace the broken regex approach with boundary-based extraction: 1. Find 'Body: "' and '"\nHeaders:' delimiters in the raw dump 2. Unescape the JSON string (\" → " and \ → \) 3. JSON.parse and return parsed.message Verified against the actual 422 response format produced by the k8s client (Body contains a JSON-encoded string with \" escaping inside). Falls back to the full raw string on any parse failure. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The previous '"\nHeaders:' boundary failed when the newline between
Body and Headers was not a real newline in the error string. Instead,
find 'Headers:' first and then use lastIndexOf('"') to locate the
closing quote of the Body value — more robust against whitespace
variations in the HttpException dump format.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ing errors
getPodConditionErrors() checks pod.status.conditions without needing
the optional 'events' list RBAC permission:
- PodScheduled=False (Unschedulable): nodeSelector mismatch, resource
requests exceed limits, no matching node
In waitForPodPhases():
- eventErrors (events-API based) is tried first; it's richer (carries
event.count and detailed message)
- conditionErrors is only added when eventErrors is empty, i.e. when
the events RBAC permission is absent — avoids duplicate output
This means FailedScheduling fast-fail now works in both cases:
- RBAC allows events → FailedScheduling event detected
- RBAC blocks events → PodScheduled=False condition detected
FailedBinding still relies on the FailedMount event path (the PVC
binding failure eventually surfaces as a FailedMount pod event).
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…mpty The previous logic only called getPodConditionErrors() when getPodEventErrors() returned [] (events RBAC unavailable). This created a race: if events take a few extra seconds to propagate, the first several polls would return eventErrors=[], run conditionErrors (and detect Unschedulable), but later polls (after events appear) would skip conditionErrors even though events may show an unrelated reason. Change: always run both checks each poll iteration. Deduplicate with a simple filter so FailedScheduling (event) + Unschedulable (condition) for the same pod don't both appear in the error output. This ensures PodScheduled=False/Unschedulable is detected reliably even when events RBAC permission is absent, covering: - FailedScheduling (nodeSelector mismatch) - FailedScheduling (resource shortage including NPU) - FailedMount (pod stuck Pending because NPU is busy) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…comment Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…lity Rename utils.ts → utils/index.ts in k8s, docker, and hooklib packages so source files match the CI coverage gate pattern '**/utils/**'. Add collectCoverageFrom config to jest.config.js in k8s and docker packages to enable coverage reporting. All existing imports resolve unchanged via TS module resolution. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Remove collectCoverage: true from jest.config.js (avoid forced coverage on every local run); coverage now opt-in via --coverage flag - Expand collectCoverageFrom to src/**/*.ts (was src/**/utils/**/*.ts) to prevent blind spots in coverage reporting - Add test:coverage script to package.json for explicit CI/local use Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
k8s/tests/k8s-utils-test.ts: - Add writeContainerStepScript: path return, env escaping, invalid workdir, invalid env key - Add prepareJobScript: path return, mkdir content, empty mounts - Add fixArgs: quoted args, single-quoted shell, plain args - Add sleep: resolves after given ms - Add listDirAllCommand: find command content, shell-quoting - Add mergeObjectMeta: label/annotation merge, throws on undefined metadata - Add useKubeScheduler: true/false/unset env var cases docker/tests/utils-test.ts: - Add checkEnvironment: passes with GITHUB_WORKSPACE set, throws when unset hooklib: add jest infrastructure (jest.config.js, jest.setup.js, tsconfig.test.json) and devDependencies (jest, ts-jest, babel-jest) hooklib/tests/utils-test.ts: - Add writeToResponseFile: string/object/null/undefined values, empty path, missing file, sequential appends - Add getInputFromStdin: mock readline, parse JSON from emitted line root package.json: include hooklib in test script Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Add index.spec.ts alongside each utils/index.ts so the CI coverage gate
(vitest, include: **/*.{test,spec}.ts) can discover and run them:
- packages/k8s/src/k8s/utils/index.spec.ts: generateContainerName, fixArgs,
sleep, listDirAllCommand, useKubeScheduler, mergeObjectMeta, writeRunScript,
writeContainerStepScript, prepareJobScript, readExtensionFromFile,
mergeContainerWithOptions, mergePodSpecWithOptions
- packages/docker/src/utils/index.spec.ts: sanitize, fixArgs,
optionsWithDockerEnvs, checkEnvironment
- packages/hooklib/src/utils/index.spec.ts: writeToResponseFile
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…inerStepScript
'/too-short'.split('/').slice(-2) yields ['', 'too-short'] (length 2),
which does not trigger the throw. Use 'tooshort' (no slash) instead:
'tooshort'.split('/').slice(-2) = ['tooshort'] (length 1) → throws.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Revert all changes except:
- packages/{k8s,docker,hooklib}/src/**/utils/index.ts (rename from utils.ts)
- packages/{k8s,docker,hooklib}/src/**/utils/index.spec.ts (vitest spec files)
These two minimal changes are sufficient for the coverage gate:
- utils/index.ts satisfies gate pattern '**/utils/**'
- index.spec.ts satisfies vitest include '**/*.spec.ts'
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
更新 package.json 和 package-lock.json 中的 uuid 依赖版本，以获取最新的安全补丁和功能改进。
…tibility refactor: move utils.ts to utils/index.ts for coverage gate compatibility
…-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)
fix(k8s): FailedScheduling errors for fast-fail and Hint
Waiting reasons: - Remove ErrImagePull: first-attempt pull failure can be a transient TLS timeout or network blip; k8s will retry and promote to ImagePullBackOff - Remove CreateContainerError: runtime errors can be transient (node restart) - Remove FailedMount from waiting reasons: dead code — k8s uses ContainerCreating as the waiting.reason when mounts fail; FailedMount surfaces only as events Event reasons: - Re-add FailedScheduling with PERMANENT_SCHEDULING_PATTERNS whitelist: fast-fail ONLY when message matches affinity/taint mismatch or PVC not found; resource shortages (Insufficient *) continue queuing until timeout New pattern for PVC not found (scheduler refuses to place pod): /persistentvolumeclaim ".+" not found/i Updated hints: ImagePullBackOff now mentions TLS/network in addition to registry and credentials; FailedScheduling event hint added. 58 tests passing. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Use [^"]+ instead of .+ in PVC not found regex to avoid matching across multiple quoted segments and prevent unnecessary backtracking - Cache compiled regex array in getPermanentSchedulingPatterns(), invalidating only when the env var value changes; avoids repeated split + RegExp construction on every FailedScheduling poll iteration Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
fix(k8s): 因网络问题TSL超时，导致的ErrImagePull，应该重试，不应该直接报错
…failures Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…and attempt info Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
tar.extract() appends into the destination directory; stale files that were deleted inside the pod (e.g. git-credentials cleaned up by actions/checkout post-job) remain on the runner side and cause a permanent hash mismatch that no retry can resolve. Clear targetRunnerPath before extraction so the local state mirrors the pod exactly. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…mPod The k8s status callback fires when the pod-side tar process exits, but tar-fs may still be writing buffered data to the local filesystem at that point. Resolving the Promise on writerStream 'finish' instead ensures all files are flushed to disk before the hash verification loop runs, preventing false mismatches where runner has fewer files than pod. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- pod-side sync: append '2>/dev/null || true' so that missing or
restricted sync binary in minimal/distroless images does not cause
the entire tar+chmod sh -c command to fail with a non-zero exit code
- runner-side sync: remove the spawn('sync') call entirely; it was
made redundant by the writerStream 'finish' event fix (222a63b)
which already guarantees all tar-fs writes are flushed to disk
before the hash verification loop starts
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…e data durability fix(k8s): PostCheckout复制时不匹配反复重试add sync after file copy to ensure data durability
…n failures When prepareJob fails (pod creation/startup fails), state.jobPod is not initialized. Subsequent runScriptStep attempts to access null.jobPod, causing TypeError: Cannot read properties of null. This fix adds early validation to throw a clear error when jobPod is null, indicating that prepareJob failed. This prevents cryptic null reference errors and improves debugging. - Add null/undefined check at runScriptStep entry point - Add test cases for null and undefined jobPod scenarios - Error message clearly indicates prepareJob dependency
…ation failures fix: add null check for jobPod in runScriptStep to handle pod creation failures
- prepare-job: format 422 pod creation errors with consistent header, separator, and actionable troubleshooting hints matching the unrecoverable-errors output style. - k8s/index.ts: include getContainerTerminatedErrors in checkUnrecoverableErrors so OOMKilled / non-zero exit failures are reported with their hints during waitForPodPhases. When phase=Failed and terminated errors exist, the error now uses the "has unrecoverable errors" format instead of the generic "is unhealthy" message.
Address code review feedback: - prepare-job.ts: refactor 422 error from single long line to array of strings joined by \n for readability and ESLint max-len compliance. - Replace Unicode box-drawing character (U+2500) with standard hyphens across all hooks (prepare-job, index, run-script-step, run-container-step) for consistent rendering across terminal emulators and log aggregators.
…container hints fix(k8s): 统一错误格式并优化提示信息unify error format and surface terminated container hints
ImagePullBackOff was in UNRECOVERABLE_WAITING_REASONS, so the hook failed the job the moment it appeared (~2nd kubelet pull attempt). During a transient network outage this killed jobs that would have recovered. ImagePullBackOff/ErrImagePull now get a bounded grace window (default 300s, configurable via ACTIONS_RUNNER_K8S_IMAGE_PULL_GRACE_SECONDS): the pull can self-heal within it, and the hook fails only once the window is exceeded without recovery. Waiting messages that positively identify a permanent cause (manifest not found, pull access denied, unauthorized, repository does not exist, ...) still fail immediately. Setting grace to 0 restores the previous instant-fail behavior. Co-Authored-By: Claude <noreply@anthropic.com>
… fail fix(k8s): grace period for image pull failures instead of instant fail
Trivy scan on PR #42 flagged js-yaml 4.3.0 with quadratic CPU consumption vulnerability in !!omap resolution (CVE-2026-59870). Upgrade to 4.3.1.
Welcome To opensourceways CommunityHey @Longwt123 , thanks for your contribution to the community. Bot Usage ManualI'm the Bot here serving you. You can find the instructions on how to interact with me at Here . That means you can comment below every pull request or issue to trigger Bot Commands. Contact GuideIf you have any questions, please contact the SIG: infratructure , |
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
Linking Issue Notice@Longwt123 , the pull request must be linked to at least one issue. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
|||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Included requirements
Validation