diff --git a/src/badfish/main.py b/src/badfish/main.py index 6eac324..be840cc 100644 --- a/src/badfish/main.py +++ b/src/badfish/main.py @@ -3129,15 +3129,18 @@ async def execute_badfish(_host, _args, logger, format_handler=None, console=Non elif clear_jobs: await badfish.clear_job_queue(force) elif check_job: - await badfish.check_schedule_job_status(check_job) + if await badfish.check_schedule_job_status(check_job) is False: + result = False elif list_jobs: await badfish.list_job_queue() elif host_type: await badfish.change_boot(host_type, interfaces_path, pxe) elif rac_reset: - await badfish.reset_idrac(wait=wait) + if await badfish.reset_idrac(wait=wait) is False: + result = False elif bmc_reset: - await badfish.reset_bmc() + if await badfish.reset_bmc() is False: + result = False elif factory_reset: await badfish.reset_bios() elif power_state: @@ -3153,13 +3156,16 @@ async def execute_badfish(_host, _args, logger, format_handler=None, console=Non elif reboot_only: await badfish.reboot_server() elif power_consumed_watts: - await badfish.get_power_consumed_watts() + if await badfish.get_power_consumed_watts() is False: + result = False elif list_interfaces: - await badfish.list_interfaces() + if await badfish.list_interfaces() is False: + result = False elif list_processors: await badfish.list_processors() elif list_gpu: - await badfish.list_gpu() + if await badfish.list_gpu() is False: + result = False elif list_memory: await badfish.list_memory() elif list_serial: @@ -3167,9 +3173,11 @@ async def execute_badfish(_host, _args, logger, format_handler=None, console=Non elif check_virtual_media: await badfish.check_virtual_media() elif mount_virtual_media: - await badfish.mount_virtual_media(mount_virtual_media) + if await badfish.mount_virtual_media(mount_virtual_media) is False: + result = False elif unmount_virtual_media: - await badfish.unmount_virtual_media() + if await badfish.unmount_virtual_media() is False: + result = False elif boot_to_virtual_media: await badfish.boot_to_virtual_media() elif check_remote_image: @@ -3197,26 +3205,35 @@ async def execute_badfish(_host, _args, logger, format_handler=None, console=Non attributes = bios_attributes if attribute_value else {attribute: value} await badfish.set_bios_attribute(attributes) elif set_bios_password: - await badfish.set_bios_password(old_password, new_password) + if await badfish.set_bios_password(old_password, new_password) is False: + result = False elif remove_bios_password: - await badfish.remove_bios_password(old_password) + if await badfish.remove_bios_password(old_password) is False: + result = False elif screenshot: await badfish.take_screenshot() elif get_scp_targets: - await badfish.get_scp_targets(get_scp_targets) + if await badfish.get_scp_targets(get_scp_targets) is False: + result = False elif export_scp: - await badfish.export_scp(export_scp, scp_targets, scp_include_read_only) + if await badfish.export_scp(export_scp, scp_targets, scp_include_read_only) is False: + result = False elif import_scp: - await badfish.import_scp(import_scp, scp_targets) + if await badfish.import_scp(import_scp, scp_targets) is False: + result = False elif get_nic_fqdds: - await badfish.get_nic_fqdds() + if await badfish.get_nic_fqdds() is False: + result = False elif get_nic_attribute: if attribute: - await badfish.get_nic_attribute_info(get_nic_attribute, attribute) + if await badfish.get_nic_attribute_info(get_nic_attribute, attribute) is False: + result = False else: - await badfish.get_nic_attribute(get_nic_attribute) + if await badfish.get_nic_attribute(get_nic_attribute) is False: + result = False elif set_nic_attribute: - await badfish.set_nic_attribute(set_nic_attribute, attribute, value) + if await badfish.set_nic_attribute(set_nic_attribute, attribute, value) is False: + result = False if pxe and not host_type: await badfish.set_next_boot_pxe() diff --git a/tests/test_exit_code_propagation.py b/tests/test_exit_code_propagation.py new file mode 100644 index 0000000..3c14270 --- /dev/null +++ b/tests/test_exit_code_propagation.py @@ -0,0 +1,90 @@ +import logging +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from badfish.main import execute_badfish + + +async def _execute_with_handler(handler, return_value, extra_args=None, flag=None): + """Run execute_badfish with a single handler stubbed to `return_value`.""" + fake_badfish = MagicMock() + setattr(fake_badfish, handler, AsyncMock(return_value=return_value)) + fake_badfish.session_id = None + fake_badfish.logger = MagicMock() + + mock_args = defaultdict(lambda: None) + mock_args.update({"u": "user", "p": "pass", "retries": 1}) + if flag is not None: + mock_args[flag] = True + if extra_args: + mock_args.update(extra_args) + + with patch("badfish.main.badfish_factory", new_callable=AsyncMock) as mock_factory: + mock_factory.return_value = fake_badfish + result = await execute_badfish("test_host", mock_args, MagicMock(spec=logging.Logger), None) + + return result, fake_badfish + + +# Handlers whose `False` means the operation *failed* and therefore must +# drive the CLI exit code to 1 (execute_badfish returns result=False). +FAILURE_HANDLERS = [ + ("get_power_consumed", "get_power_consumed_watts", None), + ("ls_interfaces", "list_interfaces", None), + ("racreset", "reset_idrac", None), + ("bmc_reset", "reset_bmc", None), + ("mount_virtual_media", "mount_virtual_media", {"mount_virtual_media": "http://img.iso"}), + ("unmount_virtual_media", "unmount_virtual_media", None), + ("check_job", "check_schedule_job_status", {"check_job": "JID_1"}), + ("set_bios_password", "set_bios_password", {"old_password": "old", "new_password": "new"}), + ("remove_bios_password", "remove_bios_password", {"old_password": "old"}), + ("get_scp_targets", "get_scp_targets", {"get_scp_targets": "export"}), + ("export_scp", "export_scp", {"export_scp": "/tmp/x.json", "scp_targets": "ALL"}), + ("import_scp", "import_scp", {"import_scp": "/tmp/x.json", "scp_targets": "ALL"}), + ("get_nic_fqdds", "get_nic_fqdds", None), + ("get_nic_attribute", "get_nic_attribute", None), + ("set_nic_attribute", "set_nic_attribute", {"attribute": "x", "value": "y"}), + ("ls_gpu", "list_gpu", None), +] + + +@pytest.mark.parametrize("flag, handler, extra_args", FAILURE_HANDLERS) +@pytest.mark.asyncio +async def test_handler_false_returns_failure_result(flag, handler, extra_args): + """A False return from an operation-failure handler must yield result=False (exit 1).""" + result, _ = await _execute_with_handler(handler, False, extra_args, flag) + assert result == ("test_host", False) + + +@pytest.mark.parametrize("flag, handler, extra_args", FAILURE_HANDLERS) +@pytest.mark.asyncio +async def test_handler_true_returns_success_result(flag, handler, extra_args): + """A True return from an operation-failure handler must yield result=True (exit 0).""" + result, _ = await _execute_with_handler(handler, True, extra_args, flag) + assert result == ("test_host", True) + + +@pytest.mark.asyncio +async def test_handler_none_success_not_treated_as_failure(): + """Handlers that return None on success (e.g. set_bios_password) must not exit 1.""" + # set_bios_password -> change_bios_password returns None on success + result, _ = await _execute_with_handler( + "set_bios_password", None, {"old_password": "old", "new_password": "new"}, "set_bios_password" + ) + assert result == ("test_host", True) + + +@pytest.mark.asyncio +async def test_check_virtual_media_false_is_informational(): + """check_virtual_media returning False (nothing mounted) is not an operation failure.""" + result, _ = await _execute_with_handler("check_virtual_media", False, None, "check_virtual_media") + assert result == ("test_host", True) + + +@pytest.mark.asyncio +async def test_check_remote_image_false_is_informational(): + """check_remote_image returning False (not attached/unsupported) is not an operation failure.""" + result, _ = await _execute_with_handler("check_remote_image", False, None, "check_remote_image") + assert result == ("test_host", True)