diff --git a/osism/commands/stress.py b/osism/commands/stress.py index 372d5e151..7370b012a 100644 --- a/osism/commands/stress.py +++ b/osism/commands/stress.py @@ -1,167 +1,49 @@ # SPDX-License-Identifier: Apache-2.0 +import argparse import subprocess from cliff.command import Command from loguru import logger +STRESS_TOOL = "/openstack-simple-stress/openstack_simple_stress/main.py" -class OpenStackStress(Command): - """Run OpenStack stress testing tool""" - def get_parser(self, prog_name): - parser = super(OpenStackStress, self).get_parser(prog_name) +class _PassThroughParser(argparse.ArgumentParser): + """Keep every argument this parser does not know for the stress tool.""" - # Boolean flags - parser.add_argument( - "--no-cleanup", - action="store_true", - help="Do not clean up resources after test", - ) - parser.add_argument( - "--debug", - action="store_true", - help="Enable debug mode", - ) - parser.add_argument( - "--no-delete", - action="store_true", - help="Do not delete resources", - ) - parser.add_argument( - "--no-volume", - action="store_true", - help="Do not create volumes", - ) - parser.add_argument( - "--no-boot-volume", - action="store_true", - help="Do not use boot volumes", - ) - parser.add_argument( - "--no-wait", - action="store_true", - help="Do not wait for resources", - ) - parser.add_argument( - "--clean", - action="store_true", - help="Clean up leftover resources matching the prefix", - ) + def parse_args(self, args=None, namespace=None): # type: ignore[override] + parsed, extra = self.parse_known_args(args, namespace) + if "--" in extra: + extra.remove("--") + # Credentials are set up for parsed.cloud; a second --cloud would make + # the tool run against a cloud whose credentials were never prepared. + if any(a == "--cloud" or a.startswith("--cloud=") for a in extra): + self.error("--cloud must come before '--'") + parsed.tool_args = extra + return parsed - # Integer parameters with defaults - parser.add_argument( - "--interval", - type=int, - default=10, - help="Interval in seconds (default: %(default)s)", - ) - parser.add_argument( - "--number", - type=int, - default=1, - help="Number of instances (default: %(default)s)", - ) - parser.add_argument( - "--parallel", - type=int, - default=1, - help="Parallel operations (default: %(default)s)", - ) - parser.add_argument( - "--timeout", - type=int, - default=600, - help="Timeout in seconds (default: %(default)s)", - ) - parser.add_argument( - "--volume-number", - type=int, - default=1, - help="Number of volumes per instance (default: %(default)s)", - ) - parser.add_argument( - "--volume-size", - type=int, - default=1, - help="Volume size in GB (default: %(default)s)", - ) - parser.add_argument( - "--boot-volume-size", - type=int, - default=20, - help="Boot volume size in GB (default: %(default)s)", - ) - # String parameters with defaults +class OpenStackStress(Command): + """Run the OpenStack stress testing tool (openstack-simple-stress). + + All options except --cloud are passed to the tool unchanged. Options that + the osism CLI itself uses (--debug, -h/--help, -v, -q, --version, + --log-file) must follow a "--": e.g. "osism openstack stress -- --help" + shows the tool's own options. + """ + + def get_parser(self, prog_name): + parser = _PassThroughParser( + prog=prog_name, + description=self.get_description(), + add_help=False, + ) parser.add_argument( "--cloud", - type=str, default="simple-stress", help="Cloud name in clouds.yaml (default: %(default)s)", ) - parser.add_argument( - "--flavor", - type=str, - default="SCS-1V-2", - help="Flavor name (default: %(default)s)", - ) - parser.add_argument( - "--image", - type=str, - default="Ubuntu 24.04", - help="Image name (default: %(default)s)", - ) - parser.add_argument( - "--subnet-cidr", - type=str, - default="10.100.0.0/16", - help="Subnet CIDR (default: %(default)s)", - ) - parser.add_argument( - "--prefix", - type=str, - default="simple-stress", - help="Resource name prefix (default: %(default)s)", - ) - parser.add_argument( - "--compute-zone", - type=str, - default="nova", - help="Compute availability zone (default: %(default)s)", - ) - parser.add_argument( - "--storage-zone", - type=str, - default="nova", - help="Storage availability zone (default: %(default)s)", - ) - parser.add_argument( - "--affinity", - type=str, - default="soft-anti-affinity", - choices=[ - "soft-affinity", - "soft-anti-affinity", - "affinity", - "anti-affinity", - ], - help="Server group policy (default: %(default)s)", - ) - parser.add_argument( - "--volume-type", - type=str, - default="__DEFAULT__", - help="Volume type (default: %(default)s)", - ) - parser.add_argument( - "--mode", - type=str, - default="rolling", - choices=["rolling", "block"], - help="Execution mode (default: %(default)s)", - ) - return parser def take_action(self, parsed_args): @@ -180,49 +62,14 @@ def take_action(self, parsed_args): ) return 1 - # Build the command command = [ "python3", - "/openstack-simple-stress/openstack_simple_stress/main.py", + STRESS_TOOL, + "--cloud", + parsed_args.cloud, + *parsed_args.tool_args, ] - # Add boolean flags - if parsed_args.no_cleanup: - command.append("--no-cleanup") - if parsed_args.debug: - command.append("--debug") - if parsed_args.no_delete: - command.append("--no-delete") - if parsed_args.no_volume: - command.append("--no-volume") - if parsed_args.no_boot_volume: - command.append("--no-boot-volume") - if parsed_args.no_wait: - command.append("--no-wait") - if parsed_args.clean: - command.append("--clean") - - # Add integer parameters - command.extend(["--interval", str(parsed_args.interval)]) - command.extend(["--number", str(parsed_args.number)]) - command.extend(["--parallel", str(parsed_args.parallel)]) - command.extend(["--timeout", str(parsed_args.timeout)]) - command.extend(["--volume-number", str(parsed_args.volume_number)]) - command.extend(["--volume-size", str(parsed_args.volume_size)]) - command.extend(["--boot-volume-size", str(parsed_args.boot_volume_size)]) - - # Add string parameters - command.extend(["--cloud", parsed_args.cloud]) - command.extend(["--flavor", parsed_args.flavor]) - command.extend(["--image", parsed_args.image]) - command.extend(["--subnet-cidr", parsed_args.subnet_cidr]) - command.extend(["--prefix", parsed_args.prefix]) - command.extend(["--compute-zone", parsed_args.compute_zone]) - command.extend(["--storage-zone", parsed_args.storage_zone]) - command.extend(["--affinity", parsed_args.affinity]) - command.extend(["--volume-type", parsed_args.volume_type]) - command.extend(["--mode", parsed_args.mode]) - logger.debug( f"Executing OpenStack stress test with command: {' '.join(command)}" ) @@ -231,9 +78,7 @@ def take_action(self, parsed_args): result = subprocess.run(command, check=False) return result.returncode except FileNotFoundError: - logger.error( - "OpenStack stress tool not found at /openstack-simple-stress/openstack_simple_stress/main.py" - ) + logger.error(f"OpenStack stress tool not found at {STRESS_TOOL}") return 1 except Exception as e: logger.error(f"Error executing OpenStack stress tool: {e}") diff --git a/tests/unit/commands/test_stress.py b/tests/unit/commands/test_stress.py index d97861ce3..497e64881 100644 --- a/tests/unit/commands/test_stress.py +++ b/tests/unit/commands/test_stress.py @@ -8,16 +8,6 @@ STRESS_TOOL = "/openstack-simple-stress/openstack_simple_stress/main.py" -BOOLEAN_FLAGS = [ - "--no-cleanup", - "--debug", - "--no-delete", - "--no-volume", - "--no-boot-volume", - "--no-wait", - "--clean", -] - def _run(args, run_mock=None, setup_success=True): """Drive OpenStackStress.take_action with mocked cloud helpers.""" @@ -33,65 +23,74 @@ def _run(args, run_mock=None, setup_success=True): return_value=(setup, MagicMock(), cleanup), ), patch("osism.commands.stress.subprocess.run", run_mock): result = cmd.take_action(parsed_args) - return result, run_mock, cleanup - - -def _flag_value(command, flag): - return command[command.index(flag) + 1] - - -def test_defaults_build_expected_command(): - result, run_mock, _ = _run([]) - - command = run_mock.call_args[0][0] - assert command[:2] == ["python3", STRESS_TOOL] - for flag in BOOLEAN_FLAGS: - assert flag not in command - - expected = { - "--interval": "10", - "--number": "1", - "--parallel": "1", - "--timeout": "600", - "--volume-number": "1", - "--volume-size": "1", - "--boot-volume-size": "20", - "--cloud": "simple-stress", - "--flavor": "SCS-1V-2", - "--image": "Ubuntu 24.04", - "--subnet-cidr": "10.100.0.0/16", - "--prefix": "simple-stress", - "--compute-zone": "nova", - "--storage-zone": "nova", - "--affinity": "soft-anti-affinity", - "--volume-type": "__DEFAULT__", - "--mode": "rolling", - } - for flag, value in expected.items(): - assert _flag_value(command, flag) == value + return result, run_mock, setup, cleanup + +def test_defaults_pass_only_the_cloud(): + result, run_mock, setup, _ = _run([]) + + assert run_mock.call_args[0][0] == [ + "python3", + STRESS_TOOL, + "--cloud", + "simple-stress", + ] + setup.assert_called_once_with("simple-stress") assert result == 0 -@pytest.mark.parametrize("flag", BOOLEAN_FLAGS) -def test_boolean_flag_appended(flag): - _, run_mock, _ = _run([flag]) - assert flag in run_mock.call_args[0][0] +def test_tool_options_are_forwarded_unchanged(): + args = [ + "--number", + "5", + "--profile", + "acceptance", + "--no-network", + "--clean", + "--yes", + ] + _, run_mock, _, _ = _run(args) + + assert run_mock.call_args[0][0][4:] == args + + +def test_cloud_is_used_for_setup_and_forwarded(): + _, run_mock, setup, _ = _run(["--cloud", "admin", "--number", "2"]) + + setup.assert_called_once_with("admin") + assert run_mock.call_args[0][0][2:] == ["--cloud", "admin", "--number", "2"] + + +def test_separator_is_removed(): + _, run_mock, _, _ = _run(["--number", "3", "--", "--debug", "--help"]) + + assert run_mock.call_args[0][0][4:] == ["--number", "3", "--debug", "--help"] + + +def test_cloud_after_separator_is_rejected(): + cmd = stress.OpenStackStress(MagicMock(), MagicMock()) + with pytest.raises(SystemExit) as exc: + cmd.get_parser("test").parse_args(["--", "--cloud", "admin"]) + assert exc.value.code == 2 + + +def test_cloud_equals_after_separator_is_rejected(): + cmd = stress.OpenStackStress(MagicMock(), MagicMock()) + with pytest.raises(SystemExit) as exc: + cmd.get_parser("test").parse_args(["--number", "2", "--", "--cloud=admin"]) + assert exc.value.code == 2 -def test_custom_values_propagated(): - _, run_mock, _ = _run(["--number", "5", "--flavor", "X", "--volume-size", "10"]) +def test_no_interval_is_imposed(): + _, run_mock, _, _ = _run([]) - command = run_mock.call_args[0][0] - assert _flag_value(command, "--number") == "5" - assert _flag_value(command, "--flavor") == "X" - assert _flag_value(command, "--volume-size") == "10" + assert "--interval" not in run_mock.call_args[0][0] -@pytest.mark.parametrize("returncode", [0, 3]) +@pytest.mark.parametrize("returncode", [0, 1, 2, 130]) def test_returncode_passed_through(returncode): run_mock = MagicMock(return_value=MagicMock(returncode=returncode)) - result, _, cleanup = _run([], run_mock=run_mock) + result, _, _, cleanup = _run([], run_mock=run_mock) assert result == returncode cleanup.assert_called_once_with(["tempfile"], "/cwd") @@ -99,7 +98,7 @@ def test_returncode_passed_through(returncode): def test_tool_not_found_returns_1(loguru_logs): run_mock = MagicMock(side_effect=FileNotFoundError()) - result, _, cleanup = _run([], run_mock=run_mock) + result, _, _, cleanup = _run([], run_mock=run_mock) assert result == 1 assert any( @@ -111,14 +110,14 @@ def test_tool_not_found_returns_1(loguru_logs): def test_generic_exception_returns_1(): run_mock = MagicMock(side_effect=RuntimeError("boom")) - result, _, cleanup = _run([], run_mock=run_mock) + result, _, _, cleanup = _run([], run_mock=run_mock) assert result == 1 cleanup.assert_called_once_with(["tempfile"], "/cwd") def test_setup_failure_returns_1(): - result, run_mock, _ = _run([], setup_success=False) + result, run_mock, _, _ = _run([], setup_success=False) assert result == 1 run_mock.assert_not_called()