telco5g: Add retries to all downloads from Web#78727
telco5g: Add retries to all downloads from Web#78727openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
WalkthroughFive CI operator telco5g step-registry scripts add transient-failure handling to external downloads: AWS CLI, Terraform, and OpenShift client archives. Each script now uses ChangesDownload Resilience via Retry Flags
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 31 minutes and 36 seconds.Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ci-operator/step-registry/telco5g/delete-interface/telco5g-delete-interface-commands.sh (1)
24-24: ⚡ Quick winConsider adding
--failso HTTP-error responses don't silently end up as the zip payload.Without
--fail, if the server returns a non-retryable HTTP error (e.g., 404 for a missing version), curl exits 0 and writes the HTML error body to/tmp/terraform.zip. The subsequentunzipthen fails with a cryptic "invalid archive" message instead of a clear download error. The same applies to the AWS CLI curl call intelco5g-add-interface-commands.sh.✨ Proposed fix
-curl -sL --retry 5 --retry-delay 10 "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip" -o /tmp/terraform.zip +curl -sLf --retry 5 --retry-delay 10 "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip" -o /tmp/terraform.zip🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/telco5g/delete-interface/telco5g-delete-interface-commands.sh` at line 24, The curl invocation that downloads Terraform (the line using TERRAFORM_VERSION and outputting to /tmp/terraform.zip in telco5g-delete-interface-commands.sh) should add the --fail flag so HTTP errors cause curl to exit non‑zero instead of writing an error HTML page; update that curl command to include --fail and do the same for the AWS CLI curl in telco5g-add-interface-commands.sh to ensure invalid HTTP responses fail fast and produce clear errors rather than an invalid zip/archive on unzip.ci-operator/step-registry/telco5g/jobs-router/telco5g-jobs-router-commands.sh (1)
48-51: ⚡ Quick winRetry flags look correct; consider adding
--failto avoid a silentnullIMG_URL.Without
--fail, if the releases API returns a non-retryable HTTP error, curl exits 0 and pipes the error body tojq.jq -r ".pullSpec"then yields"null", which is silently written into$MAINENVasT5_JOB_RELEASE_IMAGE='null'. Downstream steps would receive a bad release image reference with no error surfaced here.✨ Proposed fix (applies to both branches)
- IMG_URL=$(curl -q -L -s --retry 5 --retry-delay 10 \ + IMG_URL=$(curl -q -L -sf --retry 5 --retry-delay 10 \ "https://amd64.ocp.releases.ci.openshift.org/api/v1/releasestream/${T5CI_VERSION}.0-0.nightly/latest" \ | jq -r ".pullSpec")Also applies to: 59-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/telco5g/jobs-router/telco5g-jobs-router-commands.sh` around lines 48 - 51, The curl invocation that populates IMG_URL should include --fail and then validate the result to avoid silently writing "null"; update the curl command used to set IMG_URL (the command assigning IMG_URL via curl | jq -r ".pullSpec") to add the --fail flag and after the assignment check that IMG_URL is non-empty and not "null", and if it is, emit a clear error and exit non-zero so downstream steps don't receive a bad T5_JOB_RELEASE_IMAGE; apply the same change to the second identical curl usage later in the script.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@ci-operator/step-registry/telco5g/delete-interface/telco5g-delete-interface-commands.sh`:
- Line 24: The curl invocation that downloads Terraform (the line using
TERRAFORM_VERSION and outputting to /tmp/terraform.zip in
telco5g-delete-interface-commands.sh) should add the --fail flag so HTTP errors
cause curl to exit non‑zero instead of writing an error HTML page; update that
curl command to include --fail and do the same for the AWS CLI curl in
telco5g-add-interface-commands.sh to ensure invalid HTTP responses fail fast and
produce clear errors rather than an invalid zip/archive on unzip.
In
`@ci-operator/step-registry/telco5g/jobs-router/telco5g-jobs-router-commands.sh`:
- Around line 48-51: The curl invocation that populates IMG_URL should include
--fail and then validate the result to avoid silently writing "null"; update the
curl command used to set IMG_URL (the command assigning IMG_URL via curl | jq -r
".pullSpec") to add the --fail flag and after the assignment check that IMG_URL
is non-empty and not "null", and if it is, emit a clear error and exit non-zero
so downstream steps don't receive a bad T5_JOB_RELEASE_IMAGE; apply the same
change to the second identical curl usage later in the script.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9fe34395-9ff0-420d-8843-21175ca58a57
📒 Files selected for processing (5)
ci-operator/step-registry/telco5g/add-interface/telco5g-add-interface-commands.shci-operator/step-registry/telco5g/cnf/tests/telco5g-cnf-tests-commands.shci-operator/step-registry/telco5g/delete-interface/telco5g-delete-interface-commands.shci-operator/step-registry/telco5g/jobs-router/telco5g-jobs-router-commands.shci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.sh
|
/pj-rehearse pull-ci-openshift-sriov-network-operator-release-4.22-operator-e2e-sriov-sno periodic-ci-openshift-release-main-nightly-4.22-e2e-telco5g-cnftests |
|
@sshnaidm: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 110 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SchSeba, sshnaidm 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 |
|
/pj-rehearse ack |
|
@sshnaidm: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@sshnaidm: 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. |
0f86d58
into
openshift:main
Summary by CodeRabbit
Release Notes