Repository navigation
Conversation
The pods poller in kubectlWatchPodsLogsAndEvents did not catch a rejected kubectl call. A single transient API error (e.g. connection reset) became an unhandled rejection -> SIGTERM -> helm killed mid-upgrade, leaving the release stuck in pending-upgrade. - catch kubectl errors in the pods poller and log them as warnings - wait for 'close' instead of 'exit' so stdout is fully drained before JSON.parse (fixes spurious "Can not parse JSON output") - keep stdout newlines, capture stderr and include it in the error message - handle spawn 'error' events - skip a poll tick while the previous kubectl call is still running - snapshot existing Job UIDs before helm starts; previously the snapshot was taken on the first non-empty poll, which could include the Job just created by this upgrade and hide its failure Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JzncmhbXTJUsBEsqys93Ht
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5a6388a04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
If the initial "existing jobs" kubectl call failed, the watcher continued with an empty baseline and could treat a stale failed Job from a previous release as current, sending SIGTERM to an unrelated upgrade. - retry the baseline up to 5 times before helm starts - if it is still missing, make no failure decision until a successful poll establishes it (those Jobs are treated as pre-existing) - accept only a real list response (items array) as a baseline Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JzncmhbXTJUsBEsqys93Ht
The pre-upgrade Job snapshot runs before helm is started, and kubectl's default --request-timeout is 0 (never). A stalled API request could leave helm uninvoked and the release lock held indefinitely. - pass --request-timeout 10s to one-shot polls (job/pods), before KUBECTL_CMD_ARGS so a user-supplied value still wins - SIGKILL a one-shot kubectl process after 30s as a hard bound for cases the request timeout does not cover (e.g. hung exec credential plugin) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JzncmhbXTJUsBEsqys93Ht
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
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
This PR enhances the reliability and debuggability of the UpgradeModule's Kubernetes monitoring logic by fixing race conditions, improving error handling, and including stderr in error messages.
Key Changes
Fixed race condition in job initialization: Moved initial Job UID snapshot from the first interval tick to before helm upgrade starts. This prevents newly created Jobs from being incorrectly treated as pre-existing when the first monitoring tick lands after helm creates the Job.
Added in-flight request tracking: Introduced
inFlightflags in bothkubectlWatchPodsLogsAndEvents()andwatchJobStatus()to prevent concurrent kubectl calls from overlapping, ensuring clean state management.Improved error handling in log watching: Wrapped the auxiliary pod log watching in try-catch-finally to prevent transient kubectl/API failures from escaping as unhandled rejections, which would crash the app and kill the running helm upgrade.
Enhanced stderr capture and error messages:
interpretChildProcessExit()to accept and include stderr in error messagesImproved logging levels: Changed several
Logger.fatal()calls toLogger.warn()for transient failures (JSON parsing, API connectivity) since these are recoverable conditions, not fatal errors.Extracted common logic: Added
parseJobList()helper method to centralize Job list parsing logic.Removed obsolete state: Removed the
isFirstPollboolean flag which is no longer needed with the new initialization approach.Notable Implementation Details
https://claude.ai/code/session_01JzncmhbXTJUsBEsqys93Ht