OCPBUGS-81622: Include bootstrap gather in agent-gather archive#10501
OCPBUGS-81622: Include bootstrap gather in agent-gather archive#10501zaneb wants to merge 1 commit intoopenshift:mainfrom
Conversation
If the rendezvous host is in the process of bootstrapping the cluster, collect the same logs that would be collected by the 'gather bootstrap' installer command and incorporate them into the agent-gather archive.
|
@zaneb: This pull request references Jira Issue OCPBUGS-81622, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/data/agent/files/usr/local/bin/agent-gather`:
- Around line 124-129: The script currently unconditionally runs gunzip on
${bs_gather} even if /usr/local/bin/installer-gather.sh failed or produced no
archive; update the block that invokes installer-gather.sh and the bs_gather
variable so you check the exit status and the produced file before running
gunzip: run installer-gather.sh via TAR_FILE="${bs_gather}"
/usr/local/bin/installer-gather.sh --id ... and capture its exit code, then only
run gunzip "${bs_gather}" if the command succeeded and the file exists and is
non-empty (e.g., test -f and test -s on the bs_gather path); otherwise skip
gunzip and propagate or log the failure so the function does not falsely report
success. Ensure you reference installer-gather.sh, bs_gather, TAR_FILE and
ARTIFACTS_DIR in the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c01ad7f5-69b0-42c6-86ce-079b3fd081bf
📒 Files selected for processing (1)
data/data/agent/files/usr/local/bin/agent-gather
| if [ -f /usr/local/bin/installer-gather.sh ]; then | ||
| local bs_gather="${ARTIFACTS_DIR}/bootstrap-gather.tar.gz" | ||
| TAR_FILE="${bs_gather}" /usr/local/bin/installer-gather.sh --id "$(date '+%Y%m%d%H%M%S')" >&2 | ||
| ( >&2 echo -n ".") | ||
| gunzip "${bs_gather}" | ||
| fi |
There was a problem hiding this comment.
Guard gunzip behind a successful bootstrap gather result.
If installer-gather.sh fails (or produces no ${bs_gather}), Line 128 still runs and the function ends with " Done", resulting in a misleading success path and missing bootstrap logs in the archive.
💡 Proposed fix
- if [ -f /usr/local/bin/installer-gather.sh ]; then
+ if [ -x /usr/local/bin/installer-gather.sh ]; then
local bs_gather="${ARTIFACTS_DIR}/bootstrap-gather.tar.gz"
- TAR_FILE="${bs_gather}" /usr/local/bin/installer-gather.sh --id "$(date '+%Y%m%d%H%M%S')" >&2
- ( >&2 echo -n ".")
- gunzip "${bs_gather}"
+ if TAR_FILE="${bs_gather}" /usr/local/bin/installer-gather.sh --id "$(date '+%Y%m%d%H%M%S')" >&2 && [ -s "${bs_gather}" ]; then
+ ( >&2 echo -n ".")
+ gunzip "${bs_gather}"
+ else
+ ( >&2 echo ".. Skipping bootstrap gather archive: installer-gather failed or produced no output.")
+ fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -f /usr/local/bin/installer-gather.sh ]; then | |
| local bs_gather="${ARTIFACTS_DIR}/bootstrap-gather.tar.gz" | |
| TAR_FILE="${bs_gather}" /usr/local/bin/installer-gather.sh --id "$(date '+%Y%m%d%H%M%S')" >&2 | |
| ( >&2 echo -n ".") | |
| gunzip "${bs_gather}" | |
| fi | |
| if [ -x /usr/local/bin/installer-gather.sh ]; then | |
| local bs_gather="${ARTIFACTS_DIR}/bootstrap-gather.tar.gz" | |
| if TAR_FILE="${bs_gather}" /usr/local/bin/installer-gather.sh --id "$(date '+%Y%m%d%H%M%S')" >&2 && [ -s "${bs_gather}" ]; then | |
| ( >&2 echo -n ".") | |
| gunzip "${bs_gather}" | |
| else | |
| ( >&2 echo ".. Skipping bootstrap gather archive: installer-gather failed or produced no output.") | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/data/agent/files/usr/local/bin/agent-gather` around lines 124 - 129, The
script currently unconditionally runs gunzip on ${bs_gather} even if
/usr/local/bin/installer-gather.sh failed or produced no archive; update the
block that invokes installer-gather.sh and the bs_gather variable so you check
the exit status and the produced file before running gunzip: run
installer-gather.sh via TAR_FILE="${bs_gather}"
/usr/local/bin/installer-gather.sh --id ... and capture its exit code, then only
run gunzip "${bs_gather}" if the command succeeded and the file exists and is
non-empty (e.g., test -f and test -s on the bs_gather path); otherwise skip
gunzip and propagate or log the failure so the function does not falsely report
success. Ensure you reference installer-gather.sh, bs_gather, TAR_FILE and
ARTIFACTS_DIR in the change.
|
@zaneb: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
If the rendezvous host is in the process of bootstrapping the cluster, collect the same logs that would be collected by the 'gather bootstrap' installer command and incorporate them into the agent-gather archive.
Summary by CodeRabbit
Chores