tests: cover interface resolution against real Ansible

playbooks/test-integration.yml

@@ -47,6 +47,12 @@
           {{ python_venv_dir }}/bin/pipenv install --dev --deploy
           {{ python_venv_dir }}/bin/pipenv run pip install .
 
+          # Some integration tests resolve inventory variables the way the
+          # deployment does, by running ansible. Install it from the same
+          # requirements file the container image uses, so the version under
+          # test is the version that ships and no second pin is introduced.
+          {{ python_venv_dir }}/bin/pipenv run pip install -r requirements.ansible.txt
+
     - name: Run pytest
       ansible.builtin.shell:
         executable: /bin/bash
@@ -69,3 +75,6 @@
         # Fail the session if Redis is unreachable instead of skipping every
         # test, so a Redis-startup failure turns the job red rather than green.
         OSISM_REQUIRE_REDIS: "1"
+        # Likewise for ansible: the tests that need it skip when it is absent,
+        # which would let a broken install pass the job green.
+        OSISM_REQUIRE_ANSIBLE: "1"

tests/integration/test_rabbitmq_addresses.py

@@ -0,0 +1,201 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""Integration tests for ``get_rabbitmq_node_addresses()`` interface resolution.
+
+The unit tests in ``tests/unit/utils/test_rabbitmq.py`` cover the same scenarios
+against mocks: ``subprocess`` and the Redis client are both replaced, so what
+they verify is the shape of the calls the function makes, not whether Ansible
+agrees with it. That distinction is not academic here. The value the function
+has to resolve, ``internal_interface``, is frequently Jinja-valued, and its
+meaning is defined by Ansible's templating and by how Ansible names interface
+facts -- neither of which a mock can speak for.
+
+These tests therefore drive the public function with the real collaborators: a
+live Redis holding the cached facts, a real inventory on disk, and the real
+``ansible-inventory`` / ``ansible`` binaries. Each case is a value shape an
+operator actually writes. Because they target the public function rather than
+its internals, they stay meaningful across changes to how the resolution is
+implemented.
+
+Requires a reachable Redis (see ``conftest.py``) and ``ansible`` on PATH;
+``ansible-core`` is not a dependency of this package -- it comes from the
+container images -- so the ansible-dependent cases skip without it.
+"""
+
+import json
+import os
+import shutil
+
+import pytest
+
+from osism import utils
+from osism.utils import rabbitmq
+
+pytestmark = pytest.mark.integration
+
+
+@pytest.fixture(scope="module", autouse=True)
+def _require_ansible():
+    """Skip unless ``ansible`` is on PATH.
+
+    Set OSISM_REQUIRE_ANSIBLE to turn the skip into a failure, the way
+    OSISM_REQUIRE_REDIS does for Redis, so a job that is meant to have Ansible
+    cannot pass by skipping everything.
+    """
+    if shutil.which("ansible-inventory"):
+        return
+    if os.environ.get("OSISM_REQUIRE_ANSIBLE"):
+        pytest.fail("OSISM_REQUIRE_ANSIBLE is set but 'ansible' is not on PATH")
+    pytest.skip("ansible not on PATH")
+
+
+@pytest.fixture
+def scenario(tmp_path, monkeypatch):
+    """Build a one-host rabbitmq inventory, seed its facts, and point the code at it.
+
+    One host per scenario keeps the assertion unambiguous: the function skips
+    hosts it cannot resolve and returns the rest, so a multi-host inventory
+    would let a failure hide behind a success.
+    """
+    seeded = []
+
+    def _build(host, variables, facts):
+        inventory = tmp_path / "hosts.yml"
+        inventory.write_text(
+            "all:\n"
+            "  children:\n"
+            "    rabbitmq:\n"
+            "      hosts:\n"
+            # RFC 5737 documentation range: unreachable on purpose, so a
+            # resolution path that tried to connect would fail here rather
+            # than pass quietly.
+            f"        {host}: {{ansible_host: 192.0.2.10}}\n"
+        )
+        host_vars = tmp_path / "host_vars"
+        host_vars.mkdir(exist_ok=True)
+        (host_vars / f"{host}.yml").write_text(json.dumps(variables))
+
+        # The inventory path is hardcoded to /ansible/inventory/hosts.yml, which
+        # only exists inside the container images.
+        monkeypatch.setattr(
+            rabbitmq, "get_inventory_path", lambda *args, **kwargs: str(inventory)
+        )
+
+        key = f"ansible_facts{host}"
+        utils.redis.set(key, json.dumps(facts))
+        seeded.append(key)
+        return host
+
+    yield _build
+
+    for key in seeded:
+        utils.redis.delete(key)
+
+
+def test_literal_interface(scenario):
+    host = scenario(
+        "ctl1",
+        {"internal_interface": "eth0"},
+        {"ansible_eth0": {"ipv4": {"address": "10.0.0.5"}}},
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() == [("10.0.0.5", host)]
+
+
+def test_fact_derived_interface(scenario):
+    # internal_interface is a Jinja reference into the facts.
+    host = scenario(
+        "ctl2",
+        {"internal_interface": "{{ ansible_local.network_devices.management }}"},
+        {
+            "ansible_local": {"network_devices": {"management": "bond_mgmt"}},
+            "ansible_bond_mgmt": {"ipv4": {"address": "192.168.16.10"}},
+        },
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() == [("192.168.16.10", host)]
+
+
+def test_dotted_interface_name(scenario):
+    # Ansible replaces "-" with "_" in fact names and keeps dots, so a VLAN
+    # interface named bond0.1034 is cached as ansible_bond0.1034.
+    host = scenario(
+        "ctl3",
+        {"internal_interface": "bond0.1034"},
+        {"ansible_bond0.1034": {"ipv4": {"address": "10.74.34.12"}}},
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() == [("10.74.34.12", host)]
+
+
+def test_dashed_interface_name(scenario):
+    host = scenario(
+        "ctl4",
+        {"internal_interface": "br-ex"},
+        {"ansible_br_ex": {"ipv4": {"address": "10.74.34.13"}}},
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() == [("10.74.34.13", host)]
+
+
+@pytest.mark.xfail(
+    strict=True,
+    reason="internal_interface pointing at an inventory variable is not resolved; "
+    "the resolver only walks dotted paths through the facts (osism/issues#1425)",
+)
+def test_interface_from_inventory_variable(scenario):
+    # The shape reported by a client: internal_interface refers to an inventory
+    # variable, which is itself a literal plus a template. Nothing here is a
+    # fact, so a facts-only walk cannot resolve it.
+    host = scenario(
+        "ctl5",
+        {
+            "internal_interface": "{{ dataplane_vlan }}",
+            "dataplane_id": "1034",
+            "dataplane_vlan": "vlan{{ dataplane_id }}",
+        },
+        {"ansible_vlan1034": {"ipv4": {"address": "10.74.34.11"}}},
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() == [("10.74.34.11", host)]
+
+
+def test_missing_internal_interface_yields_no_addresses(scenario):
+    scenario("ctl6", {}, {"ansible_eth0": {"ipv4": {"address": "10.0.0.9"}}})
+    assert rabbitmq.get_rabbitmq_node_addresses() is None
+
+
+def test_interface_without_matching_fact_yields_no_addresses(scenario):
+    scenario(
+        "ctl7",
+        {"internal_interface": "vlan999"},
+        {"ansible_eth0": {"ipv4": {"address": "10.0.0.9"}}},
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() is None
+
+
+def test_interface_fact_without_ipv4_yields_no_addresses(scenario):
+    # The interface is in the facts but carries no ipv4 block, e.g. an
+    # unconfigured NIC. The address cannot be derived, so the host is skipped.
+    scenario(
+        "ctl10",
+        {"internal_interface": "eth0"},
+        {"ansible_eth0": {"macaddress": "00:00:5e:00:53:00"}},
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() is None
+
+
+def test_ipv4_without_address_yields_no_addresses(scenario):
+    scenario(
+        "ctl11",
+        {"internal_interface": "eth0"},
+        {"ansible_eth0": {"ipv4": {"netmask": "255.255.255.0"}}},
+    )
+    assert rabbitmq.get_rabbitmq_node_addresses() is None
+
+
+def test_no_facts_in_cache_yields_no_addresses(scenario, tmp_path, monkeypatch):
+    # Same inventory, but the cache entry removed: the function must report the
+    # missing facts rather than fall back to some other source.
+    host = scenario(
+        "ctl8",
+        {"internal_interface": "eth0"},
+        {"ansible_eth0": {"ipv4": {"address": "10.0.0.5"}}},
+    )
+    utils.redis.delete(f"ansible_facts{host}")
+    assert rabbitmq.get_rabbitmq_node_addresses() is None

tests/unit/utils/test_rabbitmq.py

@@ -283,43 +283,6 @@ def test_per_host_failure_keeps_addresses_of_other_hosts(
         assert rabbitmq.get_rabbitmq_node_addresses() == [("10.0.0.5", "host1")]
         _assert_error_logged(loguru_logs, "Failed to resolve address for host2")
 
-    def test_no_internal_interface_skips_host(self, setup_addresses, loguru_logs):
-        setup_addresses(
-            hosts=["host1"],
-            redis_side_effect=[_facts("ansible_eth0", "10.0.0.5")],
-            check_output=[_GROUP_LISTING, _encode({})],
-        )
-
-        assert rabbitmq.get_rabbitmq_node_addresses() is None
-        _assert_error_logged(loguru_logs, "internal_interface not found in hostvars")
-
-    def test_literal_interface_used_directly(self, setup_addresses, loguru_logs):
-        setup_addresses(
-            hosts=["host1"],
-            redis_side_effect=[_facts("ansible_eth0", "10.0.0.5")],
-            check_output=[_GROUP_LISTING, _hostvars("eth0")],
-        )
-
-        assert rabbitmq.get_rabbitmq_node_addresses() == [("10.0.0.5", "host1")]
-        assert _error_messages(loguru_logs) == []
-
-    def test_jinja_template_resolved_from_facts(self, setup_addresses, loguru_logs):
-        facts = {
-            "ansible_local": {"testbed_network_devices": {"management": "eth1"}},
-            "ansible_eth1": {"ipv4": {"address": "10.0.0.8"}},
-        }
-        setup_addresses(
-            hosts=["host1"],
-            redis_side_effect=[_encode(facts)],
-            check_output=[
-                _GROUP_LISTING,
-                _hostvars("{{ ansible_local.testbed_network_devices.management }}"),
-            ],
-        )
-
-        assert rabbitmq.get_rabbitmq_node_addresses() == [("10.0.0.8", "host1")]
-        assert _error_messages(loguru_logs) == []
-
     @pytest.mark.parametrize(
         "management_value",
         [None, {"nested": "x"}, 42],
@@ -363,64 +326,6 @@ def test_template_traversal_hits_non_dict_skips_host(
         assert rabbitmq.get_rabbitmq_node_addresses() is None
         _assert_error_logged(loguru_logs, "Could not resolve template")
 
-    @pytest.mark.parametrize(
-        "interface,fact_key",
-        [("eth0.100", "ansible_eth0.100"), ("eth-0", "ansible_eth_0")],
-    )
-    def test_interface_name_mapped_to_fact_key(
-        self, setup_addresses, loguru_logs, interface, fact_key
-    ):
-        # Facts are only stored under the key Ansible actually uses, so a
-        # correct mapping is the only way the address can be found. Ansible
-        # replaces "-" with "_" and keeps dots.
-        setup_addresses(
-            hosts=["host1"],
-            redis_side_effect=[_facts(fact_key, "10.0.0.7")],
-            check_output=[_GROUP_LISTING, _hostvars(interface)],
-        )
-
-        assert rabbitmq.get_rabbitmq_node_addresses() == [("10.0.0.7", "host1")]
-        assert _error_messages(loguru_logs) == []
-
-    def test_normalized_interface_key_missing_skips_host(
-        self, setup_addresses, loguru_logs
-    ):
-        setup_addresses(
-            hosts=["host1"],
-            redis_side_effect=[_facts("ansible_eth1", "10.0.0.5")],
-            check_output=[_GROUP_LISTING, _hostvars("eth0")],
-        )
-
-        assert rabbitmq.get_rabbitmq_node_addresses() is None
-        _assert_error_logged(loguru_logs, "not found in ansible facts")
-
-    def test_interface_without_ipv4_skips_host(self, setup_addresses, loguru_logs):
-        setup_addresses(
-            hosts=["host1"],
-            redis_side_effect=[_encode({"ansible_eth0": {"mtu": 1500}})],
-            check_output=[_GROUP_LISTING, _hostvars("eth0")],
-        )
-
-        assert rabbitmq.get_rabbitmq_node_addresses() is None
-        _assert_error_logged(loguru_logs, "No IPv4 address found")
-
-    @pytest.mark.parametrize(
-        "ipv4_info",
-        [{"address": ""}, {"netmask": "255.255.255.0"}],
-        ids=["empty_address", "missing_address"],
-    )
-    def test_ipv4_without_address_skips_host(
-        self, setup_addresses, loguru_logs, ipv4_info
-    ):
-        setup_addresses(
-            hosts=["host1"],
-            redis_side_effect=[_encode({"ansible_eth0": {"ipv4": ipv4_info}})],
-            check_output=[_GROUP_LISTING, _hostvars("eth0")],
-        )
-
-        assert rabbitmq.get_rabbitmq_node_addresses() is None
-        _assert_error_logged(loguru_logs, "No IPv4 address found")
-
     # -- aggregate results ----------------------------------------------------
 
     def test_all_hosts_skipped_returns_none(self, setup_addresses, loguru_logs):