ci: cache Galaxy collections + pip, and bound every job (#53) - #56
Conversation
`make deps` resolved against galaxy.ansible.com on every run of `ansible-lint`, `galaxy-build` and `molecule`, uncached and without retry. The galaxy-build job on #50 died on a transient `Connection reset by peer` while resolving the transitive community.crypto and passed unchanged on re-run. Cache `ansible/collections` across all three jobs under one shared key. This is not just a speed-up: given requirements already satisfied on disk, `ansible-galaxy collection install` prints "Nothing to do" and never contacts a server — verified by pointing it at an unreachable Galaxy URL — so a hit is immune to that error class rather than merely faster than it. - Split `restore`/`save` rather than the all-in-one action: the failure being defended against is Galaxy dying part way through an install, and the combined action's post step would persist that half-written tree. `save` is gated on success. - No `restore-keys`: an older cache still satisfies the `>=` ranges in requirements.yml, which would make a `GALAXY_CACHE_EPOCH` bump a silent no-op. - `GALAXY_CACHE_EPOCH` is the manual lever to force a re-resolve, since those `>=` ranges mean a warm cache otherwise freezes the resolved set until requirements.yml changes. Declared in both workflows; they must match. Cache pip via setup-python, keyed on the workflow file — no pip manifest exists and the workflow is where the package list lives. The installs stay unpinned, so this saves the download, not the PyPI round trip; PyPI is not the flaky dependency here. Add `timeout-minutes` to every job, sized off observed runtimes (molecule 45, the ci.yml jobs 5-20). Without one a hung job burns the 360-minute default, and with `cancel-in-progress` some branch-protection setups read the resulting cancelled check as "not failed". Also add each workflow to its own path filter, so a change to this cache wiring is exercised by the PR that makes it — the helm filter already claimed ci.yml for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Prevent the duplicated Galaxy cache epochs from diverging and splitting the shared cache.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR improves CI reliability and speed through dependency caching, job timeouts, and self-testing workflow filters.
Changes:
- Added Galaxy collection and pip caching.
- Bounded all CI jobs with timeouts.
- Updated workflow path filters and contributor documentation.
File summaries
| File | Summary |
|---|---|
CONTRIBUTING.md |
Documents CI caching, timeouts, and Galaxy cache refreshes. |
.github/workflows/molecule.yml |
Adds caching, timeout, and self-triggering filters; duplicated cache epochs may diverge. |
.github/workflows/ci.yml |
Adds caching, timeouts, and CI workflow path coverage. |
Review details
Suppressed comments (1)
.github/workflows/molecule.yml:14
- Because
GALAXY_CACHE_EPOCHis part of the shared key, changing onlyci.ymldoes not run this workflow's molecule job (and changing only this file does not enable ci.yml's Ansible jobs). The two values can therefore diverge without a failing check, silently splitting the cache; derive the epoch from one tracked value or add a cross-workflow equality check.
paths: ['ansible/**', '.github/workflows/molecule.yml']
push:
branches: [main]
paths: ['ansible/**', '.github/workflows/molecule.yml']
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review catch: GALAXY_CACHE_EPOCH was duplicated as an env var in ci.yml and molecule.yml, and nothing could catch them diverging — molecule.yml's `paths` do not include ci.yml, and ci.yml's `ansible` filter does not include molecule.yml, so editing the epoch in one file alone produces no failing check while silently splitting the shared cache key in two. A comment saying "bump BOTH" is documentation, not a guard. Fold the counter into ansible/requirements.yml, which the key already hashes. One file, one hash: divergence is now structurally impossible rather than merely discouraged. A bump also matches both workflows' `ansible/**` trigger, so it is exercised immediately instead of on the next unrelated PR. Also correct CONTRIBUTING.md: it claimed both caches use actions/cache's split restore/save. Only the Galaxy one does — pip uses setup-python's built-in cache and has no restore/save pair. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re: the duplicated
|
No-op. The two prior runs each started cold (the first had no cache, the second moved the key by editing requirements.yml). This run is the first with a populated cache, and is what actually demonstrates the claim the change rests on: `make deps` reporting "Nothing to do" with no galaxy.ansible.com request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Warm-cache run: the claim is now verified in CI, not just locally
Occurrences of That download is the exact call that failed #50 with Save gating confirmed: neither Pip cache restored too — and notably across a changed workflow-file hash ( Summary of the three runs
Ready for review. 🤖 Generated with Claude Code |
Closes #53.
1. Galaxy collections cache
make depsresolved againstgalaxy.ansible.comon every run ofansible-lint,galaxy-buildandmolecule, with no cache and no retry — thegalaxy-buildjob on #50 died on a transientConnection reset by peerwhile resolving the transitivecommunity.crypto, then passed unchanged on re-run.ansible/collectionsis now cached across all three jobs under one shared key.This removes the flake class rather than shortening it. With the requirements already satisfied on disk,
ansible-galaxy collection installmakes zero network calls — verified by pointing it at an unreachable Galaxy server:Three deliberate choices:
actions/cache/restore+/save, not the all-in-one. The failure being defended against is Galaxy dying part way through an install; the combined action's post step would happily persist that half-written tree.saveis gated on the restore having missed and on the deps step succeeding.restore-keys. An older cache still satisfies the>=ranges inrequirements.yml, somake depswould find nothing to do and aGALAXY_CACHE_EPOCHbump would be a silent no-op.GALAXY_CACHE_EPOCHas the refresh lever. Because those are ranges and not pins, a warm cache would otherwise freeze the resolved set (transitive collections included) untilrequirements.ymlchanges, drifting CI away from what a localmake lint-ansibleresolves. The epoch makes that refresh explicit and visible — the same manual posture.github/dependabot.ymlalready declares for Galaxy. It is declared in both workflows and must match;CONTRIBUTING.mdlists it alongside the other bump-in-two-places values.2. pip cache
Via
setup-python, keyed on the workflow file — this repo ships no pip manifest, and the workflow is literally where the package list lives. Two limits, recorded in the comments rather than glossed:--upgrade, so pip still queries the PyPI index to resolve; the cache saves the download, not the round trip. That is the right trade — PyPI is not the flaky dependency here.setup-pythonderives its own key and takes no custom suffix, soansible-lintandgalaxy-buildshare one pip cache. Their package sets overlap heavily, so whichever populates it first covers most of both.3.
timeout-minuteson every jobThe issue named
molecule; theci.ymljobs had the same gap, so all seven are bounded. Values are bounds sized off observed runtimes, not targets:moleculeJOBS=3galaxy-buildansible-linthelm/kicsactionlintchanges4. Path filters made self-testing
The ansible jobs' cache wiring now lives in
ci.yml, and the molecule job's inmolecule.yml— but neither file was in its own trigger, so this very PR would not have exercised the jobs it changes. Each workflow now lists itself, the same reason thehelmfilter already claimedci.yml.Verification
actionlint(the blocking CI gate for workflow syntax) — clean, exit 0.make lint(all pre-commit hooks) — clean.restore+ 3savesteps (a divergence would silently split the cache).actions/cachepinned tocaa29612…, confirmed to be exactly tagv5.1.0and to carry both sub-actions at that commit.What CI here proves and doesn't: this first run is a cold cache by definition, so it should log
Cache not found for input keys: galaxy-…and save at the end. The payoff is visible on the second run —Cache restored from key: galaxy-…plusNothing to do. All requested collections are already installed.with no download lines. Worth an empty follow-up commit to confirm before merging.🤖 Generated with Claude Code