test(molecule): #75 review follow-ups, and a host-wide lock on the suite

[thiras]

Follow-up to #75, which was merged before its review findings were applied. This PR applies all of them.

host-env / schema (test(molecule): act on the #75 review…)

• One of my comments in test(molecule): fix false passes in host-env/schema and shared staging in validation #75 was false. It said systemd_service rejects a bare name. It doesn't: default values satisfy that argument check, and on ansible-core 2.21.3 -a name=… returns changed: false plus the full status. The unit is now read with just name:. state: started and check_mode are gone.
• NRestarts is now documented correctly. A manual restart resets the counter to 0. That reset is why "crash, then the handler" passes and "crash, no handler" fails.
• Restart=always is now asserted. The NRestarts check depends on it. Without it, a crashed unit is started by the converge's state: started, so a missing handler again shows a new PID with NRestarts 0.
• Dead daemon vs. missed restart. "Daemon running" and "PID changed" are now separate asserts, so a daemon that is down isn't reported as a missed handler restart.
• NRestarts is compared as a string. | int turns anything unparsable into 0.
• The metrics listener is tied to the unit's MainPID via ss -ltnp, in both host-env and schema. The comments now say what the check is: a liveness check on the daemon under the rendered unit. The stub hardcodes 127.0.0.1:9090 and ignores node.toml, so the bind address itself is still only checked in default.

validation

• The per-run tempdirs are created before their blocks. A failure on the controller side now stops the play with its own error. Before, rescue: swallowed it and the final tally reported it as a validation regression.
• Leaked tempdirs are swept. Dirs older than a day, left by interrupted runs, are removed. That age is far longer than any run, so a concurrent run's dirs are never touched.
• The comment on what the old fixed path shared is corrected.

Suite lock (build(molecule): refuse to overlap two suites on one host)

#75 only removed the controller-side path collision. Overlapping suites still collided on:

• container names, which are fixed on the one Docker daemon, including across checkouts and worktrees;
• each scenario's leading destroy;
• lifecycle/cloud-init files in molecule's ephemeral dir.

make molecule and make molecule-serial now take a flock shared by every user on the host: the directory /tmp/decdn-devops-molecule.lock. It's a directory because fs.protected_regular stops users opening each other's regular files in /tmp. deps runs under the lock too, since it rewrites ansible/collections, which a running suite reads. A second suite exits 75 with a message instead of breaking the first. flock -o keeps the lock out of anything molecule leaves running. tests/scripts-test.sh covers the refusal for both targets. CI's ubuntu runners already have util-linux flock.

Verification

• make lint-ansible, make lint, and tests/scripts-test.sh all pass (including the two new lock cases).
• The full make molecule passes: 11/11 scenarios, run through the new locked path.
• A real second make molecule started during that run was refused: "another molecule suite holds /run/user/1000/decdn-devops-molecule.lock".
• Negative checks, applied temporarily and then reverted:
  • Missed restart hidden by a crash. Removed the env-checksum notify and killed the daemon in side_effect. "running" and "PID changed" both still passed, which is the old false pass. The new assert failed with Restart=always, NRestarts=1.
  • Leaked-dir sweep. Planted a 2-day-old decdn-molecule-badbin.* dir and a fresh decdn-molecule-checkbin.* dir. validation removed the old one and left the fresh one alone.
• An earlier full run hung, unrelated to this change. Three privileged systemd containers froze at boot in the same second: systemd stuck in initializing, all jobs waiting, and grafana-cloud's container died. The role's Restart decdn-node handler then waited indefinitely. The re-run passed cleanly. A suite that hangs instead of failing is a separate problem worth an issue: a restart timeout, or a boot-complete check in prepare.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Dependency installation remains outside the lock, the default lock path is not host-wide, and test failure cleanup can leak the holder process.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

• Use a host-wide default lock path across users · New
• Acquire lock before installing shared Ansible collections · New

What changed in this PR

This PR hardens Molecule validation and adds locking to prevent overlapping suite runs from corrupting shared Docker state.

Changes:

• Strengthens restart and metrics assertions.
• Isolates and cleans validation tempdirs.
• Adds suite locking, documentation, and refusal tests.

File	Summary
tests/​scripts-test.sh	Tests lock refusal behavior.
CONTRIBUTING.md	Documents Molecule locking requirements.
ansible/​molecule/​validation/​converge.yml	Isolates and cleans validation tempdirs.
ansible/​molecule/​schema/​verify.yml	Ties metrics checks to the daemon process.
ansible/​molecule/​host-env/​verify.yml	Strengthens restart and metrics checks.
ansible/​molecule/​host-env/​side_effect.yml	Validates clean pre-edit service state.
ansible/​Makefile	Adds Molecule locking and fan-out targets.

---

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