From 9b32a1d7c689c795d622aecde3d69e1fb22af9b4 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Sat, 14 Nov 2026 07:13:59 +0100 Subject: [PATCH] Read the Ansible fact cache in both key schemas ansible-core 2.19 changed how cache plugins store entries. Before 2.19 the redis cache plugin stores a host's facts as plain JSON under ansible_facts. From 2.19 on, ansible-core puts a schema prefix inside the plugin's key prefix and wraps the value, so the same host's facts land under ansible_factss1_ as {"__payload__": ""}. Values set with set_fact cacheable keep their data tags in the payload, as {"value": ..., "tags": [...], "__ansible_type": ...}. The runtimes OSISM ships straddle that line: kolla-ansible (2.20) writes the new key, osism-ansible (2.18) and ceph-ansible (2.16) write the old one, and both coexist on a running cluster. Every reader in python-osism used only the old key: - osism get facts / osism dump facts and osism get states (commands/get.py) - GET /v1/inventory/hosts/{host}/facts and .../facts/{fact} (api.py) - the RabbitMQ node address lookup (utils/rabbitmq.py), used by the RabbitMQ commands in commands/migrate.py and commands/status.py - osism reset facts --limit (commands/reset.py), which deleted only the old key - the facts freshness check run by osism apply (check_ansible_facts), which derived the host name "s1_" from new keys and then skipped their entries as malformed Once osism-ansible moves to ansible-core 2.19 or later, the facts it gathers land under the new key as well, and only ceph-ansible keeps the old one current. Tried on an OSISM 11.0.3 cluster with an image derived from osism-ansible 0.20261111.0 that only replaces ansible-core with 2.19.15: after `osism apply facts`, redis held fresh facts under ansible_factss1_ while ansible_facts was a day old, and osism dump facts showed the old data. Add osism/utils/fact_cache.py and route all of these readers through it: - decode() parses an entry in either schema, unwraps __payload__ and strips data tags. It does this itself rather than with ansible-core's private _cache_persistence decoder, so it returns plain JSON data and does not depend on ansible-core internals or on which ansible-core the reading image ships. - get_host_facts() reads both keys and returns the newer entry by ansible_date_time.epoch, since which key is current depends on which runtime gathered last; on a tie, or without timestamps, the schema-prefixed entry wins. - scan_host_keys() groups the keys of both schemas by host; the freshness check judges each host by its newest entry. - delete_host_facts() deletes both keys and also removes and s1_ from the ansible_cache_keys sorted set the redis cache plugin maintains. osism reset facts without --limit now deletes that set too and reports the number of distinct hosts. The API facts endpoints catch ValueError instead of JSONDecodeError, since decode() also rejects entries that are not a mapping. The freshness check now logs "unusable ansible_date_time.epoch" for a well-formed entry with a non-numeric epoch instead of calling it malformed; the host is still skipped. Tests: tests/unit/utils/test_fact_cache.py covers the module against fakeredis, and new tests cover the freshness check and the API with new-format entries. Existing tests that pinned the exact redis calls now expect both keys. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Roger Luethi --- osism/api.py | 18 +- osism/commands/get.py | 7 +- osism/commands/reset.py | 22 +-- osism/utils/__init__.py | 87 +++++----- osism/utils/fact_cache.py | 135 +++++++++++++++ osism/utils/rabbitmq.py | 7 +- tests/unit/commands/test_get.py | 7 +- tests/unit/commands/test_reset.py | 17 +- tests/unit/test_api_endpoints.py | 21 ++- tests/unit/utils/test_fact_cache.py | 200 ++++++++++++++++++++++ tests/unit/utils/test_init_task_output.py | 50 +++++- tests/unit/utils/test_rabbitmq.py | 15 +- 12 files changed, 498 insertions(+), 88 deletions(-) create mode 100644 osism/utils/fact_cache.py create mode 100644 tests/unit/utils/test_fact_cache.py diff --git a/osism/api.py b/osism/api.py index bd0544668..bf529c69e 100644 --- a/osism/api.py +++ b/osism/api.py @@ -7,7 +7,7 @@ import os import re import subprocess -from typing import Optional, Dict, Any, List, cast +from typing import Optional, Dict, Any, List from uuid import UUID from fastapi import ( @@ -26,6 +26,7 @@ from osism.tasks import reconciler, openstack from osism import utils +from osism.utils import fact_cache from osism.utils.inventory import get_hosts_from_inventory, get_inventory_path from osism.services.listener import BaremetalEvents from osism.services.websocket_manager import websocket_manager @@ -941,22 +942,21 @@ async def get_host_hostvar(host: str, variable: str) -> HostvarSingleResponse: async def get_host_facts(host: str) -> FactsResponse: """Get all cached Ansible facts for a specific host.""" try: - data = cast(bytes | None, utils.redis.get(f"ansible_facts{host}")) + facts_data = fact_cache.get_host_facts(utils.redis, host) - if not data: + if not facts_data: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=f"No facts found in cache for host '{host}'", ) - facts_data = json.loads(data) facts = [ FactEntry(name=name, value=value) for name, value in sorted(facts_data.items()) ] return FactsResponse(host=host, facts=facts, count=len(facts), from_cache=True) - except json.JSONDecodeError as e: + except ValueError as e: logger.error(f"Error parsing facts JSON for {host}: {str(e)}") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, @@ -980,16 +980,14 @@ async def get_host_facts(host: str) -> FactsResponse: async def get_host_fact(host: str, fact: str) -> FactSingleResponse: """Get a specific cached Ansible fact for a host.""" try: - data = cast(bytes | None, utils.redis.get(f"ansible_facts{host}")) + facts_data = fact_cache.get_host_facts(utils.redis, host) - if not data: + if not facts_data: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=f"No facts found in cache for host '{host}'", ) - facts_data = json.loads(data) - if fact not in facts_data: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -999,7 +997,7 @@ async def get_host_fact(host: str, fact: str) -> FactSingleResponse: return FactSingleResponse( host=host, name=fact, value=facts_data[fact], from_cache=True ) - except json.JSONDecodeError as e: + except ValueError as e: logger.error(f"Error parsing facts JSON for {host}: {str(e)}") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, diff --git a/osism/commands/get.py b/osism/commands/get.py index 31d756277..65b041d18 100644 --- a/osism/commands/get.py +++ b/osism/commands/get.py @@ -11,6 +11,7 @@ from tabulate import tabulate from osism import utils +from osism.utils import fact_cache from osism.utils.inventory import get_hosts_from_inventory, get_inventory_path @@ -192,9 +193,8 @@ def take_action(self, parsed_args): fact = parsed_args.fact cache = not parsed_args.no_cache - data = utils.redis.get(f"ansible_facts{host}") + data = fact_cache.get_host_facts(utils.redis, host) if data: - data = json.loads(data) table = [] if fact: @@ -355,9 +355,8 @@ def get_parser(self, prog_name): def take_action(self, parsed_args): host = parsed_args.host[0] - data = utils.redis.get(f"ansible_facts{host}") + data = fact_cache.get_host_facts(utils.redis, host) if data: - data = json.loads(data) table = [] if "ansible_local" in data and "osism" in data["ansible_local"]: diff --git a/osism/commands/reset.py b/osism/commands/reset.py index 0cf2b5862..5aa9fc5ce 100644 --- a/osism/commands/reset.py +++ b/osism/commands/reset.py @@ -8,6 +8,7 @@ from redis.exceptions import RedisError from osism import utils +from osism.utils import fact_cache from osism.utils.inventory import get_hosts_from_inventory, get_inventory_path @@ -39,23 +40,17 @@ def take_action(self, parsed_args): return self._reset_all() def _reset_all(self): - removed = 0 try: - cursor = 0 - while True: - cursor, batch = utils.redis.scan( - cursor, match="ansible_facts*", count=100 - ) - if batch: - utils.redis.delete(*batch) - removed += len(batch) - if cursor == 0: - break + hosts = fact_cache.scan_host_keys(utils.redis) + keys = [key for host_keys in hosts.values() for key in host_keys] + if keys: + utils.redis.delete(*keys) + utils.redis.delete(fact_cache.CACHE_KEYS) except RedisError as exc: logger.error(f"Failed to reset Ansible fact cache: {exc}") return 1 - logger.info(f"Removed cached facts for {removed} host(s)") + logger.info(f"Removed cached facts for {len(hosts)} host(s)") return 0 def _reset_limited(self, limit): @@ -96,9 +91,8 @@ def _reset_limited(self, limit): logger.warning("No hosts matched the given limit.") return 0 - keys = [f"ansible_facts{host}" for host in hosts] try: - deleted = utils.redis.delete(*keys) + deleted = fact_cache.delete_host_facts(utils.redis, hosts) except RedisError as exc: logger.error(f"Failed to reset Ansible fact cache: {exc}") return 1 diff --git a/osism/utils/__init__.py b/osism/utils/__init__.py index 8bcba1cfd..85604ab16 100644 --- a/osism/utils/__init__.py +++ b/osism/utils/__init__.py @@ -608,33 +608,26 @@ def is_task_locked(): def check_ansible_facts(max_age=None): """Check if Ansible facts exist in Redis and are not stale. - Scans Redis for ansible_facts* keys and checks the + Looks at every host's cached facts, in both of the fact cache's key + schemas (see osism.utils.fact_cache), and checks the newest entry's ansible_date_time.epoch field to determine freshness. Args: max_age: Maximum age in seconds (default: settings.FACTS_MAX_AGE) """ - import json + from osism.utils import fact_cache if max_age is None: max_age = settings.FACTS_MAX_AGE try: r = _init_redis() - - # Find all ansible_facts keys - keys = [] - cursor = 0 - while True: - cursor, batch = r.scan(cursor, match="ansible_facts*", count=100) - keys.extend(batch) - if cursor == 0: - break + hosts = fact_cache.scan_host_keys(r) except Exception as e: logger.warning(f"Could not check Ansible facts freshness: {e}") return - if not keys: + if not hosts: logger.warning( "No Ansible facts found in Redis cache. " "Run 'osism sync facts' to gather facts." @@ -644,43 +637,45 @@ def check_ansible_facts(max_age=None): now = time.time() stale_hosts = [] - for key in keys: - data = None - try: - key_str = key.decode() if isinstance(key, bytes) else key - hostname = key_str.replace("ansible_facts", "", 1) - - # Skip localhost and friends: their facts are never refreshed via - # 'osism sync facts', so reporting them as stale is misleading. - if hostname in LOCAL_FACT_HOSTS: - continue - - data = r.get(key) - if not data: - continue - facts = json.loads(data) - date_time = facts.get("ansible_date_time", {}) - epoch = date_time.get("epoch") - if epoch is None: - logger.debug( - f"Host '{hostname}': facts missing ansible_date_time.epoch" + for hostname, keys in hosts.items(): + # Skip localhost and friends: their facts are never refreshed via + # 'osism sync facts', so reporting them as stale is misleading. + if hostname in LOCAL_FACT_HOSTS: + continue + + entries = [] + for key in keys: + data = None + try: + data = r.get(key) + if not data: + continue + entries.append(fact_cache.decode(data)) + except (ValueError, TypeError): + truncated_value = data + if isinstance(truncated_value, (bytes, str)): + truncated_value = truncated_value[:200] + # loguru formats with str.format and knows no exc_info kwarg, so + # printf-style placeholders would be logged verbatim and the + # traceback dropped. + logger.opt(exception=True).debug( + f"Skipping malformed ansible_facts entry for key {key!r}: " + f"{truncated_value!r}" ) - continue + if not entries: + continue + + epoch = fact_cache.newest(entries).get("ansible_date_time", {}).get("epoch") + if epoch is None: + logger.debug(f"Host '{hostname}': facts missing ansible_date_time.epoch") + continue + try: age = now - float(epoch) - if age > max_age: - stale_hosts.append((hostname, int(age))) - except (json.JSONDecodeError, ValueError, TypeError): - truncated_value = data - if isinstance(truncated_value, (bytes, str)): - truncated_value = truncated_value[:200] - # loguru formats with str.format and knows no exc_info kwarg, so - # printf-style placeholders would be logged verbatim and the - # traceback dropped. - logger.opt(exception=True).debug( - f"Skipping malformed ansible_facts entry for key {key!r}: " - f"{truncated_value!r}" - ) + except (TypeError, ValueError): + logger.debug(f"Host '{hostname}': unusable ansible_date_time.epoch") continue + if age > max_age: + stale_hosts.append((hostname, int(age))) if stale_hosts: logger.warning( diff --git a/osism/utils/fact_cache.py b/osism/utils/fact_cache.py new file mode 100644 index 000000000..61a611c19 --- /dev/null +++ b/osism/utils/fact_cache.py @@ -0,0 +1,135 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Ansible's fact cache in Redis, in both of its key schemas. + +The Ansible runtimes OSISM ships straddle ansible-core 2.19, which changed how +cache plugins store entries. Before 2.19 the redis cache plugin stores a host's +facts as plain JSON under ``ansible_facts``. From 2.19 on, ansible-core +puts a schema prefix inside the plugin's key prefix and wraps the value, so the +same host's facts land under ``ansible_factss1_`` as +``{"__payload__": ""}``. Values set with ``set_fact: cacheable`` keep +their data tags in that payload, as +``{"value": ..., "tags": [...], "__ansible_type": "..."}``. + +Which key is current depends on which runtime gathered last, and both coexist +on a running cluster. Readers here look at both and take the newer entry, by +``ansible_date_time.epoch``. Tags are stripped rather than decoded with +ansible-core's private decoder, so the result is plain JSON data and does not +depend on ansible-core internals. + +The redis cache plugin also records every key it has written as a member of +the sorted set ``ansible_cache_keys`` (```` and ``s1_``), so +deleting a host's facts removes those members too. +""" + +import json + +KEY_PREFIX = "ansible_facts" +SCHEMA_PREFIX = "s1_" +CACHE_KEYS = "ansible_cache_keys" + +_PAYLOAD = "__payload__" +_TYPE = "__ansible_type" + + +def keys_for(host): + """Return both cache keys for *host*, the schema-prefixed one first.""" + return [f"{KEY_PREFIX}{SCHEMA_PREFIX}{host}", f"{KEY_PREFIX}{host}"] + + +def host_from_key(key): + """Return the host a cache key belongs to, or None for a foreign key.""" + if isinstance(key, bytes): + key = key.decode() + if not key.startswith(KEY_PREFIX): + return None + name = key[len(KEY_PREFIX) :] + if name.startswith(SCHEMA_PREFIX): + name = name[len(SCHEMA_PREFIX) :] + return name + + +def _untag(value): + if isinstance(value, dict): + if _TYPE in value: + if "value" in value: + return _untag(value["value"]) + return {k: _untag(v) for k, v in value.items() if k != _TYPE} + return {k: _untag(v) for k, v in value.items()} + if isinstance(value, list): + return [_untag(v) for v in value] + return value + + +def decode(data): + """Parse one cache entry, in either schema, into a plain dict. + + Raises ValueError (json.JSONDecodeError included) for an entry that is not + JSON or not a mapping of facts. + """ + value = json.loads(data) + if isinstance(value, dict) and _PAYLOAD in value: + value = _untag(json.loads(value[_PAYLOAD])) + if not isinstance(value, dict): + raise ValueError("fact cache entry is not a mapping") + return value + + +def _epoch(facts): + try: + return float(facts.get("ansible_date_time", {}).get("epoch")) + except (AttributeError, TypeError, ValueError): + return float("-inf") + + +def newest(entries): + """Return the entry with the newest ``ansible_date_time.epoch``. + + On a tie, or when no entry carries a timestamp, the first one wins. + """ + return max(entries, key=_epoch) + + +def get_host_facts(redis, host): + """Return the newest cached facts for *host*, or None if there are none. + + Raises ValueError if a cached entry cannot be parsed. + """ + entries = [] + for key in keys_for(host): + data = redis.get(key) + if data: + entries.append(decode(data)) + if not entries: + return None + return newest(entries) + + +def scan_host_keys(redis): + """Return ``{host: [key, ...]}`` for every fact-cache key in Redis.""" + hosts = {} + cursor = 0 + while True: + cursor, batch = redis.scan(cursor, match=f"{KEY_PREFIX}*", count=100) + for key in batch: + host = host_from_key(key) + if host is not None: + hosts.setdefault(host, []).append(key) + if cursor == 0: + break + return hosts + + +def delete_host_facts(redis, hosts): + """Delete both cache entries of each host in *hosts*. + + Also removes the hosts' members from the ``ansible_cache_keys`` set. + Returns the number of hosts that had at least one entry. + """ + removed = sum(1 for host in hosts if redis.exists(*keys_for(host))) + keys = [key for host in hosts for key in keys_for(host)] + members = [m for host in hosts for m in (f"{SCHEMA_PREFIX}{host}", host)] + if keys: + redis.delete(*keys) + redis.zrem(CACHE_KEYS, *members) + return removed diff --git a/osism/utils/rabbitmq.py b/osism/utils/rabbitmq.py index 5bb9714a6..0f3df212f 100644 --- a/osism/utils/rabbitmq.py +++ b/osism/utils/rabbitmq.py @@ -60,6 +60,7 @@ def get_rabbitmq_node_addresses(): logger.debug(f"RabbitMQ hosts: {rabbitmq_hosts}") from osism import utils + from osism.utils import fact_cache node_addresses = [] for host in rabbitmq_hosts: @@ -67,13 +68,11 @@ def get_rabbitmq_node_addresses(): # collected for the other nodes. try: # Get ansible facts from Redis cache - facts_data = utils.redis.get(f"ansible_facts{host}") - if not facts_data: + facts = fact_cache.get_host_facts(utils.redis, host) + if not facts: logger.error(f"No ansible facts found in cache for {host}") continue - facts = json.loads(facts_data) - # Resolve internal_interface and the address it carries in one # templated lookup, so that any Jinja2 shape works -- not just # the ones a hand-written resolver anticipated. diff --git a/tests/unit/commands/test_get.py b/tests/unit/commands/test_get.py index 70fcc1d0c..e57e5725a 100644 --- a/tests/unit/commands/test_get.py +++ b/tests/unit/commands/test_get.py @@ -16,7 +16,7 @@ import pprint import subprocess from datetime import datetime -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import docker @@ -335,7 +335,10 @@ def test_facts_prints_single_requested_fact(monkeypatch, capsys): cmd.take_action(parsed_args) - fake.get.assert_called_once_with("ansible_factstesthost") + assert fake.get.call_args_list == [ + call("ansible_factss1_testhost"), + call("ansible_factstesthost"), + ] out = capsys.readouterr().out assert "ansible_hostname" in out assert "'testhost'" in out diff --git a/tests/unit/commands/test_reset.py b/tests/unit/commands/test_reset.py index e76b0074d..e9bbee041 100644 --- a/tests/unit/commands/test_reset.py +++ b/tests/unit/commands/test_reset.py @@ -9,7 +9,7 @@ """ import subprocess -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import pytest from redis.exceptions import RedisError @@ -53,9 +53,10 @@ def test_facts_flushes_all_keys_when_no_limit(mock_redis, loguru_logs): assert rc == 0 mock_redis.scan.assert_called_once_with(0, match="ansible_facts*", count=100) - mock_redis.delete.assert_called_once_with( - b"ansible_factsnode1", b"ansible_factsnode2" - ) + assert mock_redis.delete.call_args_list == [ + call(b"ansible_factsnode1", b"ansible_factsnode2"), + call("ansible_cache_keys"), + ] assert any("2 host(s)" in r["message"] for r in loguru_logs) @@ -115,7 +116,13 @@ def test_facts_limit_deletes_only_selected_hosts(mock_redis): assert rc == 0 mock_redis.scan.assert_not_called() mock_redis.delete.assert_called_once_with( - "ansible_factsnode1", "ansible_factsnode2" + "ansible_factss1_node1", + "ansible_factsnode1", + "ansible_factss1_node2", + "ansible_factsnode2", + ) + mock_redis.zrem.assert_called_once_with( + "ansible_cache_keys", "s1_node1", "node1", "s1_node2", "node2" ) diff --git a/tests/unit/test_api_endpoints.py b/tests/unit/test_api_endpoints.py index b070c7a5c..19a4e7f1f 100644 --- a/tests/unit/test_api_endpoints.py +++ b/tests/unit/test_api_endpoints.py @@ -855,7 +855,26 @@ def test_get_host_facts_cache_miss(client): assert response.status_code == 404 assert response.json()["detail"] == "No facts found in cache for host 'node-1'" - fake_redis.get.assert_called_once_with("ansible_factsnode-1") + assert fake_redis.get.call_args_list == [ + call("ansible_factss1_node-1"), + call("ansible_factsnode-1"), + ] + + +def test_get_host_facts_reads_a_schema1_entry(client): + # ansible-core 2.19+ writes ansible_factss1_ as {"__payload__": ...}; + # values from `set_fact: cacheable` carry data tags. + tagged = {"value": "bar", "tags": [], "__ansible_type": "_AnsibleTaggedStr"} + entry = json.dumps({"__payload__": json.dumps({"a_fact": tagged})}).encode() + fake_redis = MagicMock() + fake_redis.get.side_effect = lambda key: ( + entry if key == "ansible_factss1_node-1" else None + ) + + response = get_with_redis(client, fake_redis, "/v1/inventory/hosts/node-1/facts") + + assert response.status_code == 200 + assert response.json()["facts"] == [{"name": "a_fact", "value": "bar"}] def test_get_host_facts_returns_sorted_facts(client): diff --git a/tests/unit/utils/test_fact_cache.py b/tests/unit/utils/test_fact_cache.py new file mode 100644 index 000000000..b5726090e --- /dev/null +++ b/tests/unit/utils/test_fact_cache.py @@ -0,0 +1,200 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for ``osism.utils.fact_cache``. + +The entries mirror what the redis cache plugin writes: ansible-core before +2.19 stores plain JSON under ``ansible_facts``; 2.19 and later store +``{"__payload__": ""}`` under ``ansible_factss1_``, with data tags +on values set by ``set_fact: cacheable``. Both kinds are recorded in the +``ansible_cache_keys`` sorted set. +""" + +import json + +import fakeredis +import pytest + +from osism.utils import fact_cache + + +def _legacy(facts): + return json.dumps(facts).encode() + + +def _schema1(facts): + return json.dumps({"__payload__": json.dumps(facts)}).encode() + + +def _facts(epoch, **extra): + return { + "ansible_date_time": {"epoch": str(epoch)}, + "ansible_hostname": "node1", + **extra, + } + + +# A value as ansible-core 2.20 stores a tagged string from `set_fact: cacheable`. +_TAGGED = { + "value": "bar", + "tags": [ + {"path": "/p.yml", "line_num": 4, "col_num": 14, "__ansible_type": "Origin"} + ], + "__ansible_type": "_AnsibleTaggedStr", +} + + +@pytest.fixture +def redis(): + return fakeredis.FakeRedis() + + +def _store(redis, key, data, member): + redis.set(key, data) + redis.zadd(fact_cache.CACHE_KEYS, {member: 1.0}) + + +# --- keys --- + + +def test_keys_for_lists_the_schema_prefixed_key_first(): + assert fact_cache.keys_for("node1") == [ + "ansible_factss1_node1", + "ansible_factsnode1", + ] + + +@pytest.mark.parametrize( + "key, host", + [ + (b"ansible_factsnode1", "node1"), + (b"ansible_factss1_node1", "node1"), + ("ansible_factsnode1", "node1"), + (b"ansible_cache_keys", None), + ], +) +def test_host_from_key(key, host): + assert fact_cache.host_from_key(key) == host + + +# --- decode --- + + +def test_decode_legacy_entry(): + assert fact_cache.decode(_legacy(_facts(1))) == _facts(1) + + +def test_decode_schema1_entry_unwraps_the_payload(): + assert fact_cache.decode(_schema1(_facts(1))) == _facts(1) + + +def test_decode_strips_data_tags(): + facts = _facts(1, foo=_TAGGED, nested={"items": [_TAGGED, 2]}) + + decoded = fact_cache.decode(_schema1(facts)) + + assert decoded["foo"] == "bar" + assert decoded["nested"] == {"items": ["bar", 2]} + + +def test_decode_drops_the_type_marker_of_a_tagged_mapping_without_value(): + facts = _facts(1, when={"iso": "2026-11-14", "__ansible_type": "datetime"}) + + assert fact_cache.decode(_schema1(facts))["when"] == {"iso": "2026-11-14"} + + +@pytest.mark.parametrize( + "data", + [b"{not json", b"[1, 2]", json.dumps({"__payload__": "{not json"}).encode()], +) +def test_decode_rejects_what_is_not_a_fact_mapping(data): + with pytest.raises(ValueError): + fact_cache.decode(data) + + +# --- get_host_facts --- + + +def test_get_host_facts_none_when_nothing_is_cached(redis): + assert fact_cache.get_host_facts(redis, "node1") is None + + +def test_get_host_facts_reads_a_legacy_entry_alone(redis): + redis.set("ansible_factsnode1", _legacy(_facts(100))) + + assert fact_cache.get_host_facts(redis, "node1") == _facts(100) + + +def test_get_host_facts_reads_a_schema1_entry_alone(redis): + redis.set("ansible_factss1_node1", _schema1(_facts(100))) + + assert fact_cache.get_host_facts(redis, "node1") == _facts(100) + + +def test_get_host_facts_prefers_the_newer_schema1_entry(redis): + redis.set("ansible_factsnode1", _legacy(_facts(100, packages={}))) + redis.set("ansible_factss1_node1", _schema1(_facts(200))) + + assert fact_cache.get_host_facts(redis, "node1") == _facts(200) + + +def test_get_host_facts_prefers_the_newer_legacy_entry(redis): + # ceph-ansible keeps writing the legacy key after osism-ansible moved on. + redis.set("ansible_factsnode1", _legacy(_facts(300))) + redis.set("ansible_factss1_node1", _schema1(_facts(200))) + + assert fact_cache.get_host_facts(redis, "node1") == _facts(300) + + +def test_get_host_facts_falls_back_to_the_entry_with_a_timestamp(redis): + redis.set("ansible_factsnode1", _legacy({"ansible_hostname": "node1"})) + redis.set("ansible_factss1_node1", _schema1(_facts(200))) + + assert fact_cache.get_host_facts(redis, "node1") == _facts(200) + + +def test_get_host_facts_raises_on_a_corrupt_entry(redis): + redis.set("ansible_factsnode1", b"{not json") + + with pytest.raises(ValueError): + fact_cache.get_host_facts(redis, "node1") + + +# --- scan_host_keys --- + + +def test_scan_host_keys_groups_both_schemas_by_host(redis): + redis.set("ansible_factsnode1", _legacy(_facts(1))) + redis.set("ansible_factss1_node1", _schema1(_facts(2))) + redis.set("ansible_factss1_node2", _schema1(_facts(3))) + redis.zadd(fact_cache.CACHE_KEYS, {"node1": 1.0}) + redis.set("unrelated", b"x") + + hosts = fact_cache.scan_host_keys(redis) + + assert sorted(hosts) == ["node1", "node2"] + assert sorted(hosts["node1"]) == [b"ansible_factsnode1", b"ansible_factss1_node1"] + assert hosts["node2"] == [b"ansible_factss1_node2"] + + +# --- delete_host_facts --- + + +def test_delete_host_facts_removes_both_entries_and_set_members(redis): + _store(redis, "ansible_factsnode1", _legacy(_facts(1)), "node1") + _store(redis, "ansible_factss1_node1", _schema1(_facts(2)), "s1_node1") + _store(redis, "ansible_factss1_node2", _schema1(_facts(3)), "s1_node2") + _store(redis, "ansible_factsnode3", _legacy(_facts(4)), "node3") + + removed = fact_cache.delete_host_facts(redis, ["node1", "node2", "absent"]) + + assert removed == 2 + assert redis.keys("ansible_facts*") == [b"ansible_factsnode3"] + assert redis.zrange(fact_cache.CACHE_KEYS, 0, -1) == [b"node3"] + + +def test_delete_host_facts_with_no_hosts_touches_nothing(redis): + _store(redis, "ansible_factsnode1", _legacy(_facts(1)), "node1") + + assert fact_cache.delete_host_facts(redis, []) == 0 + assert redis.exists("ansible_factsnode1") + assert redis.zrange(fact_cache.CACHE_KEYS, 0, -1) == [b"node1"] diff --git a/tests/unit/utils/test_init_task_output.py b/tests/unit/utils/test_init_task_output.py index 422dc0418..2e97b4332 100644 --- a/tests/unit/utils/test_init_task_output.py +++ b/tests/unit/utils/test_init_task_output.py @@ -635,6 +635,54 @@ def test_check_ansible_facts_one_fresh_host_no_warning(mocker, loguru_logs): assert not any("stale" in m for m in warnings) +def _schema1_payload(epoch): + """Return JSON bytes shaped like an ansible-core 2.19+ fact-cache entry.""" + import json + + inner = json.dumps({"ansible_date_time": {"epoch": epoch}}) + return json.dumps({"__payload__": inner}).encode("utf-8") + + +def test_check_ansible_facts_fresh_schema1_entry_beats_stale_legacy( + mocker, loguru_logs +): + # osism-ansible on ansible-core 2.19+ refreshes ansible_factss1_ + # while the legacy key ages; the host is fresh. + import time as time_mod + + now = time_mod.time() + entries = { + b"ansible_factshost-a": _facts_payload(now - 9999), + b"ansible_factss1_host-a": _schema1_payload(now - 1), + } + mock_r = mocker.MagicMock() + mock_r.scan.return_value = (0, list(entries)) + mock_r.get.side_effect = entries.get + mocker.patch("osism.utils._init_redis", return_value=mock_r) + + utils_pkg.check_ansible_facts(max_age=10) + + warnings = [r["message"] for r in loguru_logs if r["level"] == "WARNING"] + assert not any("stale" in m for m in warnings) + + +def test_check_ansible_facts_stale_schema1_entry_names_the_host(mocker, loguru_logs): + import time as time_mod + + now = time_mod.time() + mock_r = mocker.MagicMock() + mock_r.scan.return_value = (0, [b"ansible_factss1_host-a"]) + mock_r.get.return_value = _schema1_payload(now - 9999) + mocker.patch("osism.utils._init_redis", return_value=mock_r) + + utils_pkg.check_ansible_facts(max_age=10) + + warnings = [r["message"] for r in loguru_logs if r["level"] == "WARNING"] + assert any("stale for 1 host(s)" in m for m in warnings) + assert any("Host 'host-a'" in m for m in warnings) + assert not any("s1_host-a" in m for m in warnings) + + def test_check_ansible_facts_mix_of_fresh_and_stale(mocker, loguru_logs): import time as time_mod @@ -752,7 +800,7 @@ def test_check_ansible_facts_non_numeric_epoch_skipped(mocker, loguru_logs): utils_pkg.check_ansible_facts(max_age=10) debug_messages = [r["message"] for r in loguru_logs if r["level"] == "DEBUG"] - assert any("Skipping malformed ansible_facts entry" in m for m in debug_messages) + assert any("unusable ansible_date_time.epoch" in m for m in debug_messages) warnings = [r["message"] for r in loguru_logs if r["level"] == "WARNING"] assert not any("stale" in m for m in warnings) diff --git a/tests/unit/utils/test_rabbitmq.py b/tests/unit/utils/test_rabbitmq.py index 792d5c3e5..6411e3398 100644 --- a/tests/unit/utils/test_rabbitmq.py +++ b/tests/unit/utils/test_rabbitmq.py @@ -65,7 +65,18 @@ def setup_addresses(mocker): def _setup(*, hosts, redis_side_effect, check_output, resolve=None): fake_redis = mocker.MagicMock() - fake_redis.get.side_effect = redis_side_effect + # ``redis_side_effect`` lists the legacy ``ansible_facts`` + # entries in host order. The schema-prefixed key that ansible-core + # 2.19+ writes (``ansible_factss1_``) is read first and is empty + # here; the fact-cache helper's own tests cover it. + legacy_entries = iter(redis_side_effect) + + def _get(key): + if key.startswith("ansible_factss1_"): + return None + return next(legacy_entries) + + fake_redis.get.side_effect = _get # Seed the lazy attribute on the package so ``utils.redis`` resolves to # the mock without triggering ``__getattr__`` -> ``_init_redis()``. mocker.patch.dict(utils_pkg.__dict__, {"redis": fake_redis}) @@ -152,7 +163,9 @@ def test_two_hosts_returned_in_alphabetical_order( # Hosts are sorted before processing, so the cache is queried host1 # first. assert mocks.redis.get.call_args_list == [ + call("ansible_factss1_host1"), call("ansible_factshost1"), + call("ansible_factss1_host2"), call("ansible_factshost2"), ]