Repository navigation
fix(hack): avoid a SIGPIPE false-negative in the resourcesPreset check - #4204
Merged
Merged
Conversation
check-rd-presets.sh tests each expected preset with `printf '%s\n' "$enums" | grep -Fqx -- "$want"`. When grep matches it exits immediately and closes the pipe, so printf is killed by SIGPIPE (exit 141) while it still has data to write. Under `set -o pipefail` that 141 propagates as the pipeline's status, so `if ! <pipeline>` reads a preset that IS present as missing. It is a race with the pipe buffer, so it fires intermittently and mostly on the larger enums -- seen in CI on http-cache-rd, whose schema carries two 47-entry enums -- failing the "Unit & controller tests" job for unrelated PRs. Feed grep from a here-string instead of a pipe: no second process, no SIGPIPE, no pipefail interaction. Deterministic repro of the old behaviour (target at the front of a 100k-line list): the pipe form reports "missing", the here-string form finds it. Signed-off-by: Alexey Artamonov <[email protected]>
scooby87
requested review from
IvanHunters,
Andrei Kvapil (kvaps),
Aleksei Sviridkin (lexfrei),
Timofei Larkin (lllamnyp) and
myasnikovdaniil
as code owners
September 10, 2026 14:23
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe preset validation script replaces a ChangesPreset validation
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change makes preset validation reliable by avoiding false failures when grep finds a match. No merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks 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
size/XS
Andrey Kolkov (androndo)
approved these changes
Sep 10, 2026
scooby87
enabled auto-merge
September 11, 2026 07:53
Andrei Kvapil (kvaps)
approved these changes
Sep 11, 2026
Aleksei Sviridkin (lexfrei)
added a commit
that referenced
this pull request
Sep 28, 2026
## What this PR does `rd-presets-check` discards `jq` errors, so malformed schema can pass the check or appear as missing presets. It now fails with the schema path and `jq` diagnostic. An absent schema or a readable schema without `resourcesPreset` is still skipped. Regression tests use real `jq` for malformed input and partial output. They also cover the canonical preset set, exact matching and the long-list case fixed by [#4204](#4204). ### Testing - `make unit-tests -j4 --output-sync=target` passed. - Nine cases in `hack/check-rd-presets_test.bats`, run with `bats` and `dash hack/cozytest.sh`. - Both read-failure cases fail against `main`. Eleven deliberate code mutations also fail the expected test. - `shellcheck hack/check-rd-presets.sh hack/check-rd-presets_test.bats`. ### Screenshots Not applicable. ### Downstream repositories - [x] No downstream repository is affected by this change No files or make targets were moved or renamed. The change is limited to the RD checker and its tests. ### Release note ```release-note fix(tests): Fail the RD preset check when a schema cannot be read, with the schema path and jq diagnostic. ```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hack/check-rd-presets.shintermittently fails the "Unit & controller tests" CI job with a falseresourcesPreset enum missing: <preset>for a preset that is actually present — most visibly onhttp-cache-rd, and on PRs that have nothing to do with that package.Root cause
The check tests each expected preset with
printf '%s\n' "$enums" | grep -Fqx -- "$want". When grep finds the preset it exits immediately and closes the pipe, soprintfis killed by SIGPIPE (exit 141) while it still has data to write. The script runs underset -o pipefail, so that 141 becomes the pipeline's status andif ! <pipeline>records a present preset as missing. It is a race with the pipe buffer, so it fires intermittently and mostly on the larger enums (http-cache-rd's schema carries two 47-entry enums). The CI log shows the tell:check-rd-presets.sh: line 39: printf: write error: Broken pipeimmediately followed byFAIL: ... resourcesPreset enum missing: s1.micro.Fix
Feed grep from a here-string instead of a pipe — no second process, no SIGPIPE, no pipefail interaction. Deterministic reproduction (target at the front of a 100k-line list, under
set -euo pipefail): theprintf | grep -Fqxform reports the target as missing; the here-string form finds it. After the changebash hack/check-rd-presets.shpasses green.Release note
Summary by CodeRabbit