From baaf60cdbdc4a7ffcd89d72614ed2687dbbe74b2 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Wed, 2 Sep 2026 21:27:06 +0200 Subject: [PATCH 1/2] apply: keep kolla playbooks in kolla-ansible The kolla branch of Run._prepare_task lets osism-ansible claim a role whenever osism-ansible ships a playbook of that name: role not in ["common"] and "osism-ansible" in MAP_ROLE2RUNTIME and role in MAP_ROLE2RUNTIME["osism-ansible"] The override is needed: several kolla-environment playbooks live in osism-ansible rather than kolla-ansible (ansible-playbooks playbooks/kolla/: stepca, fix-gh973, nova-update-cell-mappings, openvswitch-ipfix) and must run in that runtime. But the test only asks whether osism-ansible ships the name, never which environment it ships it for, and the kolla- prefix is stripped just above. Any name kolla-ansible advertises as kolla- whose bare form osism-ansible registers for a different environment therefore collapses onto that entry and is dispatched to osism-ansible with ENVIRONMENT=kolla, where run-kolla.sh looks for /ansible/kolla/.yml, finds nothing and reports ERROR: service in environment kolla not available Exactly three names collide, all registered by osism-ansible for the generic environment: facts, gather-facts and certificates. So osism apply kolla-facts, kolla-gather-facts and kolla-certificates have all been unreachable, in every spelling, since #877 introduced the override -- kolla-certificates from two days after #612 deliberately renamed the play to make it addressable, and kolla-gather-facts from 2024-06-26, when generic/gather-facts.yml was added. Nobody noticed because all three were redundant at the time: a single fact-cache keyspace meant osism-ansible's gather served kolla too, and OSISM handles TLS separately from kolla's certificates play. Require the role to be mapped to the kolla environment as well. stepca and friends map to kolla and keep running in osism-ansible; facts, gather-facts and certificates map to generic, fail the guard and fall through to kolla-ansible, where those playbooks actually live. This matters now because ansible-core 2.19 namespaces fact-cache entries per schema, so kolla-ansible needs to write its own generation rather than read osism-ansible's, and kolla-facts is how it does that. The fix itself is version independent: these commands should work on any ansible-core. Verified against the interface maps of a live cluster: facts, kolla-facts, gather-facts, certificates and kolla-certificates now route to kolla-ansible, while stepca, fix-gh973 and nova-update-cell-mappings still route to osism-ansible, and keystone and common are unchanged. Assisted-by: Claude:claude-opus-5 Signed-off-by: Roger Luethi --- osism/commands/apply.py | 15 +++++++++++ tests/unit/commands/test_apply.py | 45 +++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) 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/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"}, From c7382ec79c9faf511b6953adff75aeb6c2e06d46 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Wed, 2 Sep 2026 21:27:23 +0200 Subject: [PATCH 2/2] facts: gather in the kolla runtime too Since ansible-core 2.19 the fact cache is namespaced per schema, so osism-ansible's gather no longer fills the keyspace kolla-ansible reads when the two straddle 2.19. kolla never gathers for itself -- the site splitter forces gather_facts: false on every emitted play -- so its generation has no writer at all. Give it one. dispatch_kolla_facts() queues /ansible/kolla-facts.yml, OSISM's own unconditional kolla-side gather, in the kolla-ansible runtime. The periodic gather_facts task calls it after the generic gather, and osism sync facts does the same so a single operator command repairs both generations; check_ansible_facts already points operators at that command, and until now its advice could not fix the kolla case. Without the periodic half a one-shot gather decays: fact_caching_timeout is 86400 while GATHER_FACTS_SCHEDULE is 43200, and only osism-ansible's generation was ever rewritten, so kolla's would expire a day after a successful deploy with no deployment in flight to explain the failure. The dispatch is guarded on kolla-ansible appearing in MAP_ROLE2RUNTIME. The kolla-ansible container is the only consumer of its own Celery queue and is gated on enable_kolla_ansible, so on a manager without it task_create_missing_queues would create the queue and every run would strand another task in Redis. The guard has to run 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. Verified on a live cluster -- in a container without /interface the map is {} and the dispatch is skipped, while osismclient sees ceph-ansible, kolla-ansible and osism-ansible. sync facts returns early if the generic gather fails, so a broken osism-ansible run is reported as-is instead of being masked by the kolla result. Assisted-by: Claude:claude-opus-5 Signed-off-by: Roger Luethi --- osism/commands/sync.py | 11 +++++ osism/tasks/ansible.py | 37 +++++++++++++++- tests/unit/commands/test_sync.py | 58 +++++++++++++++++++++++++- tests/unit/tasks/test_task_wrappers.py | 41 ++++++++++++++++++ 4 files changed, 145 insertions(+), 2 deletions(-) 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_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")