perf(molecule): run scenarios in parallel, enable pipelining

[thiras]

Why

make molecule took 595s. Profiling the default scenario (105s) showed the time is not in the role's logic but in per-task connection overhead — 258 task executions across converge+idempotence, each remote task paying ~1.2–1.9s for module transfer over docker exec (asserts, which run locally, are ~0.03s), plus ~11s/scenario of fixed docker-driver create+destroy wait.

Configuration	Full suite	-s default
before	595s	105s
+ pipelining	—	82s
+ scenarios in parallel	151s	—
+ leading destroy (final)	154s	—

What changed

Parallel scenarios (ansible/Makefile) — the six scenarios already own distinct container names, publish no host ports, and share only read-only state on the control machine, so they fan out with xargs -P. Output carries a [scenario] prefix because runs interleave; JOBS=<n> caps the width; make molecule-serial keeps the readable sequential path (it stops at the first failure, so it reports less — but legibly).

Molecule's own --workers is not usable here. I tested it: it refuses to run outside collection mode ("--workers > 1 is only supported in collection mode (galaxy.yml required)"), and adding a root galaxy.yml drops ansible-lint from production to safety profile until three metadata files are duplicated — and worker mode never runs create, so all six scenarios fail UNREACHABLE in ~40s. The Makefile records this so it isn't re-litigated.

Pipelining — community.docker.docker declares has_pipelining, and molecule's docker driver connects with exactly that plugin, so ANSIBLE_PIPELINING removes a docker exec round trip per remote task.

Dropped the dependency step — it never vendored anything. The roles invoker reads role-file, not requirements-file (which it filters out); the collections invoker resolved the relative path against the process CWD, so ../../requirements.yml pointed outside the repo. Both halves only warned — 12 warnings per run. Collections come from make deps, now a prerequisite of both molecule targets instead of an unenforced ordering convention living in the CI workflow.

Leading destroy — the docker driver creates with recreate: false, so a container orphaned by an aborted run (likelier now that six run at once) was reused, and converge + idempotence would pass trivially against an already-converged host.

acl in baseline_packages — decdn_node runs decdn key-gen and the decdn config validate gate as the unprivileged decdn user; on Debian Ansible needs ACL support to hand the temp module file across. Without it both fail with no rc at all, which the role can only report after the fact (tasks/main.yml already names acl in that message). Molecule never caught this because it connects as root, where the same hand-off is a plain chown. This one ships in the collection, so it has a CHANGELOG entry.

Fail-loud guards (AGENTS.md hard rule 4)

Scenario discovery moved from molecule to a make glob, which introduces a new way to pass without testing anything. Three guards close it:

• an empty or truncated scenario set is refused (a glob matching nothing would otherwise run zero scenarios and exit 0 — a green build that tested nothing), as is a non-positive JOBS (xargs -P 0 means unlimited);
• set -euo pipefail keeps the prefixing sed from masking molecule's exit status, and each scenario announces [<name>] SCENARIO FAILED, since the bare xargs message doesn't say which of six;
• xargs then exits non-zero and make fails.

Test plan

• make molecule — all six scenarios green, 154s (missing=0 failed=0 each), no leftover containers.
• Orphan case verified directly: left a converged container behind via molecule converge -s default, re-ran the suite; log order is destroy → create → prepare → converge, and prepare reported changed on /var/lib/decdn and /etc/decdn — on a reused container those would have been ok.
• Guards: empty SCENARIOS → refused; truncated (SCENARIOS="default schema") → refused; JOBS=0 → refused; a failing scenario → [name] SCENARIO FAILED → xargs 123 → make … Error 123.
• make lint (all pre-commit hooks) and make lint-ansible (production profile, 0 failures) green.

Notes for the reviewer

• CI runs make molecule JOBS=3 rather than the local default of one job per scenario — a hosted runner has far fewer cores and each scenario is a privileged systemd container. That cap is the one value I could not benchmark from a dev box; if this job turns flaky, drop to JOBS=1 or swap in make molecule-serial.
• acl is unverifiable by this suite by construction (molecule connects as root; baseline isn't exercised in containers) — make check against a real host is what exercises it.
• Known coverage boundary, documented in default/molecule.yml: with pipelining on, command/shell modules are fed over stdin, so the remote-tmp path behind become_user isn't exercised. Molecule never reproduced that path anyway for the root-connection reason above.
• Left for follow-up (pre-existing, surfaced by review): host-env/verify.yml:159 asserts a PID delta against a Restart=always unit; host-env/schema have no positive readiness assertion; validation stages .molecule-badbin into the shared project dir rather than its ephemeral dir.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved moderate findings concern redundant dependency installation and unsafe JOBS=00 handling; the failure-status documentation also needs correction.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Speeds Molecule validation through parallel scenarios, pipelining, safer cleanup, and clearer dependency handling.

Changes:

• Adds guarded parallel and serial Molecule targets.
• Enables pipelining and leading container destruction.
• Adds acl to baseline packages and updates documentation and CI.

File summaries

File	Summary
Makefile	Exposes Molecule execution targets.
CONTRIBUTING.md	Documents parallel and serial test execution.
ansible/roles/baseline/defaults/main.yml	Adds acl to baseline packages.
ansible/README.md	Documents Molecule workflows and scenarios.
ansible/molecule/validation/molecule.yml	Updates Molecule configuration and removes the ineffective dependency step.
ansible/molecule/slow-readiness/molecule.yml	Updates Molecule configuration and removes the ineffective dependency step.
ansible/molecule/schema/molecule.yml	Updates Molecule configuration and removes the ineffective dependency step.
ansible/molecule/host-env/molecule.yml	Updates Molecule configuration and removes the ineffective dependency step.
ansible/molecule/generate-keystore/molecule.yml	Updates Molecule configuration and removes the ineffective dependency step.
ansible/molecule/default/molecule.yml	Enables pipelining, cleanup, and removes the ineffective dependency step.
ansible/Makefile	Implements parallel execution and guards. Moderate (3 votes): reject all-zero JOBS values; moderate (1 vote): remove redundant workflow dependency installation; nit (1 vote): correct documented failure-status behavior.
ansible/galaxy/CHANGELOG.md	Records the acl package change.
AGENTS.md	Updates repository command guidance.
.github/workflows/molecule.yml	Runs capped parallel Molecule execution. Moderate (2 votes): remove the redundant standalone make deps step.

Review details

Suppressed comments (2)

ansible/Makefile:96

• The failure-status note does not match the command below: the bash -c wrapper converts every failed pipeline to exit 1, so xargs sees 1 and returns 123; it can never see the original 255 and return 124 or stop on that status. Please document the actual normalized behavior so failure diagnosis is not misleading.

#   - xargs then exits non-zero: 123 for an ordinary failure, or 124 on a status-255
#     abort, which also skips any scenario it had not started yet.

ansible/Makefile:101

• This new prerequisite makes the workflow install Galaxy collections twice: .github/workflows/molecule.yml still runs make deps before invoking this target, and this target invokes deps again. Keep the prerequisite so direct local runs remain self-contained, but remove the redundant workflow install step to avoid the extra Galaxy resolution/install during every CI run.

molecule: deps

• Files reviewed: 14/14 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[thiras]

Filed the follow-ups this PR deliberately left out, so they don't get lost with the branch:

• molecule: two host-env/schema assertions can pass for the wrong reason #51 — host-env/schema assertions that can pass for the wrong reason (PID delta against a Restart=always unit; no positive readiness assertion). Pre-existing, but parallel runs make them likelier to bite.
• molecule: validation stages .molecule-badbin into the shared project dir #52 — validation stages .molecule-badbin into the shared project dir rather than its ephemeral dir.
• CI: cache Galaxy collections + pip, and bound the molecule job #53 — cache Galaxy collections + pip in CI, and bound the molecule job with timeout-minutes. Motivated by the transient galaxy.ansible.com reset that failed galaxy-build on this PR.
• Decide whether real deploys should use Ansible pipelining too #54 — decide whether real deploys should use pipelining too, now that the suite does.

[Copilot]

🔵 Needs a closer look

Replace the non-portable sed -u usage and correct the signal-related exit-code documentation.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

ansible/Makefile:114

• sed -u is a GNU-sed-only option; the default BSD sed on macOS exits with an illegal-option error, so the new local make molecule target fails before running any scenario on otherwise usable Docker setups. Use a portable prefixing loop (or omit -u; it only affects buffering) instead of requiring GNU sed.

ansible/Makefile:99

• The wrapper normalizes every non-zero molecule | sed status to exit 1, including a molecule process terminated by a signal (for example, SIGTERM becomes 143 before the wrapper exits 1). Therefore xargs will report 123 for that case, not 125 as this comment states; please correct the documented exit-code distinction or preserve the signal status if 125 is intended.

#     otherwise cut the suite short. So: 123 when a scenario failed, 125 if one was
#     killed by a signal, and every scenario always gets to run.

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Lite