-
Notifications
You must be signed in to change notification settings - Fork 70
Boilerplate: Update to a8a3172411f3f2b8848f64333843e028ef4b3ed1 #422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| a0e42e58ed1d65bb75a848c595b34ae5553296eb | ||
| a8a3172411f3f2b8848f64333843e028ef4b3ed1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| inheritance: true | ||
| reviews: | ||
| path_filters: | ||
| # Exclude boilerplate changes from review | ||
| - "!boilerplate/**" | ||
|
|
||
| # Exclude build artifacts and dependencies | ||
| - "!build/**" | ||
| - "!.venv/**" | ||
| - "!vendor/**" | ||
|
|
||
| # Exclude test fixtures | ||
| - "!**/testdata/**" | ||
| - "!**/.test-fixtures/**" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ parameters: | |
| required: true | ||
| description: Prow periodic job name to trigger via Gangway | ||
| - name: POLL_INTERVAL | ||
| value: "60" | ||
| value: "120" | ||
| description: Seconds between status polls | ||
| - name: TIMEOUT | ||
| value: "7200" | ||
|
|
@@ -17,8 +17,11 @@ parameters: | |
| value: "5" | ||
| description: Number of times to retry the Prow job on failure before reporting failure | ||
| - name: ACTIVE_DEADLINE | ||
| value: "50400" | ||
| value: "54000" | ||
| description: Kubernetes Job deadline in seconds (must exceed all attempts plus backoff delays) | ||
| - name: INITIAL_DELAY | ||
| value: "0" | ||
| description: Seconds to sleep before the first Gangway call; stagger concurrent jobs to avoid shared rate limit saturation | ||
| - name: JOB_ENVS | ||
| value: "" | ||
| description: Comma-separated KEY=VALUE pairs passed to the Prow job | ||
|
|
@@ -53,15 +56,22 @@ objects: | |
| [[ "${TIMEOUT}" =~ ^[1-9][0-9]*$ ]] || { log "ERROR: TIMEOUT must be a positive integer"; exit 1; } | ||
| [[ "${POLL_INTERVAL}" =~ ^[1-9][0-9]*$ ]] || { log "ERROR: POLL_INTERVAL must be a positive integer"; exit 1; } | ||
| [[ "${MAX_RETRIES}" =~ ^[0-9]+$ ]] || { log "ERROR: MAX_RETRIES must be a non-negative integer"; exit 1; } | ||
| [[ "${INITIAL_DELAY}" =~ ^[0-9]+$ ]] || { log "ERROR: INITIAL_DELAY must be a non-negative integer"; exit 1; } | ||
|
|
||
| # Backoff sum: base 30s doubling each retry = 30*(2^N-1), plus 15s max jitter | ||
| if [[ "${INITIAL_DELAY}" -gt 0 ]]; then | ||
| log "Waiting ${INITIAL_DELAY}s before first Gangway call (INITIAL_DELAY)..." | ||
| sleep "${INITIAL_DELAY}" | ||
| fi | ||
|
Comment on lines
+61
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Run the deadline validation before the initial sleep.
🤖 Prompt for AI Agents |
||
|
|
||
| # Backoff sum: base 30s doubling each retry, capped at 900s, plus 15s max jitter | ||
| MAX_BACKOFF_SUM=$(( 30 * ((1 << MAX_RETRIES) - 1) + MAX_RETRIES * 15 )) | ||
| # Each attempt may overshoot TIMEOUT by up to POLL_INTERVAL + status-request | ||
| # max-time (30s) on the last poll cycle | ||
| POLL_OVERSHOOT=$(( POLL_INTERVAL + 30 )) | ||
| # Each attempt may overshoot TIMEOUT by up to max(POLL_INTERVAL, 300s max backoff) + | ||
| # status-request max-time (30s) on the last poll cycle | ||
| POLL_OVERSHOOT=$(( (POLL_INTERVAL > 300 ? POLL_INTERVAL : 300) + 30 )) | ||
| # Trigger POST max-time (60s) + worst-case Retry-After (600s) per attempt | ||
| TRIGGER_OVERHEAD=$(( 60 + 600 )) | ||
| REQUIRED_DEADLINE=$(( (MAX_RETRIES + 1) * (TIMEOUT + POLL_OVERSHOOT + TRIGGER_OVERHEAD) + MAX_BACKOFF_SUM )) | ||
| # INITIAL_DELAY is a one-time cost at job startup, not per attempt | ||
| REQUIRED_DEADLINE=$(( (MAX_RETRIES + 1) * (TIMEOUT + POLL_OVERSHOOT + TRIGGER_OVERHEAD) + MAX_BACKOFF_SUM + INITIAL_DELAY )) | ||
| if [[ "${ACTIVE_DEADLINE}" -lt "${REQUIRED_DEADLINE}" ]]; then | ||
| log "ERROR: ACTIVE_DEADLINE (${ACTIVE_DEADLINE}s) is less than the minimum required for ${MAX_RETRIES} retries with TIMEOUT=${TIMEOUT}s (need at least ${REQUIRED_DEADLINE}s)" | ||
| exit 1 | ||
|
|
@@ -114,9 +124,25 @@ objects: | |
| log "Prow logs: ${PROW_URL}" | ||
|
|
||
| END=$((SECONDS + ${TIMEOUT})) | ||
| local poll_backoff="${POLL_INTERVAL}" | ||
| while [[ $SECONDS -lt $END ]]; do | ||
| sleep "${POLL_INTERVAL}" | ||
| S=$(curl -sfSL --max-time 30 -H "Authorization: Bearer ${GANGWAY_TOKEN}" "${GW}/${ID}" | jq -r .job_status) || S=UNKNOWN | ||
| sleep "$poll_backoff" | ||
| local poll_file="/dev/shm/gw_poll.$$" | ||
| local poll_code | ||
| poll_code=$(curl -sSL --max-time 30 \ | ||
| -H "Authorization: Bearer ${GANGWAY_TOKEN}" \ | ||
| -o "$poll_file" -w '%{http_code}' \ | ||
| "${GW}/${ID}" 2>/dev/null) || poll_code=000 | ||
| if [[ "$poll_code" == "429" ]]; then | ||
| rm -f "$poll_file" | ||
| poll_backoff=$(( poll_backoff * 2 )) | ||
| [[ $poll_backoff -gt 300 ]] && poll_backoff=300 | ||
|
Comment on lines
+138
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Do not reduce the poll delay below the configured interval. The validation allows 🤖 Prompt for AI Agents |
||
| log "Rate limited polling status (429) — backing off ${poll_backoff}s" | ||
| continue | ||
| fi | ||
| poll_backoff="${POLL_INTERVAL}" | ||
| S=$(jq -r .job_status "$poll_file" 2>/dev/null) || S=UNKNOWN | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Gate status parsing on a successful poll response. The loop parses 🤖 Prompt for AI Agents |
||
| rm -f "$poll_file" | ||
| log "${S} ($((SECONDS))s)" | ||
| case $S in | ||
| SUCCESS) log "Prow logs: ${PROW_URL}"; return 0;; | ||
|
|
@@ -143,7 +169,7 @@ objects: | |
| RATE_LIMITED_WAITED=0 | ||
| else | ||
| BACKOFF=$(( 30 * (1 << (ATTEMPT - 1)) )) | ||
| [[ $BACKOFF -gt 480 ]] && BACKOFF=480 | ||
| [[ $BACKOFF -gt 900 ]] && BACKOFF=900 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Cap each term in
🤖 Prompt for AI Agents |
||
| JITTER=$(( RANDOM % 16 )) | ||
| DELAY=$(( BACKOFF + JITTER )) | ||
| log "Retrying in ${DELAY}s (backoff=${BACKOFF}s, jitter=${JITTER}s)..." | ||
|
|
@@ -166,6 +192,8 @@ objects: | |
| value: ${JOB_ENVS} | ||
| - name: MAX_RETRIES | ||
| value: ${MAX_RETRIES} | ||
| - name: INITIAL_DELAY | ||
| value: ${INITIAL_DELAY} | ||
| - name: ACTIVE_DEADLINE | ||
| value: ${ACTIVE_DEADLINE} | ||
| resources: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: openshift/rbac-permissions-operator
Length of output: 238
🏁 Script executed:
Repository: openshift/rbac-permissions-operator
Length of output: 8968
Normalize or reject leading-zero
INITIAL_DELAYvalues.When
INITIAL_DELAY=08or09, validation passes, but Bash treats the value as an invalid octal literal inREQUIRED_DEADLINEarithmetic. With-e, the container can exit before polling. Normalize the value as base 10 or reject leading zeros.🤖 Prompt for AI Agents