Skip to content

Commit cfb4961

Browse files
ArangoGutierrezArangoGutierrez
committed
fix(kwok): guard timeout literal sync, octal-safe input validation
Adds a sync-budget_test.sh case asserting each caller's job_timeout_minutes matches its timeout-minutes (drift silently re-creates the CANCELLED-without-diagnostics failure); tightens the integer guard against zero-padded octal misparse and forces base-10 arithmetic; corrects the stale 'default 18' doc claim; per-operation budget wording; derive step logs the implied kill offset. Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
1 parent 624a947 commit cfb4961

6 files changed

Lines changed: 66 additions & 19 deletions

File tree

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,14 @@ inputs:
7575
description: >-
7676
The calling job's timeout-minutes. Used to derive
7777
KWOK_SYNC_DEADLINE_EPOCH (deadline = step start + this budget - 240s
78-
diagnostics margin) so the chainsaw sync gates always finish - and
79-
print their catch-block diagnostics - before GitHub kills the job.
80-
Required with no default: every caller must wire its own
81-
timeout-minutes here, so a job that changes its timeout cannot
82-
silently drift from a stale default (a missing value fails the
83-
integer validation below loudly).
78+
diagnostics margin) so each chainsaw sync gate's budget stays within
79+
that margin, letting the gate finish - and print its catch-block
80+
diagnostics - before GitHub kills the job in the expected
81+
single-gate-dominates case (this bounds each gate operation, not the
82+
job's overall wall time). Required with no default: every caller
83+
must wire its own timeout-minutes here, so a job that changes its
84+
timeout cannot silently drift from a stale default (a missing value
85+
fails the integer validation below loudly).
8486
required: true
8587

8688
runs:
@@ -101,13 +103,13 @@ runs:
101103
# silently re-creates the CANCELLED-without-diagnostics failure
102104
# this mechanism exists to prevent.
103105
# See kwok/scripts/lib/sync-budget.sh for the consuming side.
104-
if ! [[ "${JOB_TIMEOUT_MINUTES}" =~ ^[0-9]+$ ]]; then
105-
echo "::error::job_timeout_minutes must be an integer, got '${JOB_TIMEOUT_MINUTES}'"
106+
if ! [[ "${JOB_TIMEOUT_MINUTES}" =~ ^[1-9][0-9]*$ ]]; then
107+
echo "::error::job_timeout_minutes must be a positive integer, no leading zeros, got '${JOB_TIMEOUT_MINUTES}'"
106108
exit 1
107109
fi
108-
KWOK_SYNC_DEADLINE_EPOCH=$(( $(date +%s) + JOB_TIMEOUT_MINUTES * 60 - 240 ))
110+
KWOK_SYNC_DEADLINE_EPOCH=$(( $(date +%s) + 10#${JOB_TIMEOUT_MINUTES} * 60 - 240 ))
109111
echo "KWOK_SYNC_DEADLINE_EPOCH=${KWOK_SYNC_DEADLINE_EPOCH}" >> "$GITHUB_ENV"
110-
echo "KWOK_SYNC_DEADLINE_EPOCH=${KWOK_SYNC_DEADLINE_EPOCH} (job_timeout_minutes=${JOB_TIMEOUT_MINUTES}, margin=240s)"
112+
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)"
111113
112114
- name: Set up Go
113115
uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0

‎.github/workflows/kwok-recipes.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ jobs:
323323
chainsaw_version: ${{ steps.versions.outputs.chainsaw }}
324324
chainsaw_sha256: ${{ steps.versions.outputs.chainsaw_sha256_linux_amd64 }}
325325
kind_node_image: ${{ steps.versions.outputs.kind_node_image }}
326+
# keep == this job's timeout-minutes (asserted by sync-budget_test.sh)
326327
job_timeout_minutes: '18'
327328

328329
# ── Tier 2: diff-aware accelerator tests (PR only, conditional) ──
@@ -364,6 +365,7 @@ jobs:
364365
chainsaw_version: ${{ steps.versions.outputs.chainsaw }}
365366
chainsaw_sha256: ${{ steps.versions.outputs.chainsaw_sha256_linux_amd64 }}
366367
kind_node_image: ${{ steps.versions.outputs.kind_node_image }}
368+
# keep == this job's timeout-minutes (asserted by sync-budget_test.sh)
367369
job_timeout_minutes: '18'
368370

369371
# ── Tier 3: full matrix (push to main + nightly schedule) ──

‎.github/workflows/kwok-tier3-shard.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,5 @@ jobs:
6666
chainsaw_version: ${{ steps.versions.outputs.chainsaw }}
6767
chainsaw_sha256: ${{ steps.versions.outputs.chainsaw_sha256_linux_amd64 }}
6868
kind_node_image: ${{ steps.versions.outputs.kind_node_image }}
69+
# keep == this job's timeout-minutes (asserted by sync-budget_test.sh)
6970
job_timeout_minutes: '18'

‎docs/contributor/tests.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -383,11 +383,12 @@ independent.
383383
In CI, the `kwok-test` action derives `KWOK_SYNC_DEADLINE_EPOCH` in its
384384
first step — before toolchain setup and the `aicr` build, so the anchor
385385
sits within ~60 s of job start — from its `job_timeout_minutes` input
386-
(default `18`, kept in sync with the calling job's `timeout-minutes`)
387-
minus a 240 s margin reserved for chainsaw catch-block diagnostics,
388-
pod verification, and debug-artifact upload. The input must be an
389-
integer; the step fails fast otherwise. Local runs leave it unset and
390-
keep the fixed defaults above.
386+
(required, no default — every caller must wire its own value, which
387+
must equal that caller's `timeout-minutes`; currently `18` for the
388+
KWOK jobs) minus a 240 s margin reserved for chainsaw catch-block
389+
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.
391392

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

‎kwok/scripts/lib/sync-budget.sh‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414
# KWOK_SYNC_DEADLINE_EPOCH — the absolute epoch after which the job has no
1515
# time left for sync-gate work (job timeout minus a diagnostics margin).
1616
# Each chainsaw sync gate derives its budget as min(<gate default>,
17-
# deadline − now) so chainsaw always times out — and prints its catch-block
18-
# diagnostics — BEFORE GitHub's job timeout kills the runner with a silent
19-
# CANCELLED and zero error report.
17+
# deadline − now), bounding that gate's assert/error operation so it
18+
# finishes — and prints its catch-block diagnostics — before GitHub's
19+
# job timeout kills the runner in the expected single-gate-dominates
20+
# case (this bounds each gate operation, not the job's whole wall time).
2021
#
2122
# Source guard: constants and functions only, no side effects at source
2223
# time (same contract as lib/cleanup.sh).

‎kwok/scripts/lib/sync-budget_test.sh‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,48 @@ export KWOK_SYNC_DEADLINE_EPOCH=999000
5858
out=$(compute_sync_budget 500 1000000); rc=$?
5959
check "deadline-past-fails" 1 "" "${rc}" "${out}"
6060

61+
# 7. Literal-sync guard: job_timeout_minutes must equal the SAME job's
62+
# timeout-minutes in every caller workflow. This is a hand-synced literal
63+
# (the composite action cannot read the calling job's own timeout-minutes);
64+
# drift silently re-creates the CANCELLED-without-diagnostics failure this
65+
# whole mechanism exists to prevent. Resolve workflows SCRIPT_DIR-relative
66+
# so this always tests THIS checkout, never a deployed copy.
67+
REPO_ROOT="$(cd "${SCRIPT_DIR}/../../.." && pwd)"
68+
69+
# job_timeout_sync <workflow_file> -> one "<job>:jtm=<X>,tm=<Y>" line per
70+
# job that sets job_timeout_minutes, where <X> is that value and <Y> is
71+
# the nearest preceding job-level (4-space-indented) timeout-minutes
72+
# ("unset" if none was seen for the current job).
73+
job_timeout_sync() {
74+
local file="$1" job="" job_timeout=""
75+
while IFS= read -r line; do
76+
if [[ "${line}" =~ ^\ \ ([A-Za-z0-9_-]+):[[:space:]]*$ ]]; then
77+
job="${BASH_REMATCH[1]}"
78+
job_timeout=""
79+
continue
80+
fi
81+
if [[ "${line}" =~ ^\ \ \ \ timeout-minutes:\ *([0-9]+)[[:space:]]*$ ]]; then
82+
job_timeout="${BASH_REMATCH[1]}"
83+
continue
84+
fi
85+
if [[ "${line}" =~ job_timeout_minutes:\ *\'?([0-9]+)\'? ]]; then
86+
echo "${job}:jtm=${BASH_REMATCH[1]},tm=${job_timeout:-unset}"
87+
fi
88+
done < "${file}"
89+
}
90+
91+
kwok_recipes_out=$(job_timeout_sync "${REPO_ROOT}/.github/workflows/kwok-recipes.yaml")
92+
check "kwok-recipes-tier1-job-timeout-in-sync" 0 "test-tier1:jtm=18,tm=18" \
93+
0 "$(echo "${kwok_recipes_out}" | grep '^test-tier1:')"
94+
check "kwok-recipes-tier2-job-timeout-in-sync" 0 "test-tier2:jtm=18,tm=18" \
95+
0 "$(echo "${kwok_recipes_out}" | grep '^test-tier2:')"
96+
97+
tier3_shard_out=$(job_timeout_sync "${REPO_ROOT}/.github/workflows/kwok-tier3-shard.yaml")
98+
check "kwok-tier3-shard-job-timeout-in-sync" 0 "test:jtm=18,tm=18" \
99+
0 "$(echo "${tier3_shard_out}" | grep '^test:')"
100+
61101
if (( fails > 0 )); then
62102
echo "${fails} test(s) failed"
63103
exit 1
64104
fi
65-
echo "All 6 tests passed"
105+
echo "All 9 tests passed"

0 commit comments

Comments
 (0)