From 18381b7c75826b37f2dccb04fbda07ebb6a7a1d9 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 6 Nov 2024 16:28:51 -0600 Subject: [PATCH 1/6] launchdev.sh: translate ST2TEST_* vars to ST2_* var overrides --- tools/launchdev.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 385aa3d6d0..91b9275ae0 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -97,6 +97,9 @@ function eecho() function init() { heading "Initialising system variables ..." + # Capture list of exported vars before adding any others + ST2VARS=(${!ST2_@}) + ST2_BASE_DIR="/opt/stackstorm" COMMAND_PATH=${0%/*} CURRENT_DIR=$(pwd) @@ -131,6 +134,15 @@ function init() if [ -z "$ST2_CONF" ]; then ST2_CONF=${ST2_REPO}/conf/st2.dev.conf fi + # The ST2TESTS_* vars are only for tests. ST2_* overrides the conf var directly. + if [ -n "${ST2TESTS_SYSTEM_USER}" ]; then + export ST2_SYSTEM_USER__USER="${ST2_SYSTEM_USER__USER:-${ST2TESTS_SYSTEM_USER}}" + ST2VARS+=("ST2_SYSTEM_USER__USER") + fi + if [ -n "${ST2TESTS_REDIS_HOST}" ] && [ -n "${ST2TESTS_REDIS_PORT}"]; then + export ST2_COORDINATION__URL="${ST2_COORDINATION__URL:-redis://${ST2TESTS_REDIS_HOST}:${ST2TESTS_REDIS_PORT}}" + ST2VARS+=("ST2_COORDINATION__URL") + fi ST2_CONF=$(readlink -f ${ST2_CONF}) echo -n "Using st2 config file: "; iecho "$ST2_CONF" @@ -237,6 +249,9 @@ function st2start() done local PRE_SCRIPT_VARS=() + for var_name in "${ST2VARS[@]}"; do + PRE_SCRIPT_VARS+=("${var_name}=${!var_name}") + done PRE_SCRIPT_VARS+=("ST2_CONFIG_PATH=${ST2_CONF}") # PRE_SCRIPT should not end with ';' so that using it is clear. From 1401926a8f68b9bcef4dd25e0e3ffcc13e6d3d84 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 9 Nov 2024 00:24:03 -0600 Subject: [PATCH 2/6] test: Add ?namespace=... to redis:// uri for parallel testing This will make it safer to run tests in parallel as the tests will have separate service registration and coordination locks. To do that, this adds a namespace= query param to the redis url that includes the slot number that pants creates to allow tests to run in parallel more safely. The namespace is used by the tooz library's redis driver as a prefix for all the redis keys. Thus, multiple tests can use the same redis database without clobbering each other's keys. --- conf/st2.dev.conf | 2 +- pants-plugins/uses_services/redis_rules.py | 2 +- pants-plugins/uses_services/scripts/is_redis_running.py | 2 +- st2tests/st2tests/config.py | 4 +++- tools/launchdev.sh | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/conf/st2.dev.conf b/conf/st2.dev.conf index cf2b5b6596..433e8d4973 100644 --- a/conf/st2.dev.conf +++ b/conf/st2.dev.conf @@ -87,7 +87,7 @@ protocol = udp # / cross server functionality #url = redis://localhost #url = kazoo://localhost -url = redis://127.0.0.1:6379 +url = redis://127.0.0.1:6379?namespace=_st2_dev [webui] # webui_base_url = https://mywebhost.domain diff --git a/pants-plugins/uses_services/redis_rules.py b/pants-plugins/uses_services/redis_rules.py index b53f3bd4d7..cb9dcbfb46 100644 --- a/pants-plugins/uses_services/redis_rules.py +++ b/pants-plugins/uses_services/redis_rules.py @@ -63,7 +63,7 @@ class UsesRedisRequest: @property def coord_url(self) -> str: - return f"redis://{self.host}:{self.port}" + return f"redis://{self.host}:{self.port}?namespace=_st2_test" @dataclass(frozen=True) diff --git a/pants-plugins/uses_services/scripts/is_redis_running.py b/pants-plugins/uses_services/scripts/is_redis_running.py index 4a54533381..f9c1f1f621 100644 --- a/pants-plugins/uses_services/scripts/is_redis_running.py +++ b/pants-plugins/uses_services/scripts/is_redis_running.py @@ -41,7 +41,7 @@ def _is_redis_running(coord_url: str) -> bool: # unit tests do not use redis, they use use an in-memory coordinator: "zake://" # integration tests use this url with a conf file derived from conf/st2.dev.conf - coord_url = args.get(1, "redis://127.0.0.1:6379") + coord_url = args.get(1, "redis://127.0.0.1:6379?namespace=_st2_test") is_running = _is_redis_running(coord_url) exit_code = 0 if is_running else 1 diff --git a/st2tests/st2tests/config.py b/st2tests/st2tests/config.py index 6a2e13ea89..6b0224b318 100644 --- a/st2tests/st2tests/config.py +++ b/st2tests/st2tests/config.py @@ -163,7 +163,9 @@ def _override_coordinator_opts(noop=False): redis_host = os.environ.get("ST2TESTS_REDIS_HOST", False) if redis_host: redis_port = os.environ.get("ST2TESTS_REDIS_PORT", "6379") - driver = f"redis://{redis_host}:{redis_port}" + # namespace= is the tooz redis driver's key prefix (default is "_tooz") + namespace = f"_st2_test{os.environ.get('ST2TESTS_PARALLEL_SLOT', '')}" + driver = f"redis://{redis_host}:{redis_port}?namespace={namespace}" CONF.set_override(name="url", override=driver, group="coordination") CONF.set_override(name="lock_timeout", override=1, group="coordination") diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 91b9275ae0..dfcede76f8 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -140,7 +140,7 @@ function init() ST2VARS+=("ST2_SYSTEM_USER__USER") fi if [ -n "${ST2TESTS_REDIS_HOST}" ] && [ -n "${ST2TESTS_REDIS_PORT}"]; then - export ST2_COORDINATION__URL="${ST2_COORDINATION__URL:-redis://${ST2TESTS_REDIS_HOST}:${ST2TESTS_REDIS_PORT}}" + export ST2_COORDINATION__URL="${ST2_COORDINATION__URL:-redis://${ST2TESTS_REDIS_HOST}:${ST2TESTS_REDIS_PORT}}?namespace=_st2_dev" ST2VARS+=("ST2_COORDINATION__URL") fi From 5cee8c090e74156ec4a017a0f4c6d649a068d9f5 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 11 Nov 2024 14:02:39 -0600 Subject: [PATCH 3/6] launchdev.sh: ST2TESTS_* vars have precedence over ST2_* --- pants.toml | 6 +++++- tools/launchdev.sh | 7 ++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/pants.toml b/pants.toml index 23a2c5f4a2..c388729d54 100644 --- a/pants.toml +++ b/pants.toml @@ -240,6 +240,7 @@ extra_env_vars = [ # Use this so that the test system does not require the stanley user. # For example: export ST2TESTS_SYSTEM_USER=${USER} "ST2TESTS_SYSTEM_USER", + "ST2_SYSTEM_USER__USER", # Use these to override MongoDB connection details # "ST2_DATABASE__HOST", # Tests override this with "127.0.0.1" "ST2_DATABASE__PORT", @@ -247,9 +248,12 @@ extra_env_vars = [ "ST2_DATABASE__CONNECTION_TIMEOUT", "ST2_DATABASE__USERNAME", "ST2_DATABASE__PASSWORD", - # Use these to override the redis host and port + # Use these to override Redis connection details "ST2TESTS_REDIS_HOST", "ST2TESTS_REDIS_PORT", + # "ST2_COORDINATION__URL", # Tests will override this with one of: + # "redis://{ST2TESTS_REDIS_HOST}:{ST2TESTS_REDIS_PORT}?namespace=_st2_test{ST2TESTS_PARALLEL_SLOT} + # "zake://" ] # 10 min should be more than enough even for integration tests. timeout_default = 600 # seconds diff --git a/tools/launchdev.sh b/tools/launchdev.sh index dfcede76f8..c9426b04ea 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -134,13 +134,14 @@ function init() if [ -z "$ST2_CONF" ]; then ST2_CONF=${ST2_REPO}/conf/st2.dev.conf fi - # The ST2TESTS_* vars are only for tests. ST2_* overrides the conf var directly. + # ST2_* vars directly override conf vars using oslo_config's env var feature. + # The ST2TESTS_* vars are only for tests, and take precedence over ST2_* vars. if [ -n "${ST2TESTS_SYSTEM_USER}" ]; then - export ST2_SYSTEM_USER__USER="${ST2_SYSTEM_USER__USER:-${ST2TESTS_SYSTEM_USER}}" + export ST2_SYSTEM_USER__USER="${ST2TESTS_SYSTEM_USER}" ST2VARS+=("ST2_SYSTEM_USER__USER") fi if [ -n "${ST2TESTS_REDIS_HOST}" ] && [ -n "${ST2TESTS_REDIS_PORT}"]; then - export ST2_COORDINATION__URL="${ST2_COORDINATION__URL:-redis://${ST2TESTS_REDIS_HOST}:${ST2TESTS_REDIS_PORT}}?namespace=_st2_dev" + export ST2_COORDINATION__URL="redis://${ST2TESTS_REDIS_HOST}:${ST2TESTS_REDIS_PORT}?namespace=_st2_dev" ST2VARS+=("ST2_COORDINATION__URL") fi From 500c0f6eae12c63c53377b8a14e0c71794d4acab Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 11 Nov 2024 15:09:34 -0600 Subject: [PATCH 4/6] pants-plugins/uses_services: follow new conventions in redis rules too These changes were already made for MongoDB and RabbitMQ rules. This removes a TODO comment and slightly refactors to follow the same convention. --- pants-plugins/uses_services/redis_rules.py | 32 +++++++++++++------ .../uses_services/redis_rules_test.py | 13 ++++++-- .../uses_services/scripts/is_redis_running.py | 4 +-- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/pants-plugins/uses_services/redis_rules.py b/pants-plugins/uses_services/redis_rules.py index cb9dcbfb46..4824cd45d7 100644 --- a/pants-plugins/uses_services/redis_rules.py +++ b/pants-plugins/uses_services/redis_rules.py @@ -28,6 +28,7 @@ rules as pex_rules, ) from pants.core.goals.test import TestExtraEnv +from pants.engine.env_vars import EnvironmentVars from pants.engine.fs import CreateDigest, Digest, FileContent from pants.engine.rules import collect_rules, Get, MultiGet, rule from pants.engine.process import FallibleProcessResult, ProcessCacheScope @@ -54,17 +55,30 @@ class UsesRedisRequest: # These config opts for integration tests are in: # conf/st2.dev.conf (copied to conf/st2.ci.conf) - # TODO: for int tests: set url by either modifying st2.{dev,ci}.conf on the fly or via env vars. - - # with our version of oslo.config (newer are slower) we can't directly override opts w/ environment variables. + # These can also be updated via the ST2TESTS_REDIS_* env vars. + # Integration tests should pass these changes onto subprocesses using the + # ST2_COORDINATION__* env vars (which oslo_config reads). host: str = "127.0.0.1" - port: str = "6379" + port: int = 6379 @property def coord_url(self) -> str: return f"redis://{self.host}:{self.port}?namespace=_st2_test" + @classmethod + def from_env(cls, env: EnvironmentVars) -> UsesRedisRequest: + default = cls() + host = env.get("ST2TESTS_REDIS_HOST", default.host) + port_raw = env.get("ST2TESTS_REDIS_PORT", str(default.port)) + + try: + port = int(port_raw) + except (TypeError, ValueError): + port = default.port + + return cls(host=host, port=port) + @dataclass(frozen=True) class RedisIsRunning: @@ -88,11 +102,8 @@ async def redis_is_running_for_pytest( request: PytestUsesRedisRequest, test_extra_env: TestExtraEnv, ) -> PytestPluginSetup: - redis_host = test_extra_env.env.get("ST2TESTS_REDIS_HOST", "127.0.0.1") - redis_port = test_extra_env.env.get("ST2TESTS_REDIS_PORT", "6379") - # this will raise an error if redis is not running - _ = await Get(RedisIsRunning, UsesRedisRequest(host=redis_host, port=redis_port)) + _ = await Get(RedisIsRunning, UsesRedisRequest.from_env(env=test_extra_env.env)) return PytestPluginSetup() @@ -161,7 +172,7 @@ async def redis_is_running( not_installed_clause_deb="this is one way to install it:", install_instructions_deb=dedent( """\ - sudo apt-get install -y mongodb redis + sudo apt-get install -y redis # Don't forget to start redis. """ ), @@ -170,7 +181,8 @@ async def redis_is_running( """\ You can also export the ST2TESTS_REDIS_HOST and ST2TESTS_REDIS_PORT env vars to automatically use any redis host, local or remote, - while running unit and integration tests. + while running unit and integration tests. Tests do not use any + ST2_COORDINATION__* vars at this point. """ ), ), diff --git a/pants-plugins/uses_services/redis_rules_test.py b/pants-plugins/uses_services/redis_rules_test.py index 93d7668698..ab8871c36f 100644 --- a/pants-plugins/uses_services/redis_rules_test.py +++ b/pants-plugins/uses_services/redis_rules_test.py @@ -51,7 +51,14 @@ def run_redis_is_running( "--backend-packages=uses_services", *(extra_args or ()), ], - env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + env_inherit={ + "PATH", + "PYENV_ROOT", + "HOME", + "ST2TESTS_REDIS_HOST", + "ST2TESTS_REDIS_PORT", + "ST2TESTS_PARALLEL_SLOT", + }, ) result = rule_runner.request( RedisIsRunning, @@ -62,7 +69,7 @@ def run_redis_is_running( # Warning this requires that redis be running def test_redis_is_running(rule_runner: RuleRunner) -> None: - request = UsesRedisRequest() + request = UsesRedisRequest.from_env(env=rule_runner.environment) mock_platform = platform(os="TestMock") # we are asserting that this does not raise an exception @@ -74,7 +81,7 @@ def test_redis_is_running(rule_runner: RuleRunner) -> None: def test_redis_not_running(rule_runner: RuleRunner, mock_platform: Platform) -> None: request = UsesRedisRequest( host="127.100.20.7", - port="10", # 10 is an unassigned port, unlikely to be used + port=10, # 10 is an unassigned port, unlikely to be used ) with pytest.raises(ExecutionError) as exception_info: diff --git a/pants-plugins/uses_services/scripts/is_redis_running.py b/pants-plugins/uses_services/scripts/is_redis_running.py index f9c1f1f621..9786462cea 100644 --- a/pants-plugins/uses_services/scripts/is_redis_running.py +++ b/pants-plugins/uses_services/scripts/is_redis_running.py @@ -39,8 +39,8 @@ def _is_redis_running(coord_url: str) -> bool: if __name__ == "__main__": args = dict((k, v) for k, v in enumerate(sys.argv)) - # unit tests do not use redis, they use use an in-memory coordinator: "zake://" - # integration tests use this url with a conf file derived from conf/st2.dev.conf + # unit and integration tests require a coordinator, and mostly use this redis url. + # In some cases, unit tests can also use an in-memory coordinator: "zake://" coord_url = args.get(1, "redis://127.0.0.1:6379?namespace=_st2_test") is_running = _is_redis_running(coord_url) From c394a52bb0e52fe4d15e53bccd062a29dfe51215 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 23 Nov 2024 12:09:38 -0600 Subject: [PATCH 5/6] update changelog entry --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a94babb48b..9d378adc0b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -69,7 +69,7 @@ Added 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 #6183 #6200 #6237 #6229 #6240 #6241 #6244 #6251 #6253 - #6254 #6258 #6259 #6260 #6269 #6275 #6279 #6278 + #6254 #6258 #6259 #6260 #6269 #6275 #6279 #6278 #6283 Contributed by @cognifloyd * Build of ST2 EL9 packages #6153 Contributed by @amanda11 From 0ebced6747dedd469bcc95e28680ecd808496170 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 28 Nov 2024 19:59:37 -0600 Subject: [PATCH 6/6] copy webtest pin from lockfiles/st2.lock This fixes CircleCI. webtest bumped their dep on waitress to a version that is not available to all of our interpreters. https://github.com/Pylons/webtest/commit/090c5b27863b502b91e038ddf2a5649cd5bde2d8 The lockfile has a version locked version that does not have this issue. The pin was already copied to test-requirements.txt, so this just copies it to the other requirements files as well. This is not a significant issue because webtest is only required by st2tests. --- fixed-requirements.txt | 1 + requirements.txt | 2 +- st2tests/requirements.txt | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fixed-requirements.txt b/fixed-requirements.txt index f728439328..97a1688bbb 100644 --- a/fixed-requirements.txt +++ b/fixed-requirements.txt @@ -72,6 +72,7 @@ tooz==6.3.0 # lockfiles/st2.lock has pip==24.2 wheel==0.44.0 setuptools==75.2.0 virtualenv==20.27.0 webob==1.8.9 +webtest==3.0.1 zake==0.2.2 # test requirements below bcrypt==4.2.0 diff --git a/requirements.txt b/requirements.txt index 479a970a5d..4ab082c4bf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -76,7 +76,7 @@ tooz==6.3.0 typing-extensions==4.12.2 unittest2 webob==1.8.9 -webtest +webtest==3.0.1 zake==0.2.2 zipp==3.20.2 zstandard==0.23.0 diff --git a/st2tests/requirements.txt b/st2tests/requirements.txt index fd28f0d7f3..1fd708e95d 100644 --- a/st2tests/requirements.txt +++ b/st2tests/requirements.txt @@ -14,4 +14,4 @@ psutil==6.1.0 pyrabbit rednose unittest2 -webtest +webtest==3.0.1