Skip to content

[需求] CI容器通用化k8s错误反馈机制-补充优化-的开发实现-runner-container-hooks部分#19

Closed
flysky22222 wants to merge 5 commits into
mainfrom
issue-1203-from-main
Closed

[需求] CI容器通用化k8s错误反馈机制-补充优化-的开发实现-runner-container-hooks部分#19
flysky22222 wants to merge 5 commits into
mainfrom
issue-1203-from-main

Conversation

@flysky22222

Copy link
Copy Markdown

背景

[需求] CI容器通用化k8s错误反馈机制-补充优化(runner-container-hooks)· 开发流水线 · 开发预览阶段(代码已推 + 预览已部署 + UT 已补;门禁/对抗由 PR CI 异步跑)

改动内容

fix(runner-container-hooks): add describePodFailure diagnostic output to Running-phase terminated error messages

Changes

packages/k8s/src/hooks/run-container-step.ts

  • Added describePodFailure to imports from '../k8s' (line 7)
  • Added const details = await describePodFailure(podName) call before core.error (line 116)
  • Appended \n${details} to the core.error message string (line 118)

Bug fixed: Per design §2.3 #11, Running-phase terminated errors (OOMKilled, Error, FailedPostStartHookError) previously only showed a one-line summary without the full diagnostic output (phase, ✗ markers, conditions, events). Now describePodFailure(podName) is called and its output is included in the logged error, providing complete diagnostic information consistent with how Pending-phase errors are handled in waitForPodPhases.

packages/k8s/tests/run-container-step-test.ts

  • Added import * as k8sModule from '../src/k8s' and import * as coreModule from '@actions/core' for jest.spyOn
  • Added import { RunContainerStepArgs } from 'hooklib' for test data construction
  • Added new describe block "runContainerStep includes describePodFailure output for terminated errors" with:
    • Helper podStatusWithTerminated() to construct V1PodStatus with terminated containers
    • Helper makeMinimalArgs() to construct RunContainerStepArgs without env vars
    • jest.spyOn mocks for all k8s module functions called by runContainerStep (createJob, getContainerJobPodName, waitForPodPhases, getPodLogs, waitForJobToComplete, getPodStatus, getPodByName, describePodFailure)
    • jest.spyOn on @actions/core.error to capture error messages
    • 3 test cases:
      1. OOMKilled container — verifies describePodFailure is called and output appears in core.error message
      2. Error (exit non-zero) container — same verification
      3. FailedPostStartHookError container — same verification

UT Coverage

  • New tests cover: describePodFailure call + output inclusion in core.error for all 3 Running-phase terminated error types (OOMKilled, Error, FailedPostStartHookError)
  • All 3 new tests pass; existing getContainerTerminatedErrors and getUnrecoverableTerminatedReasons tests unaffected

相关 Issue

resolve https://github.com/opensourceways/backlog/issues/1203

AI 使用声明

当前 PR 是否有 AI 参与:

Longwt123 and others added 3 commits July 1, 2026 06:34
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 describePodFailure to aggregate pod
phase, conditions, container statuses and Warning events into a single
diagnostic string. describePodWarningEvents provides best-effort retrieval of
recent Warning events (requires optional events RBAC permission, degrades
gracefully when missing). getUnrecoverableWaitingReasons allows operators to
extend the built-in fast-fail whitelist via the
ACTIONS_RUNNER_K8S_UNRECOVERABLE_WAITING_REASONS env var.

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
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: 19 new test cases covering parsePodPhase, getContainerErrors,
waitForPodPhases, getUnrecoverableWaitingReasons, describePodFailure,
describePodWarningEvents, and edge cases such as forbidden events API and
unreadable pods.
@opensourceways-bot

Copy link
Copy Markdown

Welcome To opensourceways Community

Hey @flysky22222 , thanks for your contribution to the community.

Bot Usage Manual

I'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 Guide

If you have any questions, please contact the SIG: infratructure ,
and any of the maintainers: @GeorgeCao-hw, @TangJia025, @pkking, @zhongjun2 ,
and any of the committers: @GeorgeCao-hw, @Goalina, @Hourunze1997, @JavaPythonAIForBAT, @KadenZhang3321, @LiYanghang00, @ccijunk, @drizzlezyk, @githubliuyang777, @pkking, @rosecoffe, @tfhddd, @yao-xiaobai, @zhongjun2, @zkhzkhz .

@opensourceways-bot

Copy link
Copy Markdown

CLA Signature Pass

flysky22222, thanks for your pull request. All authors of the commits have signed the CLA. 👍

@socket-security

socket-security Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedhooklib@​0.1.000000

View full report

@opensourceways-bot

Copy link
Copy Markdown
检查项 状态
敏感信息扫描
安全编码扫描
漏洞扫描
开源license合规扫描
UT测试覆盖率
开发阶段设计文档检查
流水线链接 点击跳转查看日志

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances Kubernetes runner pod diagnostics and fail-fast mechanisms by tracking unrecoverable container states, pod conditions, and warning events, alongside adding comprehensive unit tests. The review feedback highlights several critical issues: first, runContainerStep should return early with exit code 1 upon detecting terminated errors to avoid returning undefined; second, getPodConditionErrors must not treat all False conditions as unrecoverable, as Ready=False is expected during startup; third, prunePods in the catch block of prepareJob should be wrapped in a try-catch to prevent shadowing the original error; fourth, immediately failing on transient events like FailedScheduling or FailedMount is risky and should be avoided; and finally, FailedMount is not a container waiting reason and should be removed from UNRECOVERABLE_WAITING_REASONS.

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.

Comment on lines +115 to +120
if (terminatedErrors.length > 0) {
const details = await describePodFailure(podName)
core.error(
`Pod ${podName} has unrecoverable container errors:\n${terminatedErrors.join('\n')}\n${details}`
)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

严重 Bug:如果 init 容器或非最后一个容器发生未恢复的终止错误(例如 init 容器被 OOMKilled),terminatedErrors 会捕获到该错误并记录。然而,此时主容器可能尚未运行,导致 status.containerStatuses 中最后一个容器的 terminated?.exitCodeundefined。\n\n如果直接返回 exitCode(即 undefined),调用者可能会误认为步骤执行成功。\n\n建议在检测到 terminatedErrors 时,直接返回 1(或非零状态码),以确保步骤正确标记为失败。

Suggested change
if (terminatedErrors.length > 0) {
const details = await describePodFailure(podName)
core.error(
`Pod ${podName} has unrecoverable container errors:\n${terminatedErrors.join('\n')}\n${details}`
)
}
if (terminatedErrors.length > 0) {
const details = await describePodFailure(podName)
core.error(
`Pod ${podName} has unrecoverable container errors:\n${terminatedErrors.join('\n')}\n${details}`
)
return 1
}

Comment on lines +494 to +506
export function getPodConditionErrors(pod: k8s.V1Pod): string[] {
const errors: string[] = []
for (const cond of pod.status?.conditions ?? []) {
if (cond.status === 'False') {
errors.push(
`Condition ${cond.type}=False (reason: ${cond.reason ?? ''}): ${
cond.message ?? ''
}`
)
}
}
return errors
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

严重 Bug:在 getPodConditionErrors 中,任何状态为 False 的 Pod Condition 都会被视为不可恢复的错误。\n\n在 Pod 刚创建并处于 Pending 阶段时,ReadyContainersReady 条件的状态必然是 False(因为容器还在启动中)。这会导致 waitForPodPhases 在第一次轮询时就因为检测到 Ready=False 而直接抛出异常并失败,使得所有 Pod 都无法正常启动。\n\n建议修改 getPodConditionErrors,只将特定的、真正代表不可恢复错误的条件(例如 PodScheduled=FalsereasonUnschedulable)视为错误。

Suggested change
export function getPodConditionErrors(pod: k8s.V1Pod): string[] {
const errors: string[] = []
for (const cond of pod.status?.conditions ?? []) {
if (cond.status === 'False') {
errors.push(
`Condition ${cond.type}=False (reason: ${cond.reason ?? ''}): ${
cond.message ?? ''
}`
)
}
}
return errors
}
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') {
errors.push(
`Condition ${cond.type}=False (reason: ${cond.reason ?? ''}): ${
cond.message ?? ''
}`
)
}
}
return errors
}

Comment thread packages/k8s/src/hooks/prepare-job.ts Outdated
Comment on lines +102 to +104
await prunePods()
throw new Error(`pod failed to come online with error: ${err}`)
const message = err instanceof Error ? err.message : String(err)
throw new Error(`pod failed to come online:\n${message}`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

catch 块中,await prunePods() 可能会因为 K8s API 调用失败等原因抛出异常。如果它抛出异常,原本的 err 将会被掩盖(shadowed),导致排查问题时无法获取到真正的 pod failed to come online 错误。\n\n建议将 await prunePods() 包裹在 try-catch 中,并记录可能发生的错误,以确保原始错误能够被正确抛出。

    try {
      await prunePods()
    } catch (pruneErr) {
      core.error(`Failed to prune pods: ${pruneErr instanceof Error ? pruneErr.message : String(pruneErr)}`)
    }
    const message = err instanceof Error ? err.message : String(err)
    throw new Error(`pod failed to come online:\n${message}`)

Comment thread packages/k8s/src/k8s/index.ts Outdated
Comment on lines +686 to +694
const eventErrors = await getPodEventErrors(podName)
if (eventErrors.length > 0) {
const details = await describePodFailure(podName)
throw new Error(
`Pod ${podName} has unrecoverable event errors: ${eventErrors.join(
'; '
)}\n${details}`
)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

高风险设计问题:在 waitForPodPhases 中,检测到任何 FailedSchedulingFailedBindingFailedMount 警告事件时,会立即中断等待并报错失败。\n\n然而,在实际的 Kubernetes 集群中,这些警告事件往往是暂时的(transient):\n1. FailedScheduling:当集群正在进行弹性伸缩(Autoscaling)时,新节点启动需要 1-2 分钟,期间会产生 FailedScheduling 事件。如果立即报错,将导致弹性伸缩功能失效。\n2. FailedMount / FailedBinding:云硬盘挂载或 PVC 绑定通常需要几秒到几十秒的时间,期间产生这些警告是正常现象,随后会自动恢复。\n\n建议不要在轮询过程中因为这些事件而立即失败(fail-fast)。这些事件应该仅在超时(Timeout)发生后,作为诊断信息(describePodFailure)附加到错误日志中,而不是作为提前终止的触发条件。

Comment on lines +371 to +378
export const UNRECOVERABLE_WAITING_REASONS = new Set([
'ImagePullBackOff',
'ErrImagePull',
'InvalidImageName',
'CreateContainerConfigError',
'CreateContainerError',
'FailedMount'
])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

UNRECOVERABLE_WAITING_REASONS 中包含了 'FailedMount'。然而,在 Kubernetes 中,FailedMount 是一个 Pod 事件(Event)或 Condition 的 Reason,而不是容器的等待原因(Container Waiting Reason)。\n\n当卷挂载失败时,容器通常会保持在 waiting 状态,其 reason 为 ContainerCreating。因此,将 FailedMount 放在 UNRECOVERABLE_WAITING_REASONS 中是无效的,不会被触发。建议将其移除以避免混淆。

export const UNRECOVERABLE_WAITING_REASONS = new Set([
  'ImagePullBackOff',
  'ErrImagePull',
  'InvalidImageName',
  'CreateContainerConfigError',
  'CreateContainerError'
])

@opensourceways-bot

Copy link
Copy Markdown

CLA Signature Pass

flysky22222, thanks for your pull request. All authors of the commits have signed the CLA. 👍

@opensourceways-bot

Copy link
Copy Markdown
检查项 状态
敏感信息扫描
安全编码扫描
漏洞扫描
开源license合规扫描
UT测试覆盖率
开发阶段设计文档检查
流水线链接 点击跳转查看日志

@opensourceways-bot

Copy link
Copy Markdown

CLA Signature Pass

flysky22222, thanks for your pull request. All authors of the commits have signed the CLA. 👍

@opensourceways-bot

Copy link
Copy Markdown
检查项 状态
敏感信息扫描
安全编码扫描
漏洞扫描
开源license合规扫描
UT测试覆盖率
开发阶段设计文档检查
流水线链接 点击跳转查看日志

@flysky22222

Copy link
Copy Markdown
Author

Closing this PR as requested by @Longwt123 — will recreate from release/no_volumes branch instead of main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants