Add unit tests for osism/utils/rabbitmq.py

[berendt]

Closes #2232

Adds tests/unit/utils/test_rabbitmq.py covering both functions in
osism/utils/rabbitmq.py, plus one small robustness fix in
get_rabbitmq_node_addresses that the tests pin down.

Production change

Skip RabbitMQ hosts that fail address resolution. Previously a
failure while resolving a single host (corrupt facts in the Redis
cache, a failing ansible-inventory --host call, non-JSON hostvars or
an internal_interface value that is not a string) raised out of the
per-host loop and made the whole function return None, discarding the
addresses already collected for other hosts — while the expected data
problems right next to these spots (missing facts, missing interface,
missing IPv4 address) only skipped the affected host. Per-host failures
are now caught, logged and skipped like the other skip branches; the
function still returns None if no host resolves at all.

What is covered

Tests are grouped into one class per function
(TestGetRabbitmqNodeAddresses, TestLoadRabbitmqPassword), with
factory fixtures (setup_addresses, setup_password) wiring up the
collaborators.

get_rabbitmq_node_addresses()

• Inventory & host discovery: two hosts processed in alphabetical
  order (happy path returning [("10.0.0.5","host1"),("10.0.0.6","host2")]);
  empty rabbitmq group → None; first subprocess.check_output raising
  CalledProcessError; invalid JSON from the group listing
  (JSONDecodeError); outer generic exception. The inventory
  invocations are pinned: --limit rabbitmq is the only thing scoping
  the listing to the group, and the hostvars lookup must use the
  non-minified inventory (prefer_minified=False) because the minified
  variant omits hostvars.
• Per-host resolution: cache miss skips the host and continues to the
  next; partial-success tests (host1 resolves, host2 fails with corrupt
  cached facts / failing hostvars query / non-JSON hostvars / non-string
  internal_interface) pin that only the failing host is dropped;
  missing internal_interface; literal interface name used directly;
  Jinja2 template resolved via dotted-path traversal of the facts;
  template resolving to a non-string (None / dict / int); traversal
  hitting a non-dict mid-walk; interface-name normalization
  (eth0.100 → ansible_eth0_100, eth-0 → ansible_eth_0); missing
  normalized interface key; interface without ipv4; ipv4 without an
  address.
• Aggregate: all hosts skipped → None; at least one address → the
  list is returned with no error logged.

load_rabbitmq_password() — missing file; empty / invalid / non-dict
secrets; missing rabbitmq_password key; whitespace stripping; int
coercion; loader raising.

Plus a check that the RABBITMQ_USER module constant is "openstack".

Notes for the reviewer

• Redis mocking deviates from the issue's hint. The issue suggested
  mocker.patch("osism.utils.rabbitmq.utils.redis", ...), but that target
  does not resolve: redis is a lazy __getattr__ attribute on the
  osism.utils package, and the function reads it via a function-local
  from osism import utils, so the rabbitmq module never gains a utils
  global (AttributeError: module 'osism.utils.rabbitmq' has no attribute 'utils'). Instead the cached attribute is seeded on the package
  namespace with mocker.patch.dict(osism.utils.__dict__, {"redis": ...}),
  which also bypasses the lazy initializer that would otherwise open a real
  Redis connection. Reading the cache key the function builds is asserted in
  the happy-path test.
• load_yaml_file and os.path.exists are patched at the call site, as the
  issue suggests.
• Tests follow the existing mocker + loguru_logs conventions
  (see tests/unit/utils/test_init_connections.py).

Verification

• black --check and flake8 clean on both files; unit tests run in CI.

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've left some high level feedback:

• The test module is quite long and dense; consider splitting it into smaller logical sections (e.g., separate files or pytest classes for get_rabbitmq_node_addresses and load_rabbitmq_password) or introducing shared fixtures to reduce repetition and improve readability.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The test module is quite long and dense; consider splitting it into smaller logical sections (e.g., separate files or pytest classes for `get_rabbitmq_node_addresses` and `load_rabbitmq_password`) or introducing shared fixtures to reduce repetition and improve readability.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[ideaship]

Per-host failures abort the whole discovery and discard partial results — is that intended?

Inside the per-host loop, json.loads(facts_data) (corrupt cache), the --host subprocess.check_output, json.loads(result) (non-JSON), and re.match(...) on a truthy non-string internal_interface are all uncaught locally. Any one of them propagates to the function-level handlers and returns None, throwing away every address already collected for earlier hosts. That's a different blast radius than the continue skip-branches right next to them, which only drop the offending host. The existing CalledProcessError/JSONDecodeError tests only hit the first (group-listing) call, so this asymmetry isn't exercised — which also means the commit message's "every error and skip branch" / 100% coverage framing is a bit optimistic here. Mostly I'd want to know: is abort-all-after-partial-success the behavior you want, or should these continue like the skips? Whatever the answer, a partial-success test (host1 resolves, host2 fails) would pin it.

Optional: two production arguments carry correctness weight but aren't asserted, because their collaborators are mocked — so a future regression flipping either would pass silently:

• the hostvars lookup deliberately uses prefer_minified=False (the minified inventory omits hostvars, so internal_interface wouldn't resolve)
• --limit rabbitmq is the only thing scoping results to the group (get_hosts_from_inventory does no group filtering)

Each is a one-line call_args assertion if you think they're worth pinning. Not blocking.