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.