From 3ea5cd561b7d5a041c5e6fe55ecc42a85ff2e5de Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 15:02:04 -0500 Subject: [PATCH 01/14] Add support to pack venvs for pth files in st2 venv --- st2common/st2common/util/sandboxing.py | 79 +++++++++++++++---------- st2common/st2common/util/virtualenvs.py | 66 ++++++++++++++++++++- 2 files changed, 110 insertions(+), 35 deletions(-) diff --git a/st2common/st2common/util/sandboxing.py b/st2common/st2common/util/sandboxing.py index 8640985362..19f1ddc09b 100644 --- a/st2common/st2common/util/sandboxing.py +++ b/st2common/st2common/util/sandboxing.py @@ -24,6 +24,7 @@ import os import sys from sysconfig import get_path +from typing import Optional from oslo_config import cfg @@ -32,20 +33,51 @@ from st2common.content.utils import get_pack_base_path -def get_python_lib(): - """Replacement for distutil.sysconfig.get_python_lib, returns a string with the python platform lib path (to site-packages)""" - return get_path("platlib") - - __all__ = [ + "get_site_packages_dir", + "get_virtualenv_lib_path", "get_sandbox_python_binary_path", "get_sandbox_python_path", "get_sandbox_python_path_for_python_action", "get_sandbox_path", "get_sandbox_virtualenv_path", + "is_in_virtualenv", ] +def get_site_packages_dir() -> str: + """Returns a string with the python platform lib path (to site-packages).""" + # This assumes we are running in the primary st2 virtualenv (typically /opt/stackstorm/st2) + site_packages_dir = get_path("platlib") + + sys_prefix = os.path.abspath(sys.prefix) + if sys_prefix not in site_packages_dir: + raise ValueError( + f'The file with "{sys_prefix}" is not found in "{site_packages_dir}".' + ) + + return site_packages_dir + + +def get_virtualenv_lib_path(virtualenv_path: str) -> str: + """Returns the path to a virtualenv's lib/python3.* directory.""" + if not (virtualenv_path and os.path.isdir(virtualenv_path)): + raise OSError( + f"virtualenv_path must be an existing directory. virtualenv_path={virtualenv_path}" + ) + + pack_virtualenv_lib_path = os.path.join(virtualenv_path, "lib") + + virtualenv_directories = os.listdir(pack_virtualenv_lib_path) + virtualenv_directories = [ + dir_name + for dir_name in virtualenv_directories + if fnmatch.fnmatch(dir_name, "python*") + ] + + return os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0]) + + def get_sandbox_python_binary_path(pack=None): """ Return path to the Python binary for the provided pack. @@ -114,14 +146,7 @@ def get_sandbox_python_path(inherit_from_parent=True, inherit_parent_virtualenv= if inherit_parent_virtualenv and is_in_virtualenv(): # We are running inside virtualenv - site_packages_dir = get_python_lib() - - sys_prefix = os.path.abspath(sys.prefix) - if sys_prefix not in site_packages_dir: - raise ValueError( - f'The file with "{sys_prefix}" is not found in "{site_packages_dir}".' - ) - + site_packages_dir = get_site_packages_dir() sandbox_python_path.append(site_packages_dir) sandbox_python_path = ":".join(sandbox_python_path) @@ -146,30 +171,20 @@ def get_sandbox_python_path_for_python_action( virtualenv_path = get_sandbox_virtualenv_path(pack=pack) if virtualenv_path and os.path.isdir(virtualenv_path): - pack_virtualenv_lib_path = os.path.join(virtualenv_path, "lib") - - virtualenv_directories = os.listdir(pack_virtualenv_lib_path) - virtualenv_directories = [ - dir_name - for dir_name in virtualenv_directories - if fnmatch.fnmatch(dir_name, "python*") - ] - # Add the pack's lib directory (lib/python3.x) in front of the PYTHONPATH. - pack_actions_lib_paths = os.path.join( - pack_base_path, "actions", ACTION_LIBS_DIR - ) - pack_virtualenv_lib_path = os.path.join(virtualenv_path, "lib") - pack_venv_lib_directory = os.path.join( - pack_virtualenv_lib_path, virtualenv_directories[0] - ) + pack_venv_lib_directory = get_virtualenv_lib_path(virtualenv_path) # Add the pack's site-packages directory (lib/python3.x/site-packages) # in front of the Python system site-packages This is important because # we want Python 3 compatible libraries to be used from the pack virtual # environment and not system ones. pack_venv_site_packages_directory = os.path.join( - pack_virtualenv_lib_path, virtualenv_directories[0], "site-packages" + pack_venv_lib_directory, "site-packages" + ) + + # Then add the actions/lib directory in the pack. + pack_actions_lib_paths = os.path.join( + pack_base_path, "actions", ACTION_LIBS_DIR ) full_sandbox_python_path = [ @@ -185,7 +200,7 @@ def get_sandbox_python_path_for_python_action( return sandbox_python_path -def get_sandbox_virtualenv_path(pack): +def get_sandbox_virtualenv_path(pack: str) -> Optional[str]: """ Return a path to the virtual environment for the provided pack. """ @@ -194,7 +209,7 @@ def get_sandbox_virtualenv_path(pack): virtualenv_path = None else: system_base_path = cfg.CONF.system.base_path - virtualenv_path = os.path.join(system_base_path, "virtualenvs", pack) + virtualenv_path = str(os.path.join(system_base_path, "virtualenvs", pack)) return virtualenv_path diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index 188733e85c..4935e181bf 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -22,6 +22,8 @@ import os import re import shutil +from pathlib import Path +from textwrap import dedent import six from oslo_config import cfg @@ -30,6 +32,11 @@ from st2common.constants.pack import PACK_REF_WHITELIST_REGEX from st2common.constants.pack import BASE_PACK_REQUIREMENTS from st2common.constants.pack import RESERVED_PACK_LIST +from st2common.util.sandboxing import ( + get_site_packages_dir, + get_virtualenv_lib_path, + is_in_virtualenv, +) from st2common.util.shell import run_command from st2common.util.shell import quote_unix from st2common.util.compat import to_ascii @@ -52,6 +59,7 @@ def setup_pack_virtualenv( proxy_config=None, no_download=True, force_owner_group=True, + inject_parent_virtualenv_sites=True, ): """ @@ -112,7 +120,14 @@ def setup_pack_virtualenv( no_download=no_download, ) - # 2. Install base requirements which are common to all the packs + # 2. Inject the st2 site-packages dir to enable .pth file loading + if inject_parent_virtualenv_sites: + logger.debug("Injecting st2 venv site-packages via .pth file") + inject_st2_pth_into_virtualenv( + virtualenv_path=virtualenv_path, + ) + + # 3. Install base requirements which are common to all the packs logger.debug("Installing base requirements") for requirement in BASE_PACK_REQUIREMENTS: install_requirement( @@ -122,7 +137,7 @@ def setup_pack_virtualenv( logger=logger, ) - # 3. Install pack-specific requirements + # 4. Install pack-specific requirements requirements_file_path = os.path.join(pack_path, "requirements.txt") has_requirements = os.path.isfile(requirements_file_path) @@ -139,7 +154,7 @@ def setup_pack_virtualenv( else: logger.debug("No pack specific requirements found") - # 4. Set the owner group + # 5. Set the owner group if force_owner_group: apply_pack_owner_group(pack_path=virtualenv_path) @@ -252,6 +267,51 @@ def remove_virtualenv(virtualenv_path, logger=None): return True +def inject_st2_pth_into_virtualenv(virtualenv_path: str) -> None: + """ + Add a .pth file to the pack virtualenv that loads any .pth files from the st2 virtualenv. + + If the primary st2 venv (typically /opt/stackstorm/st2) contains any .pth files, + the pack's virtualenv would not load them because that directory is on the PYTHONPATH, + but it is not a "sites" directory that the "site" module will try to load from. + To work around this, we add an .pth file that registers the st2 venv's + site-packages directory as a "sites" directory. + + Sites dirs get loaded from (in order): venv, [user site-packages,] system site-packages. + After the sites dirs are loaded (including loading their .pth files), sitecustomize gets + imported. We do not use sitecustomize because we need the st2 venv's .pth files to load + after the pack venv and before system site-packages. So, we use a .pth file that loads + as late as possible. + """ + if not is_in_virtualenv(): + # If this is installed in the system-packages directory, then + # its site-packages directory should already be included. + return + + parent_site_packages_dir = get_site_packages_dir() + + contents = dedent( + # The .pth format requires that any code be on one line. + # https://docs.python.org/3/library/site.html + f"""# This file is auto-generated by StackStorm. + import site; site.addsitedir('{parent_site_packages_dir}') + """ + # TODO: Maybe use this to adjust PATH and PYTHONPATH as well. + # This could replace manipulations in python_runner and sensor process_container: + # env["PATH"] = st2common.util.sandboxing.get_sandbox_path(...) + # env["PYTHONPATH"] = st2common.util.sandboxing.get_sandbox_python_path*(...) + ) + + # .pth filenames are sorted() before loading, so use many "z"s to ensure + # any st2 virtualenv .pth files get loaded after .pth files in the pack's virtualenv. + pth_file = ( + Path(get_virtualenv_lib_path(virtualenv_path)) + / "site-packages" + / "zzzzzzzzzz__st2__.pth" + ) + pth_file.write_text(contents) + + def install_requirements( virtualenv_path, requirements_file_path, proxy_config=None, logger=None ): From 378c44a5a5ff561922af7e3963bda8a9a4f0051f Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 15:26:08 -0500 Subject: [PATCH 02/14] Fix test for renamed get_site_packages_dir function --- st2common/tests/unit/test_util_sandboxing.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 3926c9f74c..73c28aa608 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -90,8 +90,8 @@ def test_get_sandbox_path(self): result, f"{virtualenv_path}/bin/:/home/path1:/home/path2:/home/path3" ) - @mock.patch("st2common.util.sandboxing.get_python_lib") - def test_get_sandbox_python_path(self, mock_get_python_lib): + @mock.patch("st2common.util.sandboxing.get_site_packages_dir") + def test_get_sandbox_python_path(self, mock_get_site_packages_dir): # No inheritance python_path = get_sandbox_python_path( inherit_from_parent=False, inherit_parent_virtualenv=False @@ -119,7 +119,7 @@ def test_get_sandbox_python_path(self, mock_get_python_lib): # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = "/usr" - mock_get_python_lib.return_value = f"{sys.prefix}/virtualenvtest" + mock_get_site_packages_dir.return_value = f"{sys.prefix}/virtualenvtest" with mock.patch.dict(os.environ, {"PYTHONPATH": ":/data/test1:/data/test2"}): python_path = get_sandbox_python_path( @@ -132,9 +132,9 @@ def test_get_sandbox_python_path(self, mock_get_python_lib): @mock.patch("os.path.isdir", mock.Mock(return_value=True)) @mock.patch("os.listdir", mock.Mock(return_value=["python3.6"])) - @mock.patch("st2common.util.sandboxing.get_python_lib") + @mock.patch("st2common.util.sandboxing.get_site_packages_dir") def test_get_sandbox_python_path_for_python_action_no_inheritance( - self, mock_get_python_lib + self, mock_get_site_packages_dir ): # No inheritance @@ -158,9 +158,9 @@ def test_get_sandbox_python_path_for_python_action_no_inheritance( @mock.patch("os.path.isdir", mock.Mock(return_value=True)) @mock.patch("os.listdir", mock.Mock(return_value=["python3.6"])) - @mock.patch("st2common.util.sandboxing.get_python_lib") + @mock.patch("st2common.util.sandboxing.get_site_packages_dir") def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_only( - self, mock_get_python_lib + self, mock_get_site_packages_dir ): # Inherit python path from current process @@ -196,9 +196,9 @@ def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_o @mock.patch("os.path.isdir", mock.Mock(return_value=True)) @mock.patch("os.listdir", mock.Mock(return_value=["python3.6"])) - @mock.patch("st2common.util.sandboxing.get_python_lib") + @mock.patch("st2common.util.sandboxing.get_site_packages_dir") def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_and_venv( - self, mock_get_python_lib + self, mock_get_site_packages_dir ): # Inherit from current process and from virtualenv (not running inside virtualenv) @@ -237,7 +237,7 @@ def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_a # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = "/usr" - mock_get_python_lib.return_value = f"{sys.prefix}/virtualenvtest" + mock_get_site_packages_dir.return_value = f"{sys.prefix}/virtualenvtest" # Inherit python path from current process # Mock the current process python path From 32151fd77a39d44c41c8a8d3a2e92d909c7e9d34 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 15:59:10 -0500 Subject: [PATCH 03/14] more debug logs --- st2common/st2common/util/virtualenvs.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index 4935e181bf..b2f0f5664d 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -22,6 +22,7 @@ import os import re import shutil +from logging import Logger from pathlib import Path from textwrap import dedent @@ -125,6 +126,7 @@ def setup_pack_virtualenv( logger.debug("Injecting st2 venv site-packages via .pth file") inject_st2_pth_into_virtualenv( virtualenv_path=virtualenv_path, + logger=logger, ) # 3. Install base requirements which are common to all the packs @@ -267,7 +269,7 @@ def remove_virtualenv(virtualenv_path, logger=None): return True -def inject_st2_pth_into_virtualenv(virtualenv_path: str) -> None: +def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) -> None: """ Add a .pth file to the pack virtualenv that loads any .pth files from the st2 virtualenv. @@ -286,6 +288,7 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str) -> None: if not is_in_virtualenv(): # If this is installed in the system-packages directory, then # its site-packages directory should already be included. + logger.debug("Not in a virtualenv; Skipping st2 .pth injection.") return parent_site_packages_dir = get_site_packages_dir() @@ -294,7 +297,7 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str) -> None: # The .pth format requires that any code be on one line. # https://docs.python.org/3/library/site.html f"""# This file is auto-generated by StackStorm. - import site; site.addsitedir('{parent_site_packages_dir}') + import site; print(site.addsitedir('{parent_site_packages_dir}')) """ # TODO: Maybe use this to adjust PATH and PYTHONPATH as well. # This could replace manipulations in python_runner and sensor process_container: @@ -310,6 +313,7 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str) -> None: / "zzzzzzzzzz__st2__.pth" ) pth_file.write_text(contents) + logger.debug(f"injected st2 .pth: {str(pth_file)}\n{contents}\n---") def install_requirements( From a816be8ba676b591ae280a2785732d14ebf2f4dd Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 16:15:05 -0500 Subject: [PATCH 04/14] Better dedent for .pth file --- st2common/st2common/util/virtualenvs.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index b2f0f5664d..9a647d80ae 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -296,7 +296,8 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) contents = dedent( # The .pth format requires that any code be on one line. # https://docs.python.org/3/library/site.html - f"""# This file is auto-generated by StackStorm. + f"""\ + # This file is auto-generated by StackStorm. import site; print(site.addsitedir('{parent_site_packages_dir}')) """ # TODO: Maybe use this to adjust PATH and PYTHONPATH as well. @@ -313,7 +314,7 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) / "zzzzzzzzzz__st2__.pth" ) pth_file.write_text(contents) - logger.debug(f"injected st2 .pth: {str(pth_file)}\n{contents}\n---") + logger.debug(f"injected st2 .pth: {str(pth_file)}\n{contents}---End of .pth file---") def install_requirements( From 1f2e8ae4aed5daf68769e3c1ce7bd82a6c7ac083 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 17:46:59 -0500 Subject: [PATCH 05/14] .pth imports get exec()'d, so do not import site importing site has side effects which were messing up sys.path I wasn't sure what else to import, so I imported sys. An import is required to trigger the exec(). --- st2common/st2common/util/virtualenvs.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index 9a647d80ae..a15d71761f 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -296,9 +296,11 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) contents = dedent( # The .pth format requires that any code be on one line. # https://docs.python.org/3/library/site.html + # Do not 'import site' or the system libs get injected too soon. + # The line gets passed through exec() which uses the scope of site.addpackage() f"""\ # This file is auto-generated by StackStorm. - import site; print(site.addsitedir('{parent_site_packages_dir}')) + import sys; addsitedir('{parent_site_packages_dir}') """ # TODO: Maybe use this to adjust PATH and PYTHONPATH as well. # This could replace manipulations in python_runner and sensor process_container: From 0ea0e6a439d4e0a8ae094ea69fd7bb714f6190f2 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 21:25:32 -0500 Subject: [PATCH 06/14] pass known_paths to addpackage avoid duplicate entries and hopefully avoid adding system paths before the st2 paths. --- st2common/st2common/util/virtualenvs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index a15d71761f..386ddf7a90 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -300,7 +300,7 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) # The line gets passed through exec() which uses the scope of site.addpackage() f"""\ # This file is auto-generated by StackStorm. - import sys; addsitedir('{parent_site_packages_dir}') + import sys; addsitedir('{parent_site_packages_dir}', known_paths) """ # TODO: Maybe use this to adjust PATH and PYTHONPATH as well. # This could replace manipulations in python_runner and sensor process_container: From 431473a461cbcba2837584c17cbabf19df55d224 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 22:37:59 -0500 Subject: [PATCH 07/14] Drop incorrect comment The system lib paths are already in the sys.path before site gets imported. Then, all .pth files append after that. So, this is expected. --- st2common/st2common/util/virtualenvs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index 386ddf7a90..7282d6eb26 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -296,7 +296,6 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) contents = dedent( # The .pth format requires that any code be on one line. # https://docs.python.org/3/library/site.html - # Do not 'import site' or the system libs get injected too soon. # The line gets passed through exec() which uses the scope of site.addpackage() f"""\ # This file is auto-generated by StackStorm. From 7f1d9038c279f4f171b5d88b98ead40854322e6a Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 22:55:48 -0500 Subject: [PATCH 08/14] Fix sensor test pack venv creation to support .pth files --- st2common/st2common/util/virtualenvs.py | 3 ++- st2reactor/tests/integration/test_sensor_container.py | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index 7282d6eb26..038bf34b02 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -45,7 +45,7 @@ from st2common.content.utils import get_packs_base_paths from st2common.content.utils import get_pack_directory -__all__ = ["setup_pack_virtualenv"] +__all__ = ["setup_pack_virtualenv", "inject_st2_pth_into_virtualenv"] LOG = logging.getLogger(__name__) @@ -285,6 +285,7 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) after the pack venv and before system site-packages. So, we use a .pth file that loads as late as possible. """ + logger = logger or LOG if not is_in_virtualenv(): # If this is installed in the system-packages directory, then # its site-packages directory should already be included. diff --git a/st2reactor/tests/integration/test_sensor_container.py b/st2reactor/tests/integration/test_sensor_container.py index 687c9f9102..e2e13a3474 100644 --- a/st2reactor/tests/integration/test_sensor_container.py +++ b/st2reactor/tests/integration/test_sensor_container.py @@ -27,6 +27,7 @@ from st2common.models.db import db_setup from st2reactor.container.process_container import PROCESS_EXIT_TIMEOUT from st2common.util.green.shell import run_command +from st2common.util.virtualenvs import inject_st2_pth_into_virtualenv from st2common.bootstrap.sensorsregistrar import register_sensors from st2tests.base import IntegrationTestCase @@ -106,6 +107,7 @@ def setUpClass(cls): virtualenv_path, ] run_command(cmd=cmd) + inject_st2_pth_into_virtualenv(virtualenv_path) def test_child_processes_are_killed_on_sigint(self): process = self._start_sensor_container() From 031c691cc71d25697022550bb88a39ce47728a68 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 5 Apr 2024 23:03:38 -0500 Subject: [PATCH 09/14] Revert various debug bits --- st2common/st2common/util/virtualenvs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index 038bf34b02..3326c18fe0 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -316,7 +316,6 @@ def inject_st2_pth_into_virtualenv(virtualenv_path: str, logger: Logger = None) / "zzzzzzzzzz__st2__.pth" ) pth_file.write_text(contents) - logger.debug(f"injected st2 .pth: {str(pth_file)}\n{contents}---End of .pth file---") def install_requirements( From 4db35b7b8767bf4719dff7775db45d5a0b3f620e Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 6 Apr 2024 15:44:13 -0500 Subject: [PATCH 10/14] Add pth support to st2-run-pack-tests created venvs --- st2common/bin/st2-run-pack-tests | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/st2common/bin/st2-run-pack-tests b/st2common/bin/st2-run-pack-tests index beb325f7f8..52af1b4f0d 100755 --- a/st2common/bin/st2-run-pack-tests +++ b/st2common/bin/st2-run-pack-tests @@ -63,7 +63,8 @@ ENABLE_TIMING=false VIRTUALENV_ACTIVATED=false -STACKSTORM_VIRTUALENV_BIN="/opt/stackstorm/st2/bin" +STACKSTORM_VIRTUALENV="/opt/stackstorm/st2" +STACKSTORM_VIRTUALENV_BIN="${STACKSTORM_VIRTUALENV}/bin" STACKSTORM_VIRTUALENV_PYTHON_BINARY="${STACKSTORM_VIRTUALENV_BIN}/python" #################### @@ -194,6 +195,13 @@ if [ "${CREATE_VIRTUALENV}" = true ]; then mkdir -p ${VIRTUALENVS_DIR} virtualenv --no-download --system-site-packages ${VIRTUALENV_DIR} + if [ -f "${STACKSTORM_VIRTUALENV_PYTHON_BINARY}" ]; then + # ensure any .pth files in st2 venv get loaded with the pack venv too. + ST2_SITE_PACKAGES=$(${STACKSTORM_VIRTUALENV_PYTHON_BINARY} -c "import sysconfig;print(sysconfig.get_path('platlib'))") + PACK_SITE_PACKAGES=$(${VIRTUALENV_DIR}/bin/python3 -c "import sysconfig;print(sysconfig.get_path('platlib'))") + echo "import sys; addsitedir('${ST2_SITE_PACKAGES}', known_paths)" > "${PACK_SITE_PACKAGES}/zzzzzzzzzz__st2__.pth" + fi + # Activate the virtualenv activate_virtualenv From 566d7e96e3637af9413966572b421a6301e3b6ca Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 15 Apr 2024 16:16:06 -0500 Subject: [PATCH 11/14] Add pants+pex hermetic_scripts workaround to st2-run-pack-tests By default, pants+pex adds `-sE` to the virtualenv/bin scripts which prevents PYTHONPATH and other PYTHON* vars from affecting the program. st2-run-pack-tests uses PYTHONPATH, however, so bypass the script if it is hermetic. --- st2common/bin/st2-run-pack-tests | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/st2common/bin/st2-run-pack-tests b/st2common/bin/st2-run-pack-tests index 52af1b4f0d..e358aa1e63 100755 --- a/st2common/bin/st2-run-pack-tests +++ b/st2common/bin/st2-run-pack-tests @@ -354,16 +354,26 @@ if [ "${ENABLE_TIMING}" = true ]; then NOSE_OPTS+=(--with-timer) fi +NOSE=(nosetests) +if head -n 1 $(command -v nosetests) | grep -q ' -sE$'; then + # workaround pants+pex default of hermetic scripts so we can run nosetests with PYTHONPATH + if [ -f "${STACKSTORM_VIRTUALENV_PYTHON_BINARY}" ]; then + NOSE=(${STACKSTORM_VIRTUALENV_PYTHON_BINARY} -m "nose") + else + NOSE=(python3 -m "nose") + fi +fi + # Change to the pack's directory (required for test coverage reporting) pushd ${PACK_PATH} > /dev/null # Execute the tests if [ "${TEST_LOCATION}" ]; then # Run a specific test file, class or method - nosetests ${NOSE_OPTS[@]} ${TEST_LOCATION} + ${NOSE[@]} ${NOSE_OPTS[@]} ${TEST_LOCATION} else # Run all tests inside the pack - nosetests ${NOSE_OPTS[@]} ${PACK_TESTS_PATH} + ${NOSE[@]} ${NOSE_OPTS[@]} ${PACK_TESTS_PATH} fi TESTS_EXIT_CODE=$? From a740fc5934f2f1ea046da7886c41a2bfcef0fb66 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 8 Apr 2024 19:12:21 -0500 Subject: [PATCH 12/14] Avoid virtualenv/bin/st2-run-pack-tests assumption in CI If the virtualenv is built by pants, the PEP 660 wheels will not include any of the scripts (like st2-run-pack-tests). Only entry_point scripts will be installed in the virtualenv. So, do not assume that a dev venv will have st2-run-pack-tests. Also, adjust path assumptions to work with either kind of venv. --- .github/workflows/ci.yaml | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c4c3563be2..855a582790 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -253,13 +253,21 @@ jobs: run: | script -e -c "make .ci-prepare-integration" && exit 0 - name: Extend the path for upcoming tasks - run: | - echo ${HOME}/work/st2/st2/virtualenv/bin - echo ${HOME}/work/st2/st2/virtualenv/bin >> $GITHUB_PATH + # pants uses PEP 660 editable wheels to add our code to the virtualenv. + # But PEP 660 editable wheels do not include 'scripts'. + # https://peps.python.org/pep-0660/#limitations + # So, we need to include each bin dir in PATH instead of virtualenv/bin. + run: | + for component_bin in ${GITHUB_WORKSPACE}/st2*/bin; do + echo ${component_bin} | tee -a $GITHUB_PATH + done + echo ${GITHUB_WORKSPACE}/st2/virtualenv/bin | tee -a $GITHUB_PATH - name: Create symlinks to find the binaries when running st2 actions + # st2 is actually a console_script entry point, not just a 'script' + # so it IS included in the virtualenv. But, st2-run-pack-tests might not be included. run: | - ln -s ${HOME}/work/st2/st2/virtualenv/bin/st2 /usr/local/bin/st2 - ln -s ${HOME}/work/st2/st2/virtualenv/bin/st2-run-pack-tests /usr/local/bin/st2-run-pack-tests + ln -s ${GITHUB_WORKSPACE}/virtualenv/bin/st2 /usr/local/bin/st2 + ln -s ${GITHUB_WORKSPACE}/st2common/bin/st2-run-pack-tests /usr/local/bin/st2-run-pack-tests - name: Install st2client timeout-minutes: 5 run: | @@ -270,7 +278,7 @@ jobs: env: ST2_CONF: /home/runner/work/st2/st2/conf/st2.ci.conf run: | - sudo -E ST2_AUTH_TOKEN=$(st2 auth testu -p 'testp' -t) PATH=${PATH} virtualenv/bin/st2-self-check + sudo -E ST2_AUTH_TOKEN=$(st2 auth testu -p 'testp' -t) PATH=${PATH} st2common/bin/st2-self-check - name: Compress Service Logs Before upload if: ${{ failure() }} run: | From 9348b0b679b79a669c672c66171a1ee5059f9440 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 15 Apr 2024 17:03:41 -0500 Subject: [PATCH 13/14] add changelog entry --- CHANGELOG.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fd3b485148..1eae0c288b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,10 +20,12 @@ Added * Continue introducing `pants `_ to improve DX (Developer Experience) working on StackStorm, improve our security posture, and improve CI reliability thanks in part to pants' use of PEX lockfiles. This is not a user-facing addition. - #6118 #6141 #6133 #6120 #6181 + #6118 #6141 #6133 #6120 #6181 #6183 Contributed by @cognifloyd * Build of ST2 EL9 packages #6153 Contributed by @amanda11 +* Ensure `.pth` files in the st2 virtualenv get loaded by pack virtualenvs. #6183 + Contributed by @cognifloyd 3.8.1 - December 13, 2023 ------------------------- From f5b223549364f67e7c7f21194f6e34fa1c42a5ba Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 15 Apr 2024 17:07:44 -0500 Subject: [PATCH 14/14] fix path typo --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 855a582790..9a36f599e5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -261,7 +261,7 @@ jobs: for component_bin in ${GITHUB_WORKSPACE}/st2*/bin; do echo ${component_bin} | tee -a $GITHUB_PATH done - echo ${GITHUB_WORKSPACE}/st2/virtualenv/bin | tee -a $GITHUB_PATH + echo ${GITHUB_WORKSPACE}/virtualenv/bin | tee -a $GITHUB_PATH - name: Create symlinks to find the binaries when running st2 actions # st2 is actually a console_script entry point, not just a 'script' # so it IS included in the virtualenv. But, st2-run-pack-tests might not be included.