Summary
Chainsaw runs script.content as sh -c, so a block with several check_node.sh calls and no set -e gates only on its last command and every check above it is decorative. This has now been found and fixed three times in k8s-tests/operator-agent/ (#605, #673, #683), and nothing stops the next block from shipping the same way. Separately, with set -e a block stops at its first failing check, so when two checks would both fail only the first is reported, which hides the diagnostic that tells the two apart (raised on #683: a reaper that keeps the wrong count and trims from the wrong end reports only the count).
Proposal
Either or both of:
- A CI lint over
k8s-tests/ that fails any chainsaw script: block containing more than one command when it does not start with set -e (cleanup blocks that are deliberately non-gating would opt out with a marker comment). chainsaw does not appear to offer a per-test shell-options setting, so the lint is the practical route. lint-ci.yaml is the natural home.
- One check per
script: op in the assertion-heavy scenarios (simple/, reap_old_logs/, check_results/, service_restart/), so every failing check is reported independently and set -e per block becomes moot for those.
Option 2 makes option 1 unnecessary for the scenarios it touches, so they can land together or 1 can land first as the guard while 2 is done gradually.
Context
Acceptance criteria
- A new multi-command
script: block without set -e fails CI, or the affected scenarios no longer have multi-check blocks.
- Existing deliberately non-gating cleanup blocks (
rm -rf ... || true) still pass.
docs/contributing/ci-test-pools.md says which rule applies and how to opt a block out.
Summary
Chainsaw runs
script.contentassh -c, so a block with severalcheck_node.shcalls and noset -egates only on its last command and every check above it is decorative. This has now been found and fixed three times ink8s-tests/operator-agent/(#605, #673, #683), and nothing stops the next block from shipping the same way. Separately, withset -ea block stops at its first failing check, so when two checks would both fail only the first is reported, which hides the diagnostic that tells the two apart (raised on #683: a reaper that keeps the wrong count and trims from the wrong end reports only the count).Proposal
Either or both of:
k8s-tests/that fails any chainsawscript:block containing more than one command when it does not start withset -e(cleanup blocks that are deliberately non-gating would opt out with a marker comment). chainsaw does not appear to offer a per-test shell-options setting, so the lint is the practical route.lint-ci.yamlis the natural home.script:op in the assertion-heavy scenarios (simple/,reap_old_logs/,check_results/,service_restart/), so every failing check is reported independently andset -eper block becomes moot for those.Option 2 makes option 1 unnecessary for the scenarios it touches, so they can land together or 1 can land first as the guard while 2 is done gradually.
Context
simple/chainsaw-test.yaml:47andreap_old_logs/chainsaw-test.yaml:59).Acceptance criteria
script:block withoutset -efails CI, or the affected scenarios no longer have multi-check blocks.rm -rf ... || true) still pass.docs/contributing/ci-test-pools.mdsays which rule applies and how to opt a block out.