fix: propagate handler failures to CLI exit codes

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,23 +3156,28 @@ 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:
             await badfish.list_serial()
         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()

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)