From 757eaaf245b0f769731edf6d3e85a37987a8d959 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Fri, 17 Jul 2026 15:20:02 +0200 Subject: [PATCH 1/5] Add unit tests for the RabbitMQ 3-to-4 migration command Cover the Rabbitmq3to4 command: kolla configuration validation, vhost preparation, queue and exchange classification, connection draining, deletion helpers, and the check/list/delete subcommand wiring including dry-run and error paths. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- tests/unit/commands/test_migrate.py | 1010 +++++++++++++++++++++++++++ 1 file changed, 1010 insertions(+) create mode 100644 tests/unit/commands/test_migrate.py diff --git a/tests/unit/commands/test_migrate.py b/tests/unit/commands/test_migrate.py new file mode 100644 index 00000000..4fe074eb --- /dev/null +++ b/tests/unit/commands/test_migrate.py @@ -0,0 +1,1010 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for the ``osism migrate rabbitmq3to4`` command. + +The command talks to the RabbitMQ management API, so all HTTP traffic is +mocked at the module boundary (``osism.commands.migrate.requests``) while the +real ``requests.exceptions`` classes are kept in place so the ``except`` +clauses in the command still match. The inventory and password lookups that +``take_action`` imports lazily are patched at their source module, +``osism.utils.rabbitmq``. +""" + +from unittest.mock import MagicMock, call, mock_open, patch + +import pytest +import requests + +from osism.commands import migrate + +from ._helpers import make_command, parse_args + + +NODES = [("10.0.0.1", "node1"), ("10.0.0.2", "node2")] +BASE_URL = "http://10.0.0.1:15672/api" +AUTH = ("openstack", "secret") + +VALID_KOLLA_CONFIG = ( + "om_enable_rabbitmq_quorum_queues: true\n" + 'om_rpc_vhost: "openstack"\n' + 'om_notify_vhost: "openstack"\n' +) + + +def _response(status_code=200, json_data=None): + response = MagicMock() + response.status_code = status_code + response.json.return_value = json_data + return response + + +def _http_error(status_code): + return requests.exceptions.HTTPError(response=MagicMock(status_code=status_code)) + + +def _error_response(status_code): + response = _response(status_code) + response.raise_for_status.side_effect = _http_error(status_code) + return response + + +def _messages(records, level): + return [r["message"] for r in records if r["level"] == level] + + +@pytest.fixture +def mock_requests(): + """Patch the requests module used by migrate, keeping real exceptions.""" + with patch("osism.commands.migrate.requests") as mocked: + mocked.exceptions = requests.exceptions + yield mocked + + +def _take_action(cmd, parsed_args, node_addresses=NODES, password="secret"): + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=node_addresses, + ), patch("osism.utils.rabbitmq.load_rabbitmq_password", return_value=password): + return cmd.take_action(parsed_args) + + +# --- _check_kolla_configuration --- + + +@pytest.mark.parametrize( + ("content", "expected"), + [ + pytest.param(VALID_KOLLA_CONFIG, True, id="valid-double-quoted"), + pytest.param( + "om_rpc_vhost: 'openstack'\nom_notify_vhost: 'openstack'\n", + True, + id="valid-single-quoted-flag-absent", + ), + pytest.param( + "om_rpc_vhost: openstack\nom_notify_vhost: openstack\n", + True, + id="valid-unquoted", + ), + pytest.param( + " om_rpc_vhost: openstack\n om_notify_vhost: openstack\n", + True, + id="indented-lines-are-stripped", + ), + pytest.param( + "# om_enable_rabbitmq_quorum_queues: false\n" + "om_rpc_vhost: openstack\n" + "om_notify_vhost: openstack\n", + True, + id="commented-quorum-false-is-ignored", + ), + pytest.param( + "om_enable_rabbitmq_quorum_queues: false\n" + 'om_rpc_vhost: "openstack"\n' + 'om_notify_vhost: "openstack"\n', + False, + id="quorum-queues-false", + ), + pytest.param( + 'om_enable_rabbitmq_quorum_queues: "no"\n' + 'om_rpc_vhost: "openstack"\n' + 'om_notify_vhost: "openstack"\n', + False, + id="quorum-queues-no", + ), + pytest.param( + 'om_notify_vhost: "openstack"\n', + False, + id="rpc-vhost-missing", + ), + pytest.param( + 'om_rpc_vhost: "openstack"\n', + False, + id="notify-vhost-missing", + ), + ], +) +def test_check_kolla_configuration(content, expected): + cmd = make_command(migrate.Rabbitmq3to4) + with patch("builtins.open", mock_open(read_data=content)): + assert cmd._check_kolla_configuration() is expected + + +def test_check_kolla_configuration_missing_file(loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + with patch("builtins.open", side_effect=FileNotFoundError): + assert cmd._check_kolla_configuration() is False + assert any( + "Configuration file not found" in m for m in _messages(loguru_logs, "ERROR") + ) + + +def test_check_kolla_configuration_read_error(loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + with patch("builtins.open", side_effect=OSError("disk error")): + assert cmd._check_kolla_configuration() is False + assert any( + "Failed to read configuration file" in m + for m in _messages(loguru_logs, "ERROR") + ) + + +def test_check_kolla_configuration_logs_success(loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + with patch("builtins.open", mock_open(read_data=VALID_KOLLA_CONFIG)): + assert cmd._check_kolla_configuration() is True + assert any( + "Kolla configuration check passed" in m for m in _messages(loguru_logs, "INFO") + ) + + +# --- _prepare_vhost --- + + +def test_prepare_vhost_aborts_when_kolla_check_fails(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._check_kolla_configuration = MagicMock(return_value=False) + + assert cmd._prepare_vhost(BASE_URL, AUTH) is False + mock_requests.put.assert_not_called() + + +def test_prepare_vhost_dry_run_makes_no_requests(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._check_kolla_configuration = MagicMock(return_value=True) + + assert cmd._prepare_vhost(BASE_URL, AUTH, dry_run=True) is True + mock_requests.put.assert_not_called() + infos = _messages(loguru_logs, "INFO") + assert any("[DRY-RUN] Would create vhost 'openstack'" in m for m in infos) + assert any("[DRY-RUN] Would set permissions" in m for m in infos) + + +def test_prepare_vhost_creates_vhost_and_permissions(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._check_kolla_configuration = MagicMock(return_value=True) + + assert cmd._prepare_vhost(BASE_URL, AUTH) is True + assert mock_requests.put.call_args_list == [ + call( + f"{BASE_URL}/vhosts/openstack", + auth=AUTH, + json={"default_queue_type": "quorum"}, + timeout=30, + ), + call( + f"{BASE_URL}/permissions/openstack/openstack", + auth=AUTH, + json={"configure": ".*", "write": ".*", "read": ".*"}, + timeout=30, + ), + ] + + +def test_prepare_vhost_conflict_is_treated_as_success(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._check_kolla_configuration = MagicMock(return_value=True) + mock_requests.put.return_value = _error_response(409) + + assert cmd._prepare_vhost(BASE_URL, AUTH) is True + assert any( + "Vhost 'openstack' already exists" in m + for m in _messages(loguru_logs, "WARNING") + ) + + +def test_prepare_vhost_http_error_fails(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._check_kolla_configuration = MagicMock(return_value=True) + mock_requests.put.return_value = _error_response(500) + + assert cmd._prepare_vhost(BASE_URL, AUTH) is False + assert any("Failed to prepare vhost" in m for m in _messages(loguru_logs, "ERROR")) + + +def test_prepare_vhost_connection_error_fails(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._check_kolla_configuration = MagicMock(return_value=True) + mock_requests.put.side_effect = requests.exceptions.ConnectionError("boom") + + assert cmd._prepare_vhost(BASE_URL, AUTH) is False + assert any("Failed to prepare vhost" in m for m in _messages(loguru_logs, "ERROR")) + + +# --- queue classification --- + + +def test_get_classic_queues_defaults_missing_type_to_classic(): + cmd = make_command(migrate.Rabbitmq3to4) + queues = [ + {"name": "a"}, + {"name": "b", "type": "classic"}, + {"name": "c", "type": "quorum"}, + ] + + assert cmd._get_classic_queues(queues) == [ + {"name": "a"}, + {"name": "b", "type": "classic"}, + ] + + +def test_get_quorum_queues_returns_only_quorum(): + cmd = make_command(migrate.Rabbitmq3to4) + queues = [ + {"name": "a"}, + {"name": "b", "type": "classic"}, + {"name": "c", "type": "quorum"}, + ] + + assert cmd._get_quorum_queues(queues) == [{"name": "c", "type": "quorum"}] + + +@pytest.mark.parametrize( + ("service", "queue_name", "matches"), + [ + pytest.param("nova", "compute", True, id="nova-exact"), + pytest.param("nova", "compute.host1", True, id="nova-host-suffix"), + pytest.param("nova", "compute_fanout_x", True, id="nova-fanout"), + pytest.param("nova", "computex", False, id="nova-no-partial-match"), + pytest.param("designate", "reply_abc123", True, id="designate-reply-hex"), + pytest.param("designate", "reply_XYZ", False, id="designate-reply-non-hex"), + ], +) +def test_match_queues_for_service_patterns(service, queue_name, matches): + cmd = make_command(migrate.Rabbitmq3to4) + queues = [{"name": queue_name}] + + matched = cmd._match_queues_for_service(queues, service) + assert matched == (queues if matches else []) + + +def test_match_queues_for_service_unknown_service_returns_empty(): + cmd = make_command(migrate.Rabbitmq3to4) + queues = [{"name": "compute"}] + + assert cmd._match_queues_for_service(queues, "unknown-service") == [] + + +def test_match_queues_for_service_deduplicates_multi_pattern_matches(): + cmd = make_command(migrate.Rabbitmq3to4) + queues = [{"name": "dup.queue"}] + + with patch.dict( + migrate.SERVICE_QUEUE_PATTERNS, + {"testsvc": [r"^dup.*$", r"^dup\..*$"]}, + ): + matched = cmd._match_queues_for_service(queues, "testsvc") + + assert matched == queues + + +# --- _get_all_queues --- + + +def test_get_all_queues_returns_parsed_json(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + queues = [{"name": "compute", "vhost": "/"}] + mock_requests.get.return_value = _response(200, json_data=queues) + + assert cmd._get_all_queues(BASE_URL, AUTH) == queues + mock_requests.get.assert_called_once_with( + f"{BASE_URL}/queues", auth=AUTH, timeout=30 + ) + + +def test_get_all_queues_returns_none_on_error(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.side_effect = requests.exceptions.ConnectionError("boom") + + assert cmd._get_all_queues(BASE_URL, AUTH) is None + assert any("Failed to get queues" in m for m in _messages(loguru_logs, "ERROR")) + + +# --- _get_all_exchanges --- + + +def test_get_all_exchanges_without_vhost(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + exchanges = [{"name": "nova"}] + mock_requests.get.return_value = _response(200, json_data=exchanges) + + assert cmd._get_all_exchanges(BASE_URL, AUTH) == exchanges + mock_requests.get.assert_called_once_with( + f"{BASE_URL}/exchanges", auth=AUTH, timeout=30 + ) + + +def test_get_all_exchanges_encodes_vhost(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response(200, json_data=[]) + + assert cmd._get_all_exchanges(BASE_URL, AUTH, vhost="/") == [] + mock_requests.get.assert_called_once_with( + f"{BASE_URL}/exchanges/%2F", auth=AUTH, timeout=30 + ) + + +def test_get_all_exchanges_filters_default_exchanges(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + exchanges = [ + {"name": ""}, + {"name": "amq.topic"}, + {}, + {"name": "nova"}, + ] + mock_requests.get.return_value = _response(200, json_data=exchanges) + + assert cmd._get_all_exchanges(BASE_URL, AUTH) == [{"name": "nova"}] + + +def test_get_all_exchanges_returns_none_on_error(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.side_effect = requests.exceptions.ConnectionError("boom") + + assert cmd._get_all_exchanges(BASE_URL, AUTH) is None + assert any("Failed to get exchanges" in m for m in _messages(loguru_logs, "ERROR")) + + +# --- _close_queue_connections --- + + +def _consumer(connection_name): + return {"channel_details": {"connection_name": connection_name}} + + +def test_close_queue_connections_queue_not_found(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response(404) + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 0 + mock_requests.delete.assert_not_called() + + +def test_close_queue_connections_without_consumers(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response(200, json_data={}) + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 0 + mock_requests.get.assert_called_once_with( + f"{BASE_URL}/queues/%2F/compute", auth=AUTH, timeout=30 + ) + mock_requests.delete.assert_not_called() + + +def test_close_queue_connections_without_connection_names(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response( + 200, json_data={"consumer_details": [{"channel_details": {}}]} + ) + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 0 + mock_requests.delete.assert_not_called() + + +def test_close_queue_connections_dry_run_counts_unique_connections( + mock_requests, loguru_logs +): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response( + 200, + json_data={ + "consumer_details": [ + _consumer("conn1"), + _consumer("conn1"), + _consumer("conn2"), + ] + }, + ) + + closed = cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute", dry_run=True) + + assert closed == 2 + mock_requests.delete.assert_not_called() + dry_run_logs = [ + m + for m in _messages(loguru_logs, "INFO") + if "[DRY-RUN] Would close connection" in m + ] + assert len(dry_run_logs) == 2 + + +def test_close_queue_connections_counts_successful_deletes(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response( + 200, + json_data={"consumer_details": [_consumer("conn1"), _consumer("conn2")]}, + ) + mock_requests.delete.side_effect = [_response(200), _response(204)] + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 2 + assert mock_requests.delete.call_count == 2 + + +def test_close_queue_connections_encodes_connection_name(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response( + 200, + json_data={"consumer_details": [_consumer("1.2.3.4:5672 -> 5.6.7.8:5673")]}, + ) + mock_requests.delete.return_value = _response(200) + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 1 + mock_requests.delete.assert_called_once_with( + f"{BASE_URL}/connections/1.2.3.4%3A5672%20-%3E%205.6.7.8%3A5673", + auth=AUTH, + timeout=30, + ) + + +def test_close_queue_connections_skips_already_closed_connection(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response( + 200, json_data={"consumer_details": [_consumer("conn1")]} + ) + mock_requests.delete.return_value = _response(404) + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 0 + + +def test_close_queue_connections_continues_after_failed_delete( + mock_requests, loguru_logs +): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.return_value = _response( + 200, + json_data={"consumer_details": [_consumer("conn1"), _consumer("conn2")]}, + ) + mock_requests.delete.side_effect = [ + requests.exceptions.ConnectionError("boom"), + _response(200), + ] + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 1 + assert mock_requests.delete.call_count == 2 + assert any( + "Failed to close connection" in m for m in _messages(loguru_logs, "WARNING") + ) + + +def test_close_queue_connections_queue_lookup_failure_returns_zero( + mock_requests, loguru_logs +): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.get.side_effect = requests.exceptions.ConnectionError("boom") + + assert cmd._close_queue_connections(BASE_URL, AUTH, "/", "compute") == 0 + assert any( + "Failed to get queue consumers for 'compute'" in m + for m in _messages(loguru_logs, "WARNING") + ) + + +# --- _delete_queue --- + + +def test_delete_queue_closes_connections_first(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._close_queue_connections = MagicMock(return_value=2) + mock_requests.delete.return_value = _response(200) + + result = cmd._delete_queue(BASE_URL, AUTH, "/", "compute", close_connections=True) + + assert result is True + cmd._close_queue_connections.assert_called_once_with( + BASE_URL, AUTH, "/", "compute", False + ) + assert any( + "Closed 2 connection(s) for queue: compute" in m + for m in _messages(loguru_logs, "INFO") + ) + + +def test_delete_queue_does_not_close_connections_by_default(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + cmd._close_queue_connections = MagicMock() + mock_requests.delete.return_value = _response(200) + + assert cmd._delete_queue(BASE_URL, AUTH, "/", "compute") is True + cmd._close_queue_connections.assert_not_called() + + +def test_delete_queue_dry_run_makes_no_requests(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + + assert cmd._delete_queue(BASE_URL, AUTH, "/", "compute", dry_run=True) is True + mock_requests.delete.assert_not_called() + assert any( + "[DRY-RUN] Would delete queue: compute" in m + for m in _messages(loguru_logs, "INFO") + ) + + +def test_delete_queue_encodes_vhost_and_queue_name(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.return_value = _response(200) + + assert cmd._delete_queue(BASE_URL, AUTH, "/openstack", "a/b") is True + mock_requests.delete.assert_called_once_with( + f"{BASE_URL}/queues/%2Fopenstack/a%2Fb", auth=AUTH, timeout=30 + ) + + +def test_delete_queue_not_found_is_treated_as_success(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.return_value = _error_response(404) + + assert cmd._delete_queue(BASE_URL, AUTH, "/", "compute") is True + assert any( + "Queue 'compute' not found" in m for m in _messages(loguru_logs, "WARNING") + ) + + +def test_delete_queue_http_error_fails(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.return_value = _error_response(500) + + assert cmd._delete_queue(BASE_URL, AUTH, "/", "compute") is False + assert any( + "Failed to delete queue 'compute'" in m for m in _messages(loguru_logs, "ERROR") + ) + + +def test_delete_queue_connection_error_fails(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.side_effect = requests.exceptions.ConnectionError("boom") + + assert cmd._delete_queue(BASE_URL, AUTH, "/", "compute") is False + assert any( + "Failed to delete queue 'compute'" in m for m in _messages(loguru_logs, "ERROR") + ) + + +# --- _delete_exchange --- + + +def test_delete_exchange_dry_run_makes_no_requests(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + + assert cmd._delete_exchange(BASE_URL, AUTH, "/", "nova", dry_run=True) is True + mock_requests.delete.assert_not_called() + assert any( + "[DRY-RUN] Would delete exchange: nova" in m + for m in _messages(loguru_logs, "INFO") + ) + + +def test_delete_exchange_encodes_vhost_and_exchange_name(mock_requests): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.return_value = _response(200) + + assert cmd._delete_exchange(BASE_URL, AUTH, "/", "nova") is True + mock_requests.delete.assert_called_once_with( + f"{BASE_URL}/exchanges/%2F/nova", auth=AUTH, timeout=30 + ) + + +def test_delete_exchange_not_found_is_treated_as_success(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.return_value = _error_response(404) + + assert cmd._delete_exchange(BASE_URL, AUTH, "/", "nova") is True + assert any( + "Exchange 'nova' not found" in m for m in _messages(loguru_logs, "WARNING") + ) + + +def test_delete_exchange_http_error_fails(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.return_value = _error_response(500) + + assert cmd._delete_exchange(BASE_URL, AUTH, "/", "nova") is False + assert any( + "Failed to delete exchange 'nova'" in m for m in _messages(loguru_logs, "ERROR") + ) + + +def test_delete_exchange_connection_error_fails(mock_requests, loguru_logs): + cmd = make_command(migrate.Rabbitmq3to4) + mock_requests.delete.side_effect = requests.exceptions.ConnectionError("boom") + + assert cmd._delete_exchange(BASE_URL, AUTH, "/", "nova") is False + assert any( + "Failed to delete exchange 'nova'" in m for m in _messages(loguru_logs, "ERROR") + ) + + +# --- take_action: argument validation and node selection --- + + +def test_take_action_requires_command(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, []) + + with patch("osism.utils.rabbitmq.get_rabbitmq_node_addresses") as mock_nodes: + rc = cmd.take_action(parsed_args) + + assert rc == 1 + mock_nodes.assert_not_called() + assert any("must be specified" in m for m in _messages(loguru_logs, "ERROR")) + + +def test_take_action_fails_without_node_addresses(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["check"]) + + assert _take_action(cmd, parsed_args, node_addresses=None) == 1 + assert any( + "Failed to get RabbitMQ node addresses" in m + for m in _messages(loguru_logs, "ERROR") + ) + + +def test_take_action_fails_without_password(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["check"]) + + assert _take_action(cmd, parsed_args, password=None) == 1 + assert any( + "Failed to load RabbitMQ password" in m for m in _messages(loguru_logs, "ERROR") + ) + + +def test_take_action_rejects_unknown_server(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["check", "--server", "node3"]) + + assert _take_action(cmd, parsed_args) == 1 + errors = _messages(loguru_logs, "ERROR") + assert any("Server 'node3' not found" in m for m in errors) + assert any("Available: node1, node2" in m for m in errors) + + +def test_take_action_selects_requested_server(): + cmd, parsed_args = parse_args( + migrate.Rabbitmq3to4, ["prepare", "--server", "node2", "--dry-run"] + ) + cmd._prepare_vhost = MagicMock(return_value=True) + + assert _take_action(cmd, parsed_args) == 0 + cmd._prepare_vhost.assert_called_once_with("http://10.0.0.2:15672/api", AUTH, True) + + +@pytest.mark.parametrize( + ("prepare_result", "expected_rc"), + [ + pytest.param(True, 0, id="prepare-success"), + pytest.param(False, 1, id="prepare-failure"), + ], +) +def test_take_action_prepare_uses_first_node_by_default(prepare_result, expected_rc): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["prepare"]) + cmd._prepare_vhost = MagicMock(return_value=prepare_result) + + assert _take_action(cmd, parsed_args) == expected_rc + cmd._prepare_vhost.assert_called_once_with(BASE_URL, AUTH, False) + + +# --- take_action: check command --- + + +CLASSIC_ROOT = {"name": "compute", "vhost": "/", "type": "classic"} +CLASSIC_OTHER = {"name": "compute", "vhost": "/other", "type": "classic"} +QUORUM_ROOT = {"name": "conductor", "vhost": "/", "type": "quorum"} +QUORUM_OPENSTACK = {"name": "conductor", "vhost": "/openstack", "type": "quorum"} + + +@pytest.mark.parametrize( + ("queues", "expected_message"), + [ + pytest.param( + [CLASSIC_ROOT], + "Migration is REQUIRED", + id="only-classic", + ), + pytest.param( + [QUORUM_OPENSTACK], + "Migration is NOT required: Only quorum queues found", + id="only-quorum", + ), + pytest.param( + [CLASSIC_ROOT, QUORUM_OPENSTACK], + "Migration is IN PROGRESS: Classic queues in / and quorum queues", + id="classic-in-root-quorum-in-openstack", + ), + pytest.param( + [CLASSIC_ROOT, QUORUM_ROOT], + "Migration is IN PROGRESS: Classic queues in / and quorum queues", + id="legacy-quorum-in-root", + ), + pytest.param( + [CLASSIC_OTHER, QUORUM_OPENSTACK], + "Migration is IN PROGRESS: Both classic and quorum queues found", + id="mixed-outside-root", + ), + pytest.param( + [], + "Migration is NOT required: No queues found", + id="no-queues", + ), + ], +) +def test_take_action_check_verdicts(queues, expected_message, loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["check"]) + cmd._get_all_queues = MagicMock(return_value=queues) + + assert _take_action(cmd, parsed_args) == 0 + assert any(expected_message in m for m in _messages(loguru_logs, "INFO")) + + +def test_take_action_fails_when_queues_unavailable(): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["list"]) + cmd._get_all_queues = MagicMock(return_value=None) + + assert _take_action(cmd, parsed_args) == 1 + + +def test_take_action_check_fails_when_second_queue_fetch_fails(): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["check"]) + cmd._get_all_queues = MagicMock(side_effect=[[], None]) + + assert _take_action(cmd, parsed_args) == 1 + + +# --- take_action: list command --- + + +LIST_QUEUES = [ + {"name": "compute", "vhost": "/", "type": "classic", "messages": 3}, + {"name": "q-plugin", "vhost": "/", "type": "classic"}, + {"name": "conductor", "vhost": "/", "type": "quorum", "messages": 1}, + {"name": "central", "vhost": "/openstack", "type": "classic"}, +] + + +def test_take_action_list_shows_classic_queues_in_default_vhost(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["list"]) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + + assert _take_action(cmd, parsed_args) == 0 + infos = _messages(loguru_logs, "INFO") + assert any("Found 2 classic queue(s) in vhost '/':" in m for m in infos) + assert any("- compute (vhost: /, messages: 3)" in m for m in infos) + assert any("- q-plugin (vhost: /, messages: 0)" in m for m in infos) + assert not any("conductor" in m for m in infos) + assert not any("central" in m for m in infos) + + +def test_take_action_list_quorum_queues(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["list", "--quorum"]) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + + assert _take_action(cmd, parsed_args) == 0 + infos = _messages(loguru_logs, "INFO") + assert any("Found 1 quorum queue(s) in vhost '/':" in m for m in infos) + assert any("- conductor (vhost: /, messages: 1)" in m for m in infos) + + +def test_take_action_list_filters_by_service(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["list", "nova"]) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + + assert _take_action(cmd, parsed_args) == 0 + infos = _messages(loguru_logs, "INFO") + assert any( + "Found 1 classic queue(s) for service 'nova' in vhost '/':" in m for m in infos + ) + assert not any("q-plugin" in m for m in infos) + + +def test_take_action_list_filters_by_vhost(loguru_logs): + cmd, parsed_args = parse_args( + migrate.Rabbitmq3to4, ["list", "--vhost", "/openstack"] + ) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + + assert _take_action(cmd, parsed_args) == 0 + infos = _messages(loguru_logs, "INFO") + assert any("Found 1 classic queue(s) in vhost '/openstack':" in m for m in infos) + assert any("- central (vhost: /openstack, messages: 0)" in m for m in infos) + assert not any("compute" in m for m in infos) + + +def test_take_action_list_reports_no_queues(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["list", "designate"]) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + + assert _take_action(cmd, parsed_args) == 0 + assert any( + "No classic queues found for service 'designate' in vhost '/'" in m + for m in _messages(loguru_logs, "INFO") + ) + + +# --- take_action: delete command --- + + +def test_take_action_delete_deletes_all_classic_queues(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["delete"]) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + cmd._delete_queue = MagicMock(return_value=True) + + assert _take_action(cmd, parsed_args) == 0 + cmd._delete_queue.assert_has_calls( + [ + call(BASE_URL, AUTH, "/", "compute", False, True), + call(BASE_URL, AUTH, "/", "q-plugin", False, True), + ] + ) + assert cmd._delete_queue.call_count == 2 + assert any( + "Successfully deleted 2 queue(s) in vhost '/'" in m + for m in _messages(loguru_logs, "INFO") + ) + + +def test_take_action_delete_reports_failures(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["delete"]) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + cmd._delete_queue = MagicMock(side_effect=[True, False]) + + assert _take_action(cmd, parsed_args) == 1 + assert any( + "Failed to delete 1 queue(s)" in m for m in _messages(loguru_logs, "ERROR") + ) + + +def test_take_action_delete_dry_run_with_service_filter(loguru_logs): + cmd, parsed_args = parse_args( + migrate.Rabbitmq3to4, + ["delete", "nova", "--dry-run", "--no-close-connections"], + ) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + cmd._delete_queue = MagicMock(return_value=True) + + assert _take_action(cmd, parsed_args) == 0 + cmd._delete_queue.assert_called_once_with( + BASE_URL, AUTH, "/", "compute", True, False + ) + assert any( + "[DRY-RUN] Would delete 1 queue(s) for service 'nova' in vhost '/'" in m + for m in _messages(loguru_logs, "INFO") + ) + + +def test_take_action_delete_reports_no_queues(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["delete", "designate"]) + cmd._get_all_queues = MagicMock(return_value=LIST_QUEUES) + cmd._delete_queue = MagicMock() + + assert _take_action(cmd, parsed_args) == 0 + cmd._delete_queue.assert_not_called() + assert any( + "No classic queues found for service 'designate' in vhost '/'" in m + for m in _messages(loguru_logs, "INFO") + ) + + +# --- take_action: list-exchanges command --- + + +def test_take_action_list_exchanges_fails_on_error(): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["list-exchanges"]) + cmd._get_all_exchanges = MagicMock(return_value=None) + + assert _take_action(cmd, parsed_args) == 1 + + +def test_take_action_list_exchanges_reports_empty_vhost(loguru_logs): + cmd, parsed_args = parse_args( + migrate.Rabbitmq3to4, ["list-exchanges", "--vhost", "/openstack"] + ) + cmd._get_all_exchanges = MagicMock(return_value=[]) + + assert _take_action(cmd, parsed_args) == 0 + cmd._get_all_exchanges.assert_called_once_with(BASE_URL, AUTH, "/openstack") + assert any( + "No exchanges found in vhost '/openstack'" in m + for m in _messages(loguru_logs, "INFO") + ) + + +def test_take_action_list_exchanges_lists_exchanges(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["list-exchanges"]) + cmd._get_all_exchanges = MagicMock( + return_value=[ + {"name": "nova", "type": "topic", "durable": True}, + {"name": "cinder", "type": "fanout", "durable": False}, + ] + ) + + assert _take_action(cmd, parsed_args) == 0 + infos = _messages(loguru_logs, "INFO") + assert any("Found 2 exchange(s) in vhost '/':" in m for m in infos) + assert any("- nova (type: topic, durable)" in m for m in infos) + assert any("- cinder (type: fanout, transient)" in m for m in infos) + + +# --- take_action: delete-exchanges command --- + + +def test_take_action_delete_exchanges_fails_on_error(): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["delete-exchanges"]) + cmd._get_all_exchanges = MagicMock(return_value=None) + + assert _take_action(cmd, parsed_args) == 1 + + +def test_take_action_delete_exchanges_reports_empty_vhost(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["delete-exchanges"]) + cmd._get_all_exchanges = MagicMock(return_value=[]) + + assert _take_action(cmd, parsed_args) == 0 + assert any( + "No exchanges found in vhost '/'" in m for m in _messages(loguru_logs, "INFO") + ) + + +def test_take_action_delete_exchanges_only_targets_root_vhost(loguru_logs): + cmd, parsed_args = parse_args( + migrate.Rabbitmq3to4, ["delete-exchanges", "--vhost", "/openstack"] + ) + cmd._get_all_exchanges = MagicMock( + return_value=[{"name": "nova"}, {"name": "cinder"}] + ) + cmd._delete_exchange = MagicMock(return_value=True) + + assert _take_action(cmd, parsed_args) == 0 + cmd._get_all_exchanges.assert_called_once_with(BASE_URL, AUTH, "/") + cmd._delete_exchange.assert_has_calls( + [ + call(BASE_URL, AUTH, "/", "cinder", False), + call(BASE_URL, AUTH, "/", "nova", False), + ] + ) + assert any( + "Successfully deleted 2 exchange(s) in vhost '/'" in m + for m in _messages(loguru_logs, "INFO") + ) + + +def test_take_action_delete_exchanges_reports_failures(loguru_logs): + cmd, parsed_args = parse_args(migrate.Rabbitmq3to4, ["delete-exchanges"]) + cmd._get_all_exchanges = MagicMock( + return_value=[{"name": "nova"}, {"name": "cinder"}] + ) + cmd._delete_exchange = MagicMock(side_effect=[True, False]) + + assert _take_action(cmd, parsed_args) == 1 + assert any( + "Failed to delete 1 exchange(s)" in m for m in _messages(loguru_logs, "ERROR") + ) + + +def test_take_action_delete_exchanges_dry_run(loguru_logs): + cmd, parsed_args = parse_args( + migrate.Rabbitmq3to4, ["delete-exchanges", "--dry-run"] + ) + cmd._get_all_exchanges = MagicMock(return_value=[{"name": "nova"}]) + cmd._delete_exchange = MagicMock(return_value=True) + + assert _take_action(cmd, parsed_args) == 0 + cmd._delete_exchange.assert_called_once_with(BASE_URL, AUTH, "/", "nova", True) + assert any( + "[DRY-RUN] Would delete 1 exchange(s) in vhost '/'" in m + for m in _messages(loguru_logs, "INFO") + ) From f168cdb1de91d4c27b556a42f56fb3828f0299c1 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Fri, 17 Jul 2026 15:20:02 +0200 Subject: [PATCH 2/5] Add unit tests for octavia, amphora and loadbalancer commands Cover the amphora wait helpers (boot and delete polling loops), amphora restore and rotate flows including cleanup on failover errors, and the loadbalancer configuration loaders, database status resets, and the list/reset/delete commands with prompt and failover handling. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- tests/unit/commands/test_amphora.py | 207 ++++++++ tests/unit/commands/test_loadbalancer.py | 603 +++++++++++++++++++++++ tests/unit/commands/test_octavia.py | 62 +++ 3 files changed, 872 insertions(+) create mode 100644 tests/unit/commands/test_amphora.py create mode 100644 tests/unit/commands/test_loadbalancer.py create mode 100644 tests/unit/commands/test_octavia.py diff --git a/tests/unit/commands/test_amphora.py b/tests/unit/commands/test_amphora.py new file mode 100644 index 00000000..02bca862 --- /dev/null +++ b/tests/unit/commands/test_amphora.py @@ -0,0 +1,207 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for the ``osism manage amphora`` commands. + +``AmphoraRestore`` and ``AmphoraRotate`` import the cloud environment helpers +inside ``take_action``, so those are patched at their source module +``osism.tasks.openstack``. The octavia wait helpers and ``sleep`` are bound at +import time and therefore patched at ``osism.commands.amphora``. +""" + +from contextlib import contextmanager +from datetime import datetime, timedelta, timezone +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import openstack +import pytest + +from osism.commands import amphora + +from ._helpers import parse_args + + +@contextmanager +def _patched_environment(conn, setup_success=True): + """Patch the cloud helpers and octavia wait helpers used by both commands.""" + setup_return = ("pw", [], None, True) if setup_success else (None, [], None, False) + with patch( + "osism.tasks.openstack.setup_cloud_environment", return_value=setup_return + ) as setup, patch( + "osism.tasks.openstack.get_openstack_connection", return_value=conn + ) as getconn, patch( + "osism.tasks.openstack.cleanup_cloud_environment" + ) as cleanup, patch( + "osism.commands.amphora.sleep" + ) as mock_sleep, patch( + "osism.commands.amphora.wait_for_amphora_boot" + ) as boot, patch( + "osism.commands.amphora.wait_for_amphora_delete" + ) as delete: + yield SimpleNamespace( + setup=setup, + getconn=getconn, + cleanup=cleanup, + sleep=mock_sleep, + boot=boot, + delete=delete, + ) + + +def _run(command_class, args, conn, setup_success=True): + cmd, parsed_args = parse_args(command_class, args) + with _patched_environment(conn, setup_success=setup_success) as mocks: + result = cmd.take_action(parsed_args) + return result, mocks + + +def _make_amphora(amphora_id="amp-1", loadbalancer_id="lb-1", age=timedelta(days=31)): + amp = MagicMock() + amp.id = amphora_id + amp.loadbalancer_id = loadbalancer_id + amp.created_at = (datetime.now(timezone.utc) - age).isoformat() + return amp + + +# --- AmphoraRestore.take_action --- + + +def test_restore_returns_1_when_setup_fails(): + conn = MagicMock() + + result, mocks = _run(amphora.AmphoraRestore, [], conn, setup_success=False) + + assert result == 1 + mocks.getconn.assert_not_called() + mocks.cleanup.assert_not_called() + + +def test_restore_queries_all_error_amphorae_by_default(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [] + + _run(amphora.AmphoraRestore, [], conn) + + conn.load_balancer.amphorae.assert_called_once_with(status="ERROR") + + +def test_restore_scopes_query_to_loadbalancer(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [] + + _run(amphora.AmphoraRestore, ["--loadbalancer", "lb-1"], conn) + + conn.load_balancer.amphorae.assert_called_once_with( + status="ERROR", loadbalancer_id="lb-1" + ) + + +def test_restore_triggers_failover_and_waits_per_amphora(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [_make_amphora()] + + result, mocks = _run(amphora.AmphoraRestore, [], conn) + + conn.load_balancer.failover_amphora.assert_called_once_with("amp-1") + mocks.boot.assert_called_once_with(conn, "lb-1") + mocks.delete.assert_called_once_with(conn, "lb-1") + mocks.cleanup.assert_called_once_with([], None) + + +def test_restore_cleans_up_when_failover_raises(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [_make_amphora()] + conn.load_balancer.failover_amphora.side_effect = RuntimeError("boom") + cmd, parsed_args = parse_args(amphora.AmphoraRestore, []) + + with _patched_environment(conn) as mocks, pytest.raises(RuntimeError): + cmd.take_action(parsed_args) + + mocks.cleanup.assert_called_once_with([], None) + + +# --- AmphoraRotate.take_action --- + + +def test_rotate_returns_1_when_setup_fails(): + conn = MagicMock() + + result, mocks = _run(amphora.AmphoraRotate, [], conn, setup_success=False) + + assert result == 1 + mocks.getconn.assert_not_called() + mocks.cleanup.assert_not_called() + + +def test_rotate_old_amphora_triggers_loadbalancer_failover(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [_make_amphora(age=timedelta(days=31))] + + result, mocks = _run(amphora.AmphoraRotate, [], conn) + + conn.load_balancer.amphorae.assert_called_once_with(status="ALLOCATED") + conn.load_balancer.failover_load_balancer.assert_called_once_with("lb-1") + mocks.boot.assert_called_once_with(conn, "lb-1") + mocks.delete.assert_called_once_with(conn, "lb-1") + mocks.cleanup.assert_called_once_with([], None) + + +def test_rotate_skips_young_amphora_without_force(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [_make_amphora(age=timedelta(days=1))] + + result, mocks = _run(amphora.AmphoraRotate, [], conn) + + conn.load_balancer.failover_load_balancer.assert_not_called() + mocks.boot.assert_not_called() + mocks.delete.assert_not_called() + + +def test_rotate_force_rotates_regardless_of_age(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [_make_amphora(age=timedelta(days=1))] + + result, mocks = _run( + amphora.AmphoraRotate, ["--force", "--loadbalancer", "lb-1"], conn + ) + + conn.load_balancer.amphorae.assert_called_once_with( + status="ALLOCATED", loadbalancer_id="lb-1" + ) + conn.load_balancer.failover_load_balancer.assert_called_once_with("lb-1") + + +def test_rotate_handles_each_loadbalancer_once(): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [ + _make_amphora(amphora_id="amp-1"), + _make_amphora(amphora_id="amp-2"), + ] + + result, mocks = _run(amphora.AmphoraRotate, [], conn) + + conn.load_balancer.failover_load_balancer.assert_called_once_with("lb-1") + mocks.boot.assert_called_once_with(conn, "lb-1") + + +def test_rotate_conflict_keeps_loadbalancer_out_of_done(loguru_logs): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [ + _make_amphora(amphora_id="amp-1"), + _make_amphora(amphora_id="amp-2"), + ] + conn.load_balancer.failover_load_balancer.side_effect = [ + openstack.exceptions.ConflictException, + None, + ] + + result, mocks = _run(amphora.AmphoraRotate, [], conn) + + # The conflict only logs a warning; the loadbalancer is not marked as + # done, so the second amphora of the same loadbalancer retries. + assert conn.load_balancer.failover_load_balancer.call_count == 2 + mocks.boot.assert_called_once_with(conn, "lb-1") + warnings = [r for r in loguru_logs if r["level"] == "WARNING"] + assert any( + "Conflict while rotating loadbalancer lb-1" in r["message"] for r in warnings + ) diff --git a/tests/unit/commands/test_loadbalancer.py b/tests/unit/commands/test_loadbalancer.py new file mode 100644 index 00000000..e310e6d8 --- /dev/null +++ b/tests/unit/commands/test_loadbalancer.py @@ -0,0 +1,603 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for the ``osism manage loadbalancer`` commands and their helpers. + +The module-level helpers load the kolla configuration and the octavia +database password and open a MariaDB connection; the commands combine those +with the OpenStack cloud helpers. The database connection helper delegates to +``osism.utils.mariadb.connect`` (which handles the ProxySQL sharded user), so +that module is patched as a whole here. +""" + +from contextlib import contextmanager +from types import SimpleNamespace +from unittest.mock import MagicMock, call, mock_open, patch + +import pymysql +import pytest + +from osism.commands import loadbalancer + +from ._helpers import parse_args + + +# --- _load_kolla_configuration --- + + +def test_load_kolla_configuration_returns_none_when_file_missing(loguru_logs): + with patch("osism.commands.loadbalancer.os.path.exists", return_value=False): + assert loadbalancer._load_kolla_configuration() is None + + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("Configuration file not found" in r["message"] for r in errors) + + +def test_load_kolla_configuration_returns_none_on_load_error(loguru_logs): + with patch("osism.commands.loadbalancer.os.path.exists", return_value=True), patch( + "builtins.open", mock_open(read_data="key: value\n") + ), patch( + "osism.commands.loadbalancer.yaml.safe_load", side_effect=Exception("boom") + ): + assert loadbalancer._load_kolla_configuration() is None + + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("Failed to load configuration" in r["message"] for r in errors) + + +def test_load_kolla_configuration_returns_parsed_yaml(): + open_mock = mock_open(read_data="kolla_internal_vip_address: 192.0.2.10\n") + with patch("osism.commands.loadbalancer.os.path.exists", return_value=True), patch( + "builtins.open", open_mock + ): + config = loadbalancer._load_kolla_configuration() + + assert config == {"kolla_internal_vip_address": "192.0.2.10"} + open_mock.assert_called_once_with( + "/opt/configuration/environments/kolla/configuration.yml", "r" + ) + + +# --- _load_octavia_database_password --- + + +def test_load_octavia_password_returns_none_when_file_missing(loguru_logs): + with patch("osism.commands.loadbalancer.os.path.exists", return_value=False): + assert loadbalancer._load_octavia_database_password() is None + + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("Secrets file not found" in r["message"] for r in errors) + + +@pytest.mark.parametrize( + "secrets, expected", + [ + pytest.param(None, None, id="loader-returns-none"), + pytest.param(["not", "a", "dict"], None, id="not-a-dict"), + pytest.param( + { + "octavia_database_password": "octavia-pw", + "database_password": "general-pw", + }, + "octavia-pw", + id="octavia-password-preferred", + ), + pytest.param( + {"database_password": "general-pw"}, + "general-pw", + id="fallback-to-database-password", + ), + pytest.param({"other_key": "value"}, None, id="no-password-keys"), + pytest.param( + {"octavia_database_password": " padded \n"}, "padded", id="stripped" + ), + pytest.param( + {"octavia_database_password": 123456}, "123456", id="coerced-to-str" + ), + ], +) +def test_load_octavia_password_variants(secrets, expected): + with patch("osism.commands.loadbalancer.os.path.exists", return_value=True), patch( + "osism.tasks.conductor.utils.load_yaml_file", return_value=secrets + ): + assert loadbalancer._load_octavia_database_password() == expected + + +def test_load_octavia_password_returns_none_on_loader_error(loguru_logs): + with patch("osism.commands.loadbalancer.os.path.exists", return_value=True), patch( + "osism.tasks.conductor.utils.load_yaml_file", side_effect=Exception("boom") + ): + assert loadbalancer._load_octavia_database_password() is None + + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any( + "Failed to load octavia database password" in r["message"] for r in errors + ) + + +# --- _get_octavia_database_connection --- + + +@contextmanager +def _database_environment(config, password): + with patch( + "osism.commands.loadbalancer._load_kolla_configuration", return_value=config + ), patch( + "osism.commands.loadbalancer._load_octavia_database_password", + return_value=password, + ), patch( + "osism.commands.loadbalancer.mariadb" + ) as mariadb: + yield mariadb + + +def test_get_database_connection_returns_none_without_config(): + with _database_environment(None, "secret") as mariadb: + assert loadbalancer._get_octavia_database_connection() is None + + mariadb.connect.assert_not_called() + + +def test_get_database_connection_returns_none_without_vip_address(loguru_logs): + with _database_environment({}, "secret") as mariadb: + assert loadbalancer._get_octavia_database_connection() is None + + mariadb.connect.assert_not_called() + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("kolla_internal_vip_address not found" in r["message"] for r in errors) + + +def test_get_database_connection_returns_none_without_password(): + with _database_environment( + {"kolla_internal_vip_address": "192.0.2.10"}, None + ) as mariadb: + assert loadbalancer._get_octavia_database_connection() is None + + mariadb.connect.assert_not_called() + + +def test_get_database_connection_happy_path(): + with _database_environment( + {"kolla_internal_vip_address": "192.0.2.10"}, "secret" + ) as mariadb: + connection = loadbalancer._get_octavia_database_connection() + + assert connection is mariadb.connect.return_value + mariadb.connect.assert_called_once_with( + "192.0.2.10", + "octavia", + "secret", + port=3306, + database="octavia", + cursorclass=pymysql.cursors.DictCursor, + connect_timeout=10, + ) + + +def test_get_database_connection_returns_none_on_pymysql_error(loguru_logs): + with _database_environment( + {"kolla_internal_vip_address": "192.0.2.10"}, "secret" + ) as mariadb: + mariadb.connect.side_effect = pymysql.Error("boom") + assert loadbalancer._get_octavia_database_connection() is None + + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("Failed to connect to Octavia database" in r["message"] for r in errors) + + +# --- _reset_provisioning_status / _reset_operating_status --- + +RESET_STATUS_CASES = [ + pytest.param( + loadbalancer._reset_provisioning_status, + "provisioning_status", + "ACTIVE", + id="provisioning", + ), + pytest.param( + loadbalancer._reset_operating_status, + "operating_status", + "ONLINE", + id="operating", + ), +] + + +@pytest.mark.parametrize("reset, column, default_status", RESET_STATUS_CASES) +def test_reset_status_executes_update_with_default_status( + reset, column, default_status +): + database = MagicMock() + cursor = database.cursor.return_value.__enter__.return_value + + reset(database, "lb-1") + + cursor.execute.assert_called_once_with( + f"UPDATE load_balancer SET {column} = '{default_status}' WHERE id = 'lb-1';" + ) + database.commit.assert_called_once_with() + + +@pytest.mark.parametrize("reset, column, default_status", RESET_STATUS_CASES) +def test_reset_status_interpolates_custom_status(reset, column, default_status): + database = MagicMock() + cursor = database.cursor.return_value.__enter__.return_value + + reset(database, "lb-1", status="ERROR") + + cursor.execute.assert_called_once_with( + f"UPDATE load_balancer SET {column} = 'ERROR' WHERE id = 'lb-1';" + ) + database.commit.assert_called_once_with() + + +# --- LoadbalancerList.take_action --- + + +def _make_loadbalancer(provisioning_status="ACTIVE", operating_status="ONLINE"): + lb = MagicMock() + lb.id = "lb-1" + lb.name = "web" + lb.provisioning_status = provisioning_status + lb.operating_status = operating_status + lb.project_id = "project-1" + return lb + + +def _run_list(args, conn, setup_success=True): + cmd, parsed_args = parse_args(loadbalancer.LoadbalancerList, args) + setup_return = ("pw", [], None, True) if setup_success else (None, [], None, False) + setup = MagicMock(return_value=setup_return) + getconn = MagicMock(return_value=conn) + cleanup = MagicMock() + with patch( + "osism.tasks.openstack.get_cloud_helpers", + return_value=(setup, getconn, cleanup), + ): + result = cmd.take_action(parsed_args) + return result, cleanup + + +def test_list_returns_1_when_setup_fails(): + result, cleanup = _run_list([], MagicMock(), setup_success=False) + + assert result == 1 + cleanup.assert_not_called() + + +def test_list_provisioning_status_queries_three_statuses(capsys): + conn = MagicMock() + conn.load_balancer.load_balancers.return_value = [_make_loadbalancer("ERROR")] + + result, cleanup = _run_list([], conn) + + assert conn.load_balancer.load_balancers.call_args_list == [ + call(provisioning_status="PENDING_CREATE"), + call(provisioning_status="PENDING_UPDATE"), + call(provisioning_status="ERROR"), + ] + assert "lb-1" in capsys.readouterr().out + cleanup.assert_called_once_with([], None) + + +def test_list_operating_status_queries_error_only(capsys): + conn = MagicMock() + conn.load_balancer.load_balancers.return_value = [ + _make_loadbalancer(operating_status="ERROR") + ] + + _run_list(["--status-type", "operating_status"], conn) + + conn.load_balancer.load_balancers.assert_called_once_with(operating_status="ERROR") + assert "lb-1" in capsys.readouterr().out + + +def test_list_without_results_logs_message(capsys, loguru_logs): + conn = MagicMock() + conn.load_balancer.load_balancers.return_value = [] + + _run_list([], conn) + + assert capsys.readouterr().out == "" + assert any( + "No loadbalancers with problematic status found" in r["message"] + for r in loguru_logs + ) + + +# --- LoadbalancerReset / LoadbalancerDelete --- + + +@contextmanager +def _command_environment(conn, db=None, prompt_answer="yes", setup_success=True): + """Patch everything ``LoadbalancerReset``/``LoadbalancerDelete`` touch.""" + setup_return = ("pw", [], None, True) if setup_success else (None, [], None, False) + setup = MagicMock(return_value=setup_return) + getconn = MagicMock(return_value=conn) + cleanup = MagicMock() + with patch( + "osism.tasks.openstack.get_cloud_helpers", + return_value=(setup, getconn, cleanup), + ), patch( + "osism.commands.loadbalancer.prompt", return_value=prompt_answer + ) as prompt_mock, patch( + "osism.commands.loadbalancer._get_octavia_database_connection", + return_value=db, + ) as get_db, patch( + "osism.commands.loadbalancer._reset_provisioning_status" + ) as reset_provisioning, patch( + "osism.commands.loadbalancer._reset_operating_status" + ) as reset_operating, patch( + "osism.commands.loadbalancer.wait_for_amphora_boot" + ) as boot, patch( + "osism.commands.loadbalancer.sleep" + ): + yield SimpleNamespace( + getconn=getconn, + cleanup=cleanup, + prompt=prompt_mock, + get_db=get_db, + reset_provisioning=reset_provisioning, + reset_operating=reset_operating, + boot=boot, + ) + + +def _run_command(command_class, args, conn, **kwargs): + cmd, parsed_args = parse_args(command_class, args) + with _command_environment(conn, **kwargs) as mocks: + result = cmd.take_action(parsed_args) + return result, mocks + + +def _conn_for(lb): + conn = MagicMock() + conn.load_balancer.get_load_balancer.return_value = lb + return conn + + +# --- LoadbalancerReset.take_action --- + + +def test_reset_returns_1_when_setup_fails(): + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, ["lb-1"], MagicMock(), setup_success=False + ) + + assert result == 1 + mocks.getconn.assert_not_called() + + +def test_reset_returns_1_when_loadbalancer_lookup_fails(loguru_logs): + conn = MagicMock() + conn.load_balancer.get_load_balancer.side_effect = Exception("boom") + + result, mocks = _run_command(loadbalancer.LoadbalancerReset, ["lb-1"], conn) + + assert result == 1 + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("Failed to get loadbalancer lb-1" in r["message"] for r in errors) + + +def test_reset_provisioning_rejects_pending_create(loguru_logs): + conn = _conn_for(_make_loadbalancer("PENDING_CREATE")) + + result, mocks = _run_command(loadbalancer.LoadbalancerReset, ["lb-1"], conn) + + assert result == 1 + mocks.get_db.assert_not_called() + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("manage loadbalancer delete" in r["message"] for r in errors) + + +def test_reset_operating_requires_error_operating_status(): + conn = _conn_for(_make_loadbalancer("ACTIVE", "ONLINE")) + + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, + ["lb-1", "--status-type", "operating_status"], + conn, + ) + + assert result == 1 + mocks.get_db.assert_not_called() + + +def test_reset_operating_requires_active_provisioning_status(): + conn = _conn_for(_make_loadbalancer("PENDING_UPDATE", "ERROR")) + + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, + ["lb-1", "--status-type", "operating_status"], + conn, + ) + + assert result == 1 + mocks.get_db.assert_not_called() + + +def test_reset_aborts_when_prompt_declined(loguru_logs): + conn = _conn_for(_make_loadbalancer("ERROR")) + + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, ["lb-1"], conn, prompt_answer="no" + ) + + assert result == 0 + mocks.get_db.assert_not_called() + mocks.reset_provisioning.assert_not_called() + assert any("Aborted" in r["message"] for r in loguru_logs) + + +def test_reset_returns_1_without_database_connection(): + conn = _conn_for(_make_loadbalancer("ERROR")) + + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, ["lb-1", "--yes"], conn, db=None + ) + + assert result == 1 + mocks.reset_provisioning.assert_not_called() + + +def test_reset_prompts_before_reset(): + conn = _conn_for(_make_loadbalancer("PENDING_UPDATE")) + db = MagicMock() + + result, mocks = _run_command(loadbalancer.LoadbalancerReset, ["lb-1"], conn, db=db) + + mocks.prompt.assert_called_once() + mocks.reset_provisioning.assert_called_once_with(db, "lb-1") + + +def test_reset_provisioning_happy_path(): + conn = _conn_for(_make_loadbalancer("ERROR")) + db = MagicMock() + + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, ["lb-1", "--yes"], conn, db=db + ) + + assert result is None + mocks.prompt.assert_not_called() + mocks.reset_provisioning.assert_called_once_with(db, "lb-1") + mocks.reset_operating.assert_not_called() + conn.load_balancer.failover_load_balancer.assert_called_once_with("lb-1") + mocks.boot.assert_called_once_with(conn, "lb-1") + db.close.assert_called_once_with() + mocks.cleanup.assert_called_once_with([], None) + + +def test_reset_operating_happy_path(): + conn = _conn_for(_make_loadbalancer("ACTIVE", "ERROR")) + db = MagicMock() + + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, + ["lb-1", "--yes", "--status-type", "operating_status"], + conn, + db=db, + ) + + assert result is None + mocks.reset_operating.assert_called_once_with(db, "lb-1") + mocks.reset_provisioning.assert_not_called() + db.close.assert_called_once_with() + + +def test_reset_no_failover_skips_failover(): + conn = _conn_for(_make_loadbalancer("ERROR")) + db = MagicMock() + + result, mocks = _run_command( + loadbalancer.LoadbalancerReset, ["lb-1", "--yes", "--no-failover"], conn, db=db + ) + + conn.load_balancer.failover_load_balancer.assert_not_called() + mocks.boot.assert_not_called() + mocks.reset_provisioning.assert_called_once_with(db, "lb-1") + db.close.assert_called_once_with() + + +def test_reset_closes_database_when_failover_raises(): + conn = _conn_for(_make_loadbalancer("ERROR")) + conn.load_balancer.failover_load_balancer.side_effect = RuntimeError("boom") + db = MagicMock() + cmd, parsed_args = parse_args(loadbalancer.LoadbalancerReset, ["lb-1", "--yes"]) + + with _command_environment(conn, db=db) as mocks, pytest.raises(RuntimeError): + cmd.take_action(parsed_args) + + db.close.assert_called_once_with() + mocks.cleanup.assert_called_once_with([], None) + + +# --- LoadbalancerDelete.take_action --- + + +def test_delete_returns_1_when_loadbalancer_lookup_fails(loguru_logs): + conn = MagicMock() + conn.load_balancer.get_load_balancer.side_effect = Exception("boom") + + result, mocks = _run_command(loadbalancer.LoadbalancerDelete, ["lb-1"], conn) + + assert result == 1 + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("Failed to get loadbalancer lb-1" in r["message"] for r in errors) + + +def test_delete_rejects_non_pending_create_status(loguru_logs): + conn = _conn_for(_make_loadbalancer("ERROR")) + + result, mocks = _run_command(loadbalancer.LoadbalancerDelete, ["lb-1"], conn) + + assert result == 1 + mocks.get_db.assert_not_called() + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("manage loadbalancer reset" in r["message"] for r in errors) + + +def test_delete_aborts_when_prompt_declined(loguru_logs): + conn = _conn_for(_make_loadbalancer("PENDING_CREATE")) + + result, mocks = _run_command( + loadbalancer.LoadbalancerDelete, ["lb-1"], conn, prompt_answer="no" + ) + + assert result == 0 + conn.load_balancer.delete_load_balancer.assert_not_called() + assert any("Aborted" in r["message"] for r in loguru_logs) + + +def test_delete_yes_skips_prompt(): + conn = _conn_for(_make_loadbalancer("PENDING_CREATE")) + db = MagicMock() + + result, mocks = _run_command( + loadbalancer.LoadbalancerDelete, ["lb-1", "--yes"], conn, db=db + ) + + mocks.prompt.assert_not_called() + conn.load_balancer.delete_load_balancer.assert_called_once_with("lb-1") + + +def test_delete_returns_1_without_database_connection(): + conn = _conn_for(_make_loadbalancer("PENDING_CREATE")) + + result, mocks = _run_command( + loadbalancer.LoadbalancerDelete, ["lb-1", "--yes"], conn, db=None + ) + + assert result == 1 + conn.load_balancer.delete_load_balancer.assert_not_called() + + +def test_delete_sets_error_status_before_deleting(): + conn = _conn_for(_make_loadbalancer("PENDING_CREATE")) + db = MagicMock() + cmd, parsed_args = parse_args(loadbalancer.LoadbalancerDelete, ["lb-1", "--yes"]) + + with _command_environment(conn, db=db) as mocks: + # The provisioning status must be set to ERROR before the delete call. + mocks.reset_provisioning.side_effect = ( + lambda *args, **kwargs: conn.load_balancer.delete_load_balancer.assert_not_called() + ) + result = cmd.take_action(parsed_args) + + assert result is None + mocks.reset_provisioning.assert_called_once_with(db, "lb-1", status="ERROR") + conn.load_balancer.delete_load_balancer.assert_called_once_with("lb-1") + db.close.assert_called_once_with() + mocks.cleanup.assert_called_once_with([], None) + + +def test_delete_closes_database_when_delete_raises(): + conn = _conn_for(_make_loadbalancer("PENDING_CREATE")) + conn.load_balancer.delete_load_balancer.side_effect = RuntimeError("boom") + db = MagicMock() + cmd, parsed_args = parse_args(loadbalancer.LoadbalancerDelete, ["lb-1", "--yes"]) + + with _command_environment(conn, db=db) as mocks, pytest.raises(RuntimeError): + cmd.take_action(parsed_args) + + db.close.assert_called_once_with() + mocks.cleanup.assert_called_once_with([], None) diff --git a/tests/unit/commands/test_octavia.py b/tests/unit/commands/test_octavia.py new file mode 100644 index 00000000..25075612 --- /dev/null +++ b/tests/unit/commands/test_octavia.py @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for the amphora wait helpers in ``osism.commands.octavia``. + +Both helpers poll the load-balancer API until no amphora is left in the +transitional state, sleeping between polls and giving up after a fixed +timeout. ``sleep`` is patched so the timeout paths run instantly. +""" + +from unittest.mock import MagicMock, call, patch + +import pytest + +from osism.commands import octavia + +# (wait helper, transitional status queried, timeout / sleep iterations) +WAIT_CASES = [ + pytest.param(octavia.wait_for_amphora_boot, "BOOTING", 24, id="boot"), + pytest.param(octavia.wait_for_amphora_delete, "PENDING_DELETE", 12, id="delete"), +] + + +@pytest.mark.parametrize("wait, status, max_iterations", WAIT_CASES) +def test_returns_without_sleeping_when_no_amphorae(wait, status, max_iterations): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [] + + with patch("osism.commands.octavia.sleep") as mock_sleep: + wait(conn, "lb-1") + + conn.load_balancer.amphorae.assert_called_once_with( + loadbalancer_id="lb-1", status=status + ) + mock_sleep.assert_not_called() + + +@pytest.mark.parametrize("wait, status, max_iterations", WAIT_CASES) +def test_polls_until_no_amphorae_remain(wait, status, max_iterations): + conn = MagicMock() + amphora = MagicMock() + conn.load_balancer.amphorae.side_effect = [[amphora], [amphora], []] + + with patch("osism.commands.octavia.sleep") as mock_sleep: + wait(conn, "lb-1") + + assert ( + conn.load_balancer.amphorae.call_args_list + == [call(loadbalancer_id="lb-1", status=status)] * 3 + ) + assert mock_sleep.call_args_list == [call(5), call(5)] + + +@pytest.mark.parametrize("wait, status, max_iterations", WAIT_CASES) +def test_gives_up_after_timeout(wait, status, max_iterations): + conn = MagicMock() + conn.load_balancer.amphorae.return_value = [MagicMock()] + + with patch("osism.commands.octavia.sleep") as mock_sleep: + wait(conn, "lb-1") + + assert conn.load_balancer.amphorae.call_count == max_iterations + assert mock_sleep.call_count == max_iterations From 617302b3d0b499f80d893a3b0e3d17d96869ff75 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Fri, 17 Jul 2026 15:20:02 +0200 Subject: [PATCH 3/5] Extend unit tests for compute and volume commands Add coverage for compute service enable/disable, instance listing with project and domain filters, live and cold migration classification and polling, migration list query assembly, start/stop eligibility, evacuation, stuck volume listing, and the volume repair sequences. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- tests/unit/commands/test_compute.py | 545 +++++++++++++++++++++++++++- tests/unit/commands/test_volume.py | 200 +++++++++- 2 files changed, 739 insertions(+), 6 deletions(-) diff --git a/tests/unit/commands/test_compute.py b/tests/unit/commands/test_compute.py index 478de97a..efd43bb3 100644 --- a/tests/unit/commands/test_compute.py +++ b/tests/unit/commands/test_compute.py @@ -1,8 +1,10 @@ # SPDX-License-Identifier: Apache-2.0 -from unittest.mock import MagicMock, patch +import datetime +from unittest.mock import MagicMock, call, patch import openstack +import pytest from osism.commands import compute @@ -102,3 +104,544 @@ def test_migration_list_rejects_changes_since_after_changes_before(): MagicMock(), ) assert result == 1 + + +# --------------------------------------------------------------------------- +# Shared helpers for the remaining compute commands +# --------------------------------------------------------------------------- + + +def _run_command(command_class, args, conn, setup_success=True, prompt_mock=None): + """Run any compute command with mocked cloud helpers, prompt and sleep.""" + cmd = command_class(MagicMock(), MagicMock()) + parsed_args = cmd.get_parser("test").parse_args(args) + setup = MagicMock(return_value=("pw", [], None, setup_success)) + getconn = MagicMock(return_value=conn) + cleanup = MagicMock() + if prompt_mock is None: + prompt_mock = MagicMock(return_value="yes") + with patch( + "osism.tasks.openstack.get_cloud_helpers", + return_value=(setup, getconn, cleanup), + ), patch("osism.commands.compute.prompt", prompt_mock), patch( + "osism.commands.compute.time.sleep", MagicMock() + ): + return cmd.take_action(parsed_args) + + +def _service(forced_down=False): + service = MagicMock() + service.__getitem__.return_value = forced_down # service["forced_down"] + return service + + +def _server(server_id, name, status, project_id="project-1"): + server = MagicMock() + server.id = server_id + server.name = name + server.status = status + server.project_id = project_id + return server + + +def _hypervisor(hypervisor_id, name, status="enabled", state="up", uptime=None): + hypervisor = MagicMock() + data = { + "id": hypervisor_id, + "status": status, + "state": state, + "uptime": uptime, + } + hypervisor.get.side_effect = lambda key, default=None: data.get(key, default) + hypervisor.name = name + return hypervisor + + +def _polled_server(status): + server = MagicMock() + server.status = status + return server + + +# --------------------------------------------------------------------------- +# ComputeEnable +# --------------------------------------------------------------------------- + + +def test_enable_service_not_forced_down_only_enables(): + conn = MagicMock() + service = _service(forced_down=False) + conn.compute.services.return_value = iter([service]) + result = _run_command(compute.ComputeEnable, ["somehost"], conn) + assert result is None + conn.compute.update_service_forced_down.assert_not_called() + conn.compute.enable_service.assert_called_once_with( + service=service.id, host="somehost", binary="nova-compute" + ) + + +def test_enable_forced_down_service_is_forced_up_then_enabled(): + conn = MagicMock() + service = _service(forced_down=True) + conn.compute.services.return_value = iter([service]) + result = _run_command(compute.ComputeEnable, ["somehost"], conn) + assert result is None + conn.compute.update_service_forced_down.assert_called_once_with( + service=service.id, host="somehost", binary="nova-compute", forced=False + ) + conn.compute.enable_service.assert_called_once_with( + service=service.id, host="somehost", binary="nova-compute" + ) + + +def test_enable_returns_1_when_setup_fails(): + conn = MagicMock() + result = _run_command( + compute.ComputeEnable, ["somehost"], conn, setup_success=False + ) + assert result == 1 + conn.compute.enable_service.assert_not_called() + + +# --------------------------------------------------------------------------- +# ComputeDisable +# --------------------------------------------------------------------------- + + +def test_disable_uses_maintenance_reason(): + conn = MagicMock() + service = _service() + conn.compute.services.return_value = iter([service]) + result = _run_command(compute.ComputeDisable, ["somehost"], conn) + assert result is None + conn.compute.disable_service.assert_called_once_with( + service=service.id, + host="somehost", + binary="nova-compute", + disabled_reason="MAINTENANCE", + ) + + +# --------------------------------------------------------------------------- +# ComputeList +# --------------------------------------------------------------------------- + + +def test_list_host_with_project_filter(capsys): + conn = MagicMock() + server = _server("id1", "srv1", "ACTIVE", project_id="project-1") + conn.compute.servers.return_value = [server] + result = _run_command( + compute.ComputeList, ["--project", "project-1", "somehost"], conn + ) + assert result is None + conn.compute.servers.assert_called_once_with(all_projects=True, node="somehost") + # A server matching the project filter is listed without a domain lookup. + conn.identity.get_project.assert_not_called() + out = capsys.readouterr().out + assert "id1" in out + assert "srv1" in out + + +def test_list_host_with_domain_filter(capsys): + conn = MagicMock() + matching = _server("id1", "srv1", "ACTIVE", project_id="project-1") + other = _server("id2", "srv2", "ACTIVE", project_id="project-2") + conn.compute.servers.return_value = [matching, other] + + def get_project(project_id): + project = MagicMock() + project.domain_id = "domain-1" if project_id == "project-1" else "domain-2" + return project + + conn.identity.get_project.side_effect = get_project + result = _run_command( + compute.ComputeList, ["--domain", "domain-1", "somehost"], conn + ) + assert result is None + assert conn.identity.get_project.call_count == 2 + out = capsys.readouterr().out + assert "id1" in out + assert "id2" not in out + + +def test_list_host_unfiltered(capsys): + conn = MagicMock() + conn.compute.servers.return_value = [ + _server("id1", "srv1", "ACTIVE"), + _server("id2", "srv2", "SHUTOFF"), + ] + result = _run_command(compute.ComputeList, ["somehost"], conn) + assert result is None + out = capsys.readouterr().out + assert "id1" in out + assert "id2" in out + + +def test_list_details_parses_uptime(capsys): + conn = MagicMock() + conn.compute.hypervisors.return_value = [ + _hypervisor( + "hv1", + "node001", + uptime=" 16:41:00 up 5 days, 3:02, 2 users, load average: 0.98, 1.10, 1.00", + ) + ] + result = _run_command(compute.ComputeList, ["--details"], conn) + assert result is None + conn.compute.hypervisors.assert_called_once_with(details=True) + out = capsys.readouterr().out + assert "5 days, 3:02" in out + assert "0.98" in out + + +@pytest.mark.parametrize("uptime", [None, "garbage"]) +def test_list_details_missing_or_unparsable_uptime(capsys, uptime): + conn = MagicMock() + conn.compute.hypervisors.return_value = [ + _hypervisor("hv1", "node001", uptime=uptime) + ] + result = _run_command(compute.ComputeList, ["--details"], conn) + assert result is None + out = capsys.readouterr().out + row = next(line for line in out.splitlines() if "hv1" in line) + # Uptime, Load 1, Load 5 and Load 15 all fall back to the "-" placeholder. + assert row.count("-") == 4 + + +def test_list_hypervisors_without_details(capsys): + conn = MagicMock() + conn.compute.hypervisors.return_value = [_hypervisor("hv1", "node001")] + result = _run_command(compute.ComputeList, [], conn) + assert result is None + out = capsys.readouterr().out + assert "Uptime" not in out + assert "hv1" in out + assert "node001" in out + assert "enabled" in out + + +# --------------------------------------------------------------------------- +# ComputeMigrate +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("status", ["ACTIVE", "PAUSED"]) +def test_migrate_live_migrates_active_and_paused(status): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", status)] + result = _run_command( + compute.ComputeMigrate, + ["--yes", "--no-wait", "--target", "target1", "--force", "somehost"], + conn, + ) + assert result is None + conn.compute.live_migrate_server.assert_called_once_with( + "id1", host="target1", block_migration="auto", force=True + ) + conn.compute.migrate_server.assert_not_called() + # --no-wait skips the status polling loop. + conn.compute.get_server.assert_not_called() + + +def test_migrate_cold_migrates_shutoff(): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "SHUTOFF")] + result = _run_command( + compute.ComputeMigrate, + ["--yes", "--no-wait", "--target", "target1", "somehost"], + conn, + ) + assert result is None + conn.compute.migrate_server.assert_called_once_with("id1", host="target1") + conn.compute.live_migrate_server.assert_not_called() + conn.compute.get_server.assert_not_called() + + +def test_migrate_shutoff_skipped_with_no_cold_migration(loguru_logs): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "SHUTOFF")] + result = _run_command( + compute.ComputeMigrate, ["--yes", "--no-cold-migration", "somehost"], conn + ) + assert result is None + conn.compute.migrate_server.assert_not_called() + conn.compute.live_migrate_server.assert_not_called() + assert any("cannot be migrated" in r["message"] for r in loguru_logs) + + +def test_migrate_other_status_skipped(loguru_logs): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "ERROR")] + result = _run_command(compute.ComputeMigrate, ["--yes", "somehost"], conn) + assert result is None + conn.compute.migrate_server.assert_not_called() + conn.compute.live_migrate_server.assert_not_called() + assert any("cannot be migrated" in r["message"] for r in loguru_logs) + + +def test_migrate_prompt_no_skips_migration(): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "ACTIVE")] + prompt_mock = MagicMock(return_value="no") + result = _run_command( + compute.ComputeMigrate, ["somehost"], conn, prompt_mock=prompt_mock + ) + assert result is None + prompt_mock.assert_called_once() + conn.compute.live_migrate_server.assert_not_called() + conn.compute.migrate_server.assert_not_called() + + +def test_migrate_cold_wait_confirms_verify_resize(): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "SHUTOFF")] + polled = _polled_server("VERIFY_RESIZE") + conn.compute.get_server.return_value = polled + result = _run_command(compute.ComputeMigrate, ["--yes", "somehost"], conn) + assert result is None + conn.compute.confirm_server_resize.assert_called_once_with(polled) + + +def test_migrate_confirm_failure_reraises(): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "SHUTOFF")] + conn.compute.get_server.return_value = _polled_server("VERIFY_RESIZE") + conn.compute.confirm_server_resize.side_effect = RuntimeError("confirm failed") + with pytest.raises(RuntimeError): + _run_command(compute.ComputeMigrate, ["--yes", "somehost"], conn) + + +def test_migrate_live_wait_until_migration_completes(loguru_logs): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "ACTIVE")] + conn.compute.get_server.side_effect = [ + _polled_server("MIGRATING"), + _polled_server("ACTIVE"), + ] + result = _run_command(compute.ComputeMigrate, ["--yes", "somehost"], conn) + assert result is None + assert conn.compute.get_server.call_count == 2 + messages = [r["message"] for r in loguru_logs] + assert any("still in progress" in m for m in messages) + assert any("completed with status ACTIVE" in m for m in messages) + + +def test_migrate_no_instances_found(loguru_logs): + conn = MagicMock() + conn.compute.servers.return_value = [] + result = _run_command(compute.ComputeMigrate, ["--yes", "somehost"], conn) + assert result is None + assert any("No migratable instances found" in r["message"] for r in loguru_logs) + + +# --------------------------------------------------------------------------- +# ComputeMigrationList (happy path) +# --------------------------------------------------------------------------- + + +def test_migration_list_passes_resolved_filters_to_query(): + conn = MagicMock() + + domain = MagicMock() + domain.__contains__.return_value = True # "id" in domain + domain.id = "domain-id" + conn.identity.find_domain.return_value = domain + + user = MagicMock() + user.__contains__.return_value = True + user.id = "user-id" + conn.identity.find_user.return_value = user + + project = MagicMock() + project.__contains__.return_value = True + project.id = "project-id" + conn.identity.find_project.return_value = project + + server = MagicMock() + server.__contains__.return_value = True + server.id = "instance-uuid" + conn.compute.find_server.return_value = server + + conn.compute.migrations.return_value = [] + + result = _run( + [ + "--host", + "somehost", + "--server", + "srv1", + "--user", + "alice", + "--user-domain", + "somedomain", + "--project", + "someproject", + "--project-domain", + "somedomain", + "--status", + "running", + "--type", + "live-migration", + "--changes-since", + "2025-01-01T00:00:00", + "--changes-before", + "2025-01-02T00:00:00", + ], + conn, + ) + assert result is None + conn.identity.find_user.assert_called_once_with( + "alice", ignore_missing=True, domain_id="domain-id" + ) + conn.identity.find_project.assert_called_once_with( + "someproject", ignore_missing=True, domain_id="domain-id" + ) + conn.compute.find_server.assert_called_once_with( + "srv1", details=False, ignore_missing=False, all_projects=True + ) + conn.compute.migrations.assert_called_once_with( + host="somehost", + instance_uuid="instance-uuid", + status="running", + migration_type="live-migration", + user_id="user-id", + project_id="project-id", + changes_since=datetime.datetime(2025, 1, 1, 0, 0, 0), + changes_before=datetime.datetime(2025, 1, 2, 0, 0, 0), + ) + + +# --------------------------------------------------------------------------- +# ComputeStart / ComputeStop +# --------------------------------------------------------------------------- + + +def test_start_starts_only_shutoff_servers(loguru_logs): + conn = MagicMock() + conn.compute.servers.return_value = [ + _server("id1", "srv1", "SHUTOFF"), + _server("id2", "srv2", "ACTIVE"), + ] + prompt_mock = MagicMock(return_value="no") + result = _run_command( + compute.ComputeStart, ["--yes", "somehost"], conn, prompt_mock=prompt_mock + ) + assert result is None + # --yes starts without consulting the prompt. + prompt_mock.assert_not_called() + conn.compute.start_server.assert_called_once_with("id1") + assert any("cannot be started" in r["message"] for r in loguru_logs) + + +def test_start_prompt_no_skips(): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "SHUTOFF")] + prompt_mock = MagicMock(return_value="no") + result = _run_command( + compute.ComputeStart, ["somehost"], conn, prompt_mock=prompt_mock + ) + assert result is None + prompt_mock.assert_called_once() + conn.compute.start_server.assert_not_called() + + +def test_stop_stops_only_active_and_paused_servers(loguru_logs): + conn = MagicMock() + conn.compute.servers.return_value = [ + _server("id1", "srv1", "ACTIVE"), + _server("id2", "srv2", "PAUSED"), + _server("id3", "srv3", "SHUTOFF"), + ] + prompt_mock = MagicMock(return_value="no") + result = _run_command( + compute.ComputeStop, ["--yes", "somehost"], conn, prompt_mock=prompt_mock + ) + assert result is None + # --yes stops without consulting the prompt. + prompt_mock.assert_not_called() + assert conn.compute.stop_server.call_args_list == [call("id1"), call("id2")] + assert any("cannot be stopped" in r["message"] for r in loguru_logs) + + +def test_stop_prompt_no_skips(): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "ACTIVE")] + prompt_mock = MagicMock(return_value="no") + result = _run_command( + compute.ComputeStop, ["somehost"], conn, prompt_mock=prompt_mock + ) + assert result is None + prompt_mock.assert_called_once() + conn.compute.stop_server.assert_not_called() + + +# --------------------------------------------------------------------------- +# ComputeEvacuate +# --------------------------------------------------------------------------- + + +def test_evacuate_prompt_no_makes_no_changes(): + conn = MagicMock() + conn.compute.servers.return_value = [_server("id1", "srv1", "ACTIVE")] + prompt_mock = MagicMock(return_value="no") + result = _run_command( + compute.ComputeEvacuate, ["somehost"], conn, prompt_mock=prompt_mock + ) + assert result is None + prompt_mock.assert_called_once() + conn.compute.stop_server.assert_not_called() + conn.compute.update_service_forced_down.assert_not_called() + conn.compute.evacuate_server.assert_not_called() + conn.compute.disable_service.assert_not_called() + + +def test_evacuate_full_flow(loguru_logs): + conn = MagicMock() + conn.compute.servers.return_value = [ + _server("id1", "srv1", "ACTIVE"), + _server("id2", "srv2", "SHUTOFF"), + _server("id3", "srv3", "ERROR"), + ] + service = _service() + conn.compute.services.return_value = iter([service]) + # First poll: the stopped server reached SHUTOFF; second poll: the + # restarted server reached ACTIVE. + conn.compute.get_server.side_effect = [ + _polled_server("SHUTOFF"), + _polled_server("ACTIVE"), + ] + + result = _run_command( + compute.ComputeEvacuate, ["--yes", "--target", "target1", "somehost"], conn + ) + assert result is None + + # The ACTIVE server is stopped first and recorded for restart. + conn.compute.stop_server.assert_called_once_with("id1") + conn.compute.start_server.assert_called_once_with("id1") + + conn.compute.update_service_forced_down.assert_called_once_with( + service=service.id, host="somehost", binary="nova-compute", forced=True + ) + assert conn.compute.evacuate_server.call_args_list == [ + call("id1", host="target1"), + call("id2", host="target1"), + ] + conn.compute.disable_service.assert_called_once_with( + service=service.id, + host="somehost", + binary="nova-compute", + disabled_reason="EVACUATE", + ) + + # The service is forced down before any server is evacuated. + method_calls = [name for name, _args, _kwargs in conn.mock_calls] + assert method_calls.index( + "compute.update_service_forced_down" + ) < method_calls.index("compute.evacuate_server") + + assert any( + "srv3" in r["message"] and "cannot be evacuated" in r["message"] + for r in loguru_logs + ) diff --git a/tests/unit/commands/test_volume.py b/tests/unit/commands/test_volume.py index 715db028..c408affd 100644 --- a/tests/unit/commands/test_volume.py +++ b/tests/unit/commands/test_volume.py @@ -1,13 +1,18 @@ # SPDX-License-Identifier: Apache-2.0 -"""Tests for the ``osism volume list`` command. +"""Tests for the ``osism volume list`` and ``osism volume repair`` commands. -These focus on the exit-code contract: ``VolumeList.take_action`` must return a -non-zero exit status when a requested domain or project cannot be found, so a -failed lookup does not look like success. +``VolumeList`` must return a non-zero exit status when a requested domain or +project cannot be found, report volumes stuck in transitional states for more +than two hours, and list volumes per project when filtering by domain. +``VolumeRepair`` must only touch volumes that are actually stuck and honor the +confirmation prompts. """ -from unittest.mock import MagicMock, patch +from datetime import datetime, timedelta, timezone +from unittest.mock import MagicMock, call, patch + +import pytest from osism.commands import volume @@ -44,3 +49,188 @@ def test_returns_nonzero_when_project_not_found(): conn.identity.find_project.return_value = None result = _run(["--project", "p"], conn) assert result == 1 + + +# --------------------------------------------------------------------------- +# Shared helpers for the stuck-volume tests +# --------------------------------------------------------------------------- + +STUCK_STATUSES = ["detaching", "creating", "error_deleting", "deleting", "error"] + + +def _created_at(hours_ago): + # The command parses ``created_at`` with dateutil and localizes it via + # ``pytz.utc``, so the timestamp must be a naive ISO string. + return (datetime.now(timezone.utc) - timedelta(hours=hours_ago)).strftime( + "%Y-%m-%dT%H:%M:%S.%f" + ) + + +def _volume(volume_id, status, hours_ago, name="vol", volume_type="ssd"): + vol = MagicMock() + vol.id = volume_id + vol.name = name + vol.volume_type = volume_type + vol.status = status + vol.created_at = _created_at(hours_ago) + return vol + + +def _volumes_by_status(mapping): + def side_effect(all_projects=True, status=None, **kwargs): + return mapping.get(status, []) + + return side_effect + + +def _run_repair(args, conn, prompt_mock=None, sleep_mock=None): + cmd = volume.VolumeRepair(MagicMock(), MagicMock()) + parsed_args = cmd.get_parser("test").parse_args(args) + setup = MagicMock(return_value=("pw", [], None, True)) + getconn = MagicMock(return_value=conn) + cleanup = MagicMock() + if prompt_mock is None: + prompt_mock = MagicMock(return_value="yes") + if sleep_mock is None: + sleep_mock = MagicMock() + with patch( + "osism.tasks.openstack.get_cloud_helpers", + return_value=(setup, getconn, cleanup), + ), patch("osism.commands.volume.prompt", prompt_mock), patch( + "osism.commands.volume.sleep", sleep_mock + ): + return cmd.take_action(parsed_args) + + +# --------------------------------------------------------------------------- +# VolumeList +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("status", STUCK_STATUSES) +def test_list_reports_only_stuck_volumes(capsys, status): + conn = MagicMock() + stuck = _volume("stuckvol", status, hours_ago=3) + fresh = _volume("freshvol", status, hours_ago=1) + conn.block_storage.volumes.side_effect = _volumes_by_status( + {status: [stuck, fresh]} + ) + result = _run([], conn) + assert result is None + queried = {c.kwargs["status"] for c in conn.block_storage.volumes.call_args_list} + assert queried == set(STUCK_STATUSES) + out = capsys.readouterr().out + assert "stuckvol" in out + assert "freshvol" not in out + + +def test_list_domain_lists_volumes_per_project(capsys): + conn = MagicMock() + domain = MagicMock() + domain.id = "domainid" + conn.identity.find_domain.return_value = domain + + project1 = MagicMock() + project1.id = "projectid1" + project1.name = "project1" + project2 = MagicMock() + project2.id = "projectid2" + project2.name = "project2" + conn.identity.projects.return_value = [project1, project2] + + volumes = { + "projectid1": [_volume("volid1", "available", hours_ago=1)], + "projectid2": [_volume("volid2", "in-use", hours_ago=1)], + } + + def volumes_by_project(all_projects=True, project_id=None, **kwargs): + return volumes[project_id] + + conn.block_storage.volumes.side_effect = volumes_by_project + + result = _run(["--domain", "somedomain"], conn) + assert result is None + conn.identity.projects.assert_called_once_with(domain_id="domainid") + out = capsys.readouterr().out + assert "project1" in out + assert "volid1" in out + assert "project2" in out + assert "volid2" in out + + +# --------------------------------------------------------------------------- +# VolumeRepair +# --------------------------------------------------------------------------- + + +def test_repair_aborts_stuck_detaching_without_prompt(): + conn = MagicMock() + stuck = _volume("stuckvol", "detaching", hours_ago=3) + fresh = _volume("freshvol", "detaching", hours_ago=1) + conn.block_storage.volumes.side_effect = _volumes_by_status( + {"detaching": [stuck, fresh]} + ) + prompt_mock = MagicMock(return_value="yes") + result = _run_repair([], conn, prompt_mock=prompt_mock) + assert result is None + conn.block_storage.abort_volume_detaching.assert_called_once_with("stuckvol") + prompt_mock.assert_not_called() + conn.block_storage.delete_volume.assert_not_called() + + +def test_repair_deletes_stuck_creating_with_yes(): + conn = MagicMock() + conn.block_storage.volumes.side_effect = _volumes_by_status( + {"creating": [_volume("stuckvol", "creating", hours_ago=3)]} + ) + prompt_mock = MagicMock(return_value="no") + result = _run_repair(["--yes"], conn, prompt_mock=prompt_mock) + assert result is None + prompt_mock.assert_not_called() + conn.block_storage.delete_volume.assert_called_once_with("stuckvol", force=True) + + +def test_repair_stuck_creating_prompt_no_skips_delete(): + conn = MagicMock() + conn.block_storage.volumes.side_effect = _volumes_by_status( + {"creating": [_volume("stuckvol", "creating", hours_ago=3)]} + ) + prompt_mock = MagicMock(return_value="no") + result = _run_repair([], conn, prompt_mock=prompt_mock) + assert result is None + prompt_mock.assert_called_once() + conn.block_storage.delete_volume.assert_not_called() + + +def test_repair_error_deleting_prompted_regardless_of_age(): + conn = MagicMock() + # A fresh volume: ERROR_DELETING is handled without an age threshold. + conn.block_storage.volumes.side_effect = _volumes_by_status( + {"error_deleting": [_volume("brokenvol", "error_deleting", hours_ago=1)]} + ) + prompt_mock = MagicMock(return_value="yes") + result = _run_repair([], conn, prompt_mock=prompt_mock) + assert result is None + prompt_mock.assert_called_once() + conn.block_storage.delete_volume.assert_called_once_with("brokenvol", force=True) + + +def test_repair_stuck_deleting_resets_then_deletes(): + conn = MagicMock() + conn.block_storage.volumes.side_effect = _volumes_by_status( + {"deleting": [_volume("stuckvol", "deleting", hours_ago=3)]} + ) + sleep_mock = MagicMock() + manager = MagicMock() + manager.attach_mock(conn.block_storage.reset_volume_status, "reset") + manager.attach_mock(sleep_mock, "sleep") + manager.attach_mock(conn.block_storage.delete_volume, "delete") + result = _run_repair([], conn, sleep_mock=sleep_mock) + assert result is None + assert manager.mock_calls == [ + call.reset( + "stuckvol", status="available", attach_status=None, migration_status=None + ), + call.sleep(volume.SLEEP_WAIT_FOR_API), + call.delete("stuckvol", force=True), + ] From bdadd401938a4bb336f86458e19240697dd82c29 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Fri, 17 Jul 2026 15:20:02 +0200 Subject: [PATCH 4/5] Extend unit tests for status and report commands Add coverage for display_time, the Galera health checks, database and messaging status commands in log and script formats, RabbitMQ overview parsing with alarms and partitions, and the per-host memory, LLDP, BGP and bootstrap status report processing. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- tests/unit/commands/test_report.py | 346 ++++++++++++- tests/unit/commands/test_status.py | 777 ++++++++++++++++++++++++++++- 2 files changed, 1115 insertions(+), 8 deletions(-) diff --git a/tests/unit/commands/test_report.py b/tests/unit/commands/test_report.py index 61d15fa6..517cd9a1 100644 --- a/tests/unit/commands/test_report.py +++ b/tests/unit/commands/test_report.py @@ -2,14 +2,21 @@ """Tests for the ``osism report`` commands. -These focus on the exit-code contract when loading the Ansible inventory: a -command must return a non-zero exit status when the inventory query itself -cannot be run (a non-zero ansible-inventory return code, or a timeout), but -must keep returning success when the query runs fine and simply yields no -hosts. +The first group of tests covers the exit-code contract when loading the +Ansible inventory: a command must return a non-zero exit status when the +inventory query itself cannot be run (a non-zero ansible-inventory return +code, or a timeout), but must keep returning success when the query runs fine +and simply yields no hosts. + +The per-command tests then drive the SSH fan-out with mocked +``subprocess.run`` results: parsed rows must land in the printed table with +the documented defaults, while hosts whose output cannot be read or parsed +must end up in the failure summary instead of the table. """ +import json import subprocess +from contextlib import contextmanager from unittest.mock import MagicMock, patch import pytest @@ -84,3 +91,332 @@ def test_returns_success_when_inventory_is_empty(cls): result = cmd.take_action(parsed_args) assert not result + + +# --- per-host SSH fan-out helpers --- + + +def _proc(returncode=0, stdout="", stderr=""): + proc = MagicMock() + proc.returncode = returncode + proc.stdout = stdout + proc.stderr = stderr + return proc + + +@contextmanager +def _ssh_env(hosts, ssh_results): + """Patch the inventory pipeline so ``take_action`` iterates ``hosts``. + + ``subprocess.run`` first serves the ansible-inventory call, then hands out + ``ssh_results`` in order; exception instances in the list are raised. + """ + with patch( + "osism.commands.report.ensure_known_hosts_file", return_value=True + ), patch( + "osism.commands.report.get_inventory_path", + return_value="/ansible/inventory/hosts.yml", + ), patch( + "osism.commands.report.get_hosts_from_inventory", return_value=hosts + ), patch( + "osism.commands.report.resolve_host_with_fallback", + side_effect=lambda host: host, + ), patch( + "osism.commands.report.subprocess.run", + side_effect=[_proc(stdout="{}"), *ssh_results], + ): + yield + + +# --- Memory --- + + +def test_memory_sums_memory_over_hosts(capsys): + cmd, parsed_args = _make(report.Memory) + + with _ssh_env( + ["host-a", "host-b"], + [ + _proc(stdout="64\n"), + _proc(stdout="uuid-a\n"), + _proc(stdout="128\n"), + _proc(stdout="uuid-b\n"), + ], + ): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "uuid-a" in out + assert "uuid-b" in out + assert "Hosts: 2" in out + assert "Memory: 192 GB" in out + + +def test_memory_reports_uuid_as_na_when_unreadable(capsys): + cmd, parsed_args = _make(report.Memory) + + with _ssh_env(["host-a"], [_proc(stdout="64\n"), _proc(returncode=1)]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "n/a" in out + assert "Hosts: 1" in out + assert "Memory: 64 GB" in out + + +def test_memory_skips_host_when_memory_query_fails(capsys, loguru_logs): + cmd, parsed_args = _make(report.Memory) + + with _ssh_env( + ["host-a", "host-b"], + [ + _proc(returncode=1, stderr="ssh: connection refused"), + _proc(stdout="32\n"), + _proc(stdout="uuid-b\n"), + ], + ): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "Hosts: 1" in out + assert "Memory: 32 GB" in out + messages = [record["message"] for record in loguru_logs] + assert any("Failed to get memory info from host-a" in m for m in messages) + assert any("Failed to query 1 host(s): host-a" in m for m in messages) + + +def test_memory_marks_host_failed_on_timeout(loguru_logs): + cmd, parsed_args = _make(report.Memory) + + with _ssh_env(["host-a"], [subprocess.TimeoutExpired("ssh", 30)]): + result = cmd.take_action(parsed_args) + + assert not result + messages = [record["message"] for record in loguru_logs] + assert any("Timeout connecting to host-a." in m for m in messages) + assert any("Failed to query 1 host(s): host-a" in m for m in messages) + + +def test_memory_marks_host_failed_on_unparsable_output(loguru_logs): + cmd, parsed_args = _make(report.Memory) + + with _ssh_env(["host-a"], [_proc(stdout="lots\n"), _proc(stdout="uuid-a\n")]): + result = cmd.take_action(parsed_args) + + assert not result + messages = [record["message"] for record in loguru_logs] + assert any("Could not parse memory info from host-a." in m for m in messages) + assert any("Failed to query 1 host(s): host-a" in m for m in messages) + + +# --- Lldp --- + +IFACE_ETH0 = { + "chassis": {"switch-1": {"id": {"value": "aa:bb:cc"}}}, + "port": {"id": {"value": "Ethernet10"}, "descr": "downlink"}, + "age": "12 days", +} + +IFACE_ETH1 = { + "chassis": {"switch-2": {"id": {"value": "dd:ee:ff"}}}, + "port": {"id": {"value": "Ethernet20"}, "descr": "uplink"}, + "age": "3 days", +} + + +def _lldp_json(interface): + return json.dumps({"lldp": {"interface": interface}}) + + +def test_lldp_normalizes_single_interface_dict(capsys): + cmd, parsed_args = _make(report.Lldp) + + with _ssh_env(["host-a"], [_proc(stdout=_lldp_json({"eth0": IFACE_ETH0}))]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "eth0" in out + assert "switch-1" in out + assert "Ethernet10" in out + assert "Neighbors: 1" in out + + +def test_lldp_handles_interface_list(capsys): + cmd, parsed_args = _make(report.Lldp) + interfaces = [{"eth0": IFACE_ETH0}, {"eth1": IFACE_ETH1}] + + with _ssh_env(["host-a"], [_proc(stdout=_lldp_json(interfaces))]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "eth0" in out + assert "eth1" in out + assert "switch-2" in out + assert "Ethernet20" in out + assert "Neighbors: 2" in out + + +def test_lldp_defaults_missing_neighbor_details(capsys): + cmd, parsed_args = _make(report.Lldp) + + with _ssh_env(["host-a"], [_proc(stdout=_lldp_json({"eth0": {}}))]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "Neighbors: 1" in out + # Remote switch, remote port and age all fall back to "n/a". + assert out.count("n/a") >= 3 + + +def test_lldp_marks_host_failed_on_invalid_json(loguru_logs): + cmd, parsed_args = _make(report.Lldp) + + with _ssh_env(["host-a"], [_proc(stdout="not json")]): + result = cmd.take_action(parsed_args) + + assert not result + messages = [record["message"] for record in loguru_logs] + assert any("Could not parse LLDP info from host-a." in m for m in messages) + assert any("Failed to query 1 host(s): host-a" in m for m in messages) + + +# --- Bgp --- + + +def test_bgp_afi_filter_matches_case_insensitively(capsys): + cmd = report.Bgp(MagicMock(), MagicMock()) + parsed_args = cmd.get_parser("test").parse_args(["--afi", "ipv4Unicast"]) + bgp_data = { + # Deliberately differs in case from the parsed choice "ipv4Unicast". + "IPv4Unicast": { + "peers": {"10.0.0.1": {"hostname": "spine-1", "state": "Established"}} + }, + "l2VpnEvpn": { + "peers": {"10.0.0.2": {"hostname": "spine-2", "state": "Established"}} + }, + } + + with _ssh_env(["host-a"], [_proc(stdout=json.dumps(bgp_data))]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "spine-1" in out + assert "spine-2" not in out + assert "Sessions: 1" in out + + +def test_bgp_defaults_missing_peer_fields(capsys): + cmd, parsed_args = _make(report.Bgp) + bgp_data = {"ipv4Unicast": {"peers": {"10.0.0.1": {}}}} + + with _ssh_env(["host-a"], [_proc(stdout=json.dumps(bgp_data))]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "10.0.0.1" in out + assert "n/a" in out + assert "Sessions: 1" in out + assert "Established: 0/1" in out + + +def test_bgp_counts_established_sessions(capsys): + cmd, parsed_args = _make(report.Bgp) + bgp_data = { + "ipv4Unicast": { + "peers": { + "10.0.0.1": {"state": "Established"}, + "10.0.0.2": {"state": "Active"}, + } + } + } + + with _ssh_env(["host-a"], [_proc(stdout=json.dumps(bgp_data))]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "Sessions: 2" in out + assert "Established: 1/2" in out + + +# --- Status --- + + +def test_status_parses_bootstrap_facts(capsys): + cmd, parsed_args = _make(report.Status) + fact = "[bootstrap]\nstatus = True\ntimestamp = 2026-01-01T00:00:00Z\n" + + with _ssh_env(["host-a"], [_proc(stdout=fact)]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "host-a" in out + assert "True" in out + assert "2026-01-01T00:00:00Z" in out + assert "Hosts: 1" in out + + +def test_status_falls_back_when_section_missing(capsys): + cmd, parsed_args = _make(report.Status) + + with _ssh_env(["host-a"], [_proc(stdout="[other]\nstatus = True\n")]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "False" in out + assert "n/a" in out + assert "Hosts: 1" in out + + +def test_status_reports_unreachable_host_as_false(capsys, loguru_logs): + cmd, parsed_args = _make(report.Status) + + with _ssh_env(["host-a"], [_proc(returncode=1)]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "host-a" in out + assert "False" in out + assert "n/a" in out + assert "Hosts: 1" in out + # An unreachable host is reported in the table, not as a query failure. + assert not any("Failed to query" in record["message"] for record in loguru_logs) + + +def test_status_filter_drops_non_matching_rows(capsys): + cmd = report.Status(MagicMock(), MagicMock()) + parsed_args = cmd.get_parser("test").parse_args(["bootstrap", "--status", "True"]) + fact_a = "[bootstrap]\nstatus = True\ntimestamp = t1\n" + fact_b = "[bootstrap]\nstatus = False\ntimestamp = t2\n" + + with _ssh_env(["host-a", "host-b"], [_proc(stdout=fact_a), _proc(stdout=fact_b)]): + result = cmd.take_action(parsed_args) + + assert not result + out = capsys.readouterr().out + assert "host-a" in out + assert "host-b" not in out + assert "Hosts: 1" in out + + +def test_status_marks_host_failed_on_unparsable_facts(loguru_logs): + cmd, parsed_args = _make(report.Status) + + with _ssh_env(["host-a"], [_proc(stdout="status True without a section header\n")]): + result = cmd.take_action(parsed_args) + + assert not result + messages = [record["message"] for record in loguru_logs] + assert any("Could not parse bootstrap info from host-a." in m for m in messages) + assert any("Failed to query 1 host(s): host-a" in m for m in messages) diff --git a/tests/unit/commands/test_status.py b/tests/unit/commands/test_status.py index fb2589fb..56dbe003 100644 --- a/tests/unit/commands/test_status.py +++ b/tests/unit/commands/test_status.py @@ -2,12 +2,22 @@ """Tests for the ``osism status`` commands. -A request for an unknown resource type is invalid input and must yield a -non-zero exit status rather than falling through to an implicit success. +Covers the ``display_time`` helper, the Celery worker overview (``Run``), the +MariaDB Galera cluster validation (``Database``) and the RabbitMQ cluster +validation (``Messaging``). All external services are replaced by mocks: the +MariaDB connection object, the RabbitMQ management API (``requests.get``) and +the inventory/password helpers from ``osism.utils.rabbitmq``. The commands +must return non-zero whenever a prerequisite (configuration, password, +connectivity) is missing or a health check fails, and zero only for a clean +cluster. """ import argparse -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, mock_open, patch + +import pymysql +import pytest +import requests from osism.commands import status @@ -20,3 +30,764 @@ def test_run_returns_1_for_unknown_resource_type(): result = cmd.take_action(parsed_args) assert result == 1 + + +# --- display_time --- + + +@pytest.mark.parametrize( + ("seconds", "granularity", "expected"), + [ + (0, 2, ""), + (1, 2, "1 second"), + (90061, 2, "1 day, 1 hour"), + (90061, 5, "1 day, 1 hour, 1 minute, 1 second"), + (604800, 2, "1 week"), + ], +) +def test_display_time(seconds, granularity, expected): + assert status.display_time(seconds, granularity) == expected + + +# --- Run (workers) --- + + +def test_run_workers_reports_uptime_and_reachability(capsys): + cmd = status.Run(MagicMock(), MagicMock()) + parsed_args = argparse.Namespace(type=["workers"]) + + inspector = MagicMock() + inspector.stats.return_value = { + "worker-b": {"uptime": 90061}, + "worker-a": {"uptime": 1}, + } + inspector.ping.side_effect = lambda destination: ( + [{"worker-a": {"ok": "pong"}}] if destination == ["worker-a"] else None + ) + + with patch("celery.Celery") as celery_cls: + celery_cls.return_value.control.inspect.return_value = inspector + result = cmd.take_action(parsed_args) + + assert not result + lines = capsys.readouterr().out.splitlines() + line_a = next(line for line in lines if "worker-a" in line) + line_b = next(line for line in lines if "worker-b" in line) + assert "1 second" in line_a + assert "REACHABLE" in line_a + assert "NOT REACHABLE" not in line_a + assert "1 day, 1 hour" in line_b + assert "NOT REACHABLE" in line_b + + +# --- Database helpers --- + + +def _database(format="log"): + cmd = status.Database(MagicMock(), MagicMock()) + return cmd, cmd.get_parser("test").parse_args(["--format", format]) + + +HEALTHY_WSREP_ROWS = [ + ("wsrep_cluster_status", "Primary"), + ("wsrep_connected", "ON"), + ("wsrep_ready", "ON"), + ("wsrep_cluster_size", "3"), + ("wsrep_local_state_comment", "Synced"), +] + +GENERAL_ROWS = [ + ("Uptime", "90061"), + ("Threads_connected", "5"), + ("Threads_running", "1"), + ("Questions", "1000"), + ("Slow_queries", "0"), + ("Aborted_connects", "0"), +] + + +def _galera_connection(wsrep_rows, general_rows=GENERAL_ROWS): + """Mock a PyMySQL connection serving the two SHOW STATUS queries.""" + connection = MagicMock() + cursor = connection.cursor.return_value.__enter__.return_value + cursor.fetchall.side_effect = [list(wsrep_rows), list(general_rows)] + return connection + + +def _override(rows, name, value): + return [(n, value if n == name else v) for n, v in rows] + + +# --- Database._load_kolla_configuration --- + + +def test_load_kolla_configuration_returns_none_when_file_missing(loguru_logs): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=False): + assert cmd._load_kolla_configuration() is None + + assert any( + "Configuration file not found" in record["message"] for record in loguru_logs + ) + + +def test_load_kolla_configuration_parses_yaml(): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=True), patch( + "builtins.open", + mock_open(read_data="kolla_internal_vip_address: 192.168.16.9\n"), + ): + config = cmd._load_kolla_configuration() + + assert config == {"kolla_internal_vip_address": "192.168.16.9"} + + +def test_load_kolla_configuration_returns_none_on_read_error(loguru_logs): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=True), patch( + "builtins.open", side_effect=OSError("permission denied") + ): + assert cmd._load_kolla_configuration() is None + + assert any( + "Failed to load configuration" in record["message"] for record in loguru_logs + ) + + +# --- Database._load_database_password --- + + +def test_load_database_password_returns_none_when_secrets_file_missing(loguru_logs): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=False): + assert cmd._load_database_password() is None + + assert any("Secrets file not found" in record["message"] for record in loguru_logs) + + +@pytest.mark.parametrize("secrets", [None, {}, "not-a-mapping", ["a", "b"]]) +def test_load_database_password_rejects_invalid_secrets(secrets, loguru_logs): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=True), patch( + "osism.tasks.conductor.utils.load_yaml_file", return_value=secrets + ): + assert cmd._load_database_password() is None + + assert any( + "Empty or invalid secrets file" in record["message"] for record in loguru_logs + ) + + +def test_load_database_password_requires_database_password_key(loguru_logs): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=True), patch( + "osism.tasks.conductor.utils.load_yaml_file", + return_value={"another_password": "x"}, + ): + assert cmd._load_database_password() is None + + assert any( + "database_password not found" in record["message"] for record in loguru_logs + ) + + +@pytest.mark.parametrize( + ("raw", "expected"), + [(12345, "12345"), (" s3cret\n", "s3cret")], +) +def test_load_database_password_coerces_and_strips(raw, expected): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=True), patch( + "osism.tasks.conductor.utils.load_yaml_file", + return_value={"database_password": raw}, + ): + assert cmd._load_database_password() == expected + + +def test_load_database_password_returns_none_when_loader_raises(loguru_logs): + cmd, _ = _database() + + with patch("osism.commands.status.os.path.exists", return_value=True), patch( + "osism.tasks.conductor.utils.load_yaml_file", + side_effect=RuntimeError("vault broken"), + ): + assert cmd._load_database_password() is None + + assert any( + "Failed to load database password" in record["message"] + for record in loguru_logs + ) + + +# --- Database._check_galera_status --- + + +def test_check_galera_status_healthy_cluster_has_no_findings(): + cmd, _ = _database() + connection = _galera_connection(HEALTHY_WSREP_ROWS) + + results, errors, warnings = cmd._check_galera_status(connection) + + assert errors == [] + assert warnings == [] + assert results["cluster_status"] == "Primary" + assert results["connected"] == "ON" + assert results["ready"] == "ON" + assert results["cluster_size"] == "3" + assert results["local_state"] == "Synced" + assert results["uptime"] == "90061" + assert results["threads_connected"] == "5" + + +@pytest.mark.parametrize( + ("name", "value", "fragment"), + [ + ("wsrep_cluster_status", "non-Primary", "expected 'Primary'"), + ("wsrep_connected", "OFF", "Cluster connected is 'OFF'"), + ("wsrep_ready", "OFF", "Cluster ready is 'OFF'"), + ("wsrep_cluster_size", "0", "Cluster size is 0"), + ("wsrep_cluster_size", "many", "Invalid cluster size: many"), + ("wsrep_local_state_comment", "Donor/Desynced", "expected 'Synced'"), + ], +) +def test_check_galera_status_reports_each_failing_check(name, value, fragment): + cmd, _ = _database() + connection = _galera_connection(_override(HEALTHY_WSREP_ROWS, name, value)) + + _, errors, _ = cmd._check_galera_status(connection) + + assert len(errors) == 1 + assert fragment in errors[0] + + +@pytest.mark.parametrize( + ("name", "value", "fragment"), + [ + ("wsrep_flow_control_paused", "0.10", "Flow control paused"), + ("wsrep_local_recv_queue_avg", "0.75", "receive queue average"), + ("wsrep_local_cert_failures", "2", "Certification failures: 2"), + ], +) +def test_check_galera_status_warns_on_replication_pressure(name, value, fragment): + cmd, _ = _database() + connection = _galera_connection(HEALTHY_WSREP_ROWS + [(name, value)]) + + _, errors, warnings = cmd._check_galera_status(connection) + + assert errors == [] + assert len(warnings) == 1 + assert fragment in warnings[0] + + +def test_check_galera_status_ignores_non_numeric_metrics(): + cmd, _ = _database() + connection = _galera_connection( + HEALTHY_WSREP_ROWS + + [ + ("wsrep_flow_control_paused", "unknown"), + ("wsrep_local_recv_queue_avg", "unknown"), + ("wsrep_local_cert_failures", "unknown"), + ] + ) + + _, errors, warnings = cmd._check_galera_status(connection) + + assert errors == [] + assert warnings == [] + + +def test_check_galera_status_reports_query_failure_once(): + cmd, _ = _database() + connection = MagicMock() + connection.cursor.side_effect = RuntimeError("connection lost") + + _, errors, warnings = cmd._check_galera_status(connection) + + assert errors == ["Failed to query Galera status: connection lost"] + assert warnings == [] + + +# --- Database.take_action --- + + +def test_database_returns_1_when_configuration_missing(): + cmd, parsed_args = _database() + + with patch.object(status.Database, "_load_kolla_configuration", return_value=None): + assert cmd.take_action(parsed_args) == 1 + + +def test_database_returns_1_without_vip_address(): + cmd, parsed_args = _database() + + with patch.object(status.Database, "_load_kolla_configuration", return_value={}): + assert cmd.take_action(parsed_args) == 1 + + +def test_database_returns_1_when_password_unavailable(): + cmd, parsed_args = _database() + + with patch.object( + status.Database, + "_load_kolla_configuration", + return_value={"kolla_internal_vip_address": "192.168.16.9"}, + ), patch.object(status.Database, "_load_database_password", return_value=None): + assert cmd.take_action(parsed_args) == 1 + + +@pytest.mark.parametrize("format", ["log", "script"]) +def test_database_returns_1_on_connection_error(format, capsys, loguru_logs): + cmd, parsed_args = _database(format) + + with patch.object( + status.Database, + "_load_kolla_configuration", + return_value={"kolla_internal_vip_address": "192.168.16.9"}, + ), patch.object( + status.Database, "_load_database_password", return_value="s3cret" + ), patch( + "osism.utils.mariadb.connect", side_effect=pymysql.Error("vip unreachable") + ): + result = cmd.take_action(parsed_args) + + assert result == 1 + if format == "script": + assert "FAILED: Connection error - vip unreachable" in capsys.readouterr().out + else: + assert any( + "Failed to connect to MariaDB" in record["message"] + for record in loguru_logs + ) + + +@pytest.mark.parametrize("format", ["log", "script"]) +def test_database_passes_on_healthy_cluster(format, capsys, loguru_logs): + cmd, parsed_args = _database(format) + connection = _galera_connection(HEALTHY_WSREP_ROWS) + + with patch.object( + status.Database, + "_load_kolla_configuration", + return_value={"kolla_internal_vip_address": "192.168.16.9"}, + ), patch.object( + status.Database, "_load_database_password", return_value="s3cret" + ), patch( + "osism.utils.mariadb.connect", return_value=connection + ) as connect_mock: + result = cmd.take_action(parsed_args) + + assert result == 0 + connect_mock.assert_called_once_with( + "192.168.16.9", "root", "s3cret", port=3306, connect_timeout=10 + ) + connection.close.assert_called_once_with() + if format == "script": + assert "PASSED" in capsys.readouterr().out + else: + assert any( + "MariaDB Galera Cluster validation PASSED" in record["message"] + for record in loguru_logs + ) + + +@pytest.mark.parametrize("format", ["log", "script"]) +def test_database_fails_on_unhealthy_cluster(format, capsys, loguru_logs): + cmd, parsed_args = _database(format) + connection = _galera_connection( + _override(HEALTHY_WSREP_ROWS, "wsrep_cluster_status", "non-Primary") + ) + + with patch.object( + status.Database, + "_load_kolla_configuration", + return_value={"kolla_internal_vip_address": "192.168.16.9"}, + ), patch.object( + status.Database, "_load_database_password", return_value="s3cret" + ), patch( + "osism.utils.mariadb.connect", return_value=connection + ): + result = cmd.take_action(parsed_args) + + assert result == 1 + connection.close.assert_called_once_with() + if format == "script": + out = capsys.readouterr().out + assert "FAILED" in out + assert "- Cluster status is 'non-Primary'" in out + else: + assert any( + "MariaDB Galera Cluster validation FAILED" in record["message"] + for record in loguru_logs + ) + + +# --- Messaging helpers --- + + +def _messaging(argv=None): + cmd = status.Messaging(MagicMock(), MagicMock()) + return cmd, cmd.get_parser("test").parse_args(argv or []) + + +def _response(payload): + response = MagicMock() + response.json.return_value = payload + return response + + +OVERVIEW = { + "rabbitmq_version": "3.13.7", + "erlang_version": "26.2.5", + "cluster_name": "rabbit@testbed", + "object_totals": {"connections": 10, "channels": 20, "queues": 30}, + "queue_totals": { + "messages": 6, + "messages_ready": 4, + "messages_unacknowledged": 2, + }, + "message_stats": { + "publish_details": {"rate": 1.5}, + "deliver_get_details": {"rate": 2.5}, + }, +} + + +def _node(name, **overrides): + node = { + "name": name, + "running": True, + "mem_alarm": False, + "disk_free_alarm": False, + "partitions": [], + "disk_free": 50 * 1024**3, + "disk_free_limit": 10 * 1024**3, + "mem_used": 2 * 1024**3, + "mem_limit": 8 * 1024**3, + "fd_used": 100, + "fd_total": 1048576, + "sockets_used": 10, + "sockets_total": 943626, + } + node.update(overrides) + return node + + +def _rabbitmq_api(nodes, overview=OVERVIEW, alarms=None): + """Responses for the three management API calls, in request order.""" + return [ + _response(overview), + _response(nodes), + _response(alarms or {"status": "ok"}), + ] + + +def _node_results(**overrides): + """A fully healthy per-node result dict, as the log format consumes it.""" + results = { + "cluster_name": "rabbit@testbed", + "rabbitmq_version": "3.13.7", + "erlang_version": "26.2.5", + "nodes": ["rabbit@node-a"], + "running_nodes": ["rabbit@node-a"], + "partitioned_nodes": [], + "alarms": [], + "cluster_size": 1, + "total_connections": 10, + "total_channels": 20, + "total_queues": 30, + "total_messages": 6, + "messages_ready": 4, + "messages_unacked": 2, + "publish_rate": 1.5, + "deliver_rate": 2.5, + "disk_free": 50 * 1024**3, + "disk_free_limit": 10 * 1024**3, + "mem_used": 2 * 1024**3, + "mem_limit": 8 * 1024**3, + "fd_used": 100, + "fd_total": 1048576, + "sockets_used": 10, + "sockets_total": 943626, + } + results.update(overrides) + return results + + +# --- Messaging._check_rabbitmq_status --- + + +def test_check_rabbitmq_status_parses_overview(): + cmd, _ = _messaging() + + with patch("requests.get", side_effect=_rabbitmq_api([_node("rabbit@node-a")])): + results, errors = cmd._check_rabbitmq_status("10.0.0.5", "openstack", "pw") + + assert errors == [] + assert results["rabbitmq_version"] == "3.13.7" + assert results["erlang_version"] == "26.2.5" + assert results["cluster_name"] == "rabbit@testbed" + assert results["total_connections"] == 10 + assert results["total_channels"] == 20 + assert results["total_queues"] == 30 + assert results["total_messages"] == 6 + assert results["messages_ready"] == 4 + assert results["messages_unacked"] == 2 + assert results["publish_rate"] == 1.5 + assert results["deliver_rate"] == 2.5 + assert results["cluster_size"] == 1 + assert results["running_nodes"] == ["rabbit@node-a"] + + +def test_check_rabbitmq_status_flags_stopped_node(): + cmd, _ = _messaging() + nodes = [_node("rabbit@node-a"), _node("rabbit@node-b", running=False)] + + with patch("requests.get", side_effect=_rabbitmq_api(nodes)): + results, errors = cmd._check_rabbitmq_status("10.0.0.5", "openstack", "pw") + + assert errors == ["Node 'rabbit@node-b' is not running"] + assert results["nodes"] == ["rabbit@node-a", "rabbit@node-b"] + assert results["running_nodes"] == ["rabbit@node-a"] + + +def test_check_rabbitmq_status_flags_resource_alarms(): + cmd, _ = _messaging() + nodes = [_node("rabbit@node-a", mem_alarm=True, disk_free_alarm=True)] + + with patch("requests.get", side_effect=_rabbitmq_api(nodes)): + _, errors = cmd._check_rabbitmq_status("10.0.0.5", "openstack", "pw") + + assert "Memory alarm on node 'rabbit@node-a'" in errors + assert "Disk free alarm on node 'rabbit@node-a'" in errors + + +def test_check_rabbitmq_status_treats_partitions_as_critical(): + cmd, _ = _messaging() + nodes = [_node("rabbit@node-a", partitions=["rabbit@node-b"])] + + with patch("requests.get", side_effect=_rabbitmq_api(nodes)): + results, errors = cmd._check_rabbitmq_status("10.0.0.5", "openstack", "pw") + + assert results["partitioned_nodes"] == ["rabbit@node-a"] + assert len(errors) == 1 + assert errors[0].startswith("CRITICAL: Node 'rabbit@node-a' has partitions") + + +def test_check_rabbitmq_status_reads_resources_from_target_node(): + cmd, _ = _messaging() + nodes = [ + _node("rabbit@node-a", disk_free=111), + _node("rabbit@node-b", disk_free=222), + ] + + with patch("requests.get", side_effect=_rabbitmq_api(nodes)): + results, errors = cmd._check_rabbitmq_status( + "10.0.0.5", "openstack", "pw", target_host="node-b" + ) + + assert errors == [] + assert results["disk_free"] == 222 + + +def test_check_rabbitmq_status_defaults_resources_to_first_node(): + cmd, _ = _messaging() + nodes = [ + _node("rabbit@node-a", disk_free=111), + _node("rabbit@node-b", disk_free=222), + ] + + with patch("requests.get", side_effect=_rabbitmq_api(nodes)): + results, _ = cmd._check_rabbitmq_status("10.0.0.5", "openstack", "pw") + + assert results["disk_free"] == 111 + + +def test_check_rabbitmq_status_collects_all_endpoint_failures(): + cmd, _ = _messaging() + + with patch( + "requests.get", + side_effect=[ + requests.exceptions.RequestException("overview down"), + requests.exceptions.RequestException("nodes down"), + requests.exceptions.RequestException("alarms down"), + ], + ): + _, errors = cmd._check_rabbitmq_status("10.0.0.5", "openstack", "pw") + + assert len(errors) == 3 + assert "Failed to get overview: overview down" in errors + assert "Failed to get nodes information: nodes down" in errors + assert "Failed to check health alarms: alarms down" in errors + + +def test_check_rabbitmq_status_reports_health_alarms(): + cmd, _ = _messaging() + api = _rabbitmq_api( + [_node("rabbit@node-a")], + alarms={"status": "failed", "alarms": ["file_descriptor_limit"]}, + ) + + with patch("requests.get", side_effect=api): + results, errors = cmd._check_rabbitmq_status("10.0.0.5", "openstack", "pw") + + assert results["alarms"] == ["file_descriptor_limit"] + assert "Alarm: file_descriptor_limit" in errors + + +# --- Messaging.take_action --- + + +def test_messaging_returns_1_without_node_addresses(loguru_logs): + cmd, parsed_args = _messaging() + + with patch("osism.utils.rabbitmq.get_rabbitmq_node_addresses", return_value=None): + assert cmd.take_action(parsed_args) == 1 + + assert any( + "Failed to get RabbitMQ node addresses" in record["message"] + for record in loguru_logs + ) + + +def test_messaging_warns_about_unknown_filter_host(loguru_logs): + cmd, parsed_args = _messaging(["node-a", "ghost"]) + + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=[("10.0.0.5", "node-a")], + ), patch( + "osism.utils.rabbitmq.load_rabbitmq_password", return_value="pw" + ), patch.object( + status.Messaging, "_check_rabbitmq_status", return_value=(_node_results(), []) + ): + result = cmd.take_action(parsed_args) + + assert result == 0 + assert any( + "Host 'ghost' not found in rabbitmq group" in record["message"] + for record in loguru_logs + ) + + +def test_messaging_returns_1_when_no_filter_host_matches(loguru_logs): + cmd, parsed_args = _messaging(["ghost"]) + + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=[("10.0.0.5", "node-a")], + ): + assert cmd.take_action(parsed_args) == 1 + + assert any( + "None of the specified hosts found" in record["message"] + for record in loguru_logs + ) + + +def test_messaging_returns_1_without_password(loguru_logs): + cmd, parsed_args = _messaging() + + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=[("10.0.0.5", "node-a")], + ), patch("osism.utils.rabbitmq.load_rabbitmq_password", return_value=None): + assert cmd.take_action(parsed_args) == 1 + + assert any( + "Failed to load RabbitMQ password" in record["message"] + for record in loguru_logs + ) + + +def test_messaging_script_format_lists_node_errors(capsys): + cmd, parsed_args = _messaging(["--format", "script"]) + + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=[("10.0.0.5", "node-a")], + ), patch( + "osism.utils.rabbitmq.load_rabbitmq_password", return_value="pw" + ), patch.object( + status.Messaging, + "_check_rabbitmq_status", + return_value=(_node_results(), ["Node 'rabbit@node-a' is not running"]), + ): + result = cmd.take_action(parsed_args) + + assert result == 1 + out = capsys.readouterr().out + assert "FAILED" in out + assert "- [node-a] Node 'rabbit@node-a' is not running" in out + + +def test_messaging_script_format_passes_when_clean(capsys): + cmd, parsed_args = _messaging(["--format", "script"]) + + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=[("10.0.0.5", "node-a")], + ), patch( + "osism.utils.rabbitmq.load_rabbitmq_password", return_value="pw" + ), patch.object( + status.Messaging, "_check_rabbitmq_status", return_value=(_node_results(), []) + ): + result = cmd.take_action(parsed_args) + + assert result == 0 + assert "PASSED" in capsys.readouterr().out + + +def test_messaging_log_format_passes_when_clean(loguru_logs): + cmd, parsed_args = _messaging() + + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=[("10.0.0.5", "node-a")], + ), patch( + "osism.utils.rabbitmq.load_rabbitmq_password", return_value="pw" + ), patch.object( + status.Messaging, "_check_rabbitmq_status", return_value=(_node_results(), []) + ): + result = cmd.take_action(parsed_args) + + assert result == 0 + assert any( + "RabbitMQ Cluster validation PASSED" in record["message"] + for record in loguru_logs + ) + + +def test_messaging_log_format_fails_on_node_errors(loguru_logs): + cmd, parsed_args = _messaging() + results = _node_results( + partitioned_nodes=["rabbit@node-a"], alarms=["file_descriptor_limit"] + ) + + with patch( + "osism.utils.rabbitmq.get_rabbitmq_node_addresses", + return_value=[("10.0.0.5", "node-a")], + ), patch( + "osism.utils.rabbitmq.load_rabbitmq_password", return_value="pw" + ), patch.object( + status.Messaging, + "_check_rabbitmq_status", + return_value=(results, ["CRITICAL: Node 'rabbit@node-a' has partitions"]), + ): + result = cmd.take_action(parsed_args) + + assert result == 1 + messages = [record["message"] for record in loguru_logs] + assert any("Partitioned Nodes: rabbit@node-a" in message for message in messages) + assert any( + "[node-a] CRITICAL: Node 'rabbit@node-a' has partitions" in message + for message in messages + ) + assert any("RabbitMQ Cluster validation FAILED" in message for message in messages) From f405b5fad06cebb4ef4c76e01f4b42117214ecbb Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Fri, 17 Jul 2026 15:20:02 +0200 Subject: [PATCH 5/5] Add manage command wiring tests and extend vault command tests Assert the Celery task signatures built by the image, flavor, dnsmasq and project manage commands, and cover vault password set/unset and the vault check failure chain including the Fernet key round-trip. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- tests/unit/commands/test_loadbalancer.py | 1 - tests/unit/commands/test_manage_commands.py | 445 ++++++++++++++++++++ tests/unit/commands/test_migrate.py | 1 - tests/unit/commands/test_vault.py | 350 +++++++++++++++ 4 files changed, 795 insertions(+), 2 deletions(-) create mode 100644 tests/unit/commands/test_manage_commands.py diff --git a/tests/unit/commands/test_loadbalancer.py b/tests/unit/commands/test_loadbalancer.py index e310e6d8..fb9c0593 100644 --- a/tests/unit/commands/test_loadbalancer.py +++ b/tests/unit/commands/test_loadbalancer.py @@ -20,7 +20,6 @@ from ._helpers import parse_args - # --- _load_kolla_configuration --- diff --git a/tests/unit/commands/test_manage_commands.py b/tests/unit/commands/test_manage_commands.py new file mode 100644 index 00000000..ed72a332 --- /dev/null +++ b/tests/unit/commands/test_manage_commands.py @@ -0,0 +1,445 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Argument-assembly wiring tests for the ``osism manage`` commands. + +Each command builds a Celery task signature via ``.si(...)`` and hands the +resulting task to ``handle_task``. These tests parse a real argument vector, +patch the task modules and assert on the exact argument list the signature +receives - no broker is involved. Validator wiring to ``fetch_text`` is +covered separately in ``test_manage_wiring.py``. +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from osism.commands import manage + +from ._helpers import assert_not_called_before_lock_check, parse_args + + +def _run_image_command(command_class, argv, fetch_bodies): + """Drive an Image* command with mocked fetch and task plumbing. + + Returns ``(mock_fetch, mock_manager, mock_handle)``; the image-manager + signature call is reachable via ``mock_manager.si.call_args``. + """ + cmd, parsed_args = parse_args(command_class, argv) + + with patch.object(manage.utils, "check_task_lock_and_exit") as mock_check, patch( + "osism.commands.manage.fetch_text" + ) as mock_fetch, patch("osism.tasks.openstack.image_manager") as mock_im, patch( + "osism.tasks.handle_task" + ) as mock_handle: + mock_check.side_effect = assert_not_called_before_lock_check(mock_im.si) + mock_fetch.side_effect = fetch_bodies + mock_im.si.return_value.apply_async.return_value = MagicMock(task_id="x") + mock_handle.return_value = 0 + cmd.take_action(parsed_args) + + mock_check.assert_called_once() + return mock_fetch, mock_im, mock_handle + + +def _run_task_command(command_class, argv, task_target): + """Drive a command whose take_action only schedules a Celery task. + + Returns ``(mock_task, mock_handle, task)`` where ``task`` is the object + ``apply_async`` produced and ``handle_task`` received. + """ + cmd, parsed_args = parse_args(command_class, argv) + + with patch.object(manage.utils, "check_task_lock_and_exit") as mock_check, patch( + task_target + ) as mock_task, patch("osism.tasks.handle_task") as mock_handle: + mock_check.side_effect = assert_not_called_before_lock_check(mock_task.si) + task = MagicMock(task_id="x") + mock_task.si.return_value.apply_async.return_value = task + mock_handle.return_value = 0 + cmd.take_action(parsed_args) + + mock_check.assert_called_once() + return mock_task, mock_handle, task + + +# --- ImageClusterapi.take_action --- + + +def test_clusterapi_default_iterates_all_supported_releases(): + fetch_bodies = [] + for release in manage.SUPPORTED_CLUSTERAPI_K8S_IMAGES: + fetch_bodies.append(f"2026-04-12 ubuntu-2404-kube-v{release}.5.qcow2") + fetch_bodies.append("a" * 64) + + mock_fetch, mock_im, _ = _run_image_command( + manage.ImageClusterapi, + ["--no-wait", "--base-url", "https://example.com/capi/"], + fetch_bodies, + ) + + assert mock_fetch.call_count == 6 + marker_urls = [call.args[0] for call in mock_fetch.call_args_list[::2]] + assert marker_urls == [ + f"https://example.com/capi/last-{release}" + for release in manage.SUPPORTED_CLUSTERAPI_K8S_IMAGES + ] + configs = mock_im.si.call_args.kwargs["configs"] + assert len(configs) == len(manage.SUPPORTED_CLUSTERAPI_K8S_IMAGES) + + +@pytest.mark.parametrize( + "extra_argv, expected_tail", + [ + ([], ()), + (["--tag", "stable"], ("--tag", "stable")), + (["--dry-run"], ("--dry-run",)), + (["--tag", "stable", "--dry-run"], ("--tag", "stable", "--dry-run")), + ], + ids=["defaults", "tag", "dry-run", "tag-and-dry-run"], +) +def test_clusterapi_builds_image_manager_arguments(extra_argv, expected_tail): + fetch_bodies = ["2026-04-12 ubuntu-2404-kube-v1.33.1.qcow2", "a" * 64] + + _, mock_im, _ = _run_image_command( + manage.ImageClusterapi, + ["--no-wait", "--filter", "1.33"] + extra_argv, + fetch_bodies, + ) + + expected = ( + "--cloud", + "admin", + "--filter", + "ubuntu-capi-image", + "--stuck-retry", + "1", + ) + expected_tail + assert mock_im.si.call_args.args == expected + assert mock_im.si.call_args.kwargs["cloud"] == "admin" + + +def test_clusterapi_extracts_version_from_image_name(): + fetch_bodies = ["2026-04-12 ubuntu-2404-kube-v1.33.1.qcow2", "a" * 64] + + _, mock_im, _ = _run_image_command( + manage.ImageClusterapi, + ["--no-wait", "--filter", "1.33"], + fetch_bodies, + ) + + (config,) = mock_im.si.call_args.kwargs["configs"] + assert 'version: "v1.33.1"' in config + assert f"checksum: \"sha256:{'a' * 64}\"" in config + assert "build_date: 2026-04-12" in config + + +# --- ImageGardenlinux.take_action --- + + +def test_gardenlinux_filter_uses_unknown_builddate_placeholder(): + _, mock_im, _ = _run_image_command( + manage.ImageGardenlinux, + [ + "--no-wait", + "--base-url", + "https://example.com/gardenlinux/", + "--filter", + "1877.2", + ], + ["a" * 64], + ) + + (config,) = mock_im.si.call_args.kwargs["configs"] + assert 'version: "1877.2"' in config + assert "build_date: unknown" in config + + +def test_gardenlinux_default_uses_supported_versions_builddate(): + mock_fetch, mock_im, _ = _run_image_command( + manage.ImageGardenlinux, + ["--no-wait", "--base-url", "https://example.com/gardenlinux/"], + ["a" * 64] * len(manage.SUPPORTED_GARDENLINUX_VERSIONS), + ) + + assert mock_fetch.call_args.args[0] == ( + "https://example.com/gardenlinux/1877.7/" + "openstack-gardener_prod-amd64-1877.7.qcow2.sha256" + ) + (config,) = mock_im.si.call_args.kwargs["configs"] + assert 'version: "1877.7"' in config + assert "build_date: 2025-11-14" in config + + +# --- Images.take_action --- + + +@pytest.mark.parametrize( + "argv, expected", + [ + ( + ["--no-wait"], + ( + "--cloud", + "admin", + "--hide", + "--images", + "/etc/images", + "--stuck-retry", + "1", + ), + ), + ( + ["--no-wait", "--delete"], + ( + "--cloud", + "admin", + "--delete", + "--yes-i-really-know-what-i-do", + "--hide", + "--images", + "/etc/images", + "--stuck-retry", + "1", + ), + ), + ( + ["--no-wait", "--images", "/srv/images"], + ( + "--cloud", + "admin", + "--hide", + "--images", + "/srv/images", + "--stuck-retry", + "1", + ), + ), + ], + ids=["defaults", "delete", "custom-images-path"], +) +def test_images_builds_image_manager_arguments(argv, expected): + mock_im, _, _ = _run_task_command( + manage.Images, argv, "osism.tasks.openstack.image_manager" + ) + + assert mock_im.si.call_args.args == expected + assert mock_im.si.call_args.kwargs == {"cloud": "admin"} + + +# --- Flavors.take_action --- + + +@pytest.mark.parametrize( + "argv, expected", + [ + (["--no-wait"], ("--name", "local", "--cloud", "admin")), + ( + ["--no-wait", "--recommended"], + ("--name", "local", "--cloud", "admin", "--recommended"), + ), + ( + ["--no-wait", "--name", "url", "--url", "https://example.com/flavors.yml"], + ( + "--name", + "url", + "--cloud", + "admin", + "--url", + "https://example.com/flavors.yml", + ), + ), + ], + ids=["defaults", "recommended", "name-and-url"], +) +def test_flavors_builds_flavor_manager_arguments(argv, expected): + mock_fm, _, _ = _run_task_command( + manage.Flavors, argv, "osism.tasks.openstack.flavor_manager" + ) + + assert mock_fm.si.call_args.args == expected + assert mock_fm.si.call_args.kwargs == {"cloud": "admin"} + + +# --- Dnsmasq.take_action --- + + +@pytest.mark.parametrize( + "argv, expected_wait", + [([], True), (["--no-wait"], False)], + ids=["wait", "no-wait"], +) +def test_dnsmasq_runs_infrastructure_playbook(argv, expected_wait): + mock_run, mock_handle, task = _run_task_command( + manage.Dnsmasq, argv, "osism.tasks.ansible.run" + ) + + mock_run.si.assert_called_once_with("infrastructure", "dnsmasq", []) + mock_handle.assert_called_once_with(task, expected_wait, format="log", timeout=300) + + +# --- ProjectCreate.take_action --- + +PROJECT_CREATE_DEFAULT_ARGUMENTS = ( + "--assign-admin-user", + "--create-admin-user", + "--nocreate-domain", + "--nocreate-user", + "--nocreate-application-credential", + "--domain-name-prefix", + "--nohas-service-network", + "--has-public-network", + "--has-shared-images", + "--norandom", + "--nomanaged-network-resources", + "--password-length", + "16", + "--quota-multiplier", + "1", + "--quota-router", + "1", + "--admin-domain", + "default", + "--cloud", + "admin", + "--domain", + "default", + "--name", + "sandbox", + "--public-network", + "public", + "--quota-class", + "basic", +) + + +def test_project_create_defaults_render_flag_pairs(): + mock_pm, _, _ = _run_task_command( + manage.ProjectCreate, ["--no-wait"], "osism.tasks.openstack.project_manager" + ) + + assert mock_pm.si.call_args.args == PROJECT_CREATE_DEFAULT_ARGUMENTS + assert mock_pm.si.call_args.kwargs == {"cloud": "admin"} + + +def test_project_create_negative_flags_flip_defaults(): + mock_pm, _, _ = _run_task_command( + manage.ProjectCreate, + ["--no-wait", "--nocreate-admin-user", "--create-domain"], + "osism.tasks.openstack.project_manager", + ) + + args = mock_pm.si.call_args.args + assert "--nocreate-admin-user" in args + assert "--create-admin-user" not in args + assert "--create-domain" in args + assert "--nocreate-domain" not in args + + +def test_project_create_includes_optional_arguments_only_when_set(): + mock_pm, _, _ = _run_task_command( + manage.ProjectCreate, + [ + "--no-wait", + "--quota-multiplier-compute", + "2", + "--quota-multiplier-network", + "3", + "--quota-multiplier-storage", + "4", + "--internal-id", + "b1a2", + "--owner", + "operations", + "--password", + "s3cret", + "--service-network-cidr", + "192.168.0.0/24", + ], + "osism.tasks.openstack.project_manager", + ) + + # The quota-multiplier options are inserted right after "--quota-router 1" + # (index 17 in the default tuple), before the string arguments. + expected = ( + PROJECT_CREATE_DEFAULT_ARGUMENTS[:17] + + ( + "--quota-multiplier-compute", + "2", + "--quota-multiplier-network", + "3", + "--quota-multiplier-storage", + "4", + ) + + PROJECT_CREATE_DEFAULT_ARGUMENTS[17:] + + ( + "--internal-id", + "b1a2", + "--owner", + "operations", + "--password", + "s3cret", + "--service-network-cidr", + "192.168.0.0/24", + ) + ) + assert mock_pm.si.call_args.args == expected + + +# --- ProjectSync.take_action --- + +PROJECT_SYNC_DEFAULT_ARGUMENTS = ( + "--noassign-admin-user", + "--nodry-run", + "--nomanage-endpoints", + "--nomanage-homeprojects", + "--manage-privatevolumetypes", + "--manage-privateflavors", + "--admin-domain", + "default", + "--classes", + "etc/classes.yml", + "--endpoints", + "etc/endpoints.yml", + "--cloud", + "admin", +) + + +def test_project_sync_defaults_render_flag_pairs(): + mock_pm, _, _ = _run_task_command( + manage.ProjectSync, ["--no-wait"], "osism.tasks.openstack.project_manager_sync" + ) + + assert mock_pm.si.call_args.args == PROJECT_SYNC_DEFAULT_ARGUMENTS + assert mock_pm.si.call_args.kwargs == {"cloud": "admin"} + + +def test_project_sync_appends_domain_and_name_only_when_given(): + mock_pm, _, _ = _run_task_command( + manage.ProjectSync, + ["--no-wait", "--domain", "d1", "--name", "p1"], + "osism.tasks.openstack.project_manager_sync", + ) + + assert mock_pm.si.call_args.args == PROJECT_SYNC_DEFAULT_ARGUMENTS + ( + "--domain", + "d1", + "--name", + "p1", + ) + + +def test_project_sync_positive_flags_flip_defaults(): + mock_pm, _, _ = _run_task_command( + manage.ProjectSync, + ["--no-wait", "--assign-admin-user", "--dry-run", "--nomanage-privateflavors"], + "osism.tasks.openstack.project_manager_sync", + ) + + args = mock_pm.si.call_args.args + assert "--assign-admin-user" in args + assert "--noassign-admin-user" not in args + assert "--dry-run" in args + assert "--nodry-run" not in args + assert "--nomanage-privateflavors" in args + assert "--manage-privateflavors" not in args diff --git a/tests/unit/commands/test_migrate.py b/tests/unit/commands/test_migrate.py index 4fe074eb..da0c9396 100644 --- a/tests/unit/commands/test_migrate.py +++ b/tests/unit/commands/test_migrate.py @@ -19,7 +19,6 @@ from ._helpers import make_command, parse_args - NODES = [("10.0.0.1", "node1"), ("10.0.0.2", "node2")] BASE_URL = "http://10.0.0.1:15672/api" AUTH = ("openstack", "secret") diff --git a/tests/unit/commands/test_vault.py b/tests/unit/commands/test_vault.py index de50a09e..20cf7690 100644 --- a/tests/unit/commands/test_vault.py +++ b/tests/unit/commands/test_vault.py @@ -5,6 +5,7 @@ from unittest.mock import MagicMock, mock_open, patch import pytest +from cryptography.fernet import Fernet from osism.commands import vault @@ -13,6 +14,21 @@ def _make_view(): return vault.View(MagicMock(), MagicMock()) +@pytest.fixture +def mock_redis(): + """Provide a mock Redis client wherever the vault commands resolve it. + + ``osism.utils.redis`` is a lazily-initialised module attribute that opens + a real connection on first access, so patch both the attribute and its + initialiser to keep the test offline. + """ + client = MagicMock() + with patch("osism.utils._init_redis", return_value=client), patch( + "osism.commands.vault.utils.redis", client, create=True + ): + yield client + + # --- View.take_action --- @@ -168,3 +184,337 @@ def test_decrypt_propagates_exit_code(tmp_path): rc = cmd.take_action(parsed_args) assert rc == 1 + + +# --- SetPassword.take_action --- + + +def _make_set_password(monkeypatch, tmp_path): + """Build a SetPassword command whose keyfile lives under tmp_path. + + Returns the ``(command, parsed_args, keyfile)`` triple; the keyfile is + not created, tests write it themselves when a pre-existing key is needed. + """ + keyfile = tmp_path / "ansible_vault_password.key" + monkeypatch.setattr(vault.SetPassword, "keyfile", str(keyfile)) + cmd = vault.SetPassword(MagicMock(), MagicMock()) + parsed_args = cmd.get_parser("test").parse_args([]) + return cmd, parsed_args, keyfile + + +def _piped_stdin(text): + stdin = MagicMock() + stdin.isatty.return_value = False + stdin.read.return_value = text + return stdin + + +def test_set_password_reuses_existing_keyfile(monkeypatch, tmp_path, mock_redis): + cmd, parsed_args, keyfile = _make_set_password(monkeypatch, tmp_path) + key = Fernet.generate_key() + keyfile.write_text(key.decode("utf-8")) + + with patch("sys.stdin", _piped_stdin("hunter2\n")), patch( + "cryptography.fernet.Fernet.generate_key" + ) as mock_generate: + cmd.take_action(parsed_args) + + mock_generate.assert_not_called() + assert keyfile.read_text() == key.decode("utf-8") + name, token = mock_redis.set.call_args.args + assert name == "ansible_vault_password" + assert Fernet(key).decrypt(token) == b"hunter2" + + +def test_set_password_writes_generated_key_when_keyfile_missing( + monkeypatch, tmp_path, mock_redis +): + cmd, parsed_args, keyfile = _make_set_password(monkeypatch, tmp_path) + key = Fernet.generate_key() + + with patch("sys.stdin", _piped_stdin("hunter2\n")), patch( + "cryptography.fernet.Fernet.generate_key", return_value=key + ): + cmd.take_action(parsed_args) + + assert keyfile.read_text() == key.decode("utf-8") + name, token = mock_redis.set.call_args.args + assert name == "ansible_vault_password" + assert Fernet(key).decrypt(token) == b"hunter2" + + +def test_set_password_reads_piped_stdin_without_prompting( + monkeypatch, tmp_path, mock_redis +): + cmd, parsed_args, keyfile = _make_set_password(monkeypatch, tmp_path) + key = Fernet.generate_key() + keyfile.write_text(key.decode("utf-8")) + + with patch("sys.stdin", _piped_stdin(" spacey-secret \n")), patch( + "prompt_toolkit.prompt" + ) as mock_prompt: + cmd.take_action(parsed_args) + + mock_prompt.assert_not_called() + _, token = mock_redis.set.call_args.args + assert Fernet(key).decrypt(token) == b"spacey-secret" + + +def test_set_password_prompts_on_tty(monkeypatch, tmp_path, mock_redis): + cmd, parsed_args, keyfile = _make_set_password(monkeypatch, tmp_path) + key = Fernet.generate_key() + keyfile.write_text(key.decode("utf-8")) + stdin = MagicMock() + stdin.isatty.return_value = True + + with patch("sys.stdin", stdin), patch( + "prompt_toolkit.prompt", return_value="tty-secret" + ) as mock_prompt: + cmd.take_action(parsed_args) + + mock_prompt.assert_called_once_with("Ansible Vault password: ", is_password=True) + _, token = mock_redis.set.call_args.args + assert Fernet(key).decrypt(token) == b"tty-secret" + + +# --- UnsetPassword.take_action --- + + +def test_unset_password_deletes_redis_key(mock_redis): + cmd = vault.UnsetPassword(MagicMock(), MagicMock()) + parsed_args = cmd.get_parser("test").parse_args([]) + + cmd.take_action(parsed_args) + + mock_redis.delete.assert_called_once_with("ansible_vault_password") + + +# --- Check._find_secrets_file --- + + +def _make_check(): + return vault.Check(MagicMock(), MagicMock()) + + +def test_find_secrets_file_returns_first_existing_search_path(): + target = vault.SECRETS_SEARCH_PATHS[1] + + with patch( + "osism.commands.vault.os.path.isfile", side_effect=lambda p: p == target + ), patch("osism.commands.vault.glob.glob") as mock_glob: + found = _make_check()._find_secrets_file() + + assert found == target + mock_glob.assert_not_called() + + +def test_find_secrets_file_falls_back_to_glob(): + matches = [ + "/opt/configuration/environments/custom2/secrets.yml", + "/opt/configuration/environments/other/secrets.yml", + ] + + with patch("osism.commands.vault.os.path.isfile", return_value=False), patch( + "osism.commands.vault.glob.glob", return_value=matches + ) as mock_glob: + found = _make_check()._find_secrets_file() + + assert found == matches[0] + mock_glob.assert_called_once_with( + "/opt/configuration/environments/**/secrets.yml", recursive=True + ) + + +def test_find_secrets_file_returns_none_when_nothing_found(): + with patch("osism.commands.vault.os.path.isfile", return_value=False), patch( + "osism.commands.vault.glob.glob", return_value=[] + ): + assert _make_check()._find_secrets_file() is None + + +# --- Check.take_action --- + + +def _make_check_keyfile(monkeypatch, tmp_path, content=None): + """Point ``Check.keyfile`` at a tmp_path file, optionally writing content.""" + keyfile = tmp_path / "ansible_vault_password.key" + if content is not None: + keyfile.write_text(content) + monkeypatch.setattr(vault.Check, "keyfile", str(keyfile)) + return keyfile + + +def _setup_valid_chain(monkeypatch, tmp_path, mock_redis, password=b"secret"): + """Real Fernet key on disk plus a matching encrypted password in Redis.""" + key = Fernet.generate_key() + _make_check_keyfile(monkeypatch, tmp_path, key.decode("utf-8")) + mock_redis.get.return_value = Fernet(key).encrypt(password) + return key + + +def _parse_check(args): + cmd = _make_check() + return cmd, cmd.get_parser("test").parse_args(args) + + +def test_check_fails_when_keyfile_missing(monkeypatch, tmp_path, mock_redis, capsys): + _make_check_keyfile(monkeypatch, tmp_path) + cmd, parsed_args = _parse_check(["--format", "script"]) + + rc = cmd.take_action(parsed_args) + + assert rc == 1 + assert capsys.readouterr().out == "FAILED: keyfile_missing\n" + mock_redis.get.assert_not_called() + + +def test_check_fails_on_invalid_fernet_key(monkeypatch, tmp_path, mock_redis, capsys): + _make_check_keyfile(monkeypatch, tmp_path, "not-a-fernet-key") + cmd, parsed_args = _parse_check(["--format", "script"]) + + rc = cmd.take_action(parsed_args) + + assert rc == 1 + assert capsys.readouterr().out == "FAILED: invalid_keyfile\n" + mock_redis.get.assert_not_called() + + +def test_check_fails_when_password_not_set(monkeypatch, tmp_path, mock_redis, capsys): + _make_check_keyfile(monkeypatch, tmp_path, Fernet.generate_key().decode("utf-8")) + mock_redis.get.return_value = None + cmd, parsed_args = _parse_check(["--format", "script"]) + + rc = cmd.take_action(parsed_args) + + assert rc == 1 + assert capsys.readouterr().out == "FAILED: password_not_set\n" + + +def test_check_fails_when_token_cannot_be_decrypted( + monkeypatch, tmp_path, mock_redis, capsys +): + # Token encrypted with a different key than the one on disk, as after a + # keyfile regeneration. + _make_check_keyfile(monkeypatch, tmp_path, Fernet.generate_key().decode("utf-8")) + mock_redis.get.return_value = Fernet(Fernet.generate_key()).encrypt(b"secret") + cmd, parsed_args = _parse_check(["--format", "script"]) + + rc = cmd.take_action(parsed_args) + + assert rc == 1 + assert capsys.readouterr().out == "FAILED: decryption_failed\n" + + +def test_check_fails_on_whitespace_only_password( + monkeypatch, tmp_path, mock_redis, capsys +): + _setup_valid_chain(monkeypatch, tmp_path, mock_redis, password=b" \t ") + cmd, parsed_args = _parse_check(["--format", "script"]) + + rc = cmd.take_action(parsed_args) + + assert rc == 1 + assert capsys.readouterr().out == "FAILED: password_empty\n" + + +def test_check_reports_wrong_password_for_undecryptable_file( + monkeypatch, tmp_path, mock_redis, capsys +): + _setup_valid_chain(monkeypatch, tmp_path, mock_redis) + secrets = tmp_path / "secrets.yml" + secrets.write_bytes(b"$ANSIBLE_VAULT;1.1;AES256\nciphertext\n") + cmd, parsed_args = _parse_check(["--format", "script", "--path", str(secrets)]) + + # The unit-test environment stubs the ansible package (see + # tests/conftest.py), so real vault decryption is impossible; mock the + # VaultLib interaction instead. + with patch("ansible.parsing.vault.VaultLib") as mock_vaultlib: + mock_vaultlib.return_value.is_encrypted.return_value = True + mock_vaultlib.return_value.decrypt.side_effect = Exception( + "HMAC verification failed" + ) + rc = cmd.take_action(parsed_args) + + assert rc == 1 + assert capsys.readouterr().out == "FAILED: wrong_password\n" + + +def test_check_passes_with_valid_chain_and_encrypted_file( + monkeypatch, tmp_path, mock_redis, capsys +): + _setup_valid_chain(monkeypatch, tmp_path, mock_redis) + secrets = tmp_path / "secrets.yml" + secrets.write_bytes(b"$ANSIBLE_VAULT;1.1;AES256\nciphertext\n") + cmd, parsed_args = _parse_check(["--format", "script", "--path", str(secrets)]) + + with patch("ansible.parsing.vault.VaultLib") as mock_vaultlib: + mock_vaultlib.return_value.is_encrypted.return_value = True + mock_vaultlib.return_value.decrypt.return_value = b"key: value\n" + rc = cmd.take_action(parsed_args) + + assert rc == 0 + assert capsys.readouterr().out == "PASSED\n" + mock_vaultlib.return_value.decrypt.assert_called_once_with( + b"$ANSIBLE_VAULT;1.1;AES256\nciphertext\n" + ) + + +def test_check_warns_when_no_secrets_file_found( + monkeypatch, tmp_path, mock_redis, loguru_logs +): + _setup_valid_chain(monkeypatch, tmp_path, mock_redis) + cmd, parsed_args = _parse_check(["--format", "log"]) + + with patch.object(vault.Check, "_find_secrets_file", return_value=None): + rc = cmd.take_action(parsed_args) + + assert rc == 0 + warnings = [r for r in loguru_logs if r["level"] == "WARNING"] + assert any("No secrets.yml file found" in r["message"] for r in warnings) + + +def test_check_resolves_relative_path_against_opt_configuration( + monkeypatch, tmp_path, mock_redis, capsys +): + _setup_valid_chain(monkeypatch, tmp_path, mock_redis) + cmd, parsed_args = _parse_check( + ["--format", "script", "--path", "environments/kolla/secrets.yml"] + ) + + expected = "/opt/configuration/environments/kolla/secrets.yml" + real_open = open + opened = [] + + def fake_open(path, *args, **kwargs): + # Intercept only the resolved secrets path; the keyfile read in + # step 2 must keep going to the real tmp_path file. + if path == expected: + opened.append(path) + return mock_open(read_data=b"$ANSIBLE_VAULT;1.1;AES256\nciphertext\n")() + return real_open(path, *args, **kwargs) + + with patch("osism.commands.vault.open", fake_open, create=True), patch( + "ansible.parsing.vault.VaultLib" + ) as mock_vaultlib: + mock_vaultlib.return_value.is_encrypted.return_value = True + mock_vaultlib.return_value.decrypt.return_value = b"key: value\n" + rc = cmd.take_action(parsed_args) + + assert rc == 0 + assert opened == [expected] + assert capsys.readouterr().out == "PASSED\n" + + +def test_check_skips_decryption_test_for_plain_file( + monkeypatch, tmp_path, mock_redis, loguru_logs +): + _setup_valid_chain(monkeypatch, tmp_path, mock_redis) + secrets = tmp_path / "plain.yml" + secrets.write_bytes(b"key: value\n") + cmd, parsed_args = _parse_check(["--format", "log", "--path", str(secrets)]) + + rc = cmd.take_action(parsed_args) + + assert rc == 0 + warnings = [r for r in loguru_logs if r["level"] == "WARNING"] + assert any("not vault-encrypted" in r["message"] for r in warnings)