diff --git a/osism/commands/baremetal.py b/osism/commands/baremetal.py index 52451728b..659bc1a56 100644 --- a/osism/commands/baremetal.py +++ b/osism/commands/baremetal.py @@ -163,13 +163,13 @@ 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( "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 @@ -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: @@ -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 @@ -714,13 +714,13 @@ 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( "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 @@ -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: @@ -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): @@ -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 @@ -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: @@ -1182,13 +1182,13 @@ 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( "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"}] @@ -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: @@ -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 @@ -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: @@ -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 @@ -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") @@ -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 @@ -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" @@ -1616,13 +1616,13 @@ 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( "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 @@ -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..7c1e1d9c3 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( @@ -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 @@ -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 8e0be1658..42fa1f4e4 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( @@ -536,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"], @@ -585,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 @@ -631,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/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/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/osism/commands/server.py b/osism/commands/server.py index eb11cecff..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" @@ -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/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/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/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/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_baremetal.py b/tests/unit/commands/test_baremetal.py new file mode 100644 index 000000000..bf254b8d0 --- /dev/null +++ b/tests/unit/commands/test_baremetal.py @@ -0,0 +1,186 @@ +# 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 + + +# --- 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 + + +# --- 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 + + +# 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 diff --git a/tests/unit/commands/test_compute.py b/tests/unit/commands/test_compute.py new file mode 100644 index 000000000..478de97ab --- /dev/null +++ b/tests/unit/commands/test_compute.py @@ -0,0 +1,104 @@ +# 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 + + +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 + + +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_netbox.py b/tests/unit/commands/test_netbox.py new file mode 100644 index 000000000..1a1d748c7 --- /dev/null +++ b/tests/unit/commands/test_netbox.py @@ -0,0 +1,79 @@ +# 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 + + +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_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_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 diff --git a/tests/unit/commands/test_server.py b/tests/unit/commands/test_server.py new file mode 100644 index 000000000..edbe0eccd --- /dev/null +++ b/tests/unit/commands/test_server.py @@ -0,0 +1,73 @@ +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 + + +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 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 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 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 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