tasks: abort the collection chain when a play fails

[ideaship]

osism apply <collection> keeps dispatching dependent roles after an early play has already failed permanently. A nutshell deploy whose keystone bootstrap dies in the first two minutes runs to the 4h30m Zuul job timeout and reports zero failed tasks.

The bug

run_ansible_in_environment captures the Ansible exit code, writes it to the Redis output stream and to the execution-history JSON — and then returns the accumulated output string. Nothing raises, so Celery marks the task SUCCESS regardless of the rc and fires the next link in the chain.

The single-role path never noticed, because it reads the rc back out of the Redis stream (handle_task → fetch_task_output) rather than out of the Celery result. The collection path has only Celery's view of the task, and Celery's view was always SUCCESS.

Last seen in the 2026-08-27 periodic-midnight run, in both the current and next lanes — and the cleanest instance on record, because no failure detection subtlety is involved:

• keystone's bootstrap container failed permanently at 01:02:41
• five service-ks-register plays (designate, barbican, neutron, placement, magnum) then each failed after 5 retries with DiscoveryFailure … Service Unavailable (HTTP 503)
• from 01:04:07 to 04:30:26 the console log is nothing but two celery tasks in STARTED and a poll loop
• job-output.json records 0 failed tasks; both jobs TIMED_OUT at 4h31m

Builds: 7e89e00b (current), 3b6c5232 (next). Ansible reported terminal failures 3h26m before the kill.

The fix

Raise AnsibleFailure on a non-zero rc, placed after log_play_execution() and finish_task_output(). The history record and the streamed rc are unchanged, so every user-visible exit code on the single-role path stays exactly as before; the only difference there is that the task now ends in FAILURE rather than SUCCESS. Because osism/tasks/__init__.py is shared, this covers ansible.run, ceph.run, kolla.run and kubernetes.run alike. The lock release and the per-task SSH ControlPath cleanup already live in finally blocks, so the raise passes through them.

The exception message carries its own context (worker, environment, role, rc): with the JSON result serializer Celery stores only the exception type and its string form, and the play output reaches the operator over the Redis stream rather than the result backend.

That alone does not make a deploy fail. osism wait branched on PENDING, SUCCESS and STARTED only, so a FAILURE task fell through every branch, was dropped from the poll queue without being re-queued, and rc stayed 0. testbed/scripts/deploy-in-a-nutshell.sh runs osism wait --output --refresh 20 under set -e, so an aborted chain would still have been reported as a successful deploy. A FAILURE/REVOKED branch now sets rc = 1. It deliberately does not call result.get() even under --output: Celery re-raises the task's exception from there, which would replace the exit code with a traceback.

handle_loadbalancer_task is the only place that calls .get() on one of these tasks, so it is the only place the new exception can surface in the CLI. It absorbs AnsibleFailure — the rc has already been read from the output stream by handle_task, so the exception carries nothing new — and absorbs only that type. get(propagate=False) would have swallowed every exception and erased the existing, deliberate behaviour that an unexpected group failure propagates (pinned by test_handle_loadbalancer_task_group_failure_propagates).

Second commit, an independent latent bug found in the same blast radius: handle_collection fell off the end of the function and returned None. take_action tests that with if rc != 0, and None != 0 is True, so a successfully scheduled collection was treated as a failure and the remaining // segments were silently skipped — osism apply nutshell//myrole dropped myrole without a word, still exiting 0. The repo already pinned this as a strict xfail; it now passes and the marker is removed.

Verification

Chain abortion, against a real broker. The load-bearing assumption is that raising actually stops the nested chain(task, group(...)) tree that _handle_collection builds. Verified with a real Celery 5.6.3 worker against a throwaway Redis, on a tree mirroring that structure:

run	tasks that executed
baseline, nothing fails	common, keystone, mariadb, memcached, openvswitch, ovn
first task raises	common, openvswitch, ovn

The dependent subtree is skipped and the independent sibling branch still completes — which is the intended behaviour, not just a stopped chain. Also confirmed directly that the resulting state string is literally FAILURE and that result.get() re-raises, which is what the two points above depend on.

Tests. 2905 passed, 2 xfailed (both pre-existing and unrelated). New coverage: the raise happens after the rc is logged and published; the message carries its context; the redlock and SSH ControlPath dir are still released on the failure path; FAILURE/REVOKED set a non-zero exit; a failure is not reset by a later successful task in the queue; --output does not call .get() on a failed task; script format prints the failure state; handle_loadbalancer_task absorbs AnsibleFailure but still propagates others.

flake8, black and mypy pass at the versions pinned in .zuul.yaml.

Not in scope

This does not bound how long a play waits for something that never arrives. The kolla-wait-for-nova / kolla-wait-for-keystone plays have a 5h09m worst case of their own — an independent defect that co-fires in exactly these builds, and that would not fix this one (with the wait bounded, the chain still carries on, every task still reports SUCCESS, and the deploy still reports success):

• wait-for: bound the port wait with a timeout container-image-kolla-ansible#943

One intended side effect: the periodic gather_facts task also runs through this function, so a failed facts run now reports FAILURE instead of SUCCESS. That is the correct state for it and nothing chains off it.

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the new failure propagation is wrong, a non-zero play result can abort a collection after earlier tasks have already changed production, leaving a partially applied deployment that reverting this code will not undo. The resulting infrastructure state is bounded and can be repaired or rerun, but it may already cause an outage or require manual remediation.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}