SRE-4015 ci: run full PR verification only on the stack tip - #533
ryon-jensen wants to merge 3 commits into
Conversation
Rebased on master; #534 has landedpipeline-lib#534 (SRE-4019) merged as The rebase was clean — the two PRs touch no files in common. Locally: 30 tests pass, This is the cheap verification run referred to earlier: with build-only downstream testing on master, this PR's matrix should complete in minutes-to-about-an-hour per axis rather than the hours-to-days that forced build 1 to be aborted. Note for reviewers on the required checksThis PR will still show as blocked, and it is not caused by this change. Branch protection requires five checks that cannot report, because the stages producing them are hard-disabled in the
They are disabled on Follow-upOnce this lands, the matching |
GitHub stacked PRs open one pull request per layer, and today every
layer runs the complete DAOS verification pipeline. For a stack N deep
that is N full runs of work to validate a single change set, even
though only the tip contains the cumulative result that will land.
Add the primitives needed to make the pipeline stack aware:
prStack() fetches and caches the GitHub REST 'stack' object for
the current PR, failing open to an empty map so a
GitHub outage cannot break unrelated builds
isStackTip() true for non-stacked PRs and for the top layer of a
stack, honouring the CI_STACK_TIP_ONLY parameter and a
Skip-stack-optimization commit pragma as escape hatches
targetBranch() resolves the branch the change will actually land on
targetBranch() also fixes a latent correctness bug. For a mid-stack PR
env.CHANGE_TARGET is the parent layer's branch, not the landing branch,
so repository selection, package versions and stage skipping were all
computed against the wrong branch. The stack's base.ref is authoritative
and is now preferred when present.
skipStage() and skipFunctionalTestStage() skip everything on a mid-stack
PR except a small allow list of cheap checks plus a compile-only build,
so a broken layer still fails fast without paying for hardware testing.
githubAccess() is promoted out of setupDownstreamTesting so prStack can
share the same credential binding.
Signed-off-by: Ryon Jensen <ryon.jensen@hpe.com>
3341507 to
028d3f6
Compare
A status check context is published from inside the body of the stage that produces it, so a stage that does not run never reports it and GitHub blocks the pull request forever waiting for it. That makes a pull request in the middle of a stack unmergeable. Its expensive stages are deliberately skipped because the verification runs on the tip of the stack instead, so ten of the twenty contexts daos master requires are never published against it. Measured on a real three deep stack, the bottom pull request published four contexts and reported a mergeable state of blocked. As each layer merges, the one above it retargets to master and hits the same wall. Add notifySkippedRequiredChecks(), which reads the required contexts of the branch the pull request lands on and publishes a successful status for each one whose stage was deliberately not run, with a description saying why it is green. Contexts without a test/ or build/ prefix are not published by Jenkins and are left alone, so the GitHub Actions checks, which still run in full on a mid-stack pull request, are never touched. It is meant to be called before the stages run. Statuses are last-write-wins per context, so a stage that does run overwrites the status published here with its real result, including a real failure. It fails open throughout: an unprotected branch, a token that cannot read the protection settings and an unreachable API all publish nothing rather than failing the build. Signed-off-by: Ryon Jensen <ryon.jensen@hpe.com>
A mid-stack pull request is only proven to compile, so it should compile on both toolchains that the project ships. Building only EL 9 left a Leap 15 build break invisible until the tip of the stack ran, which is exactly the feedback the layer build is meant to give. Add 'Build on Leap 15' to stack_mid_stages(). It is a compile-only stage of the same cost class as 'Build on EL 9'. This also means notifySkippedRequiredChecks() no longer synthesizes 'build/Build on Leap 15': the stage runs, so it publishes its own real result. Signed-off-by: Ryon Jensen <ryon.jensen@hpe.com>
Note
The red build on this PR is not caused by this PR. Build 3 FAILED at
125 min with exactly one failing stage,
Test Library— the downstreamdaosrelease/2.6 axis. That axis fails inNLT on EL 8.8(UNSTABLE),a pre-existing breakage on 2.6 that was already observed and documented on
the same axis during SRE-4019 (#534), before any of this branch's code
existed. It needs its own ticket.
Every one of this library's own 27 executed stages passed, including
JUnit Tests,Self Unit TestandCommit Pragma tests. The other fivedownstream axes —
master,release/2.8,weekly-testing,weekly-2.6-testing,weekly-2.8-testing— are all SUCCESS.This PR will also need an admin merge regardless, per SRE-3162: master
requires five contexts whose stages are hard-disabled with
expression { false }, so no pipeline-lib PR can go green on its own.GitHub stacked PRs open one pull request per layer, and today every
layer runs the complete DAOS verification pipeline. For a stack N deep
that is N full runs of work to validate a single change set, even
though only the tip contains the cumulative result that will land.
Add the primitives needed to make the pipeline stack aware:
prStack() fetches and caches the GitHub REST 'stack' object for
the current PR, failing open to an empty map so a
GitHub outage cannot break unrelated builds
isStackTip() true for non-stacked PRs and for the top layer of a
stack, honouring the CI_STACK_TIP_ONLY parameter and a
Skip-stack-optimization commit pragma as escape hatches
targetBranch() resolves the branch the change will actually land on
targetBranch() also fixes a latent correctness bug. For a mid-stack PR
env.CHANGE_TARGET is the parent layer's branch, not the landing branch,
so repository selection, package versions and stage skipping were all
computed against the wrong branch. The stack's base.ref is authoritative
and is now preferred when present.
skipStage() and skipFunctionalTestStage() skip everything on a mid-stack
PR except a small allow list of cheap checks plus a compile-only build,
so a broken layer still fails fast without paying for hardware testing.
githubAccess() is promoted out of setupDownstreamTesting so prStack can
share the same credential binding.
Signed-off-by: Ryon Jensen ryon.jensen@hpe.com
Validation
Exercised against a real 3-deep stack in
daos-stack/daos(stack 19005, PRs #19002-#19004, now closed and their branches deleted):prStack()The mid-stack layers ran only
Cancel Previous Builds,Pre-build,Python Bandit check,BuildandBuild on EL 9.targetBranch()was confirmed to fix the landing-branch bug: on PR #19003, whoseCHANGE_TARGETwasryon-jensen/test_stacked_prs_1, the build ranci/gen_commit_pragmas.py --target origin/master.Two things were found and fixed during that validation, both of which the fail-open path absorbed without breaking a build:
prStack()is called while the Jenkinsfile is still loading, before any node exists, sosh()threwMissingContextVariableException.HttpURLConnectionwas rejected forsetRequestProperty.Both are why the lookup now uses only the
httpRequestandreadJSONsteps.Follow-up commit: letting the skipped layers go green
Skipping a stage is not free on a protected branch. Every Jenkins GitHub
status context is published from inside its stage body —
sconsBuild,runTest,buildRpmandprovisionNodesall callscmNotify/stepResultwith a
context:. A stage that never executes never publishes anything, soGitHub leaves the context as "Expected — waiting for status to be reported"
indefinitely. There is no end-of-build backfill anywhere in this library.
daos-stack/daosmaster requires 20 contexts, so without this the mid-stacklayers of a stack could never be merged. Measured on the original test stack
19005: mid-stack PR #19002 published 4 of the 20 and sat at
mergeable_state: blocked.vars/notifySkippedRequiredChecks.groovyreads the required status-checkcontexts of the branch the change will actually land on (via
targetBranch()),maps each
test/- orbuild/-prefixed context back to its stage name, andpublishes a
SUCCESSstatus for each one whose stage was deliberatelyskipped. The description names the stack and the layer, so the reason is
visible on the PR rather than buried in a build log.
It no-ops unless
CHANGE_IDandDAOS_JENKINS_NOTIFY_STATUSare both set,and fails open on any exception — the same posture as
prStack().Ordering is load-bearing. The 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. Publishing afterwards would mask genuine failures.
Contexts without a
test/orbuild/prefix are left alone: they are notpublished by this pipeline. On daos master that means the six GitHub Actions
checks, which continue to run in full on every layer.
Which stages a mid-stack layer still runs
stack_mid_stages()now includesBuild on Leap 15alongsideBuild on EL 9. A mid-stack layer is only ever proven to compile, so itshould compile on both toolchains the project ships; building only EL 9 left
a Leap 15 break invisible until the tip ran, which is exactly the feedback
the layer build exists to give. It is a compile-only stage of the same cost
class, and it added 3.8 min.
The daos Jenkinsfile carries the matching entry in its own mid-stack allow
list, since master and 2.8 gate stages through
shouldStageRun()rather thanthis library's
skipStage().This also means
build/Build on Leap 15is no longer synthesized: the stageruns, so it publishes its own real result.
Validation
Against a fresh 2-deep stack (stack 19075, daos PRs #19073 and #19074):
Ten contexts were synthesized, all of them the required
test/*ones.build/Build on EL 9andbuild/Build on Leap 15stayed real, because bothstages are on the mid-stack allow list. The layer finished in 6.6 min.
src/test/groovy/testNotifySkippedRequiredChecks.groovyadds 10 tests; 40pass in total,
spotlessCheckclean.Note
daos-stack/daosneeds the companion change, currently daos PR #19073 onbranch
ryon-jensen/SRE-4015, which callsisStackTip(),targetBranch()and
notifySkippedRequiredChecks(). This must land first or thatJenkinsfile will not load.