diff --git a/osism/commands/apply.py b/osism/commands/apply.py index ab9cd81c7..12d946d62 100644 --- a/osism/commands/apply.py +++ b/osism/commands/apply.py @@ -338,10 +338,25 @@ def _prepare_task( kolla_arguments = [f"-e kolla_action={action}"] + arguments + # Some kolla-environment playbooks are implemented in osism-ansible + # rather than kolla-ansible (ansible-playbooks playbooks/kolla/: + # stepca, fix-gh973, nova-update-cell-mappings, ...), so they have to + # run in the osism-ansible runtime. Requiring the role to be mapped + # to the kolla environment is what keeps that override from also + # swallowing roles osism-ansible registers for a DIFFERENT + # environment: it ships generic/{facts,gather-facts,certificates}.yml + # and therefore advertises those names as `generic`, while + # kolla-ansible advertises kolla-{facts,gather-facts,certificates}. + # Since the `kolla-` prefix is stripped above, without the + # environment check every one of those collapsed onto the generic + # entry and was dispatched to osism-ansible with ENVIRONMENT=kolla, + # where run-kolla.sh finds no such playbook and reports + # "service in environment kolla not available". if ( role not in ["common"] and "osism-ansible" in MAP_ROLE2RUNTIME and role in MAP_ROLE2RUNTIME["osism-ansible"] + and MAP_ROLE2ENVIRONMENT.get(role) == "kolla" ): t = ansible.run.si( environment, role, arguments, auto_release_time=task_timeout diff --git a/osism/commands/sync.py b/osism/commands/sync.py index 47c70943b..322674a42 100644 --- a/osism/commands/sync.py +++ b/osism/commands/sync.py @@ -29,6 +29,17 @@ def take_action(self, parsed_args): "generic", "gather-facts", arguments, auto_release_time=3600 ) rc = handle_task(t) + if rc != 0: + return rc + + # The gather above writes only osism-ansible's own fact-cache + # generation. Since ansible-core 2.19 namespaces cache entries per + # schema, kolla-ansible cannot read those entries when it sits on the + # other side of 2.19 -- and it never gathers for itself. Refresh its + # generation too, so one command repairs the whole cache. + kolla_task = ansible.dispatch_kolla_facts() + if kolla_task is not None: + rc = handle_task(kolla_task) return rc diff --git a/osism/tasks/ansible.py b/osism/tasks/ansible.py index 44e6eb9e1..cdabd581d 100644 --- a/osism/tasks/ansible.py +++ b/osism/tasks/ansible.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 from celery import Celery +from loguru import logger from osism import settings, utils from osism.tasks import Config, run_ansible_in_environment @@ -20,11 +21,45 @@ def setup_periodic_tasks(sender, **kwargs): ) +def dispatch_kolla_facts(): + """Queue a fact gather in the kolla-ansible runtime, if that runtime exists. + + ansible-core 2.19 namespaces fact-cache entries per schema (`s1_`), so + a runtime above 2.19 cannot read entries a runtime below it wrote. kolla + consumes the cache *exclusively* -- OSISM does not run kolla's site.yml, and + split-kolla-ansible-site.py forces `gather_facts: false` on every emitted + play -- so whenever kolla-ansible and osism-ansible sit on opposite sides of + 2.19, kolla starves unless its own runtime writes the cache. /ansible/kolla-facts.yml + is that writer. + + Guarded, because the kolla-ansible container is the only consumer of the + kolla-ansible queue and is `enable_kolla_ansible`-gated: dispatching on a + manager without it would leave a task in redis on every run, forever. + + The check has to happen HERE, in a worker, rather than in + setup_periodic_tasks: /interface is not mounted into the beat container, so + MAP_ROLE2RUNTIME is empty there and the guard would never pass. + """ + from osism.data.playbooks import MAP_ROLE2RUNTIME + from osism.tasks import kolla + + if "kolla-ansible" not in MAP_ROLE2RUNTIME: + logger.info( + "kolla-ansible runtime not available, skipping the kolla fact gather" + ) + return None + + return kolla.run.delay("kolla", "facts", [], auto_release_time=3600) + + @app.task(bind=True, name="osism.tasks.ansible.gather_facts") def gather_facts(self, publish=True): - return run_ansible_in_environment( + result = run_ansible_in_environment( self.request.id, "osism-ansible", "generic", "facts", [], publish, False ) + # The generic gather only ever fills osism-ansible's own cache generation. + dispatch_kolla_facts() + return result @app.task(bind=True, name="osism.tasks.ansible.run") diff --git a/tests/unit/commands/test_apply.py b/tests/unit/commands/test_apply.py index edb6dc977..1245079b9 100644 --- a/tests/unit/commands/test_apply.py +++ b/tests/unit/commands/test_apply.py @@ -231,6 +231,51 @@ def test_prepare_task_kolla_role_in_osism_ansible_runtime(task_mocks): assert t is task_mocks.ansible_run.si.return_value +def test_prepare_task_kolla_facts_reaches_kolla_ansible(task_mocks): + """A kolla-ansible playbook whose bare name collides with an osism-ansible + playbook registered for a DIFFERENT environment must still reach + kolla-ansible. osism-ansible ships generic/facts.yml, so it advertises + `facts -> generic`; kolla-ansible advertises `kolla-facts -> kolla`. The + runtime override may only claim roles osism-ansible registered *for kolla* + (stepca, fix-gh973, ...), never a generic one.""" + _set_playbook_maps( + role2environment={"facts": "generic", "kolla-facts": "kolla"}, + role2runtime={"osism-ansible": ["facts"], "kolla-ansible": ["kolla-facts"]}, + ) + cmd = make_command(apply.Run) + + _prepare_task(cmd, role="facts", environment="kolla") + _prepare_task(cmd, role="kolla-facts") + + assert task_mocks.kolla_run.si.call_count == 2 + task_mocks.kolla_run.si.assert_has_calls( + [ + call("kolla", "facts", ["-e kolla_action=deploy"], auto_release_time=3600), + call("kolla", "facts", ["-e kolla_action=deploy"], auto_release_time=3600), + ] + ) + task_mocks.ansible_run.si.assert_not_called() + + +def test_prepare_task_kolla_environment_osism_role_still_overrides(task_mocks): + """The override's real purpose keeps working: a role osism-ansible registers + *for the kolla environment* (playbooks/kolla/stepca.yml) runs in + osism-ansible, not kolla-ansible.""" + _set_playbook_maps( + role2environment={"stepca": "kolla"}, + role2runtime={"osism-ansible": ["stepca"]}, + ) + cmd = make_command(apply.Run) + + t = _prepare_task(cmd, role="stepca") + + task_mocks.ansible_run.si.assert_called_once_with( + "kolla", "stepca", [], auto_release_time=3600 + ) + task_mocks.kolla_run.si.assert_not_called() + assert t is task_mocks.ansible_run.si.return_value + + def test_prepare_task_common_role_stays_in_kolla(task_mocks): _set_playbook_maps( role2environment={"common": "kolla"}, diff --git a/tests/unit/commands/test_sync.py b/tests/unit/commands/test_sync.py index e0452a82f..00fc80ea9 100644 --- a/tests/unit/commands/test_sync.py +++ b/tests/unit/commands/test_sync.py @@ -16,7 +16,7 @@ import json import subprocess import tarfile -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import pytest import requests @@ -49,6 +49,62 @@ def test_facts_schedules_gather_facts_and_returns_rc(rc): assert result == rc +def test_facts_also_refreshes_the_kolla_generation(): + """`osism sync facts` is the command check_ansible_facts advertises, so it + has to repair both cache generations, not just osism-ansible's.""" + cmd, parsed_args = parse_args(sync.Facts, []) + + with patch("osism.commands.sync.utils.check_task_lock_and_exit"), patch( + "osism.tasks.ansible.run.delay" + ) as mock_delay, patch( + "osism.tasks.ansible.dispatch_kolla_facts" + ) as mock_dispatch, patch( + "osism.tasks.handle_task", side_effect=[0, 0] + ) as mock_handle: + result = cmd.take_action(parsed_args) + + mock_dispatch.assert_called_once_with() + assert mock_handle.call_args_list == [ + call(mock_delay.return_value), + call(mock_dispatch.return_value), + ] + assert result == 0 + + +def test_facts_returns_early_when_the_generic_gather_fails(): + """A failed generic gather is reported as-is; the kolla gather is not + attempted, since its own reachability is a separate question.""" + cmd, parsed_args = parse_args(sync.Facts, []) + + with patch("osism.commands.sync.utils.check_task_lock_and_exit"), patch( + "osism.tasks.ansible.run.delay" + ), patch("osism.tasks.ansible.dispatch_kolla_facts") as mock_dispatch, patch( + "osism.tasks.handle_task", return_value=2 + ): + result = cmd.take_action(parsed_args) + + assert result == 2 + mock_dispatch.assert_not_called() + + +def test_facts_skips_the_kolla_handle_when_runtime_absent(): + """When dispatch is skipped (no kolla-ansible container) there is no second + task to wait on.""" + cmd, parsed_args = parse_args(sync.Facts, []) + + with patch("osism.commands.sync.utils.check_task_lock_and_exit"), patch( + "osism.tasks.ansible.run.delay" + ) as mock_delay, patch( + "osism.tasks.ansible.dispatch_kolla_facts", return_value=None + ), patch( + "osism.tasks.handle_task", return_value=0 + ) as mock_handle: + result = cmd.take_action(parsed_args) + + mock_handle.assert_called_once_with(mock_delay.return_value) + assert result == 0 + + # --- CephKeys.take_action --- diff --git a/tests/unit/tasks/test_task_wrappers.py b/tests/unit/tasks/test_task_wrappers.py index 78a47baa6..e09313e82 100644 --- a/tests/unit/tasks/test_task_wrappers.py +++ b/tests/unit/tasks/test_task_wrappers.py @@ -235,6 +235,47 @@ def test_gather_facts_delegates_with_defaults(mocker): check.assert_not_called() +def test_dispatch_kolla_facts_skips_without_the_kolla_runtime(mocker, capsys): + """No kolla-ansible container means nothing consumes the kolla-ansible + queue, so dispatching would strand a task in redis on every run.""" + from osism.data import playbooks + + playbooks.MAP_ROLE2RUNTIME = {"osism-ansible": ["facts"]} + try: + delay = mocker.patch("osism.tasks.kolla.run.delay") + assert ansible.dispatch_kolla_facts() is None + delay.assert_not_called() + finally: + playbooks._reset_caches() + + +def test_dispatch_kolla_facts_queues_when_runtime_present(mocker): + """With kolla-ansible present, gather in its runtime so the cache generation + it can actually read gets written (it never gathers for itself).""" + from osism.data import playbooks + + playbooks.MAP_ROLE2RUNTIME = {"kolla-ansible": ["kolla-facts"]} + try: + delay = mocker.patch("osism.tasks.kolla.run.delay") + result = ansible.dispatch_kolla_facts() + delay.assert_called_once_with("kolla", "facts", [], auto_release_time=3600) + assert result is delay.return_value + finally: + playbooks._reset_caches() + + +def test_gather_facts_also_dispatches_the_kolla_gather(mocker): + """The periodic gather must cover both cache generations, or the kolla one + expires 24 h after a deploy with nothing to rewrite it.""" + mocker.patch( + "osism.tasks.ansible.run_ansible_in_environment", return_value="RESULT" + ) + dispatch = mocker.patch("osism.tasks.ansible.dispatch_kolla_facts") + + assert ansible.gather_facts.__wrapped__() == "RESULT" + dispatch.assert_called_once_with() + + def test_gather_facts_forwards_publish_false(mocker): """``publish=False`` is forwarded as the sixth positional argument.""" delegate = mocker.patch("osism.tasks.ansible.run_ansible_in_environment")