feat: surface container errors and fix runner for ARC kubernetes-novolume mode#16
Merged
Longwt123 merged 18 commits intoJun 30, 2026
Merged
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>
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. |
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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces detailed pod failure diagnostics and fast-fail capabilities for unrecoverable container errors (such as ImagePullBackOff) when waiting for runner pods, along with in-cluster namespace detection from the ServiceAccount file. Feedback focuses on optimizing the Dockerfile using the --chown flag in the COPY instruction, caching the detected namespace to avoid redundant synchronous file reads, passing the pre-fetched pod object to describePodFailure to prevent duplicate API calls, and using optional chaining when accessing event items to prevent potential runtime errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
- 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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
- 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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
- 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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
…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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
…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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
…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>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
Collaborator
Author
|
/check-pr |
Merge Verification Failed@Longwt123, this pr is not mergeable and the reasons are below: Label BlockList
Not Enough Labels
Label Usage Tips
|
|
/lgtm |
Review Code Feedback
Tips
|
Review Code Feedback
Tips
|
|
/lgtm |
Review Code Feedback
Tips
|
Collaborator
Author
|
/check-pr |
Merge Verification Failed@Longwt123, this pr is not mergeable and the reasons are below: Label BlockList
Not Enough Labels
Label Usage Tips
|
Collaborator
Author
|
/check-pr |
Merge Verification Failed@Longwt123, this pr is not mergeable and the reasons are below: Label BlockList
Not Enough Labels
Label Usage Tips
|
Collaborator
Author
|
/check-pr |
Merge Verification Failed@Longwt123, this pr is not mergeable and the reasons are below: Not Enough Labels
Label Usage Tips
|
Collaborator
Author
|
/retest |
1 similar comment
Collaborator
Author
|
/retest |
|
|||||||||||||||||
…comment Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
CLA Signature PassLongwt123, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
|
|||||||||||||||||
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
release/no_volumes(more complete thanmain— includes init-container, no-volumes, non-root support)release/no_volumesChanges
packages/k8s/src/k8s/index.ts:UNRECOVERABLE_WAITING_REASONS: surfaceImagePullBackOff,ErrImagePull, etc. as immediate errors instead of waiting for timeoutgetContainerErrors()/describePodFailure(): detailed container failure diagnosticswaitForPodPhases(): check container errors on each poll loop iteration, improve error message formatnamespace(): add in-cluster fallback — reads/var/run/secrets/kubernetes.io/serviceaccount/namespacewhen kubeconfig context has no namespace (fixes silent failure in ARC ephemeral runner setup)readPod(): extracted fromgetPodPhase()to allow returning the full pod object for error inspectionDockerfile:ghcr.nju.edu.cnmirror for China cluster access--platform=linux/amd64to ensure amd64 image regardless of build host archUSER root+chown runner:runnerbefore switching back toUSER runnerTest plan
linux-aarch64-test-hookrunner setascend-gha-runnersnamespacenamespace()works withoutACTIONS_RUNNER_KUBERNETES_NAMESPACEenv var (reads SA file)🤖 Generated with Claude Code
resolve https://github.com/opensourceways/backlog/issues/1102