fix(template): Make "Finished Build and Publish" gate reflect real build outcome - #614
fix(template): Make "Finished Build and Publish" gate reflect real build outcome#614lfrancke wants to merge 1 commit into
Conversation
…ild outcome The `finished` job is the single required status check for the build workflow. It had no `if:` clause, so a failed dependency caused GitHub to *skip* it rather than fail it - and branch protection treats a skipped required check as passing. A broken build (e.g. a failed publish-index-manifest) therefore became mergeable. Two fixes: - Add `if: always()` so the gate always runs and reports a real success/failure conclusion. - List every leaf job directly in `needs` and fail the gate on any `failure`/`cancelled` result. Previously publish-index-manifest was only a transitive dependency (via openshift-preflight-check), so its failure would not surface in `needs.*.result`. `skipped` is tolerated, since jobs skip legitimately on merge_group events, forks, and when detect-changes finds no relevant changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NickLarsenNZ
left a comment
NickLarsenNZ
left a comment
There was a problem hiding this comment.
Has this been tested on one of the operator repos?
|
No. Do you want me to? I'm never sure how this can be tested. Is it okay to open a draft PR with these changes? Will they pick up the "new actions" from within the PR? |
It's best to try it on one before rolling it out to all and having to come back to fix things. Just a branch is enough, if you want to manually trigger it. Otherwise a draft PR if you want to see what happens on PR. |
|
@NickLarsenNZ Previously this would still be mergable as we've seen in that run during our release. Now it's not.
As soon as you tell me that you looked at it I can fix that draft PR so it builds again and then it should be mergable. |
2 participants
Problem
The
finishedjob (Finished Build and Publish) is the single required status check for the build workflow. We did this so we don't have to list every step in between as required.During our 26.7 release we saw a build that succeeded even though it shouldn't have: https://github.com/stackabletech/listener-operator/actions/runs/29825092091
Fix
Github considers skipped jobs as succeeded rather than failing the job. So we need to make sure our
finishedjob always runs.This PR changes it by introducing an
if: always()to the jobif: always()so the gate always runs and reports a real success/failure conclusion.needsand fail on anyfailure/cancelledresult.publish-index-manifestwas previously only a transitive dependency (viaopenshift-preflight-check), so its failure would not surface inneeds.*.resulteven withalways().skippedis tolerated on purpose, jobs skip legitimately onmerge_groupevents, forks, and whendetect-changesfinds no relevant changes.