fix(ci): guard the matrix deployment jobs against an empty job list - #2095
Conversation
wait-for-deployment builds its matrix from build-helmreleases.outputs.staging_helmreleases, and create-auto-merge-prod-pr from updated_prod_cluster_names. Neither checked the value for '[]' first, unlike every other matrix job in this file (services-build and helm-charts-build both guard on != '[]' && != ''). A matrix that expands to zero combinations cannot produce a job, so the reusable-workflow call reports failure. On PR #2092 that surfaced as a gate failure with no failing job anywhere: 15 of the pipeline's 16 jobs were success or skipped, wait-for-deployment had no check run at all, and alls-green received gooddata-ruby-lcm-promote-pipeline result=failure with empty outputs. The trigger was #2094. Before it, create-staging-pr failed outright when there was nothing to deploy, so wait-for-deployment's !contains(needs.*.result, 'failure') was false and the job was skipped before its matrix was ever evaluated -- it showed up as 'skipped' in run 32728855771. Once create-staging-pr correctly succeeded on a no-op, the condition passed, the matrix was evaluated for the first time in this state, and it was empty. #2094 did not introduce this, it removed the failure that was masking it. staging_helmreleases is empty whenever stg1 already runs the built chart, which is the normal state after a previous attempt delivered its staging PR -- gitops-deployments#26557 in this case. create-auto-merge-prod-pr carries the same defect. It is latent here because the gate sets AUTO_MERGE: false, but it is live for any consumer that auto-merges into an already up-to-date production cluster. JIRA: GRIF-518
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe workflow adds checks that prevent deployment wait and automatic production PR jobs from running when their related output lists are empty arrays or empty strings. ChangesLCM output guards
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The change makes empty CI deployment matrices skip cleanly instead of reporting a false failure; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) Comment |
2 participants
Symptom
PR #2092's gate failed twice on a base that already contained #2093 and #2094 (32836679962, 32837479726) — with no failing job anywhere. Every job was
successorskipped, yetready-to-mergewent red:alls-greenreceived"gooddata-ruby-lcm-promote-pipeline": {"result": "failure", "outputs": {}}while all 15 of its visible children were green or skipped.Root cause
wait-for-deploymentbuilds its matrix frombuild-helmreleases.outputs.staging_helmreleasesand never checked it for'[]'. A matrix expanding to zero combinations cannot produce a job, so the reusable-workflow call reportsfailure.The evidence lines up exactly:
build-helmreleasesloggedThere is nothing to update for chart lcm-bricks in cluster 62, skipping..., andstaging_hrs=$(echo "$hrs_per_cluster" | jq -c '."62" // []')therefore emittedstaging_helmreleases=[].936ec244). The one missing entirely iswait-for-deployment— note thatpromote-to-stable,lcm-integration-e2eand the rest are all present asskipped, so absence here is distinct from being skipped.wait-for-deploymententry exists in the downloaded run log archive either.Why it appeared only after #2094
This is not a regression from #2094 — that PR removed the failure that was masking it.
wait-for-deployment's condition includes!contains(needs.*.result, 'failure'), andcreate-staging-pris one of itsneeds.create-staging-prwait-for-deploymentifskipped— run 32728855771[]failurestaging_helmreleasesis empty whenever stg1 already runs the built chart — the normal state after an earlier attempt delivered its staging PR, which is what gitops-deployments#26557 did here at 11:07Z on 24 Aug.Fix
Apply the convention the rest of this file already follows.
services-build(lines 98-99) andhelm-charts-build(151-152) both guard with!= '[]' && != ''; the two jobs consumingbuild-helmreleasesoutputs were the only ones that did not.create-auto-merge-prod-prhas the identical defect onupdated_prod_cluster_names. It is latent in this repo because the gate setsAUTO_MERGE: false, but it is live for any consumer that auto-merges into an already up-to-date production cluster — so it is fixed here too rather than left to be rediscovered.After this change all six matrix jobs are guarded:
Expected effect on #2092
wait-for-deploymentskips cleanly →lcm-integration-e2eskips →promote-to-stableruns (it needs onlybuild-helmreleases), publishingstable/lcm-bricks:2.0.9→create-manual-merge-prod-propens a real prod PR →ready-to-mergepasses viaallowed-skips.wait-for-deploymentnever got a runner that run, so the rollout was never confirmed green. Check the stg1lcm-bricksHelmRelease is Ready before merging the prod PR.Note on upstream
These workflows are a verbatim fork of
gooddata/github-actions/.github/workflows/ii-*, which 23 other repos consume. Upstream still carries this bug plus the two fixed in #2093 and #2094. Worth porting all three.Testing
lcm-pipeline.yamlparses (ruby -ryaml, 16 jobs). Everystrategy.matrixjob was enumerated programmatically and checked for a matching!= '[]'guard — output above. The empty-matrix path is only exercisable by a real chart change through the merge queue; #2092 is that change.🤖 Generated with Claude Code
Summary by CodeRabbit