Unit tests for osism/tasks/{ansible,ceph,kolla,kubernetes,reconciler}.py — thin task wrappers

[berendt]

Background

Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 5 (#2199). This issue covers the five thin Celery task wrapper modules osism/tasks/ansible.py (57 LOC), osism/tasks/ceph.py (31 LOC), osism/tasks/kolla.py (31 LOC), osism/tasks/kubernetes.py (31 LOC) and osism/tasks/reconciler.py (90 LOC). The first four delegate to osism.tasks.run_ansible_in_environment with a hard-coded worker name; reconciler shells out to /run.sh under a Redlock. Each module also creates its own Celery app configured from osism.tasks.Config and (for ansible/reconciler) registers a periodic task via the on_after_configure signal.

Scope

Add a single combined tests/unit/tasks/test_task_wrappers.py. One file is cleaner than five here: ceph/kolla/kubernetes are byte-for-byte identical except for the worker string and can share one parametrized test class, and the two setup_periodic_tasks variants share a fixture. If the file grows uncomfortably during implementation, split reconciler out into tests/unit/tasks/test_reconciler.py.

There is no existing coverage: tests/unit/tasks/ only contains the conductor/ subpackage, and tests/unit/commands/test_reconciler.py merely patches osism.tasks.reconciler.run.delay (the command layer), not the task bodies. run_command is not used by any module in scope (only by netbox/openstack tasks, tracked separately) — delegation here is exclusively to run_ansible_in_environment and subprocess.Popen.

Test targets

App configuration & queue routing — ansible.py:8, ceph.py:8, kolla.py:8, kubernetes.py:8, reconciler.py:12

No patching needed; importing the modules and reading app.conf is safe (verified: the lazy osism.utils.__getattr__ keeps Redis untouched and the on_after_configure receiver does not fire on plain import/conf access). Parametrize over the five modules:

• app.conf.task_default_queue == "default" (Config applied via config_from_object)
• app.conf.task_routes maps the module pattern to the expected queue: osism.tasks.ansible.* → osism-ansible, osism.tasks.ceph.* → ceph-ansible, osism.tasks.kolla.* → kolla-ansible, osism.tasks.kubernetes.* → kubernetes, osism.tasks.reconciler.* → reconciler
• Tasks are registered under their explicit names in app.tasks: osism.tasks.ansible.gather_facts, osism.tasks.ansible.run, osism.tasks.ansible.noop, osism.tasks.ceph.run, osism.tasks.kolla.run, osism.tasks.kubernetes.run, osism.tasks.reconciler.run, osism.tasks.reconciler.run_on_change

setup_periodic_tasks() (ansible) — ansible.py:13

Patch osism.tasks.ansible.utils.create_redlock and osism.tasks.ansible.settings.GATHER_FACTS_SCHEDULE (module attribute, read at call time); sender is a MagicMock. Call the receiver directly — it is a plain function.

• Schedule > 0, lock.acquire(timeout=10) returns True → sender.add_periodic_task(schedule, <signature>, expires=10) called once; the signature is gather_facts.s()
• Schedule = 0 → add_periodic_task not called; lock.acquire not called either (short-circuit and), but create_redlock is still called with key lock_osism_tasks_ansible_setup_periodic_tasks
• Schedule > 0, acquire returns False → add_periodic_task not called

gather_facts() — ansible.py:24

Patch osism.tasks.ansible.run_ansible_in_environment.

• Default call → delegates with exactly (request_id, "osism-ansible", "generic", "facts", [], True, False) — seven positional args, auto_release_time left at the delegate's default
• publish=False forwarded as sixth argument
• Return value of run_ansible_in_environment passed through unchanged
• No check_task_lock_and_exit call (unlike run — assert the patched lock check stays uncalled)

run() (ansible) — ansible.py:31

Patch osism.tasks.ansible.run_ansible_in_environment and osism.tasks.ansible.utils.check_task_lock_and_exit.

• Happy path: arguments forwarded positionally as (request_id, "osism-ansible", environment, playbook, arguments, publish, locking, auto_release_time)
• Defaults: publish=True, locking=False, auto_release_time=3600
• check_task_lock_and_exit is invoked before delegation; when it raises SystemExit (production behavior is exit(1)), run_ansible_in_environment is never called — use pytest.raises(SystemExit)
• Return value passthrough

noop() — ansible.py:56

• Returns True (one-liner; include for the module's coverage target)

run() (ceph/kolla/kubernetes, parametrized) — ceph.py:18, kolla.py:18, kubernetes.py:18

Patch <module>.run_ansible_in_environment and <module>.utils.check_task_lock_and_exit per parametrized module.

• Worker string per module: ceph → "ceph-ansible", kolla → "kolla-ansible", kubernetes → "osism-kubernetes" (note: deliberately differs from the queue name kubernetes)
• locking is hard-coded False (seventh positional arg) — these tasks expose no locking parameter, unlike ansible.run
• auto_release_time forwarded, default 3600; publish default True
• check_task_lock_and_exit raising SystemExit short-circuits delegation
• setup_periodic_tasks (ceph.py:13, kolla.py:13, kubernetes.py:13) is pass — call once with a MagicMock sender, assert the sender is untouched

setup_periodic_tasks() (reconciler) — reconciler.py:17

Same three cases as the ansible variant, with osism.tasks.reconciler.settings.INVENTORY_RECONCILER_SCHEDULE, lock key lock_osism_tasks_reconciler_setup_periodic_tasks, and run_on_change.s() as the scheduled signature.

run() (reconciler) — reconciler.py:28

Patch osism.tasks.reconciler.utils.check_task_lock_and_exit, osism.tasks.reconciler.utils.create_redlock, osism.tasks.reconciler.subprocess.Popen, osism.tasks.reconciler.utils.push_task_output, osism.tasks.reconciler.utils.finish_task_output.

• create_redlock called with key="lock_osism_tasks_reconciler_run", auto_release_time=60
• lock.acquire(timeout=20) returns False → Popen never called, task returns None
• Lock acquired, publish=True: Popen called with "/run.sh", shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT and an env kwarg (copy of os.environ); each stdout line is forwarded via push_task_output(request_id, line); p.wait(timeout=60); finish_task_output(request_id, rc=<wait return value>)
• publish=False: no push_task_output / finish_task_output calls, p.wait still called
• lock.release() raising pottery.ReleaseUnlockedLock → warning logged, exception does not propagate (set the real exception class as side_effect on the mock lock's release)
• check_task_lock_and_exit raising SystemExit → nothing else executes (no create_redlock, no Popen)

run_on_change() — reconciler.py:70

Same patch set minus the output helpers.

• create_redlock called with key="lock_osism_tasks_reconciler_run_on_change", auto_release_time=60
• Not acquired → no Popen
• Acquired → Popen("/run.sh", shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) — no env kwarg this time; p.wait() without timeout; no output publishing at all
• No task-lock check here (unlike run) — assert the patched check_task_lock_and_exit stays uncalled
• ReleaseUnlockedLock on release handled with a warning

Mocking hints

• No broker needed. All tasks use bind=True; two equivalent invocation styles (verified against the installed Celery):
  • task.__wrapped__(...) — already bound to the task instance, so call it without a self argument: ansible.run.__wrapped__("env", "playbook", []). Inside, self.request.id is None; assert delegation with request_id=None.
  • task.apply(args=[...], task_id="test-id") — runs eagerly in-process and sets self.request.id = "test-id", useful where the forwarded request id should be a real value (e.g. push_task_output assertions). SystemExit is a BaseException and propagates through apply(), so pytest.raises(SystemExit) works with either style.
• utils is imported as a module object (from osism import utils), so mocker.patch("osism.tasks.ansible.utils.check_task_lock_and_exit") and mocker.patch("osism.utils.check_task_lock_and_exit") hit the same attribute — either works.
• run_ansible_in_environment is imported into each wrapper's namespace (from osism.tasks import ...), so it must be patched at the wrapper, e.g. osism.tasks.ceph.run_ansible_in_environment, not at osism.tasks.
• For reconciler.run with publish=True, the code wraps p.stdout in io.TextIOWrapper(..., encoding="utf-8") — a plain MagicMock will not iterate. Give the mocked Popen return value a real byte stream:

  proc = mocker.MagicMock()
  proc.stdout = io.BytesIO(b"line one\nline two\n")
  proc.wait.return_value = 0
  mocker.patch("osism.tasks.reconciler.subprocess.Popen", return_value=proc)

• pottery is imported lazily inside the task bodies (from pottery import ReleaseUnlockedLock); import the real class in the test and use it as side_effect — do not patch pottery.
• Patch the schedule settings on the settings module (mocker.patch.object(osism.settings, "GATHER_FACTS_SCHEDULE", 0) or via the wrapper's reference) — they are read at call time, not at import time. Defaults are env-derived (43200.0 / 600.0), so never assert the literal default.

Definition of Done

• tests/unit/tasks/test_task_wrappers.py created
• All listed cases covered
• pytest --cov=osism.tasks.ansible --cov=osism.tasks.ceph --cov=osism.tasks.kolla --cov=osism.tasks.kubernetes --cov=osism.tasks.reconciler shows ≥ 95 %
• pipenv run pytest tests/unit/tasks/test_task_wrappers.py passes locally
• flake8, mypy, python-black remain green
• Zuul job python-osism-unit-tests passes

Dependencies

• Part of Meta: Unit test coverage for osism/ #2199 (meta). Blocked by Introduce foundation for unit tests (pytest + Zuul CI integration) #2192 / PR Introduce foundation for unit tests (pytest + Zuul CI) #2193.

[berendt]

Implementation Plan (issue #2354)

Summary

• Add one new test module, tests/unit/tasks/test_task_wrappers.py, covering the five thin Celery wrapper modules exactly as the issue's Test targets prescribe: per-app config/queue-routing assertions parametrized over all five apps, the two real setup_periodic_tasks receivers plus the three pass variants, gather_facts/run/noop in ansible.py, one parametrized block for the byte-identical ceph/kolla/kubernetes run tasks, and the two subprocess-driving reconciler tasks. All delegation is asserted against patched run_ansible_in_environment / utils.* / subprocess.Popen; no broker, no Redis, no filesystem. This is the smallest change set: a single file (the issue explicitly prefers one combined file), no production-code changes, no new dependencies (pottery, celery, pytest-mock are already pinned), and tests/unit/tasks/__init__.py already exists.

Ground-Truth Notes

• osism/tasks/ansible.py — confirmed, 57 LOC: app at line 8, setup_periodic_tasks at 13 (lock key lock_osism_tasks_ansible_setup_periodic_tasks, reads settings.GATHER_FACTS_SCHEDULE at call time, lock.acquire(timeout=10), sender.add_periodic_task(schedule, gather_facts.s(), expires=10)), gather_facts at 24 (delegates with 7 positional args, no lock check), run at 31 (lock check then 8 positional args), noop at 56 (return True).
• osism/tasks/ceph.py / kolla.py / kubernetes.py — confirmed, 31 LOC each, byte-identical except worker string: app at line 8, setup_periodic_tasks at 13 (pass), run at 18 delegating with workers "ceph-ansible" / "kolla-ansible" / "osism-kubernetes", locking hard-coded False as 7th positional arg, auto_release_time default 3600.
• osism/tasks/reconciler.py — confirmed, 90 LOC: app at 12, setup_periodic_tasks at 17 (key lock_osism_tasks_reconciler_setup_periodic_tasks, INVENTORY_RECONCILER_SCHEDULE, run_on_change.s()), run at 28 (lock check → create_redlock(key="lock_osism_tasks_reconciler_run", auto_release_time=60) → acquire(timeout=20) → Popen("/run.sh", shell=True, stdout=PIPE, stderr=STDOUT, env=os.environ.copy()) → per-line push_task_output via io.TextIOWrapper when publish → p.wait(timeout=60) → finish_task_output(rc=rc) when publish → ReleaseUnlockedLock swallowed with a warning), run_on_change at 70 (key lock_osism_tasks_reconciler_run_on_change, no lock check, no env kwarg, bare p.wait(), no output publishing).
• osism/tasks/__init__.py — confirmed: Config at line 22 with task_default_queue = "default" (line 32) and task_routes (lines 34–43) mapping the five patterns to osism-ansible / ceph-ansible / kolla-ansible / kubernetes / reconciler; run_ansible_in_environment at line 149 with signature (request_id, worker, environment, role, arguments, publish=True, locking=False, auto_release_time=3600, ssh_retries=3). Note: task_track_started is now plain True (line 33) — the 1-tuple typo flagged in sibling Unit tests for osism/tasks/__init__.py #2353 has already been fixed and is pinned by tests/unit/tasks/test_config.py.
• osism/settings.py — confirmed: GATHER_FACTS_SCHEDULE (line 34, float, env-derived default 43200.0), INVENTORY_RECONCILER_SCHEDULE (line 40, float, default 600.0). Both read at receiver call time via the shared osism.settings module object, so patching per test works.
• osism/utils/__init__.py — confirmed (line numbers have drifted from earlier revisions but all symbols exist): push_task_output at 443, finish_task_output at 447, create_redlock at 477 (default auto_release_time=3600), check_task_lock_and_exit at 682 (calls exit(1) → SystemExit), lazy __getattr__ at 700. utils is imported as a module object into every wrapper, so mocker.patch("osism.tasks.<mod>.utils.<name>") hits the shared attribute as the issue's hints state.
• tests/unit/tasks/ — confirmed: __init__.py already exists (nothing to create); contains only conductor/ tests plus test_config.py (covers Config.broker_url/result_backend precedence and task_track_started only — no overlap with the per-app app.conf targets here). tests/unit/tasks/test_task_wrappers.py does not exist yet.
• tests/unit/commands/test_reconciler.py — confirmed: patches osism.tasks.reconciler.run.delay (command layer only), no coverage of the task bodies; no duplication.
• tests/conftest.py — confirmed: installs ansible stubs and provides the loguru_logs fixture (needed for the ReleaseUnlockedLock warning assertions; caplog cannot see loguru).
• Dependencies — confirmed: celery[redis]==5.6.3 and pottery==3.0.1 are runtime deps (importing the real ReleaseUnlockedLock in tests is fine); pytest==9.1.1, pytest-mock==3.15.1, pytest-cov==7.1.0 in Pipfile [dev-packages]. Pytest config in setup.cfg:179 (testpaths = tests/unit, --strict-markers).
• Zuul — confirmed: job python-osism-unit-tests at .zuul.yaml:134 runs pipenv run pytest (playbooks/test-unit.yml); flake8, mypy, python-black run as separate check jobs.
• Invocation styles — the issue states both task.__wrapped__(...) (bound, no self) and task.apply(args=..., task_id=...) were verified against the installed Celery; this is consistent with Celery 5.6.3 internals (__wrapped__ is a class attribute holding the run function, so instance access binds it; the eager tracer captures only Exception, so SystemExit propagates).

Work Breakdown Coverage

The issue has no numbered work-package list; its contract is the nine "Test targets" sections plus the Definition of Done. Each is covered:

• App configuration & queue routing (ansible/ceph/kolla/kubernetes/reconciler) — Change Set item 1, Test Plan block A, Commit 1.
• setup_periodic_tasks() (ansible) — ansible.py:13 — Test Plan block B (parametrized with the reconciler variant, sharing one variant table as the issue suggests).
• gather_facts() — ansible.py:24 — Test Plan block C.
• run() (ansible) — ansible.py:31 — Test Plan block D.
• noop() — ansible.py:56 — Test Plan block E.
• run() (ceph/kolla/kubernetes, parametrized) + their pass setup_periodic_tasks — Test Plan blocks F and B3.
• setup_periodic_tasks() (reconciler) — reconciler.py:17 — Test Plan block B (second variant).
• run() (reconciler) — reconciler.py:28 — Test Plan block G.
• run_on_change() — reconciler.py:70 — Test Plan block H.
• DoD items (file created, all cases covered, coverage ≥ 95 %, local pytest, linters, Zuul) — Verification Commands section; Documentation Plan is "none" by convention (see below).
  Out-of-scope by issue text: run_command consumers (owned by Unit tests for osism/tasks/netbox.py #2355 / Unit tests for osism/tasks/openstack.py #2356) and run_ansible_in_environment internals (owned by Unit tests for osism/tasks/__init__.py #2353 / PR Add unit tests for osism/tasks/__init__.py #2434) — here the delegate is always patched.

User Stories

• As an OSISM developer working on the Celery task layer, I want unit tests that pin the wrappers' queue routing, delegation signatures, and locking semantics, so that regressions (e.g. a reordered positional argument or a changed lock key) fail CI instead of surfacing in production workers — derived (the issue itself carries no User Stories section; part of the Meta: Unit test coverage for osism/ #2199 coverage rollout).

Change Set

• tests/unit/tasks/test_task_wrappers.py — create: single combined test module (per the issue's Scope). Structure, following the conventions of tests/unit/tasks/conductor/test_netbox.py (SPDX header, module docstring, pytest.param(..., id=...) variant tables, section-divider comments, _has_log helper for loguru_logs):
  • Imports: io, os, subprocess, unittest.mock.MagicMock/call, pytest, from pottery import ReleaseUnlockedLock, from osism.tasks import ansible, ceph, kolla, kubernetes, reconciler.
  • Variant tables: APP_VARIANTS (module, route pattern, queue, expected registered task names — osism.tasks.ansible.gather_facts/.run/.noop, osism.tasks.ceph.run, osism.tasks.kolla.run, osism.tasks.kubernetes.run, osism.tasks.reconciler.run/.run_on_change); PERIODIC_VARIANTS (module, settings attr name, lock key, scheduled task object) for ansible + reconciler; WORKER_RUN_VARIANTS (module, worker string) for ceph/kolla/kubernetes.
  • Patch targets exactly as verified: osism.tasks.<mod>.run_ansible_in_environment (import-site), osism.tasks.<mod>.utils.<helper> (shared osism.utils module attribute), osism.tasks.<mod>.settings.<SCHEDULE> (shared osism.settings attribute, e.g. via mocker.patch.object(module.settings, attr, 123.0) — never asserting the env-derived defaults), osism.tasks.reconciler.subprocess.Popen.
  • Invocation: task.__wrapped__(...) for everything except the reconciler publish=True case, which uses reconciler.run.apply(task_id="test-id") so push_task_output/finish_task_output see a real request id.

Commit Sequence

1. Add unit tests for osism/tasks thin task wrappers — adds tests/unit/tasks/test_task_wrappers.py; body: covers app config/queue routing, periodic-task receivers, and the run/gather_facts/noop/run_on_change task bodies of osism/tasks/{ansible,ceph,kolla,kubernetes,reconciler}.py with all external effects (ansible runner, Redis locks, subprocess, task-output stream) mocked; end with the trailer Assisted-by: Claude:<implementer model id> (never Co-Authored-By), and do not reference reviews/PRs in the message, per repo convention.

Test Plan

All tests live in tests/unit/tasks/test_task_wrappers.py (this whole change is the test plan; it verifies the derived user story). Blocks and the behavior each locks down:

A — App configuration & queue routing (no patching; safe per the lazy osism.utils.__getattr__; parametrized over APP_VARIANTS):

• test_app_uses_default_queue — module.app.conf.task_default_queue == "default" for all five apps (proves config_from_object(Config) applied).
• test_app_routes_module_pattern_to_queue — app.conf.task_routes["osism.tasks.<mod>.*"] == {"queue": <queue>} with the five pattern→queue pairs from osism/tasks/__init__.py:34.
• test_tasks_registered_under_explicit_names — each expected explicit name is in module.app.tasks (accessing app.tasks finalizes the app; no broker involved).

B — setup_periodic_tasks receivers (called directly as plain functions with a MagicMock sender; parametrized over PERIODIC_VARIANTS):

• test_setup_periodic_tasks_schedules_when_lock_acquired — patched schedule (e.g. 123.0) and lock.acquire(timeout=10) → True ⇒ create_redlock called once with exactly key=<lock key>; sender.add_periodic_task called once with (123.0, <task>.s()) and expires=10 (compare the signature via == module.gather_facts.s() / == module.run_on_change.s() — Celery Signature equality is dict-based).
• test_setup_periodic_tasks_skips_when_schedule_disabled — schedule 0 ⇒ add_periodic_task not called and lock.acquire not called (short-circuit and), but create_redlock still called once with the key.
• test_setup_periodic_tasks_skips_when_lock_not_acquired — schedule > 0, acquire → False ⇒ add_periodic_task not called.
• B3: test_setup_periodic_tasks_is_noop (parametrized ceph/kolla/kubernetes) — call once with a MagicMock sender; sender.mock_calls == [].

C — ansible.gather_facts (patch osism.tasks.ansible.run_ansible_in_environment and osism.tasks.ansible.utils.check_task_lock_and_exit):

• test_gather_facts_delegates_with_defaults — gather_facts.__wrapped__() ⇒ delegate called once with exactly (None, "osism-ansible", "generic", "facts", [], True, False) (seven positional args, auto_release_time left to the delegate's default; request_id is None via __wrapped__); return value passed through unchanged; the patched check_task_lock_and_exit stays uncalled.
• test_gather_facts_forwards_publish_false — publish=False arrives as the sixth positional argument.

D — ansible.run (patch delegate + utils.check_task_lock_and_exit):

• test_ansible_run_defaults — run.__wrapped__("env1", "site", ["-l", "x"]) ⇒ (None, "osism-ansible", "env1", "site", ["-l", "x"], True, False, 3600); lock check called once; return passthrough.
• test_ansible_run_forwards_arguments — explicit publish=False, locking=True, auto_release_time=60 forwarded positionally.
• test_ansible_run_aborts_when_task_lock_active — lock check side_effect=SystemExit(1) ⇒ pytest.raises(SystemExit), delegate never called (proves the check runs before delegation).

E — ansible.noop:

• test_noop_returns_true — noop.__wrapped__() is True.

F — ceph/kolla/kubernetes run (parametrized over WORKER_RUN_VARIANTS; patch f"{module.__name__}.run_ansible_in_environment" and f"{module.__name__}.utils.check_task_lock_and_exit"):

• test_worker_run_defaults — run.__wrapped__("env1", "pb", ["-v"]) ⇒ (None, <worker>, "env1", "pb", ["-v"], True, False, 3600) — pins the per-module worker string (kubernetes deliberately "osism-kubernetes", not the queue name) and locking hard-coded False as seventh positional arg; return passthrough.
• test_worker_run_forwards_overrides — publish=False, auto_release_time=120 forwarded.
• test_worker_run_aborts_when_task_lock_active — SystemExit short-circuits delegation.

G — reconciler.run (patch osism.tasks.reconciler.utils.check_task_lock_and_exit / .utils.create_redlock / .utils.push_task_output / .utils.finish_task_output / osism.tasks.reconciler.subprocess.Popen):

• test_reconciler_run_aborts_when_task_lock_active — SystemExit from the lock check ⇒ neither create_redlock nor Popen called.
• test_reconciler_run_returns_none_when_lock_not_acquired — create_redlock called with exactly key="lock_osism_tasks_reconciler_run", auto_release_time=60; lock.acquire called with timeout=20; acquire → False ⇒ Popen never called, task returns None.
• test_reconciler_run_publishes_output — via reconciler.run.apply(task_id="test-id") with a fake process (proc.stdout = io.BytesIO(b"line one\nline two\n"), proc.wait.return_value = 0): Popen called once with ("/run.sh",), shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, and an env kwarg equal to a copy of os.environ; push_task_output called exactly with ("test-id", "line one\n") then ("test-id", "line two\n"); proc.wait called with timeout=60; finish_task_output("test-id", rc=0); lock.release() called once.
• test_reconciler_run_without_publish_skips_output — run.__wrapped__(publish=False) ⇒ no push_task_output/finish_task_output calls, proc.wait still called with timeout=60.
• test_reconciler_run_warns_when_lock_already_released — lock.release.side_effect set to a real instantiated ReleaseUnlockedLock (see Risks for construction) ⇒ no exception propagates; loguru_logs contains a WARNING with substring "auto-released".

H — reconciler.run_on_change (same patch set minus the output helpers, which are patched only to assert they stay uncalled):

• test_run_on_change_returns_none_when_lock_not_acquired — create_redlock called with key="lock_osism_tasks_reconciler_run_on_change", auto_release_time=60; no Popen.
• test_run_on_change_runs_script_without_publishing — popen.assert_called_once_with("/run.sh", shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) (the exact-match assertion also proves no env kwarg); proc.wait called with no arguments; push_task_output/finish_task_output and the patched check_task_lock_and_exit all stay uncalled.
• test_run_on_change_warns_when_lock_already_released — ReleaseUnlockedLock on release ⇒ warning logged, no propagation.

Expected coverage: every line of the five modules is either executed at import or exercised by A–H, so --cov should report ~100 % (DoD threshold ≥ 95 %).

Documentation Plan

• none — test-only change with no user-visible behavior. CHANGELOG.md records user-facing features/dependency bumps only; the merged sibling test PRs (e.g. Add unit tests for osism/utils semaphore, redlock and task locks #2349, Add unit tests for osism.utils.ssh #2350) added no changelog entries, and no README/docs section enumerates test modules.

Verification Commands

• pipenv install --dev --deploy (once, to mirror the CI environment; pipenv run pip install . if osism isn't importable in the venv)
• pipenv run pytest tests/unit/tasks/test_task_wrappers.py
• pipenv run pytest --cov=osism.tasks.ansible --cov=osism.tasks.ceph --cov=osism.tasks.kolla --cov=osism.tasks.kubernetes --cov=osism.tasks.reconciler tests/unit/tasks/test_task_wrappers.py — confirm ≥ 95 % per module
• pipenv run pytest — full unit suite (testpaths = tests/unit), guards against cross-test leakage through the shared osism.settings/osism.utils module objects
• Note the repo owner's workflow: local test runs may be skipped in favor of CI — the authoritative gates are the Zuul check jobs python-osism-unit-tests, flake8, mypy, python-black (.zuul.yaml:134 ff.). black/flake8/mypy are not in the Pipfile dev-packages; if run locally, install ad hoc (pipenv run pip install black flake8 → black --check tests/unit/tasks/test_task_wrappers.py, flake8 tests/unit/tasks/test_task_wrappers.py), otherwise let Zuul verify.

Risks & Open Questions

• pottery.ReleaseUnlockedLock construction: in pottery 3.0.1 the PrimitiveError base takes (key, masters), so side_effect must be an instance, e.g. ReleaseUnlockedLock(key="lock_osism_tasks_reconciler_run", masters=set()) — passing the bare class would make mock raise it with no args and fail with TypeError. Implementer should confirm the exact signature with pipenv run python -c "import inspect, pottery; print(inspect.signature(pottery.ReleaseUnlockedLock.__init__))" and adjust the keyword names if they differ.
• on_after_configure receiver may fire once per app: contrary to the issue's parenthetical ("the receiver does not fire on plain import/conf access"), Celery loads the deferred config on first app.conf/app.tasks/task-proxy access and then sends on_after_configure, so the real setup_periodic_tasks may run once with the unpatched utils.create_redlock. This is harmless: Celery's signal dispatch catches and logs receiver exceptions, the Redis ping to the unresolvable host redis fails fast, and already-merged tests (e.g. tests/unit/commands/test_reconciler.py patching osism.tasks.reconciler.run.delay) exercise exactly this path with green CI. Expect at most a stray logged connection error during collection; no plan change needed, but the implementer shouldn't mistake that log line for a failure.
• Signature-equality assertion relies on Celery Signature.__eq__ (dict comparison) for gather_facts.s() / run_on_change.s(); if it ever proves brittle, fall back to asserting call_args.args[1]["task"] == "<explicit task name>".
• Coordination with open PR Add unit tests for osism/tasks/__init__.py #2434 (issue Unit tests for osism/tasks/__init__.py #2353): it adds tests/unit/tasks/test_init.py in the same directory; no file overlap with test_task_wrappers.py and tests/unit/tasks/__init__.py already exists on main, so merge order is irrelevant.
• File size: ~25 test functions (~50 executions) in one module is comfortable; if it grows uncomfortable during implementation, the issue pre-authorizes splitting reconciler out into tests/unit/tasks/test_reconciler.py — keep it as a fallback, not the default.

Status

STATUS: PLAN_READY

---

Implementation plan generated by planwerk-agent 4c14036 implement with Claude

[berendt]

Implementation Report (issue #2354)

Work Breakdown Coverage

The issue has no numbered work-package list; its contract is the nine "Test targets" sections plus the Definition of Done. Every section is delivered in the single new file tests/unit/tasks/test_task_wrappers.py (commit 7d892fc):

• App configuration & queue routing (all 5 modules) — done — test_app_uses_default_queue, test_app_routes_module_pattern_to_queue, test_tasks_registered_under_explicit_names, parametrized over APP_VARIANTS (all 8 explicit task names asserted).
• setup_periodic_tasks() (ansible) — done — three cases parametrized in PERIODIC_VARIANTS; schedule=0 asserts create_redlock still called with the key while acquire is not (short-circuit and); acquired case asserts gather_facts.s() + expires=10.
• gather_facts() — done — test_gather_facts_delegates_with_defaults (7 positional args, check_task_lock_and_exit stays uncalled), test_gather_facts_forwards_publish_false.
• run() (ansible) — done — defaults (8 args, check before delegation), forwards_arguments, and test_ansible_run_aborts_when_task_lock_active (SystemExit → delegate never called).
• noop() — done — test_noop_returns_true.
• run() (ceph/kolla/kubernetes) — done — parametrized over WORKER_RUN_VARIANTS: per-module worker string, hard-coded locking=False 7th arg, override forwarding, SystemExit short-circuit; test_setup_periodic_tasks_is_noop covers the pass receivers.
• setup_periodic_tasks() (reconciler) — done — second PERIODIC_VARIANTS entry (INVENTORY_RECONCILER_SCHEDULE, reconciler lock key, run_on_change.s()).
• run() (reconciler) — done — abort/not-acquired/publish/no-publish/ReleaseUnlockedLock cases; publish path via run.apply(task_id="test-id").
• run_on_change() — done — not-acquired, script-without-publishing (exact Popen match proves no env kwarg; bare p.wait(); lock check stays uncalled), ReleaseUnlockedLock warning.

Acceptance Criteria (Definition of Done)

• tests/unit/tasks/test_task_wrappers.py created
  • Status: satisfied
  • Evidence: new file, 535 lines, commit 7d892fc.
• All listed cases covered
  • Status: satisfied
  • Evidence: 47 parametrized tests spanning all nine Test-target sections (see Work Breakdown above); edge/error paths covered — e.g. test_ansible_run_aborts_when_task_lock_active, test_reconciler_run_aborts_when_task_lock_active, test_reconciler_run_warns_when_lock_already_released.
• Coverage ≥ 95 % on the five modules
  • Status: satisfied
  • Evidence: --cov run reports 100 % for ansible.py, ceph.py, kolla.py, kubernetes.py, reconciler.py.
• pytest tests/unit/tasks/test_task_wrappers.py passes locally
  • Status: satisfied
  • Evidence: 47 passed in 10.21s.
• flake8, mypy, python-black remain green
  • Status: satisfied
  • Evidence: black --check clean; flake8 clean; mypy produces the same 4 pre-existing source-stub errors as the existing test_config.py baseline and zero new errors from the new file.
• Zuul job python-osism-unit-tests passes
  • Status: satisfied (locally verified; CI is authoritative) — full local unit suite shows only 5 pre-existing, environment-caused failures (fakeredis lacking Lua eval + a cliff issue in test_openstack.py) that fail identically without the new file.

Commits

• 7d892fc Add unit tests for osism/tasks thin task wrappers

Local verification

• pipenv run pytest tests/unit/tasks/test_task_wrappers.py -q — pass (47 passed)
• pipenv run pytest ... --cov=osism.tasks.{ansible,ceph,kolla,kubernetes,reconciler} --cov-report=term-missing — pass (100 % each module)
• pipenv run pytest (full unit suite) — 1488 passed, 5 failed; the 5 failures are pre-existing and environment-caused (fakeredis without Lua eval; cliff command-manager in test_openstack.py), confirmed to fail in isolation without the new file
• pipenv run black --check tests/unit/tasks/test_task_wrappers.py — pass
• pipenv run flake8 tests/unit/tasks/test_task_wrappers.py — pass
• pipenv run mypy --ignore-missing-imports tests/unit/tasks/test_task_wrappers.py — pass (no new errors; identical to existing test-file baseline)

Branch

• implement/issue-2354-task-wrapper-tests (committed, not pushed — the finalize step opens the PR)

Deviations from the issue

• none — the implementation follows the plan and the issue's Test targets exactly. (Verified the issue's "no patching needed" claim for block A empirically: accessing app.conf/app.tasks is fast and touches no Redis. A few additive assertions were included where they strengthen a case without changing scope — e.g. asserting lock.release() on the reconciler happy path and result is None for run_on_change when the lock is not acquired.)

Status

STATUS: DONE

---

Implementation report generated by planwerk-agent 4c14036 implement with Claude