From ef97d2bdf83f267a48fa698743fc4f83bb66970a Mon Sep 17 00:00:00 2001
From: Roger Luethi <luethi@osism.tech>
Date: Mon, 8 Jun 2026 10:26:35 +0200
Subject: [PATCH 1/7] Return non-zero on task-wait timeouts

Several commands wait for a Celery task's output, hit TimeoutError,
log the timeout, and then fall through to the end of take_action with
an implicit None return. cliff turns that into exit code 0
(return_code = self.take_action(parsed_args) or 0), so a command that
gave up waiting for its task still reports success. That makes the
affected commands unsafe in set -e scripts and && chains, where a
timed-out sync silently looks like it completed.

This is the same class of bug PR #2313 fixed for osism get, but on the
timeout path rather than the inventory-query path. The affected sites:

- reconciler.Sync
- validate.Run._handle_task (the value take_action returns)
- netbox.Ironic
- netbox.Sync

Each now returns 1 from the TimeoutError handler, so a timeout yields a
non-zero exit. Adds unit tests asserting exit 1 when fetch_task_output
raises TimeoutError, mirroring PR #2313's failure-path tests.

Related-Bug: #2314
AI-assisted: Claude Code
Signed-off-by: Roger Luethi <luethi@osism.tech>
---
 osism/commands/netbox.py               |  2 ++
 osism/commands/reconciler.py           |  1 +
 osism/commands/validate.py             |  1 +
 tests/unit/commands/test_netbox.py     | 42 ++++++++++++++++++++++++++
 tests/unit/commands/test_reconciler.py | 27 +++++++++++++++++
 tests/unit/commands/test_validate.py   | 27 +++++++++++++++++
 6 files changed, 100 insertions(+)
 create mode 100644 tests/unit/commands/test_netbox.py
 create mode 100644 tests/unit/commands/test_reconciler.py
 create mode 100644 tests/unit/commands/test_validate.py

diff --git a/osism/commands/netbox.py b/osism/commands/netbox.py
index 8e0be1658..139511bc7 100644
--- a/osism/commands/netbox.py
+++ b/osism/commands/netbox.py
@@ -102,6 +102,7 @@ def take_action(self, parsed_args):
                     logger.error(
                         f"Timeout while waiting for further output of task {task.task_id} (sync ironic)"
                     )
+                return 1
         else:
             if node_name:
                 logger.info(
@@ -376,6 +377,7 @@ def take_action(self, parsed_args):
                     logger.error(
                         f"Timeout while waiting for further output of task {task.task_id} (sync netbox)"
                     )
+                return 1
         else:
             if node_name:
                 logger.info(
diff --git a/osism/commands/reconciler.py b/osism/commands/reconciler.py
index d559d8e6c..4f3fdbd41 100644
--- a/osism/commands/reconciler.py
+++ b/osism/commands/reconciler.py
@@ -64,6 +64,7 @@ def take_action(self, parsed_args):
                 logger.error(
                     f"Timeout while waiting for further output of task {t.task_id} (sync inventory)"
                 )
+                return 1
         else:
             logger.info(
                 f"Task {t.task_id} (sync inventory) is running in background. No more output."
diff --git a/osism/commands/validate.py b/osism/commands/validate.py
index 38c1a9c11..ef463ad75 100644
--- a/osism/commands/validate.py
+++ b/osism/commands/validate.py
@@ -60,6 +60,7 @@ def _handle_task(self, t, wait, format, timeout, playbook):
                 logger.error(
                     f"Timeout while waiting for further output of task {t.task_id} (sync inventory)"
                 )
+                return 1
         else:
             if format == "log":
                 logger.info(
diff --git a/tests/unit/commands/test_netbox.py b/tests/unit/commands/test_netbox.py
new file mode 100644
index 000000000..be4deef81
--- /dev/null
+++ b/tests/unit/commands/test_netbox.py
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""Tests for the ``osism netbox`` commands.
+
+These focus on the exit-code contract: a command must return a non-zero exit
+status when it gives up waiting for a task (a timeout is an operational
+failure), rather than falling through to an implicit success.
+"""
+
+from unittest.mock import MagicMock, patch
+
+from osism.commands import netbox
+
+
+def test_ironic_returns_nonzero_on_task_timeout():
+    cmd = netbox.Ironic(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args([])
+
+    with patch("osism.commands.netbox.utils.check_task_lock_and_exit"), patch(
+        "osism.tasks.conductor.sync_ironic.delay", return_value=MagicMock()
+    ), patch(
+        "osism.commands.netbox.utils.fetch_task_output",
+        side_effect=TimeoutError,
+    ):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
+
+
+def test_sync_returns_nonzero_on_task_timeout():
+    cmd = netbox.Sync(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args([])
+
+    with patch("osism.commands.netbox.utils.check_task_lock_and_exit"), patch(
+        "osism.tasks.conductor.sync_netbox.delay", return_value=MagicMock()
+    ), patch(
+        "osism.commands.netbox.utils.fetch_task_output",
+        side_effect=TimeoutError,
+    ):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
diff --git a/tests/unit/commands/test_reconciler.py b/tests/unit/commands/test_reconciler.py
new file mode 100644
index 000000000..319489ae1
--- /dev/null
+++ b/tests/unit/commands/test_reconciler.py
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""Tests for the ``osism reconciler`` commands.
+
+These focus on the exit-code contract: a command must return a non-zero exit
+status when it gives up waiting for a task (a timeout is an operational
+failure), rather than falling through to an implicit success.
+"""
+
+from unittest.mock import MagicMock, patch
+
+from osism.commands import reconciler
+
+
+def test_sync_returns_nonzero_on_task_timeout():
+    cmd = reconciler.Sync(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args([])
+
+    with patch("osism.commands.reconciler.utils.check_task_lock_and_exit"), patch(
+        "osism.tasks.reconciler.run.delay", return_value=MagicMock()
+    ), patch(
+        "osism.commands.reconciler.utils.fetch_task_output",
+        side_effect=TimeoutError,
+    ):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
diff --git a/tests/unit/commands/test_validate.py b/tests/unit/commands/test_validate.py
new file mode 100644
index 000000000..add9a1648
--- /dev/null
+++ b/tests/unit/commands/test_validate.py
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""Tests for the ``osism validate`` commands.
+
+These focus on the exit-code contract: ``_handle_task`` is what ``take_action``
+returns, so a timeout while waiting for task output must yield a non-zero exit
+status rather than an implicit ``None`` (exit 0).
+"""
+
+from unittest.mock import MagicMock, patch
+
+from osism.commands import validate
+
+
+def test_handle_task_returns_nonzero_on_timeout():
+    cmd = validate.Run(MagicMock(), MagicMock())
+    task = MagicMock()
+
+    with patch(
+        "osism.commands.validate.utils.fetch_task_output",
+        side_effect=TimeoutError,
+    ):
+        result = cmd._handle_task(
+            task, wait=True, format="log", timeout=1, playbook="validate-x"
+        )
+
+    assert result == 1

From 28a2297c50e27f304a9e20fbcc60ceaba94459d9 Mon Sep 17 00:00:00 2001
From: Roger Luethi <luethi@osism.tech>
Date: Mon, 8 Jun 2026 10:30:05 +0200
Subject: [PATCH 2/7] Fix wait exit code and timeout handling for --live

osism wait returned an exit code only from a single mid-loop return,
guarded by len(task_ids) == 1 and evaluated after task_ids.pop(). That
return was reached wrongly, or not at all, in every case:

- --live, single task: after the pop len(task_ids) == 0, so the guard
  never fired. Whether the stream finished or timed out, control fell
  off the end of take_action and cliff turned the implicit None into
  exit 0. A timeout was logged but ignored.
- --live, two tasks: the first iteration left len(task_ids) == 1 and
  hit the guard after processing only the first task, skipping the
  second. On a timeout in that iteration rc was never assigned, raising
  UnboundLocalError.
- --live, three or more tasks: the guard fired after processing the
  second-to-last task, skipping the last task. If that fetch timed out,
  it returned an earlier task's rc, or raised UnboundLocalError when rc
  had not yet been assigned.
- non-live polling: the loop drained and fell off the end, relying on
  cliff to convert the implicit None return into exit 0.

Initialise rc = 0 before the loop, preserve a non-zero result once it
is seen (a later successful fetch does not overwrite it), set rc = 1 on
TimeoutError, drop the broken len(task_ids) == 1 short-circuit, and
return rc once all tasks have been processed. A timed-out or failed
live task now yields a non-zero exit, every task is processed, and a
successful wait exits 0.

Adds unit tests covering the --live timeout (single and multiple
tasks), a non-zero task rc, and a successful task.

Part of issue #2314 (Category A, wait.py).

Related-Bug: #2314
AI-assisted: Claude Code
Signed-off-by: Roger Luethi <luethi@osism.tech>
---
 osism/commands/wait.py           | 11 +++--
 tests/unit/commands/test_wait.py | 71 ++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 4 deletions(-)
 create mode 100644 tests/unit/commands/test_wait.py

diff --git a/osism/commands/wait.py b/osism/commands/wait.py
index 0b91e28dc..990925c2c 100644
--- a/osism/commands/wait.py
+++ b/osism/commands/wait.py
@@ -83,6 +83,7 @@ def take_action(self, parsed_args):
             do_refresh = True
 
         tmp_task_ids = []
+        rc = 0
         while task_ids or do_refresh:
             if task_ids:
                 task_id = task_ids.pop()
@@ -121,14 +122,14 @@ def take_action(self, parsed_args):
                     if live:
                         utils.redis.ping()
                         try:
-                            rc = utils.fetch_task_output(task_id)
+                            task_rc = utils.fetch_task_output(task_id)
+                            if task_rc:
+                                rc = task_rc
                         except TimeoutError:
                             logger.error(
                                 f"Timeout while waiting for further output of task {task_id}"
                             )
-
-                        if len(task_ids) == 1:
-                            return rc
+                            rc = 1
                     else:
                         tmp_task_ids.insert(0, task_id)
 
@@ -154,3 +155,5 @@ def take_action(self, parsed_args):
                     tmp_task_ids = []
                 else:
                     do_refresh = False
+
+        return rc
diff --git a/tests/unit/commands/test_wait.py b/tests/unit/commands/test_wait.py
new file mode 100644
index 000000000..869042aa9
--- /dev/null
+++ b/tests/unit/commands/test_wait.py
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""Tests for the ``osism wait`` command.
+
+These focus on the exit-code contract for the ``--live`` path, which streams a
+STARTED task's output and should propagate that task's result as the process
+exit code:
+
+- a timeout while streaming is an operational failure -> non-zero exit;
+- a task that finishes with a non-zero rc -> that rc;
+- a task that finishes successfully -> exit 0.
+
+The pre-fix code only returned an exit code under a ``len(task_ids) == 1``
+guard, which never fired for a single task (so a timeout was ignored) and
+raised ``UnboundLocalError`` on a timeout with two tasks.
+"""
+
+from unittest.mock import MagicMock, patch
+
+from osism.commands import wait
+
+
+def _run(args, *, state, fetch):
+    cmd = wait.Run(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(args)
+
+    result_obj = MagicMock()
+    result_obj.state = state
+
+    with patch("celery.Celery"), patch(
+        "celery.result.AsyncResult", return_value=result_obj
+    ), patch("osism.utils._init_redis", return_value=MagicMock()), patch(
+        "osism.commands.wait.utils.fetch_task_output", **fetch
+    ):
+        return cmd.take_action(parsed_args)
+
+
+def test_live_returns_nonzero_on_timeout_single_task():
+    result = _run(
+        ["taskid1", "--live"],
+        state="STARTED",
+        fetch={"side_effect": TimeoutError},
+    )
+    assert result == 1
+
+
+def test_live_returns_nonzero_on_timeout_multiple_tasks():
+    result = _run(
+        ["taskid1", "taskid2", "--live"],
+        state="STARTED",
+        fetch={"side_effect": TimeoutError},
+    )
+    assert result == 1
+
+
+def test_live_returns_task_rc_when_task_fails():
+    result = _run(
+        ["taskid1", "--live"],
+        state="STARTED",
+        fetch={"return_value": 2},
+    )
+    assert result == 2
+
+
+def test_live_returns_zero_when_task_succeeds():
+    result = _run(
+        ["taskid1", "--live"],
+        state="STARTED",
+        fetch={"return_value": 0},
+    )
+    assert result == 0

From 0ddff7de91e221b7b7e64829ef5980fc47f72bd2 Mon Sep 17 00:00:00 2001
From: Roger Luethi <luethi@osism.tech>
Date: Mon, 8 Jun 2026 10:31:55 +0200
Subject: [PATCH 3/7] Return non-zero on inventory query failures in report

osism report memory, lldp, bgp and status each load the Ansible
inventory by running ansible-inventory via subprocess.run. On a
non-zero return code ("Error loading inventory.") and on
subprocess.TimeoutExpired ("Timeout loading inventory.") they logged
the error and then bare-returned None. cliff turns that into exit
code 0 (return_code = self.take_action(parsed_args) or 0), so a report
whose inventory could not be loaded at all still exited successfully.
That makes these commands unsafe in set -e scripts and && chains.

These are the direct twins of the bug PR #2313 fixed for osism get.
All eight inventory-load failure paths now return 1.

The neighbouring "No hosts found in inventory." branch is deliberately
left at exit 0: the query ran fine and simply returned no hosts, which
is a valid empty result rather than an operational failure, matching
the convention established in PR #2313.

Adds unit tests for all four commands covering the failure path (a
non-zero ansible-inventory return code yields exit 1), the timeout
path (TimeoutExpired yields exit 1) and the empty-result path (an empty
inventory yields exit 0).

Part of issue #2314 (Category B).

Related-Bug: #2314
AI-assisted: Claude Code
Signed-off-by: Roger Luethi <luethi@osism.tech>
---
 osism/commands/report.py           | 16 +++---
 tests/unit/commands/test_report.py | 86 ++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 8 deletions(-)
 create mode 100644 tests/unit/commands/test_report.py

diff --git a/osism/commands/report.py b/osism/commands/report.py
index 67a13cdcf..141886002 100644
--- a/osism/commands/report.py
+++ b/osism/commands/report.py
@@ -51,10 +51,10 @@ def take_action(self, parsed_args):
 
             if result.returncode != 0:
                 logger.error("Error loading inventory.")
-                return
+                return 1
         except subprocess.TimeoutExpired:
             logger.error("Timeout loading inventory.")
-            return
+            return 1
 
         data = json.loads(result.stdout)
         hosts = get_hosts_from_inventory(data)
@@ -189,10 +189,10 @@ def take_action(self, parsed_args):
 
             if result.returncode != 0:
                 logger.error("Error loading inventory.")
-                return
+                return 1
         except subprocess.TimeoutExpired:
             logger.error("Timeout loading inventory.")
-            return
+            return 1
 
         data = json.loads(result.stdout)
         hosts = get_hosts_from_inventory(data)
@@ -358,10 +358,10 @@ def take_action(self, parsed_args):
 
             if result.returncode != 0:
                 logger.error("Error loading inventory.")
-                return
+                return 1
         except subprocess.TimeoutExpired:
             logger.error("Timeout loading inventory.")
-            return
+            return 1
 
         data = json.loads(result.stdout)
         hosts = get_hosts_from_inventory(data)
@@ -535,10 +535,10 @@ def take_action(self, parsed_args):
 
             if result.returncode != 0:
                 logger.error("Error loading inventory.")
-                return
+                return 1
         except subprocess.TimeoutExpired:
             logger.error("Timeout loading inventory.")
-            return
+            return 1
 
         data = json.loads(result.stdout)
         hosts = get_hosts_from_inventory(data)
diff --git a/tests/unit/commands/test_report.py b/tests/unit/commands/test_report.py
new file mode 100644
index 000000000..61d15fa68
--- /dev/null
+++ b/tests/unit/commands/test_report.py
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""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.
+"""
+
+import subprocess
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from osism.commands import report
+
+COMMANDS = [report.Memory, report.Lldp, report.Bgp, report.Status]
+
+# Status requires a positional "type"; the others take no required args.
+ARGS = {report.Status: ["bootstrap"]}
+
+
+def _make(cls):
+    cmd = cls(MagicMock(), MagicMock())
+    return cmd, cmd.get_parser("test").parse_args(ARGS.get(cls, []))
+
+
+@pytest.mark.parametrize("cls", COMMANDS)
+def test_returns_nonzero_when_inventory_load_fails(cls):
+    cmd, parsed_args = _make(cls)
+    failed = MagicMock()
+    failed.returncode = 1
+
+    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.subprocess.run", return_value=failed
+    ):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
+
+
+@pytest.mark.parametrize("cls", COMMANDS)
+def test_returns_nonzero_when_inventory_load_times_out(cls):
+    cmd, parsed_args = _make(cls)
+
+    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.subprocess.run",
+        side_effect=subprocess.TimeoutExpired("ansible-inventory", 30),
+    ):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
+
+
+@pytest.mark.parametrize("cls", COMMANDS)
+def test_returns_success_when_inventory_is_empty(cls):
+    cmd, parsed_args = _make(cls)
+    ok = MagicMock()
+    ok.returncode = 0
+    ok.stdout = "{}"
+
+    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.subprocess.run", return_value=ok
+    ), patch(
+        "osism.commands.report.get_hosts_from_inventory", return_value=[]
+    ):
+        result = cmd.take_action(parsed_args)
+
+    assert not result

From a295f5357252afc4f65690793727fa9a7e0b838a Mon Sep 17 00:00:00 2001
From: Roger Luethi <luethi@osism.tech>
Date: Mon, 8 Jun 2026 10:40:34 +0200
Subject: [PATCH 4/7] Return non-zero on failed lookups in CLI commands

Several commands look up a resource, log an error or warning when it
cannot be found, and then bare-return None. cliff turns that into exit
code 0 (return_code = self.take_action(parsed_args) or 0), so a failed
lookup silently looks like success and the commands are unsafe in
set -e scripts and && chains.

All the affected not-found / unresolved-lookup paths now return 1:

- compute.ComputeMigrationList: no user domain, no user, no project
  domain, no project, multiple servers matched, no server found.
- server.ServerList: no user domain, no user, domain not found,
  project domain not found, project not found.
- volume.VolumeList: domain not found, project domain not found,
  project not found.
- netbox.Console / netbox.Dump: NetBox integration not configured
  (the operation cannot run without it), and Dump's device not found.
- baremetal Deploy, Undeploy, BurnIn, Clean, Provide, MaintenanceSet,
  MaintenanceUnset, PowerOn, PowerOff and Delete: the single named node
  could not be found (conn.baremetal.find_node returned nothing).

Adds unit tests for each command driving the not-found path and
asserting exit 1, mirroring PR #2313.

Part of issue #2314 (Category C and Category D not-found lookups).

Related-Bug: #2314
AI-assisted: Claude Code
Signed-off-by: Roger Luethi <luethi@osism.tech>
---
 osism/commands/baremetal.py           | 20 ++++-----
 osism/commands/compute.py             | 12 +++---
 osism/commands/netbox.py              |  6 +--
 osism/commands/server.py              | 10 ++---
 osism/commands/volume.py              |  6 +--
 tests/unit/commands/test_baremetal.py | 46 ++++++++++++++++++++
 tests/unit/commands/test_compute.py   | 62 +++++++++++++++++++++++++++
 tests/unit/commands/test_netbox.py    | 37 ++++++++++++++++
 tests/unit/commands/test_server.py    | 51 ++++++++++++++++++++++
 tests/unit/commands/test_volume.py    | 46 ++++++++++++++++++++
 10 files changed, 269 insertions(+), 27 deletions(-)
 create mode 100644 tests/unit/commands/test_baremetal.py
 create mode 100644 tests/unit/commands/test_compute.py
 create mode 100644 tests/unit/commands/test_server.py
 create mode 100644 tests/unit/commands/test_volume.py

diff --git a/osism/commands/baremetal.py b/osism/commands/baremetal.py
index 52451728b..36fb3b50f 100644
--- a/osism/commands/baremetal.py
+++ b/osism/commands/baremetal.py
@@ -192,7 +192,7 @@ def take_action(self, parsed_args):
                 node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
                 if not node:
                     logger.warning(f"Could not find node {name}")
-                    return
+                    return 1
                 deploy_nodes = [node]
 
             for node in deploy_nodes:
@@ -743,7 +743,7 @@ def take_action(self, parsed_args):
                 )
                 if not node:
                     logger.warning(f"Could not find node {name}")
-                    return
+                    return 1
                 deploy_nodes = [node]
 
             for node in deploy_nodes:
@@ -1054,7 +1054,7 @@ def take_action(self, parsed_args):
                 node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
                 if not node:
                     logger.warning(f"Could not find node {name}")
-                    return
+                    return 1
                 burn_in_nodes = [node]
 
             for node in burn_in_nodes:
@@ -1211,7 +1211,7 @@ def take_action(self, parsed_args):
                 node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
                 if not node:
                     logger.warning(f"Could not find node {name}")
-                    return
+                    return 1
                 clean_nodes = [node]
 
             for node in clean_nodes:
@@ -1335,7 +1335,7 @@ def take_action(self, parsed_args):
                 node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
                 if not node:
                     logger.warning(f"Could not find node {name}")
-                    return
+                    return 1
                 provide_nodes = [node]
 
             for node in provide_nodes:
@@ -1405,7 +1405,7 @@ def take_action(self, parsed_args):
             node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
             if not node:
                 logger.warning(f"Could not find node {name}")
-                return
+                return 1
             try:
                 conn.baremetal.set_node_maintenance(node, reason=reason)
             except Exception as exc:
@@ -1453,7 +1453,7 @@ def take_action(self, parsed_args):
             node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
             if not node:
                 logger.warning(f"Could not find node {name}")
-                return
+                return 1
             try:
                 conn.baremetal.unset_node_maintenance(node)
             except Exception as exc:
@@ -1505,7 +1505,7 @@ def take_action(self, parsed_args):
             node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
             if not node:
                 logger.warning(f"Could not find node {name}")
-                return
+                return 1
 
             try:
                 conn.baremetal.set_node_power_state(node.id, "power on")
@@ -1564,7 +1564,7 @@ def take_action(self, parsed_args):
             node = conn.baremetal.find_node(name, ignore_missing=True, details=True)
             if not node:
                 logger.warning(f"Could not find node {name}")
-                return
+                return 1
 
             target = "soft power off" if soft else "power off"
 
@@ -1645,7 +1645,7 @@ def take_action(self, parsed_args):
                 )
                 if not node:
                     logger.warning(f"Could not find node {name}")
-                    return
+                    return 1
                 delete_nodes = [node]
 
             for node in delete_nodes:
diff --git a/osism/commands/compute.py b/osism/commands/compute.py
index ed63162e7..a07dc0a38 100644
--- a/osism/commands/compute.py
+++ b/osism/commands/compute.py
@@ -689,14 +689,14 @@ def take_action(self, parsed_args):
                         user_query = dict(domain_id=u_d.id)
                     else:
                         logger.error(f"No domain found for {user_domain}")
-                        return
+                        return 1
 
                 u = conn.identity.find_user(user, ignore_missing=True, **user_query)
                 if u and "id" in u:
                     user_id = u.id
                 else:
                     logger.error(f"No user found for {user}")
-                    return
+                    return 1
 
             project_id = None
             if project:
@@ -708,7 +708,7 @@ def take_action(self, parsed_args):
                         project_query = dict(domain_id=p_d.id)
                     else:
                         logger.error(f"No domain found for {project_domain}")
-                        return
+                        return 1
 
                 p = conn.identity.find_project(
                     project, ignore_missing=True, **project_query
@@ -717,7 +717,7 @@ def take_action(self, parsed_args):
                     project_id = p.id
                 else:
                     logger.error(f"No project found for {project}")
-                    return
+                    return 1
 
             instance_uuid = None
             if server:
@@ -731,10 +731,10 @@ def take_action(self, parsed_args):
                         raise openstack.exceptions.NotFoundException
                 except openstack.exceptions.DuplicateResource:
                     logger.error(f"Multiple servers where found for {server}")
-                    return
+                    return 1
                 except openstack.exceptions.NotFoundException:
                     logger.error(f"No server found for {server}")
-                    return
+                    return 1
 
             query = {}
             if host:
diff --git a/osism/commands/netbox.py b/osism/commands/netbox.py
index 139511bc7..42fa1f4e4 100644
--- a/osism/commands/netbox.py
+++ b/osism/commands/netbox.py
@@ -538,7 +538,7 @@ def take_action(self, parsed_args):
 
             if not token or not url:
                 logger.error("NetBox integration not configured.")
-                return
+                return 1
 
             subprocess.call(
                 ["/usr/local/bin/nbcli", "init"],
@@ -587,7 +587,7 @@ def take_action(self, parsed_args):
         # Check if NetBox connection is available
         if not utils.nb:
             logger.error("NetBox integration not configured.")
-            return
+            return 1
 
         device = None
 
@@ -633,7 +633,7 @@ def take_action(self, parsed_args):
 
         if not device:
             logger.error(f"Device '{host}' not found in NetBox.")
-            return
+            return 1
 
         # Prepare table data for display
         table = []
diff --git a/osism/commands/server.py b/osism/commands/server.py
index eb11cecff..205a51818 100644
--- a/osism/commands/server.py
+++ b/osism/commands/server.py
@@ -182,20 +182,20 @@ def take_action(self, parsed_args):
                         user_query = dict(domain_id=u_d.id)
                     else:
                         logger.error(f"No domain found for {user_domain}")
-                        return
+                        return 1
 
                 u = conn.identity.find_user(user, ignore_missing=True, **user_query)
                 if u and "id" in u:
                     user_id = u.id
                 else:
                     logger.error(f"No user found for {user}")
-                    return
+                    return 1
 
             if domain:
                 _domain = conn.identity.find_domain(domain)
                 if not _domain:
                     logger.error(f"Domain {domain} not found")
-                    return
+                    return 1
                 projects = list(conn.identity.projects(domain_id=_domain.id))
 
                 for project in projects:
@@ -234,14 +234,14 @@ def take_action(self, parsed_args):
                     _project_domain = conn.identity.find_domain(project_domain)
                     if not _project_domain:
                         logger.error(f"Project domain {project_domain} not found")
-                        return
+                        return 1
                     query = {"domain_id": _project_domain.id}
                     _project = conn.identity.find_project(project, **query)
                 else:
                     _project = conn.identity.find_project(project)
                 if not _project:
                     logger.error(f"Project {project} not found")
-                    return
+                    return 1
                 query = {"project_id": _project.id}
 
                 # Get domain name from project
diff --git a/osism/commands/volume.py b/osism/commands/volume.py
index 42903246c..705cc3da1 100644
--- a/osism/commands/volume.py
+++ b/osism/commands/volume.py
@@ -67,7 +67,7 @@ def take_action(self, parsed_args):
                 _domain = conn.identity.find_domain(domain)
                 if not _domain:
                     logger.error(f"Domain {domain} not found")
-                    return
+                    return 1
                 projects = list(conn.identity.projects(domain_id=_domain.id))
 
                 for project in projects:
@@ -106,14 +106,14 @@ def take_action(self, parsed_args):
                     _project_domain = conn.identity.find_domain(project_domain)
                     if not _project_domain:
                         logger.error(f"Project domain {project_domain} not found")
-                        return
+                        return 1
                     query = {"domain_id": _project_domain.id}
                     _project = conn.identity.find_project(project, **query)
                 else:
                     _project = conn.identity.find_project(project)
                 if not _project:
                     logger.error(f"Project {project} not found")
-                    return
+                    return 1
                 query = {"project_id": _project.id}
 
                 for volume in conn.block_storage.volumes(all_projects=True, **query):
diff --git a/tests/unit/commands/test_baremetal.py b/tests/unit/commands/test_baremetal.py
new file mode 100644
index 000000000..6b50c54c5
--- /dev/null
+++ b/tests/unit/commands/test_baremetal.py
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: Apache-2.0
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from osism.commands import baremetal
+
+# Each of these command classes follows the identical pattern: when the
+# requested node cannot be found, the command logs a warning and must return
+# a non-zero exit code so a failed lookup is not reported as success.
+NOT_FOUND_COMMANDS = [
+    baremetal.BaremetalDeploy,
+    baremetal.BaremetalUndeploy,
+    baremetal.BaremetalBurnIn,
+    baremetal.BaremetalClean,
+    baremetal.BaremetalProvide,
+    baremetal.BaremetalMaintenanceSet,
+    baremetal.BaremetalMaintenanceUnset,
+    baremetal.BaremetalPowerOn,
+    baremetal.BaremetalPowerOff,
+    baremetal.BaremetalDelete,
+]
+
+
+def _run_not_found(cls):
+    cmd = cls(MagicMock(), MagicMock())
+    # Select the single-node path by naming one node (and not using --all).
+    parsed_args = cmd.get_parser("test").parse_args(["node1"])
+
+    conn = MagicMock()
+    conn.baremetal.find_node.return_value = None
+
+    setup = MagicMock(return_value=("pw", [], None, True))
+    getconn = MagicMock(return_value=conn)
+    cleanup = MagicMock()
+    with patch(
+        "osism.tasks.openstack.get_cloud_helpers",
+        return_value=(setup, getconn, cleanup),
+    ):
+        return cmd.take_action(parsed_args)
+
+
+@pytest.mark.parametrize("cls", NOT_FOUND_COMMANDS)
+def test_node_not_found_returns_1(cls):
+    assert _run_not_found(cls) == 1
diff --git a/tests/unit/commands/test_compute.py b/tests/unit/commands/test_compute.py
new file mode 100644
index 000000000..d1239a729
--- /dev/null
+++ b/tests/unit/commands/test_compute.py
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: Apache-2.0
+
+from unittest.mock import MagicMock, patch
+
+import openstack
+
+from osism.commands import compute
+
+
+def _run(args, conn):
+    cmd = compute.ComputeMigrationList(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()
+    with patch(
+        "osism.tasks.openstack.get_cloud_helpers",
+        return_value=(setup, getconn, cleanup),
+    ):
+        return cmd.take_action(parsed_args)
+
+
+def test_no_user_domain_returns_1():
+    conn = MagicMock()
+    conn.identity.find_domain.return_value = None
+    result = _run(["--user", "u", "--user-domain", "d"], conn)
+    assert result == 1
+
+
+def test_no_user_returns_1():
+    conn = MagicMock()
+    conn.identity.find_user.return_value = None
+    result = _run(["--user", "u"], conn)
+    assert result == 1
+
+
+def test_no_project_domain_returns_1():
+    conn = MagicMock()
+    conn.identity.find_domain.return_value = None
+    result = _run(["--project", "p", "--project-domain", "d"], conn)
+    assert result == 1
+
+
+def test_no_project_returns_1():
+    conn = MagicMock()
+    conn.identity.find_project.return_value = None
+    result = _run(["--project", "p"], conn)
+    assert result == 1
+
+
+def test_multiple_servers_returns_1():
+    conn = MagicMock()
+    conn.compute.find_server.side_effect = openstack.exceptions.DuplicateResource
+    result = _run(["--server", "s"], conn)
+    assert result == 1
+
+
+def test_no_server_returns_1():
+    conn = MagicMock()
+    conn.compute.find_server.return_value = None
+    result = _run(["--server", "s"], conn)
+    assert result == 1
diff --git a/tests/unit/commands/test_netbox.py b/tests/unit/commands/test_netbox.py
index be4deef81..1a1d748c7 100644
--- a/tests/unit/commands/test_netbox.py
+++ b/tests/unit/commands/test_netbox.py
@@ -40,3 +40,40 @@ def test_sync_returns_nonzero_on_task_timeout():
         result = cmd.take_action(parsed_args)
 
     assert result == 1
+
+
+def test_dump_returns_nonzero_when_netbox_not_configured():
+    cmd = netbox.Dump(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["somehost"])
+
+    with patch.dict("osism.utils.__dict__", {"nb": None}):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
+
+
+def test_dump_returns_nonzero_when_device_not_found():
+    cmd = netbox.Dump(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["somehost"])
+
+    fake_nb = MagicMock()
+    fake_nb.dcim.devices.filter.return_value = []
+
+    with patch.dict("osism.utils.__dict__", {"nb": fake_nb}):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
+
+
+def test_console_returns_nonzero_when_netbox_not_configured():
+    cmd = netbox.Console(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["info"])
+
+    with patch("osism.commands.netbox.os.path.exists", return_value=False), patch(
+        "osism.commands.netbox.os.mkdir"
+    ), patch("osism.commands.netbox.os.environ.get", return_value=None), patch(
+        "builtins.open", side_effect=FileNotFoundError
+    ):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1
diff --git a/tests/unit/commands/test_server.py b/tests/unit/commands/test_server.py
new file mode 100644
index 000000000..f2a84ed95
--- /dev/null
+++ b/tests/unit/commands/test_server.py
@@ -0,0 +1,51 @@
+from unittest.mock import MagicMock, patch
+
+from osism.commands import server
+
+
+def _run(args, conn):
+    cmd = server.ServerList(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()
+    with patch(
+        "osism.tasks.openstack.get_cloud_helpers",
+        return_value=(setup, getconn, cleanup),
+    ):
+        return cmd.take_action(parsed_args)
+
+
+def test_no_domain_found_for_user_domain_returns_1():
+    conn = MagicMock()
+    conn.identity.find_domain.return_value = None
+    result = _run(["--user", "u", "--user-domain", "d"], conn)
+    assert result == 1
+
+
+def test_no_user_found_returns_1():
+    conn = MagicMock()
+    conn.identity.find_user.return_value = None
+    result = _run(["--user", "u"], conn)
+    assert result == 1
+
+
+def test_domain_not_found_returns_1():
+    conn = MagicMock()
+    conn.identity.find_domain.return_value = None
+    result = _run(["--domain", "d"], conn)
+    assert result == 1
+
+
+def test_project_domain_not_found_returns_1():
+    conn = MagicMock()
+    conn.identity.find_domain.return_value = None
+    result = _run(["--project", "p", "--project-domain", "d"], conn)
+    assert result == 1
+
+
+def test_project_not_found_returns_1():
+    conn = MagicMock()
+    conn.identity.find_project.return_value = None
+    result = _run(["--project", "p"], conn)
+    assert result == 1
diff --git a/tests/unit/commands/test_volume.py b/tests/unit/commands/test_volume.py
new file mode 100644
index 000000000..715db028a
--- /dev/null
+++ b/tests/unit/commands/test_volume.py
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""Tests for the ``osism volume list`` command.
+
+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.
+"""
+
+from unittest.mock import MagicMock, patch
+
+from osism.commands import volume
+
+
+def _run(args, conn):
+    cmd = volume.VolumeList(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()
+    with patch(
+        "osism.tasks.openstack.get_cloud_helpers",
+        return_value=(setup, getconn, cleanup),
+    ):
+        return cmd.take_action(parsed_args)
+
+
+def test_returns_nonzero_when_domain_not_found():
+    conn = MagicMock()
+    conn.identity.find_domain.return_value = None
+    result = _run(["--domain", "d"], conn)
+    assert result == 1
+
+
+def test_returns_nonzero_when_project_domain_not_found():
+    conn = MagicMock()
+    conn.identity.find_domain.return_value = None
+    result = _run(["--project", "p", "--project-domain", "d"], conn)
+    assert result == 1
+
+
+def test_returns_nonzero_when_project_not_found():
+    conn = MagicMock()
+    conn.identity.find_project.return_value = None
+    result = _run(["--project", "p"], conn)
+    assert result == 1

From 9d58ca41aa0f64f504b37e7c458b3e4d46826914 Mon Sep 17 00:00:00 2001
From: Roger Luethi <luethi@osism.tech>
Date: Mon, 8 Jun 2026 10:45:49 +0200
Subject: [PATCH 5/7] Return non-zero on precondition and operational failures

Several commands hit a runtime failure -- an unmet precondition, a
required integration being absent, a resource missing from a backend,
or an exception they catch -- log it, and then bare-return None. cliff
turns that into exit code 0 (return_code = self.take_action(parsed_args)
or 0), so the failed operation looks like success and the commands are
unsafe in set -e scripts and && chains.

The affected paths now return 1:

- server.ServerMigrate: the server is not in ACTIVE or PAUSED state and
  therefore cannot be live migrated. The requested migration cannot run,
  so this precondition failure is no longer reported as success (it was
  logged at info level, which is why it was easy to miss).
- compute.ComputeEnable: clearing force-down is rejected with
  BadRequestException because `done` evacuation records still remain on
  the host. The except handler bare-returned; it now returns 1.
- baremetal.BaremetalDump: the node could not be found in Ironic, the
  NetBox integration is not available, or the device could not be found
  in NetBox.
- baremetal.BaremetalPing: the NetBox integration is not available, the
  named device was not found in NetBox, or the ping operation raised an
  exception caught by the outer handler.

Adds unit tests driving each failure path and asserting exit 1.

Part of issue #2314 (Category C precondition/exception sites and the
remaining baremetal operational/not-found failures).

Related-Bug: #2314
AI-assisted: Claude Code
Signed-off-by: Roger Luethi <luethi@osism.tech>
---
 osism/commands/baremetal.py           | 12 ++---
 osism/commands/compute.py             |  2 +-
 osism/commands/server.py              |  2 +-
 tests/unit/commands/test_baremetal.py | 73 +++++++++++++++++++++++++++
 tests/unit/commands/test_compute.py   | 27 ++++++++++
 tests/unit/commands/test_server.py    | 22 ++++++++
 6 files changed, 130 insertions(+), 8 deletions(-)

diff --git a/osism/commands/baremetal.py b/osism/commands/baremetal.py
index 36fb3b50f..33b5a07c4 100644
--- a/osism/commands/baremetal.py
+++ b/osism/commands/baremetal.py
@@ -451,7 +451,7 @@ def take_action(self, parsed_args):
 
                 if not node:
                     logger.error(f"Could not find node {name} in Ironic")
-                    return
+                    return 1
 
                 # Get default vars from NetBox local_context_data if available
                 default_vars = {}
@@ -570,7 +570,7 @@ def take_action(self, parsed_args):
             # Check if NetBox connection is available
             if not utils.nb:
                 logger.error("NetBox connection not available")
-                return
+                return 1
 
             try:
                 # Try to find device by name first
@@ -585,7 +585,7 @@ def take_action(self, parsed_args):
                 # If device not found, error out
                 if not device:
                     logger.error(f"Could not find device {name} in NetBox")
-                    return
+                    return 1
 
                 # Get default vars from NetBox local_context_data if available.
                 # Remove frr_parameters and netplan_parameters as they are
@@ -865,14 +865,14 @@ def take_action(self, parsed_args):
 
         if not utils.nb:
             logger.error("NetBox connection not available")
-            return
+            return 1
 
         try:
             if name:
                 devices = [utils.nb.dcim.devices.get(name=name)]
                 if not devices[0]:
                     logger.error(f"Device {name} not found in NetBox")
-                    return
+                    return 1
             else:
                 # Use the NETBOX_FILTER_CONDUCTOR_IRONIC setting to get devices
                 devices = set()
@@ -958,7 +958,7 @@ def take_action(self, parsed_args):
 
         except Exception as e:
             logger.error(f"Error during ping operation: {e}")
-            return
+            return 1
 
 
 class BaremetalBurnIn(Command):
diff --git a/osism/commands/compute.py b/osism/commands/compute.py
index a07dc0a38..67a4066da 100644
--- a/osism/commands/compute.py
+++ b/osism/commands/compute.py
@@ -67,7 +67,7 @@ def take_action(self, parsed_args):
                         "has been restarted, allowing these records to move to `completed` "
                         "before retrying this request."
                     )
-                    return
+                    return 1
 
             logger.info(f"Enabling nova-compute binary @ {host} ({service.id})")
             conn.compute.enable_service(
diff --git a/osism/commands/server.py b/osism/commands/server.py
index 205a51818..532af3701 100644
--- a/osism/commands/server.py
+++ b/osism/commands/server.py
@@ -79,7 +79,7 @@ def take_action(self, parsed_args):
                 logger.info(
                     f"{server[0]} ({server[1]}) in status {server[2]} cannot be live migrated"
                 )
-                return
+                return 1
 
             if yes:
                 answer = "yes"
diff --git a/tests/unit/commands/test_baremetal.py b/tests/unit/commands/test_baremetal.py
index 6b50c54c5..4650deec5 100644
--- a/tests/unit/commands/test_baremetal.py
+++ b/tests/unit/commands/test_baremetal.py
@@ -44,3 +44,76 @@ def _run_not_found(cls):
 @pytest.mark.parametrize("cls", NOT_FOUND_COMMANDS)
 def test_node_not_found_returns_1(cls):
     assert _run_not_found(cls) == 1
+
+
+# --- BaremetalDump failure paths ---
+
+
+def test_dump_ironic_node_not_found_returns_1():
+    cmd = baremetal.BaremetalDump(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["node1", "--ironic"])
+
+    conn = MagicMock()
+    conn.baremetal.find_node.return_value = None
+
+    setup = MagicMock(return_value=("pw", [], None, True))
+    getconn = MagicMock(return_value=conn)
+    cleanup = MagicMock()
+    with patch(
+        "osism.tasks.openstack.get_cloud_helpers",
+        return_value=(setup, getconn, cleanup),
+    ):
+        assert cmd.take_action(parsed_args) == 1
+
+
+def test_dump_netbox_unavailable_returns_1():
+    cmd = baremetal.BaremetalDump(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["node1"])
+
+    with patch.dict("osism.utils.__dict__", {"nb": None}):
+        assert cmd.take_action(parsed_args) == 1
+
+
+def test_dump_device_not_found_returns_1():
+    cmd = baremetal.BaremetalDump(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["node1"])
+
+    fake_nb = MagicMock()
+    fake_nb.dcim.devices.get.return_value = None
+    fake_nb.dcim.devices.filter.return_value = []
+
+    with patch.dict("osism.utils.__dict__", {"nb": fake_nb}):
+        assert cmd.take_action(parsed_args) == 1
+
+
+# --- BaremetalPing failure paths ---
+
+
+def test_ping_netbox_unavailable_returns_1():
+    cmd = baremetal.BaremetalPing(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["node1"])
+
+    with patch.dict("osism.utils.__dict__", {"nb": None}):
+        assert cmd.take_action(parsed_args) == 1
+
+
+def test_ping_device_not_found_returns_1():
+    cmd = baremetal.BaremetalPing(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["node1"])
+
+    fake_nb = MagicMock()
+    fake_nb.dcim.devices.get.return_value = None
+
+    with patch.dict("osism.utils.__dict__", {"nb": fake_nb}):
+        assert cmd.take_action(parsed_args) == 1
+
+
+def test_ping_exception_returns_1():
+    cmd = baremetal.BaremetalPing(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["node1"])
+
+    fake_nb = MagicMock()
+    fake_nb.dcim.devices.get.side_effect = Exception("boom")
+
+    with patch.dict("osism.utils.__dict__", {"nb": fake_nb}):
+        assert cmd.take_action(parsed_args) == 1
diff --git a/tests/unit/commands/test_compute.py b/tests/unit/commands/test_compute.py
index d1239a729..740c7d2b6 100644
--- a/tests/unit/commands/test_compute.py
+++ b/tests/unit/commands/test_compute.py
@@ -60,3 +60,30 @@ def test_no_server_returns_1():
     conn.compute.find_server.return_value = None
     result = _run(["--server", "s"], conn)
     assert result == 1
+
+
+def _run_enable(args, conn):
+    cmd = compute.ComputeEnable(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()
+    with patch(
+        "osism.tasks.openstack.get_cloud_helpers",
+        return_value=(setup, getconn, cleanup),
+    ):
+        return cmd.take_action(parsed_args)
+
+
+def test_enable_returns_1_when_force_up_rejected():
+    # A service that is forced_down, where clearing force-down is rejected
+    # because `done` evacuation records remain.
+    conn = MagicMock()
+    service = MagicMock()
+    service.__getitem__.return_value = True  # service["forced_down"] is True
+    conn.compute.services.return_value = iter([service])
+    conn.compute.update_service_forced_down.side_effect = (
+        openstack.exceptions.BadRequestException
+    )
+    result = _run_enable(["somehost"], conn)
+    assert result == 1
diff --git a/tests/unit/commands/test_server.py b/tests/unit/commands/test_server.py
index f2a84ed95..edbe0eccd 100644
--- a/tests/unit/commands/test_server.py
+++ b/tests/unit/commands/test_server.py
@@ -49,3 +49,25 @@ def test_project_not_found_returns_1():
     conn.identity.find_project.return_value = None
     result = _run(["--project", "p"], conn)
     assert result == 1
+
+
+def _run_migrate(args, conn):
+    cmd = server.ServerMigrate(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()
+    with patch(
+        "osism.tasks.openstack.get_cloud_helpers",
+        return_value=(setup, getconn, cleanup),
+    ):
+        return cmd.take_action(parsed_args)
+
+
+def test_migrate_returns_1_when_server_not_active_or_paused():
+    conn = MagicMock()
+    conn.compute.get_server.return_value = MagicMock(
+        id="i1", name="n1", status="SHUTOFF"
+    )
+    result = _run_migrate(["someinstance"], conn)
+    assert result == 1

From 0972943968be098e9ba4c715d4a2320973f72410 Mon Sep 17 00:00:00 2001
From: Roger Luethi <luethi@osism.tech>
Date: Mon, 8 Jun 2026 10:49:05 +0200
Subject: [PATCH 6/7] Return non-zero on invalid command arguments

Several commands reject invalid input -- a missing required argument, an
unknown resource type, or mutually inconsistent options -- by logging an
error and then bare-returning None. cliff turns that into exit code 0
(return_code = self.take_action(parsed_args) or 0), so the command
refuses to do anything yet still reports success. Returning a non-zero
status on bad input is the conventional CLI behaviour and makes the
commands safe to use in set -e scripts and && chains.

Unlike a failed lookup, these paths never attempt the operation; they
reject the invocation up front. This is therefore a deliberate behaviour
change for the argument-validation paths, distinct from the correctness
fixes for operations that actually ran and failed.

The affected paths now return 1:

- baremetal Deploy, Undeploy, BurnIn, Clean, Provide and Delete: no node
  name was given and --all was not set; and BurnIn additionally when no
  burn-in stressor was selected.
- baremetal PowerOn and PowerOff: no node name was given (these commands
  do not support --all).
- status.Run: an unknown resource type was requested.
- compute.ComputeMigrationList: --changes-since is later than
  --changes-before.

Adds unit tests for each validation path asserting exit 1.

Part of issue #2314 (Category C argument-validation paths).

Related-Bug: #2314
AI-assisted: Claude Code
Signed-off-by: Roger Luethi <luethi@osism.tech>
---
 osism/commands/baremetal.py           | 18 ++++++-------
 osism/commands/compute.py             |  2 +-
 osism/commands/status.py              |  1 +
 tests/unit/commands/test_baremetal.py | 37 +++++++++++++++++++++++++++
 tests/unit/commands/test_compute.py   | 15 +++++++++++
 tests/unit/commands/test_status.py    | 22 ++++++++++++++++
 6 files changed, 85 insertions(+), 10 deletions(-)
 create mode 100644 tests/unit/commands/test_status.py

diff --git a/osism/commands/baremetal.py b/osism/commands/baremetal.py
index 33b5a07c4..a8da70d49 100644
--- a/osism/commands/baremetal.py
+++ b/osism/commands/baremetal.py
@@ -163,7 +163,7 @@ def take_action(self, parsed_args):
 
         if not all_nodes and not name:
             logger.error("Please specify a node name or use --all")
-            return
+            return 1
 
         if all_nodes and rebuild and not yes_i_really_really_mean_it:
             logger.error(
@@ -714,7 +714,7 @@ def take_action(self, parsed_args):
 
         if not all_nodes and not name:
             logger.error("Please specify a node name or use --all")
-            return
+            return 1
 
         if all_nodes and not yes_i_really_really_mean_it:
             logger.error(
@@ -1023,7 +1023,7 @@ def take_action(self, parsed_args):
 
         if not all_nodes and not name:
             logger.error("Please specify a node name or use --all")
-            return
+            return 1
 
         clean_steps = []
         for step, activated in stressor.items():
@@ -1033,7 +1033,7 @@ def take_action(self, parsed_args):
             logger.error(
                 f"Please specify at least one of {', '.join(stressor.keys())} for burn-in"
             )
-            return
+            return 1
 
         from osism.tasks.openstack import get_cloud_helpers
 
@@ -1182,7 +1182,7 @@ def take_action(self, parsed_args):
 
         if not all_nodes and not name:
             logger.error("Please specify a node name or use --all")
-            return
+            return 1
 
         if all_nodes and not yes_i_really_really_mean_it:
             logger.error(
@@ -1314,7 +1314,7 @@ def take_action(self, parsed_args):
 
         if not all_nodes and not name:
             logger.error("Please specify a node name or use --all")
-            return
+            return 1
 
         from osism.tasks.openstack import get_cloud_helpers
 
@@ -1488,7 +1488,7 @@ def take_action(self, parsed_args):
 
         if not name:
             logger.error("Please specify a node name")
-            return
+            return 1
 
         from osism.tasks.openstack import get_cloud_helpers
 
@@ -1547,7 +1547,7 @@ def take_action(self, parsed_args):
 
         if not name:
             logger.error("Please specify a node name")
-            return
+            return 1
 
         from osism.tasks.openstack import get_cloud_helpers
 
@@ -1616,7 +1616,7 @@ def take_action(self, parsed_args):
 
         if not all_nodes and not name:
             logger.error("Please specify a node name or use --all")
-            return
+            return 1
 
         if all_nodes and not yes_i_really_really_mean_it:
             logger.error(
diff --git a/osism/commands/compute.py b/osism/commands/compute.py
index 67a4066da..7c1e1d9c3 100644
--- a/osism/commands/compute.py
+++ b/osism/commands/compute.py
@@ -663,7 +663,7 @@ def take_action(self, parsed_args):
                 logger.error(
                     "changes-since needs to be less or equal to changes-before"
                 )
-                return
+                return 1
 
         import openstack
         from osism.tasks.openstack import get_cloud_helpers
diff --git a/osism/commands/status.py b/osism/commands/status.py
index 7e967d40f..da92052a5 100644
--- a/osism/commands/status.py
+++ b/osism/commands/status.py
@@ -69,6 +69,7 @@ def take_action(self, parsed_args):
             )
         else:
             logger.error(f"Unknown resource type '{type_of_resource}'")
+            return 1
 
 
 class Database(Command):
diff --git a/tests/unit/commands/test_baremetal.py b/tests/unit/commands/test_baremetal.py
index 4650deec5..5f94902bf 100644
--- a/tests/unit/commands/test_baremetal.py
+++ b/tests/unit/commands/test_baremetal.py
@@ -117,3 +117,40 @@ def test_ping_exception_returns_1():
 
     with patch.dict("osism.utils.__dict__", {"nb": fake_nb}):
         assert cmd.take_action(parsed_args) == 1
+
+
+# --- Argument-validation failure paths ---
+#
+# These commands validate their arguments at the very top of take_action,
+# before any cloud setup. When neither a node name nor --all is given (or, for
+# the power commands, when no node name is given) the command must report a
+# non-zero exit code rather than silently succeeding. No mocking is required
+# because the validation branch returns before any infrastructure access.
+MISSING_NODE_COMMANDS = [
+    baremetal.BaremetalDeploy,
+    baremetal.BaremetalUndeploy,
+    baremetal.BaremetalBurnIn,
+    baremetal.BaremetalClean,
+    baremetal.BaremetalProvide,
+    baremetal.BaremetalPowerOn,
+    baremetal.BaremetalPowerOff,
+    baremetal.BaremetalDelete,
+]
+
+
+@pytest.mark.parametrize("cls", MISSING_NODE_COMMANDS)
+def test_missing_node_argument_returns_1(cls):
+    cmd = cls(MagicMock(), MagicMock())
+    # Neither a node name nor --all: the argument-validation branch fires.
+    parsed_args = cmd.get_parser("test").parse_args([])
+    assert cmd.take_action(parsed_args) == 1
+
+
+def test_burnin_no_stressor_returns_1():
+    cmd = baremetal.BaremetalBurnIn(MagicMock(), MagicMock())
+    # Select a node so the node check passes, but disable every stressor so the
+    # "at least one stressor" validation branch fires before any cloud setup.
+    parsed_args = cmd.get_parser("test").parse_args(
+        ["node1", "--no-cpu", "--no-memory", "--no-disk"]
+    )
+    assert cmd.take_action(parsed_args) == 1
diff --git a/tests/unit/commands/test_compute.py b/tests/unit/commands/test_compute.py
index 740c7d2b6..478de97ab 100644
--- a/tests/unit/commands/test_compute.py
+++ b/tests/unit/commands/test_compute.py
@@ -87,3 +87,18 @@ def test_enable_returns_1_when_force_up_rejected():
     )
     result = _run_enable(["somehost"], conn)
     assert result == 1
+
+
+def test_migration_list_rejects_changes_since_after_changes_before():
+    # changes-since must be <= changes-before; this is invalid input and the
+    # check runs before any cloud setup is reached.
+    result = _run(
+        [
+            "--changes-since",
+            "2025-01-02T00:00:00",
+            "--changes-before",
+            "2025-01-01T00:00:00",
+        ],
+        MagicMock(),
+    )
+    assert result == 1
diff --git a/tests/unit/commands/test_status.py b/tests/unit/commands/test_status.py
new file mode 100644
index 000000000..fb2589fb3
--- /dev/null
+++ b/tests/unit/commands/test_status.py
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: Apache-2.0
+
+"""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.
+"""
+
+import argparse
+from unittest.mock import MagicMock, patch
+
+from osism.commands import status
+
+
+def test_run_returns_1_for_unknown_resource_type():
+    cmd = status.Run(MagicMock(), MagicMock())
+    parsed_args = argparse.Namespace(type=["bogus"])
+
+    with patch("celery.Celery"):
+        result = cmd.take_action(parsed_args)
+
+    assert result == 1

From 8278470d34f0e1cc0830e06149621426fc25c41a Mon Sep 17 00:00:00 2001
From: Roger Luethi <luethi@osism.tech>
Date: Mon, 8 Jun 2026 11:52:01 +0200
Subject: [PATCH 7/7] Return non-zero on unconfirmed destructive --all

The destructive baremetal commands let an operator target every node at
once with --all, guarded by a required --yes-i-really-really-mean-it
confirmation. When --all was given without that confirmation, the
command logged the "Please confirm ..." message and bare-returned None.
cliff turns that into exit code 0 (return_code =
self.take_action(parsed_args) or 0), so a destructive command that
refused to run for want of confirmation still reported success.

Returning a non-zero status when the command declines to act is the
conventional CLI behaviour and lets wrappers and set -e scripts tell a
refused invocation apart from a completed one. Like the
argument-validation change, these paths never attempt the operation, so
this is a deliberate behaviour change.

The confirmation guards in BaremetalDeploy (rebuild), BaremetalUndeploy,
BaremetalClean and BaremetalDelete now return 1.

The BaremetalBurnIn active-node confirmation is intentionally not
changed: it `continue`s to the next node inside a per-node loop rather
than refusing the whole command, so it is a different case.

Adds unit tests asserting exit 1 when --all is given without
confirmation.

Part of issue #2314 (Category C unconfirmed-yes paths).

Related-Bug: #2314
AI-assisted: Claude Code
Signed-off-by: Roger Luethi <luethi@osism.tech>
---
 osism/commands/baremetal.py           |  8 +++----
 tests/unit/commands/test_baremetal.py | 30 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/osism/commands/baremetal.py b/osism/commands/baremetal.py
index a8da70d49..659bc1a56 100644
--- a/osism/commands/baremetal.py
+++ b/osism/commands/baremetal.py
@@ -169,7 +169,7 @@ def take_action(self, parsed_args):
             logger.error(
                 "Please confirm that you wish to rebuild all nodes by specifying '--yes-i-really-really-mean-it'"
             )
-            return
+            return 1
 
         import openstack
         from osism.tasks.openstack import get_cloud_helpers
@@ -720,7 +720,7 @@ def take_action(self, parsed_args):
             logger.error(
                 "Please confirm that you wish to undeploy all nodes by specifying '--yes-i-really-really-mean-it'"
             )
-            return
+            return 1
 
         from osism.tasks.openstack import get_cloud_helpers
 
@@ -1188,7 +1188,7 @@ def take_action(self, parsed_args):
             logger.error(
                 "Please confirm that you wish to clean all nodes by specifying '--yes-i-really-really-mean-it'"
             )
-            return
+            return 1
 
         clean_steps = [{"interface": "deploy", "step": "erase_devices"}]
 
@@ -1622,7 +1622,7 @@ def take_action(self, parsed_args):
             logger.error(
                 "Please confirm that you wish to delete all nodes by specifying '--yes-i-really-really-mean-it'"
             )
-            return
+            return 1
 
         from osism.tasks.openstack import get_cloud_helpers
 
diff --git a/tests/unit/commands/test_baremetal.py b/tests/unit/commands/test_baremetal.py
index 5f94902bf..bf254b8d0 100644
--- a/tests/unit/commands/test_baremetal.py
+++ b/tests/unit/commands/test_baremetal.py
@@ -154,3 +154,33 @@ def test_burnin_no_stressor_returns_1():
         ["node1", "--no-cpu", "--no-memory", "--no-disk"]
     )
     assert cmd.take_action(parsed_args) == 1
+
+
+# When --all is requested for a destructive operation without the
+# --yes-i-really-really-mean-it confirmation, the command refuses to proceed
+# and must return a non-zero exit code rather than reporting success. The
+# confirmation guard runs before any cloud setup, so no mocking is needed.
+
+
+def test_deploy_all_without_confirmation_returns_1():
+    cmd = baremetal.BaremetalDeploy(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["--all", "--rebuild"])
+    assert cmd.take_action(parsed_args) == 1
+
+
+def test_undeploy_all_without_confirmation_returns_1():
+    cmd = baremetal.BaremetalUndeploy(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["--all"])
+    assert cmd.take_action(parsed_args) == 1
+
+
+def test_clean_all_without_confirmation_returns_1():
+    cmd = baremetal.BaremetalClean(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["--all"])
+    assert cmd.take_action(parsed_args) == 1
+
+
+def test_delete_all_without_confirmation_returns_1():
+    cmd = baremetal.BaremetalDelete(MagicMock(), MagicMock())
+    parsed_args = cmd.get_parser("test").parse_args(["--all"])
+    assert cmd.take_action(parsed_args) == 1