Skip to content

Commit 7138684

Browse files
committed
ci: address review on the locked base bake lane
Four points from the review of autowarefoundation#7238, all in validate-lockfiles.yaml. The require-label call site had no run-condition, unlike the three that already exist in health-check-ansible.yaml, health-check-ansible-artifacts.yaml and health-check-pr.yaml. All four publish the same required `require-label / require-label` context, and without run-condition this one reports red -- not skipped -- on a PR that reaches the workflow without touching a bake input. Add a changed-files job and wire it in. Scoping that job to the bake's own inputs also settles the trigger argument the review raised: one `paths:` filter was serving both an eight-second lint and two cold container builds, which want opposite filters. It now only has to be wide enough for whichever job could be affected, and each job narrows from there. Markdown is excluded outright, since 19 READMEs live under ansible/roles/** and the previous comment claimed they were already outside the filter. ansible-galaxy-requirements is added: base.Dockerfile bind-mounts it and it decides which collection the playbook resolves, so both sibling health-check workflows watch it. The matrix emitted `build-locked-base (humble)` and `(jazzy)` with no aggregator, so no stable name existed for a branch ruleset to require. Add one, mirroring install-dev-env-required. setup-buildx-action moves to v4, matching the three other call sites in this repository. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
1 parent 8f60853 commit 7138684

1 file changed

Lines changed: 66 additions & 10 deletions

File tree

‎.github/workflows/validate-lockfiles.yaml‎

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,19 @@ on:
77
- synchronize
88
- reopened
99
- labeled
10-
# Only what can change the outcome of a locked build, plus the scripts that
11-
# generate and validate the lockfiles themselves. Deliberately narrower than
12-
# ansible/** and docker/**: both READMEs and docker/examples live under
13-
# those, and a docs-only PR must not trigger two cold container builds.
10+
# Wide enough for whichever of the two jobs below could be affected. They no
11+
# longer share one gate: the cheap lint runs on anything reaching this
12+
# workflow, and the two cold container builds gate themselves on a
13+
# changed-files job scoped to what the `base` target actually consumes.
14+
# Markdown is excluded outright — 19 READMEs live under ansible/roles/**,
15+
# and a docs-only PR must start neither job.
1416
paths:
15-
- ansible/roles/**
16-
- ansible/vars/**
17-
- ansible/playbooks/**
18-
- ansible/scripts/**
17+
- ansible/**
18+
- ansible-galaxy-requirements.yaml
1919
- docker/*.Dockerfile
2020
- docker/docker-bake.hcl
2121
- .github/workflows/validate-lockfiles.yaml
22+
- "!**/*.md"
2223

2324
concurrency:
2425
group: ${{ github.workflow }}-${{ github.ref }}
@@ -42,13 +43,49 @@ jobs:
4243
- name: Validate lockfiles (format + digest/lockfile coverage)
4344
run: ./ansible/scripts/validate_lockfiles.sh
4445

46+
# What a locked `base` bake resolves against: the roles install_rmw runs, the
47+
# lockfile it reads, the collection those roles are resolved from, and the two
48+
# docker files the bake itself consumes. Deliberately narrower than the
49+
# workflow trigger above — ansible/scripts/** and the other four playbooks
50+
# reach the lint but no bake.
51+
changed-files:
52+
runs-on: ubuntu-latest
53+
outputs:
54+
should-run: ${{ steps.check.outputs.any_modified }}
55+
steps:
56+
- uses: actions/checkout@v6
57+
with:
58+
fetch-depth: 0
59+
60+
- id: check
61+
uses: step-security/changed-files@v47
62+
with:
63+
files: |
64+
ansible/roles/**
65+
ansible/vars/**
66+
ansible/playbooks/install_rmw.yaml
67+
ansible-galaxy-requirements.yaml
68+
docker/base.Dockerfile
69+
docker/docker-bake.hcl
70+
.github/workflows/validate-lockfiles.yaml
71+
files_ignore: |
72+
ansible/roles/artifacts/**
73+
**/*.md
74+
4575
# The locked base bakes below are two cold, uncached container builds. Put
4676
# them behind the same opt-in label the repo already uses for its other
47-
# expensive lanes (see health-check-ansible.yaml).
77+
# expensive lanes, wired to run-condition exactly as health-check-ansible.yaml,
78+
# health-check-ansible-artifacts.yaml and health-check-pr.yaml wire theirs.
79+
# Without run-condition this call site reports red — not skipped — on any PR
80+
# that reaches the workflow without touching a bake input, and all four sites
81+
# publish the same required `require-label / require-label` context.
4882
require-label:
83+
needs: changed-files
84+
if: ${{ always() }}
4985
uses: autowarefoundation/autoware-github-actions/.github/workflows/require-label.yaml@v1
5086
with:
5187
label: run:health-check
88+
run-condition: ${{ needs.changed-files.outputs.should-run == 'true' }}
5289

5390
build-locked-base:
5491
needs: require-label
@@ -63,7 +100,26 @@ jobs:
63100
uses: actions/checkout@v6
64101

65102
- name: Set up Docker Buildx
66-
uses: docker/setup-buildx-action@v3
103+
uses: docker/setup-buildx-action@v4
67104

68105
- name: Build the locked base image
69106
run: USE_LOCKFILE=true ROS_DISTRO=${{ matrix.rosdistro }} docker buildx bake -f docker/docker-bake.hcl base
107+
108+
# The matrix above emits one context per distro, none of which is a stable
109+
# name a branch ruleset can require. Aggregate them into one, the same way
110+
# health-check-ansible.yaml aggregates install-dev-env, so this lane can be
111+
# promoted to a required check without the name changing again when a distro
112+
# is added or dropped.
113+
build-locked-base-required:
114+
needs: build-locked-base
115+
if: ${{ !cancelled() }}
116+
runs-on: ubuntu-latest
117+
steps:
118+
- name: Verify build-locked-base outcome
119+
run: |
120+
result="${{ needs.build-locked-base.result }}"
121+
echo "build-locked-base: $result"
122+
case "$result" in
123+
success|skipped) exit 0 ;;
124+
*) exit 1 ;;
125+
esac

0 commit comments

Comments
 (0)