From 8ec9843bb39a8beef10e8c8a6579c9707913d526 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 24 Nov 2023 22:02:52 +0000 Subject: [PATCH] Fix/De-complicate the performance test scenarios Signed-off-by: Pedro Algarvio --- .../pytests/scenarios/performance/conftest.py | 37 +-- .../scenarios/performance/test_performance.py | 260 +++++++++--------- 2 files changed, 132 insertions(+), 165 deletions(-) diff --git a/tests/pytests/scenarios/performance/conftest.py b/tests/pytests/scenarios/performance/conftest.py index d156535ff1d..13fbb831d7c 100644 --- a/tests/pytests/scenarios/performance/conftest.py +++ b/tests/pytests/scenarios/performance/conftest.py @@ -5,17 +5,10 @@ import logging import shutil import pytest -from saltfactories.daemons.container import Container +from saltfactories.utils import random_string -import salt.utils.path from tests.support.sminion import create_sminion -docker = pytest.importorskip("docker") -# pylint: disable=3rd-party-module-not-gated,no-name-in-module -from docker.errors import DockerException # isort:skip - -# pylint: enable=3rd-party-module-not-gated,no-name-in-module - pytestmark = [ pytest.mark.slow_test, pytest.mark.skip_if_binaries_missing("docker"), @@ -26,36 +19,18 @@ log = logging.getLogger(__name__) @pytest.fixture(scope="session") -def docker_client(): - if docker is None: - pytest.skip("The docker python library is not available") - - if salt.utils.path.which("docker") is None: - pytest.skip("The docker binary is not available") - try: - client = docker.from_env() - connectable = Container.client_connectable(client) - if connectable is not True: # pragma: no cover - pytest.skip(connectable) - return client - except DockerException: - pytest.skip("Failed to get a connection to docker running on the system") +def docker_network_name(): + return random_string("salt-perf-", uppercase=False) @pytest.fixture(scope="session") -def network(): - return "salt-performance" - - -@pytest.fixture(scope="session") -def host_docker_network_ip_address(network): +def host_docker_network_ip_address(docker_network_name): sminion = create_sminion() - network_name = network network_subnet = "10.0.21.0/24" network_gateway = "10.0.21.1" try: ret = sminion.states.docker_network.present( - network_name, + docker_network_name, driver="bridge", ipam_pools=[{"subnet": network_subnet, "gateway": network_gateway}], ) @@ -66,7 +41,7 @@ def host_docker_network_ip_address(network): pytest.skip("Failed to create docker network: {}".format(ret)) yield network_gateway finally: - sminion.states.docker_network.absent(network_name) + sminion.states.docker_network.absent(docker_network_name) @pytest.fixture(scope="session") diff --git a/tests/pytests/scenarios/performance/test_performance.py b/tests/pytests/scenarios/performance/test_performance.py index 85b92ed986e..22aad753bda 100644 --- a/tests/pytests/scenarios/performance/test_performance.py +++ b/tests/pytests/scenarios/performance/test_performance.py @@ -1,7 +1,9 @@ +import logging import os import shutil -import time +import sys +import attr import pytest from pytestshellutils.utils import ports from saltfactories.daemons import master @@ -9,32 +11,34 @@ from saltfactories.daemons.container import SaltDaemon, SaltMinion from saltfactories.utils import random_string from salt.version import SaltVersionsInfo, __version__ +from tests.conftest import CODE_DIR -pytestmark = [pytest.mark.skip_if_binaries_missing("docker")] +log = logging.getLogger(__name__) + +pytestmark = [ + pytest.mark.skip_if_binaries_missing("docker"), +] -class ContainerMaster(SaltDaemon, master.SaltMaster): +@attr.s(kw_only=True, slots=True) +class SaltMaster(SaltDaemon, master.SaltMaster): """ - Containerized salt master that has no check events + Salt minion daemon implementation running in a docker container. """ def get_display_name(self): + """ + Returns a human readable name for the factory. + """ return master.SaltMaster.get_display_name(self) def get_check_events(self): - return [] + """ + Return salt events to check. - -class ContainerMinion(SaltMinion): - """ - Containerized salt minion that has no check events - """ - - def get_check_events(self): - return [] - - -# ---------------------- Previous Version Setup ---------------------- + Return a list of tuples in the form of `(master_id, event_tag)` check against to ensure the daemon is running + """ + return master.SaltMaster.get_check_events(self) @pytest.fixture @@ -49,7 +53,7 @@ def curr_version(): @pytest.fixture def prev_master_id(): - return random_string("master-performance-prev-", uppercase=False) + return random_string("master-perf-prev-", uppercase=False) @pytest.fixture @@ -57,9 +61,8 @@ def prev_master( request, salt_factories, host_docker_network_ip_address, - network, + docker_network_name, prev_version, - docker_client, prev_master_id, ): root_dir = salt_factories.get_root_dir_for_daemon(prev_master_id) @@ -69,35 +72,36 @@ def prev_master( config_defaults = { "root_dir": str(root_dir), "transport": request.config.getoption("--transport"), - "user": False, + "user": "root", } - publish_port = ports.get_unused_localhost_port() - ret_port = ports.get_unused_localhost_port() config_overrides = { + "open_mode": True, "interface": "0.0.0.0", - "publish_port": publish_port, - "ret_port": ret_port, + "publish_port": ports.get_unused_localhost_port(), + "ret_port": ports.get_unused_localhost_port(), "log_level_logfile": "quiet", "pytest-master": { "log": {"host": host_docker_network_ip_address}, + "returner_address": {"host": host_docker_network_ip_address}, }, } factory = salt_factories.salt_master_daemon( prev_master_id, + name=prev_master_id, defaults=config_defaults, overrides=config_overrides, - factory_class=ContainerMaster, - image="ghcr.io/saltstack/salt-ci-containers/salt:{}".format(prev_version), + factory_class=SaltMaster, base_script_args=["--log-level=debug"], + image=f"ghcr.io/saltstack/salt-ci-containers/salt:{prev_version}", container_run_kwargs={ - "network": network, + "network": docker_network_name, "hostname": prev_master_id, }, - docker_client=docker_client, - name=prev_master_id, start_timeout=120, max_start_attempts=1, + pull_before_start=True, + skip_on_pull_failure=True, skip_if_docker_client_not_connectable=True, ) with factory.started(): @@ -122,7 +126,7 @@ def prev_salt_run_cli(prev_master): @pytest.fixture def prev_minion_id(): return random_string( - "minion-performance-prev-", + "minion-perf-prev-", uppercase=False, ) @@ -131,34 +135,37 @@ def prev_minion_id(): def prev_minion( prev_minion_id, prev_master, - docker_client, prev_version, host_docker_network_ip_address, - network, - prev_master_id, + docker_network_name, ): config_overrides = { - "master": prev_master_id, - "user": False, - "pytest-minion": {"log": {"host": host_docker_network_ip_address}}, + "master": prev_master.id, + "open_mode": True, + "user": "root", + "pytest-minion": { + "log": {"host": host_docker_network_ip_address}, + "returner_address": {"host": host_docker_network_ip_address}, + }, } factory = prev_master.salt_minion_daemon( prev_minion_id, - overrides=config_overrides, - factory_class=ContainerMinion, - # SaltMinion kwargs name=prev_minion_id, - image="ghcr.io/saltstack/salt-ci-containers/salt:{}".format(prev_version), - docker_client=docker_client, - start_timeout=120, - pull_before_start=False, - skip_if_docker_client_not_connectable=True, + overrides=config_overrides, + factory_class=SaltMinion, + base_script_args=["--log-level=debug"], + image=f"ghcr.io/saltstack/salt-ci-containers/salt:{prev_version}", container_run_kwargs={ - "network": network, + "network": docker_network_name, "hostname": prev_minion_id, }, + start_timeout=60, max_start_attempts=1, + pull_before_start=True, + skip_on_pull_failure=True, + skip_if_docker_client_not_connectable=True, ) + factory.python_executable = "python3" factory.after_terminate( pytest.helpers.remove_stale_minion_key, prev_master, factory.id ) @@ -172,21 +179,38 @@ def prev_sls(sls_contents, state_tree, tmp_path): location = tmp_path / "prev" / "testfile" location.parent.mkdir() with pytest.helpers.temp_file( - "{}.sls".format(sls_name), sls_contents.format(path=str(location)), state_tree + f"{sls_name}.sls", sls_contents.format(path=str(location)), state_tree ): yield sls_name -# ---------------------- Current Version Setup ---------------------- +def _install_salt_in_container(container): + ret = container.run( + "python3", + "-c", + "import sys; sys.stderr.write('{}.{}'.format(*sys.version_info))", + ) + assert ret.returncode == 0 + if not ret.stdout: + requirements_py_version = "{}.{}".format(*sys.version_info) + else: + requirements_py_version = ret.stdout.strip() - -def _install_local_salt(factory): - factory.run("pip install /saltcode") + ret = container.run( + "python3", + "-m", + "pip", + "install", + f"--constraint=/salt/requirements/static/ci/py{requirements_py_version}/linux.txt", + "/salt", + ) + log.debug("Install Salt in the container: %s", ret) + assert ret.returncode == 0 @pytest.fixture def curr_master_id(): - return random_string("master-performance-", uppercase=False) + return random_string("master-perf-curr-", uppercase=False) @pytest.fixture @@ -194,8 +218,7 @@ def curr_master( request, salt_factories, host_docker_network_ip_address, - network, - docker_client, + docker_network_name, curr_master_id, ): root_dir = salt_factories.get_root_dir_for_daemon(curr_master_id) @@ -205,43 +228,46 @@ def curr_master( config_defaults = { "root_dir": str(root_dir), "transport": request.config.getoption("--transport"), - "user": False, + "user": "root", } publish_port = ports.get_unused_localhost_port() ret_port = ports.get_unused_localhost_port() config_overrides = { + "open_mode": True, "interface": "0.0.0.0", "publish_port": publish_port, "ret_port": ret_port, "log_level_logfile": "quiet", "pytest-master": { "log": {"host": host_docker_network_ip_address}, + "returner_address": {"host": host_docker_network_ip_address}, }, } factory = salt_factories.salt_master_daemon( curr_master_id, + name=curr_master_id, defaults=config_defaults, overrides=config_overrides, - factory_class=ContainerMaster, - image="ghcr.io/saltstack/salt-ci-containers/salt:current", + factory_class=SaltMaster, base_script_args=["--log-level=debug"], + image="ghcr.io/saltstack/salt-ci-containers/salt:current", container_run_kwargs={ - "network": network, + "network": docker_network_name, "hostname": curr_master_id, # Bind the current code to a directory for pip installing "volumes": { - os.environ["REPO_ROOT_DIR"]: {"bind": "/saltcode", "mode": "z"} + str(CODE_DIR): {"bind": "/salt", "mode": "z"}, }, }, - docker_client=docker_client, - name=curr_master_id, start_timeout=120, max_start_attempts=1, + pull_before_start=True, + skip_on_pull_failure=True, skip_if_docker_client_not_connectable=True, ) - factory.before_start(_install_local_salt, factory) + factory.before_start(_install_salt_in_container, factory) with factory.started(): yield factory @@ -264,7 +290,7 @@ def curr_salt_key_cli(curr_master): @pytest.fixture def curr_minion_id(): return random_string( - "minion-performance-curr-", + "minion-perf-curr-", uppercase=False, ) @@ -273,38 +299,40 @@ def curr_minion_id(): def curr_minion( curr_minion_id, curr_master, - docker_client, host_docker_network_ip_address, - network, - curr_master_id, + docker_network_name, ): config_overrides = { - "master": curr_master_id, - "user": False, - "pytest-minion": {"log": {"host": host_docker_network_ip_address}}, + "master": curr_master.id, + "open_mode": True, + "user": "root", + "pytest-minion": { + "log": {"host": host_docker_network_ip_address}, + "returner_address": {"host": host_docker_network_ip_address}, + }, } factory = curr_master.salt_minion_daemon( curr_minion_id, - overrides=config_overrides, - factory_class=ContainerMinion, - # SaltMinion kwargs name=curr_minion_id, + overrides=config_overrides, + factory_class=SaltMinion, + base_script_args=["--log-level=debug"], image="ghcr.io/saltstack/salt-ci-containers/salt:current", - docker_client=docker_client, - start_timeout=120, - pull_before_start=False, - skip_if_docker_client_not_connectable=True, container_run_kwargs={ - "network": network, + "network": docker_network_name, "hostname": curr_minion_id, # Bind the current code to a directory for pip installing "volumes": { - os.environ["REPO_ROOT_DIR"]: {"bind": "/saltcode", "mode": "z"} + str(CODE_DIR): {"bind": "/salt", "mode": "z"}, }, }, + start_timeout=120, max_start_attempts=1, + pull_before_start=True, + skip_on_pull_failure=True, + skip_if_docker_client_not_connectable=True, ) - factory.before_start(_install_local_salt, factory) + factory.before_start(_install_salt_in_container, factory) factory.after_terminate( pytest.helpers.remove_stale_minion_key, curr_master, factory.id ) @@ -318,25 +346,25 @@ def curr_sls(sls_contents, state_tree, tmp_path): location = tmp_path / "curr" / "testfile" location.parent.mkdir() with pytest.helpers.temp_file( - "{}.sls".format(sls_name), sls_contents.format(path=str(location)), state_tree + f"{sls_name}.sls", sls_contents.format(path=str(location)), state_tree ): yield sls_name -def _wait_for_stdout(expected, func, *args, timeout=120, **kwargs): - start = time.time() - while time.time() < start + timeout: - ret = func(*args, **kwargs) - if ret and ret.stdout and expected in ret.stdout: - break - time.sleep(1) - else: - pytest.skip( - f"Skipping test, one or more daemons failed to start: {expected} not found in {ret}" - ) +@pytest.fixture +def perf_state_name(state_tree, curr_master, prev_master): + + # Copy all of the needed files to both master file roots directories + subdir = random_string("perf-state-") + shutil.copytree( + state_tree, os.path.join(curr_master.config["file_roots"]["base"][0], subdir) + ) + shutil.copytree( + state_tree, os.path.join(prev_master.config["file_roots"]["base"][0], subdir) + ) + return subdir -@pytest.mark.flaky(max_runs=4) def test_performance( prev_salt_cli, prev_minion, @@ -353,48 +381,8 @@ def test_performance( prev_sls, curr_sls, curr_version, + perf_state_name, ): - # Copy all of the needed files to both master file roots directories - subdir = random_string("performance-") - shutil.copytree( - state_tree, os.path.join(curr_master.config["file_roots"]["base"][0], subdir) - ) - shutil.copytree( - state_tree, os.path.join(prev_master.config["file_roots"]["base"][0], subdir) - ) - - # Wait for the old master and minion to start - _wait_for_stdout( - prev_version, prev_master.run, *prev_salt_run_cli.cmdline("--version") - ) - salt_key_cmd = [ - comp - for comp in prev_salt_key_cli.cmdline("-Ay") - if not comp.startswith("--log-level") - ] - _wait_for_stdout(prev_minion.id, prev_master.run, *salt_key_cmd) - _wait_for_stdout( - "Salt: {}".format(prev_version), - prev_master.run, - *prev_salt_cli.cmdline("test.versions", minion_tgt=prev_minion.id), - ) - - # Wait for the new master and minion to start - _wait_for_stdout( - curr_version, curr_master.run, *curr_salt_run_cli.cmdline("--version") - ) - curr_key_cmd = [ - comp - for comp in curr_salt_key_cli.cmdline("-Ay") - if not comp.startswith("--log-level") - ] - _wait_for_stdout(curr_minion.id, curr_master.run, *curr_key_cmd) - _wait_for_stdout( - "Salt: {}".format(curr_version), - curr_master.run, - *curr_salt_cli.cmdline("test.versions", minion_tgt=curr_minion.id), - ) - # Let's now apply the states applies = os.environ.get("SALT_PERFORMANCE_TEST_APPLIES", 3) @@ -423,7 +411,9 @@ def test_performance( for _ in range(applies): prev_state_ret = prev_master.run( *prev_salt_cli.cmdline( - "state.apply", f"{subdir}.{prev_sls}", minion_tgt=prev_minion.id + "state.apply", + f"{perf_state_name}.{prev_sls}", + minion_tgt=prev_minion.id, ) ) prev_duration += _gather_durations(prev_state_ret, prev_minion.id) @@ -431,7 +421,9 @@ def test_performance( for _ in range(applies): curr_state_ret = curr_master.run( *curr_salt_cli.cmdline( - "state.apply", f"{subdir}.{curr_sls}", minion_tgt=curr_minion.id + "state.apply", + f"{perf_state_name}.{curr_sls}", + minion_tgt=curr_minion.id, ) ) curr_duration += _gather_durations(curr_state_ret, curr_minion.id)