This repository can take down the fleet

Most engineering organisations have worked out how to review application code. There are tests, there is a staging deploy, there is a rollback button, and everybody understands roughly how bad a bad merge can be.

Ansible repositories usually inherit none of that, and the blast radius is larger.

An application deploy changes one service. It runs on the hosts that run that service, it can usually be rolled back by redeploying the previous artefact, and its failure mode is that one thing stops working.

A merge to an Ansible repository can change every service on every host. The hosts: line decides the reach, and a two-character edit to it turns a change to the web tier into a change to the estate. There is no artefact to roll back to, because the change was applied in place — the previous state is whatever the machines happened to have, and nothing recorded it.

That asymmetry is the premise of this part. The review standards for this repository should be at least as strict as for the application code it configures, and in most organisations they are considerably weaker.

What a reviewer is actually looking for

Not style. ansible-lint reads style, and a human doing it is a human not doing the thing only a human can do. Four questions, in this order.

1. What is the scope? Which hosts, how many, and did that number change? The relevant lines are hosts:, --limit in any wrapper or CI job, serial, delegate_to, run_once, and any change to inventory group membership. A diff that moves one host between groups can double a blast radius while touching two lines.

2. Is it idempotent? Will the second run report changed=0? The usual offenders are command and shell without creates, removes or a changed_when, and any task whose result depends on the current time or on a remote value that varies. Part XII covers the mechanics; the review question is narrower — has this been run twice, and what did the second run say?

3. Is it safe to rerun mid-incident? Different from idempotent. A play can be perfectly idempotent and still restart a service on every run because a handler fires from an unrelated task. At 03:00 somebody will run this to fix something else, and the question is what else it will do while they are not watching.

4. What is the rollback? For a configuration change, usually “revert the commit and rerun”. For a package upgrade, a data migration, a deleted file or a rotated credential, usually nothing — and that is an acceptable answer as long as it is stated. A change whose rollback nobody thought about is a change whose rollback will be invented under pressure.

$ ansible-playbook -i inventories/production playbooks/webservers.yml --list-hosts

playbook: playbooks/webservers.yml

play #1 (webservers): Configure web tier	TAGS: []
  pattern: ['webservers']
  hosts (2):
    web-prod-01.example.com
    web-prod-02.example.com

Run it on the base branch and on the change, and attach both. A difference in that host count is the most consequential thing a review of this repository can find, and it is invisible in every other artefact.

Reading YAML is not review

Here is the uncomfortable part. A careful reviewer reading a diff carefully will still miss most of what matters, because the diff does not contain it.

Consider a one-line change to a template:

-worker_processes {{ webserver_worker_processes }};
+worker_processes auto;

Everything a reviewer needs is absent from those two lines:

• Which hosts get this? Not in the diff — it is in the play, the inventory and the --limit the deploy job passes.
• What does auto resolve to on the production machines? Not in the diff — it depends on the CPU count of hosts the reviewer has never seen.
• Does this restart anything? Not in the diff — it depends on whether the template task notifies a handler, and on whether that handler reloads or restarts. Part XVI’s distinction between the two is the difference between a blip and an outage.
• Was webserver_worker_processes used anywhere else? Not in the diff. Removing its last consumer makes it dead configuration nobody will clean up.

A reviewer can chase every one of those by opening four other files. Most will not, on most pull requests, and no process survives depending on that.

A review checklist for an Ansible pull request

1. Scope: which hosts, how many, and did the number change? Look at hosts:, group membership, --limit in the deploy job, serial, delegate_to and run_once. Ask for --list-hosts output if the answer is not obvious.
2. Idempotence: has it been run twice, and did the second run report changed=0? Every command or shell task needs creates, removes or changed_when, or an explicit reason why not.
3. Rerun safety: what happens if someone runs this mid-incident to fix something unrelated? In particular, which handlers fire, and do they reload or restart?
4. Check-mode evidence: is a --check --diff output attached, and does it match what the description claims the change does?
5. Secrets: no plaintext credentials, no secrets in a diff, no new file that should have been vaulted. The scanner is the backstop, not the review.
6. Pinning: if requirements.yml changed, is the new version pinned exactly, and has the pipeline run against it?
7. Rollback: stated explicitly, including "there is none" where that is the truth. Note anything irreversible - migrations, rotations, deletions.
8. Removal done properly: a deleted task does not undo anything. Removal means state: absent for a release, then deletion.
9. On call: who is on call when this merges, and do they know? A change that merges on Friday and runs on Sunday needs someone who is expecting it.

Knowledge check