OSAC-1155: fix CI boot failures in refresh script - #212
openshift-merge-bot[bot] merged 2 commits into
Conversation
|
@omer-vishlitzky: This pull request references OSAC-1155 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. 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. |
WalkthroughTwo deployment scripts are hardened: the AAP fulfillment launcher now waits for job template and project sync and adds robust launch retries; the post-snapshot refresh script resolves the pull-secret path and improves CDI and MetalLB readiness and application flows. ChangesDeployment Script Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Risk severity: Low–Moderate — changes affect deployment readiness and retry behavior; incorrect timeouts or checks could cause delayed or failed rollouts, but fixes improve error detection and reduce silent failure modes. Possibly related PRs
Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/hold |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/prepare-fulfillment-service.sh`:
- Around line 50-57: The check that waits for a valid AAP project revision is a
no-op because jq's '.current_update // empty' never outputs the literal string
"null", so the guard in the retry_until call returns immediately; update the
retry condition in the block that references PROJECT_ID, retry_until, AAP_TOKEN,
AAP_URL and .current_update to test for a non-empty value instead (e.g., ensure
the curl|jq pipeline yields a non-empty string) so retry_until only succeeds
when .current_update is present and non-empty; keep the same retry_until usage
and error/warning handling but change the jq expression or the comparison to
assert non-empty output rather than comparing to the literal "null".
- Around line 61-69: The loop creates LAUNCH_ERR with mktemp on every attempt
but never reads or deletes it; change the launch logic (where LAUNCH_ERR,
JOB_RESPONSE, mktemp, and curl are used) to capture curl's stderr into a single
temp file or a variable, read and print its contents when curl fails or when
JOB_RESPONSE lacks an expected id, and always rm the temp file after each
iteration (use a trap or explicit rm in both success and failure branches) to
avoid leaking /tmp files and to surface connection-level diagnostics alongside
JOB_RESPONSE and HTTP_BODY.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 14790b3b-b19b-48ce-a2b0-8dc2d736d54a
📒 Files selected for processing (2)
scripts/prepare-fulfillment-service.shscripts/refresh-after-snapshot.sh
0cbc2f2 to
471261e
Compare
MetalLB webhook TLS: rotate cert and wait for Service endpoints before patching IPAddressPool. Image preflight: verify ghcr.io/osac-project images exist after kustomize apply. Fail immediately if missing. AAP publish-templates: wait for project sync before launching job, log HTTP errors, wait for controller-task deployment stability. CDI rollout: retry on API server timeouts, run waits in parallel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
471261e to
b03668c
Compare
|
/retest |
1 similar comment
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danmanor, jhernand, omer-vishlitzky The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
/unhold |
|
/override ci/prow/images ci/prow/e2e-vmaas |
|
@omer-vishlitzky: Overrode contexts on behalf of omer-vishlitzky: ci/prow/e2e-vmaas, ci/prow/images 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 kubernetes-sigs/prow repository. |
4 participants
Summary
https://redhat.atlassian.net/browse/OSAC-1155
Follow-up to PR #206. Addresses remaining boot failure patterns found in post-merge CI analysis:
2>/dev/null). FixesMissing a revision to run due to failed project updateerrors.oc rollout statuscalls withretry_commandto handle transient API server timeouts under load. Run CDI deployment waits in parallel instead of sequentially.Test plan
bash -nsyntax check passes on both scripts🤖 Generated with Claude Code
Summary by CodeRabbit