Skip to content

Commit 62f2268

Browse files
fix(kwok): fail fast on unworkable timeout margin at derive time
The derive step validated job_timeout_minutes as a positive integer but never checked that the resulting sync-gate budget (job_timeout_minutes*60 - 240s margin) clears the SYNC_BUDGET_FLOOR_SECONDS floor consumed by compute_sync_budget. A misconfigured caller (small job_timeout_minutes, or a future bump to the 240s margin) would compute an already-past or barely-positive deadline that only surfaced at the first sync gate — after Go setup, kind prep, and make build had already burned CI minutes. Add the same floor check at derive time so misconfiguration fails before any of that work starts. The 120 literal is hand-synced with SYNC_BUDGET_FLOOR_SECONDS in kwok/scripts/lib/sync-budget.sh and is guarded by sync-budget_test.sh, same contract as the existing 240s margin sync check. Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
1 parent 2135446 commit 62f2268

3 files changed

Lines changed: 55 additions & 3 deletions

File tree

.github/actions/kwok-test/action.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ runs:
107107
echo "::error::job_timeout_minutes must be a positive integer, no leading zeros, got '${JOB_TIMEOUT_MINUTES}'"
108108
exit 1
109109
fi
110+
# 120 must equal SYNC_BUDGET_FLOOR_SECONDS in kwok/scripts/lib/sync-budget.sh
111+
# (hand-synced literal, guarded by sync-budget_test.sh — same contract as the
112+
# 240s margin). A budget under the floor would compute an already-unusable
113+
# deadline that only fails later, inside the job, after setup burned CI minutes.
114+
if (( 10#${JOB_TIMEOUT_MINUTES} * 60 - 240 < 120 )); then
115+
echo "::error::job_timeout_minutes=${JOB_TIMEOUT_MINUTES} leaves no usable sync budget after the 240s diagnostics margin (need >= 6)"
116+
exit 1
117+
fi
110118
KWOK_SYNC_DEADLINE_EPOCH=$(( $(date +%s) + 10#${JOB_TIMEOUT_MINUTES} * 60 - 240 ))
111119
echo "KWOK_SYNC_DEADLINE_EPOCH=${KWOK_SYNC_DEADLINE_EPOCH}" >> "$GITHUB_ENV"
112120
echo "KWOK_SYNC_DEADLINE_EPOCH=${KWOK_SYNC_DEADLINE_EPOCH} (job_timeout_minutes=${JOB_TIMEOUT_MINUTES}, margin=240s, job kill in ~$(( 10#${JOB_TIMEOUT_MINUTES} * 60 ))s)"

docs/contributor/tests.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,10 @@ sits within ~60 s of job start — from its `job_timeout_minutes` input
387387
must equal that caller's `timeout-minutes`; currently `18` for the
388388
KWOK jobs) minus a 240 s margin reserved for chainsaw catch-block
389389
diagnostics, pod verification, and debug-artifact upload. The input
390-
must be a positive integer with no leading zeros; the step fails fast
391-
otherwise. Local runs leave it unset and keep the fixed defaults above.
390+
must be a positive integer with no leading zeros, and must leave at
391+
least 120 s of usable budget after the 240 s margin (i.e. `>= 6`); the
392+
step fails fast otherwise. Local runs leave it unset and keep the
393+
fixed defaults above.
392394

393395
The Git-source lanes (`flux-git`, `argocd-git`) additionally honor
394396
`KWOK_GITEA_HOST_PORT` (default `3300`), `KWOK_GITEA_USER` (default

kwok/scripts/lib/sync-budget_test.sh

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,50 @@ tier3_shard_out=$(job_timeout_sync "${REPO_ROOT}/.github/workflows/kwok-tier3-sh
9898
check "kwok-tier3-shard-job-timeout-in-sync" 0 "test:jtm=18,tm=18" \
9999
0 "$(echo "${tier3_shard_out}" | grep '^test:')"
100100

101+
# 10-11. Margin-floor guard in the "Derive sync-gate deadline" step
102+
# (action.yml): job_timeout_minutes must leave >= SYNC_BUDGET_FLOOR_SECONDS
103+
# of usable budget after the 240s diagnostics margin, or the step must fail
104+
# fast before toolchain setup + make build burn CI minutes. Extracted
105+
# straight from action.yml (not reimplemented here) so this always exercises
106+
# THIS checkout's actual CI logic, never a stale copy.
107+
ACTION_YML="${REPO_ROOT}/.github/actions/kwok-test/action.yml"
108+
109+
# extract_derive_step <file> -> the dedented run: block of the "Derive
110+
# sync-gate deadline" step.
111+
extract_derive_step() {
112+
local file="$1"
113+
awk '
114+
/^ - name: Derive sync-gate deadline$/ { in_step=1; next }
115+
in_step && /^ run: \|$/ { in_run=1; next }
116+
in_run && /^ - name:/ { exit }
117+
in_run { sub(/^ /, ""); print }
118+
' "${file}"
119+
}
120+
121+
# run_derive_step <job_timeout_minutes> -> sets got_rc, got_env_file
122+
# (caller-owned temp file, removed by caller). -eo pipefail mirrors the
123+
# composite step's actual shell (`shell: bash` -> bash -eo pipefail) so a
124+
# future set -e interaction cannot diverge between CI and this harness.
125+
run_derive_step() {
126+
local jtm="$1"
127+
got_env_file=$(mktemp)
128+
JOB_TIMEOUT_MINUTES="${jtm}" GITHUB_ENV="${got_env_file}" \
129+
bash -eo pipefail -c "$(extract_derive_step "${ACTION_YML}")" > /dev/null 2>&1
130+
got_rc=$?
131+
}
132+
133+
run_derive_step 5
134+
check "derive-step-below-floor-fails" 1 "" "${got_rc}" ""
135+
rm -f "${got_env_file}"
136+
137+
run_derive_step 6
138+
env_has_deadline="no"
139+
grep -q '^KWOK_SYNC_DEADLINE_EPOCH=' "${got_env_file}" && env_has_deadline="yes"
140+
check "derive-step-at-floor-boundary-succeeds" 0 "yes" "${got_rc}" "${env_has_deadline}"
141+
rm -f "${got_env_file}"
142+
101143
if (( fails > 0 )); then
102144
echo "${fails} test(s) failed"
103145
exit 1
104146
fi
105-
echo "All 9 tests passed"
147+
echo "All 11 tests passed"

0 commit comments

Comments
 (0)