task/ansible: mark nodes down on hardware failures

[djgalloway]

Fixes: https://tracker.ceph.com/issues/75669
Related: https://tracker.ceph.com/issues/75208

What

We already mark a node down after it fails to reimage 10 times in a row. When ceph-cm-ansible's Ensure we found enough OSD disks task fails, though, we know right away that the node is missing a disk — and every job that lands on it until someone notices will fail the same way.

This scans the ansible failure log for messages that mean the hardware itself is broken and marks the affected nodes down immediately.

• FailureAnalyzer.find_hardware_failures() parses the failure log (keyed by hostname) and returns {hostname: msg} for failures matching a known hardware-failure pattern. The one pattern for now is Wanted N disks of ... but only matched M. Whitespace is collapsed before matching because ansible wraps that message across lines.
• Ansible._handle_failure() feeds the hits to a new _mark_hardware_failures_down() hook — a no-op in the base class, implemented in CephLab as lock_ops.update_lock(hostname, status='down'). This mirrors the existing _set_status() split. It's wrapped in its own try/except so a lock server problem can't disturb the existing failure-reporting path.

Notes for reviewers

Two deliberate choices:

The lock description is left untouched. The dispatcher's 10-strikes path sets description='reimage failed 10 times', but it runs after unlocking. Here the job still holds the lock, and unlock_targets() refuses to unlock a node whose description no longer matches archive_path — so setting it would leave the dead node locked forever. Only up is flipped; the reason goes to the log.

We match on the message, not the task name. ceph-cm-ansible's callback_plugins/failure_log.py only records the result dict (msg, changed, ...), never the task name. Verified against a real ansible_failures.yaml from the tracker and against roles/testnode/tasks/configure_lvm.yml. Keying off the task name instead would require a callback plugin change in ceph-cm-ansible first.

Testing

8 new cases in tests/task/test_ansible.py: analyzer coverage (the verbatim message from job 116107, a line-wrapped variant, a results list, a non-matching failure, an empty log) plus end-to-end checks that CephLab marks the node down, Ansible does not, and a non-hardware failure does not.

tests/task/test_ansible.py: 105 passed, 6 skipped. Full suite: 1222 passed, 3 failed — the 3 are in tests/orchestra/test_connection.py and fail identically on a clean checkout of main.

[kshtsk]

The feature looks interesting, though I haven't read deeply what the criteria of marking host down.
But we should not do any calls to paddles from any task, otherwise it makes the teuthology run depended on not necessary infrastructure. The actual node manipulation are supposed to be done on supervisor side. Also, would be probably a good idea to have an option to disable this behavior on config level.

[kshtsk]

Also if we want this mechanism incorporated it is better to put the documentation into teuthology how all the stuff supposed to work, and that it is relied on ansible task or/and on ceph-cm-ansible role task and what format is expected to trigger the node marking down, etc.

[djgalloway]

Thanks — both points addressed, force-pushed.

No paddles calls from the task. The ansible task now only does what it always did: analyze the failure log for the error message, and archive it to <archive_path>/ansible_failures.yaml. All node manipulation moved to the supervisor — check_for_hardware_failures_and_mark_down() in teuthology/dispatcher/supervisor.py reads that archived file back after the job process exits and calls lock_ops.update_lock(), right next to the existing check_for_reimage_failures_and_mark_down(). A running job no longer depends on the lock server for this.

The supervisor also only marks down nodes that are actually targets of the job it just ran, and only the up status is changed — the description is deliberately left alone, since unlock_targets() refuses to unlock a node whose description no longer matches the archive path.

Config option. mark_down_on_hardware_failure (default true), checked by the supervisor, documented in docs/siteconfig.rst.

Docs. New docs/node_health.rst, linked from the index, covering both mark-down mechanisms: where the failure log comes from (ceph-cm-ansible's failure_log callback plugin plus the ANSIBLE_FAILURE_LOG env var the ansible task sets), the expected YAML format with an example, the patterns that trigger it and where they live, and the caveat that this couples us to the exact wording of a task in ceph-cm-ansible (Ensure we found enough OSD disks in roles/testnode/tasks/configure_lvm.yml) — the callback plugin records the result dict, not the task name, so matching has to be on the message text.

To your first line, the criteria: exactly one pattern today, Wanted \d+ disks? of .+ but only matched \d+, matched case-insensitively against msg with whitespace collapsed first.

New tests in tests/dispatcher/test_hardware_failure_mark_machine_down.py cover the supervisor path (marks down, ignores unrelated failures, no log, host not a target, disabled by config, unparseable log). Full suite: 1225 passed, 3 pre-existing failures in tests/orchestra/test_connection.py that also fail on a clean main. ruff check clean, docs build clean.

[kshtsk]

I wonder if it would be better to move patterns to config based on machine types, so instead of

mark_down_on_hardware_failure: true

We can have:

disable_targets:
  ansible_failure_patterns:
    trial:
      - "Wanted \d+ disks? of .+ but only matched \d+"

or this is over engineered?

[djgalloway]

Well, in your example, both the smithi and trials have that check. gibba would if we were using them too. So we'd have to duplicate the entries.

[kshtsk]

In this case yaml allows to copy data using anchors:

disable_targets:
  ansible_failure_patterns:
    trial: &missing_or_dead
      - "Wanted \d+ disks? of .+ but only matched \d+"
    gibba: *missing_or_dead
    smithi: *missing_or_dead

[djgalloway]

TIL about yaml anchors

[kshtsk]

I think it is worth to add swift comment that we should try to make the targets down always before unlocking.

[djgalloway]

I'm not sure what you mean. A comment in the actual code or a log?

[kshtsk]

I meant in the code, so if the one refactor code take this into account.

[kshtsk]

Hey David,
thanks for updating with new changes, this looks really great. I've got some minor question though.
Have you tried to test this in sepia?

[djgalloway]

Hey David, thanks for updating with new changes, this looks really great. I've got some minor question though. Have you tried to test this in sepia?

https://pulpito.ceph.com/dgalloway-2026-08-10_21:48:40-smoke-main-distro-default-trial/453342/

Yes, also added second commit to change the description.