fix(ci): stop check-rd-presets reporting present presets as missing - #4194
Andrei Kvapil (kvaps) wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
area/ci
|
The red is the harness, not the fix.
#4191 rewrites the same membership check to a here-string, so whichever of the two lands second conflicts on that line. It also replaces the |
The membership check piped printf into `grep -Fqx` under `set -o pipefail`. `grep -q` exits on its first match, which can close the pipe while printf is still in write(2); printf then fails with EPIPE, pipefail makes the pipeline non-zero, and the negation reads that as "not found". A preset was therefore reported missing precisely because it matched first. It is a scheduling race, so it fired under `make -j4` in CI and not on an idle machine. The cost was not cosmetic: `Unit & controller tests` gates `finalize`, which gates `e2e`, so a false failure here skipped e2e and turned the required `E2E Tests` context red, blocking merges on unrelated pull requests. Feed grep from a here-string instead. No pipeline, nothing for pipefail to judge, no writer to receive EPIPE. This is the only script under hack/ that combined pipefail with a `| grep -q`. The tests pin the two outcomes the check owes, a complete enum passing and an incomplete one failing with the missing value named, rather than trying to win the race: a test that only reddens when the scheduler cooperates is the flaky kind this repo does not add. The fixture is written with jq alone so it does not depend on which yq dialect is installed, and it creates its temp dir in the test body because hack/cozytest.sh knows @test and nothing else. Refs #4193 Signed-off-by: Andrei Kvapil <[email protected]>
c04ab14 to
964bd83
Compare
2 participants
What this PR does
hack/check-rd-presets.shsometimes reports a preset as missing when it is in the file. Seen on #4185:c1.smallis in that file, and the script passes when I run it on the same head.The check was
printf '%s\n' "$enums" | grep -Fqx -- "$want"in a script withset -o pipefail.grep -qexits on the first match and closes the pipe while printf is still writing, printf fails with EPIPE, pipefail makes the pipeline non-zero, and!reads that as not found. So a preset gets reported missing because it matched first. It is a race, so it hits undermake -j4in CI and not on a laptop.It costs more than a red tick.
Unit & controller testsis inneedsforfinalize,finalizegatese2e, so the false failure skips e2e ande2e-reportposts the requiredE2E Testscontext as failed. #4185, #4138 and #3539 are red this way right now. #4109, #4097, #3802 and #3594 are not.Fix is a here-string instead of the pipe. This is the only script under
hack/withpipefailand| grep -q.hack/check-rd-presets.batscovers it, picked up by thehack/*.batsglob inMakefile:161. It pins the two outcomes the check owes, a complete enum passing and an incomplete one failing with the missing value named. It does not try to win the race: a test that only reddens when the scheduler cooperates is the flaky kind this repo does not add, and the CI log in #4178 is the evidence for the bug. The fixture is written with jq alone so it does not depend on which yq dialect is installed, and it makes its temp dir in the test body becausehack/cozytest.shknows@testand nothing else.shellcheck is clean and the check still passes on the tree.
Refs #4178
Screenshots
Not a UI change.
Downstream repositories
Nothing under
packages/, no API type, no values schema, no release asset, no Talos version. One CI check script edited in place and one test file added next to it. Nothing moved or renamed, andbats-unit-testsalready globbedhack/*.bats.Release note