Skip to content

test/e2e: add default 60s backoff for 429 without Retry-After header - #631

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:masterfrom
redhat-chai-bot:gangway-bridge-429-default-backoff
Sep 5, 2026
Merged

test/e2e: add default 60s backoff for 429 without Retry-After header#631
openshift-merge-bot[bot] merged 1 commit into
openshift:masterfrom
redhat-chai-bot:gangway-bridge-429-default-backoff

Conversation

@redhat-chai-bot

@redhat-chai-bot redhat-chai-bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Add a default 60-second backoff in the gangway-bridge template's 429 rate-limit handler when no valid Retry-After header is present. This prevents the script from immediately falling through to the outer retry loop with only the base exponential backoff, which can be too short to survive rate limit windows.

Problem

When the Gangway API returns HTTP 429 without a Retry-After header, the script's 429 handler logs the event but does no waiting at all -- it returns immediately to the outer retry loop. The outer loop then applies its own exponential backoff (starting at 30s), but:

  1. With MAX_RETRIES=1 (as configured in SAPM), this gives only one 30s wait before exhausting all attempts
  2. The RATE_LIMITED_WAITED flag is never set, so the outer loop's backoff is always applied on top (instead of being skipped)
  3. A 30s wait may not be sufficient for rate limits to clear, especially when multiple concurrent jobs are hitting Gangway

Changes

In the else branch of the Retry-After header check (both boilerplate/openshift/golang-osd-e2e/gangway-bridge-template.yml and test/e2e/gangway-bridge-template.yml):

  • Added sleep 60 -- a reasonable default wait when no Retry-After header is available
  • Set RATE_LIMITED_WAITED=1 -- signals the outer retry loop to skip its own backoff (avoiding double-waiting)
  • Updated log message to indicate the default backoff is being used

Note on boilerplate

Both the boilerplate source (boilerplate/openshift/golang-osd-e2e/gangway-bridge-template.yml) and the generated output (test/e2e/gangway-bridge-template.yml) are updated in this PR. The true upstream is the openshift/boilerplate repository -- a parallel change there may be needed to prevent this fix from being overwritten on the next boilerplate sync.


AI-generated. Review for accuracy.

@dustman9000 requested in Slack thread

Summary by CodeRabbit

  • Bug Fixes
    • Added a 60-second fallback wait when Gangway returns a rate-limit response without a valid retry interval.
    • The bridge now records completion of the wait before attempting the request again.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Team

Run ID: 23b934a2-45ff-4601-b6a1-2153e23132ce

📥 Commits

Reviewing files that changed from the base of the PR and between 0cd59ae and 1399d67.

⛔ Files ignored due to path filters (1)
  • boilerplate/openshift/golang-osd-e2e/gangway-bridge-template.yml is excluded by !boilerplate/**
📒 Files selected for processing (1)
  • test/e2e/gangway-bridge-template.yml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The Gangway bridge now waits 60 seconds when HTTP 429 responses lack a valid Retry-After header. It records the delay as handled before retrying.

Changes

Gangway rate-limit handling

Layer / File(s) Summary
Default rate-limit backoff
test/e2e/gangway-bridge-template.yml
The invalid or missing Retry-After path now sleeps for 60 seconds and marks the rate-limit wait as handled before retrying.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 1399d

429 responses without a valid Retry-After value now wait 60 seconds before retrying, without adding a second retry delay. The change is ready to merge.

Suggested reviewers: dustman9000

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a default 60-second backoff when a 429 response lacks a valid Retry-After header.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PASS: The pull request changes only two Gangway YAML templates. The diff adds a log message, sleep 60, and RATE_LIMITED_WAITED=1; it does not add or change any Ginkgo It, Describe, Context, …
Test Structure And Quality ✅ Passed PASS: The pull request changes only two YAML Gangway bridge templates. The diff contains no Go or Ginkgo test changes, and it introduces no It block, cluster resource setup, Eventually/Consistently wa…
Microshift Test Compatibility ✅ Passed The check is not applicable. The pull request changes only two YAML templates and adds shell backoff handling. The diff adds no Ginkgo tests or test declarations, and it introduces no OpenShift API, n…
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS — The pull request changes only the Gangway bridge YAML templates. The exact diff adds sleep 60, RATE_LIMITED_WAITED=1, and a log message in the HTTP 429 handler. It adds no Ginkgo tests (`It…
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The pull request changes only the Gangway bridge Job templates. The diff adds log, sleep, and RATE_LIMITED_WAITED shell statements in the HTTP 429 handler. It adds no node selectors, affinity, t…
Ote Binary Stdout Contract ✅ Passed PASS: The pull request changes only YAML-embedded Bash templates. The added log call writes through echo ... >&2; sleep 60 and RATE_LIMITED_WAITED=1 do not write to stdout. The template runs `…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS. The pull request changes only two YAML Gangway bridge templates. The exact diff adds a 60-second shell sleep, a log message, and RATE_LIMITED_WAITED=1 in the existing HTTP 429 handler. It adds…
No-Weak-Crypto ✅ Passed The pull request changes only the Gangway 429 fallback in the boilerplate and generated YAML templates. The added code logs a default backoff, sleeps for 60 seconds, and sets RATE_LIMITED_WAITED=1. Th…
Container-Privileges ✅ Passed The pull request changes only the 429 handler in two OpenShift templates. The added lines log a default backoff, run sleep 60, and set RATE_LIMITED_WAITED=1. No privileged container, hostPID, host…
No-Sensitive-Data-In-Logs ✅ Passed PASS: The pull request changes only the 429 fallback log text and adds sleep 60 plus RATE_LIMITED_WAITED=1 in both template copies. The new log contains only the HTTP status and fixed backoff desc…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

@redhat-chai-bot: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@dustman9000

Copy link
Copy Markdown
Member

/lgtm
/approve

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2026
@openshift-ci

openshift-ci Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dustman9000, redhat-chai-bot

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit d5ff1c2 into openshift:master Sep 5, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants