SRE-4015 ci: run full PR verification only on the stack tip - #19073
ryon-jensen wants to merge 1 commit into
Conversation
|
Errors are Unable to load ticket data |
0bb3125 to
0ad939b
Compare
|
This PR is open for review, and I would appreciate feedback now. Its CI will not succeed until pipeline-lib #533 lands, because this branch depends on the new shared-library steps introduced there. |
daltonbohning
left a comment
There was a problem hiding this comment.
Do you know why the current tip is failing?
I think overall this approach is okay for now and we can revisit if people start using stacked PRs more and need different behavior. One thing I can think of is that people might want to run more than just a simple build on mid-stack PRs.
| // for a pull request that is not the top of its stack. Building is | ||
| // all that is needed there. | ||
| /* groovylint-disable-next-line UnnecessaryGetter */ | ||
| if (isStackTip()) { |
There was a problem hiding this comment.
Do you know if the when { ... expression syntax works in this context? And if so, maybe it's possible to write this as multiple steps, where we only run the Generate RPMs step on when { expression ... isStackTip?
And then similar for other stages
Signed-off-by: Ryon Jensen <ryon.jensen@hpe.com>
0ad939b to
f2bb58e
Compare
Three-layer stack verification against
|
| Layer | PR | Detected as | Stages run | Jenkins contexts | Wall time |
|---|---|---|---|---|---|
| 1 of 3 | #19073 (this) | mid-stack | compile only | 14/14 green (4 real + 10 synthesized) | 6m 49s |
| 2 of 3 | #19074 | mid-stack | compile only | 14/14 green (4 real + 10 synthesized) | 7m 58s |
| 3 of 3 | #19137 | tip | full matrix | real results | hours |
Mid-stack layers
prStack: PR 19073 is layer 1 of 3 in stack 19075 onto master
updateRunStage: Detected mid-stack PR, only building
Running: Build on EL 9 (reason: mid-stack PR)
Running: Build on Leap 15 (reason: mid-stack PR)
Skipping: Unit Test (reason: mid-stack PR)
Skipping: Functional on EL 9 (reason: mid-stack PR)
Skipping: Functional Hardware Medium MD on SSD (reason: mid-stack PR)
...
notifySkippedRequiredChecks: publishing success for 'test/Unit Test': Runs on the tip of stack 19075, this is layer 1 of 3
Both mid-stack layers end with all 14 Jenkins-published required contexts green:
four reported for real by the stages that ran (build/Build on EL 9,
build/Build on Leap 15, test/Python Bandit check,
continuous-integration/jenkins/pr-head) and ten synthesized with a description
saying why they are green.
Tip layer
The previous two-layer test could not prove this half. Its tip carried
Skip-test, Skip-unit-tests and Skip-test-hardware pragmas to spare CI
capacity, so it was only ever shown not to take the mid-stack path. #19137
deliberately carries no skip pragmas (verified by running the commit messages
through pragmasToMap() — the only pragma on any of the three layers is
signed-off-by), so the full path is exercised for the first time:
prStack: PR 19137 is layer 3 of 3 in stack 19075 onto master
Running: Unit Test (reason: parameter selection or default)
Running: NLT (reason: parameter selection or default)
Running: Unit Test with memcheck (reason: parameter selection or default)
Running: Functional on EL 9 (reason: parameter selection or default)
Running: Fault injection testing (reason: parameter selection or default)
Running: Test RPMs on EL 9 (reason: parameter selection or default)
Running: Functional Hardware Medium MD on SSD (reason: parameter selection or default)
...
Every decision is attributed to parameter selection or default — never
mid-stack PR — so the tip is untouched by this change and behaves exactly as an
ordinary PR. Unit Test, Unit Test bdev, NLT and memcheck have already passed;
the functional and hardware stages are still running.
Notes for reviewers
- The filler deltas on layers 2 and 3 touch
ci/*.shrather thandocs/
deliberately: a docs-only delta would take thedocOnlyChange()path and skip
stages, which would have hidden the behaviour under test. - The stack shape is read once at pipeline startup. SRE-4015 ci: run full PR verification only on the stack tip #19073's first run after the
rebase reportedlayer 1 of 2because it started before SRE-4015 ci: stack layer 3 marker for CI verification #19137 joined the
stack; behaviour was still correct, and a rebuild reportslayer 1 of 3. - The six outstanding required checks on this PR are GitHub Actions, not
Jenkins. Actions run in full on every layer of a stack and are untouched by
this change.
#19074 and #19137 are throwaway scaffolding and will be closed along with the
stack once this lands.
Stack layer 1 of 2. Two related changes for SRE-4015.
1. Run full PR verification only on the tip of a stack.
updateRunStage()now consults
isStackTip(). On a layer below the tip it drops everythingexcept a small allow list of cheap checks plus a compile-only build, so a
broken layer still fails fast without paying for functional or hardware
testing. A stack N deep used to cost N full runs to validate one change set.
2. Report the required checks that skipping leaves behind. Every Jenkins
status context is published from inside its stage body, so a skipped stage
never publishes anything and GitHub leaves the context as "Expected —
waiting for status to be reported" forever. master requires 20 contexts, so
without this a mid-stack layer could never be merged. The Jenkinsfile now
collects the stage names it skipped and hands them to the new pipeline-lib
step
notifySkippedRequiredChecks(), which publishes aSUCCESSfor eachcorresponding required context, describing which stack and layer it belongs
to.
Those synthetic statuses are published before the stages run. Commit
statuses are last-write-wins per context, so any stage that does execute
overwrites its synthetic value with the real result, including a real
failure.
docs/dev/contributing.mdgains a "Required checks on a stacked pullrequest" section explaining the behaviour to contributors.
Validation
Stack 19075 (this PR and #19074), build 2:
SUCCESS in 6.6 min. Ten contexts synthesized, all of them the required
test/*ones.build/Build on EL 9andbuild/Build on Leap 15stayed real,since both stages are on the mid-stack allow list.
Before this can land
937e903— it pins@Librarytopipeline-lib@ryon-jensen/stacked-pr-tip-onlyso the unmerged librarycode could be exercised. Do not land that pin.
mid-stack for testing.
Draft: opened to validate CI behaviour.