fix(controller): handle multi-container pod exit codes#5460
Conversation
Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request introduces support for handling multiple non-zero exit codes from container statuses within a pod by adding an ExitCodes field to the Request struct and updating policy matching logic. The reviewer's feedback consistently recommends representing these exit codes as a slice of integers ([]int32) rather than a comma-separated string. This change would make the implementation more type-safe, efficient, and idiomatic in Go, eliminating unnecessary string allocations and parsing overhead across the controller logic and unit tests.
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.
|
Hey @JesseStutler pls take a look, Kept the fix scoped to regular containers only, as discussed earlier. ExitCodes is kept as an internal string because apis.Request is used as a workqueue item and must stay comparable. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
vcjob pod failure handling only used
ContainerStatuses[0]when setting the exit code for lifecycle policy matching.That breaks multi-container pods when the first container exits successfully and another regular container fails with the configured
exitCode.This PR scans all terminated regular container statuses, records non-zero exit codes, and lets
LifecyclePolicy.ExitCodematch any of them while keeping the existing policy order.Init containers are intentionally not included here, per the issue discussion.
Which issue(s) this PR fixes:
Fixes #5452
Special notes for your reviewer:
Kept
ExitCodefor existing request behavior and added internal multi-code matching without changing the public API.Does this PR introduce a user-facing change?