Make tests safer to run in parallel by changing the Redis key namespace (#6283)

Author: cognifloyd

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 #6282
+  #6254 #6258 #6259 #6260 #6269 #6275 #6279 #6278 #6282 #6283
   Contributed by @cognifloyd
 * Build of ST2 EL9 packages #6153
   Contributed by @amanda11

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

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

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,16 +55,29 @@ 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}"
+        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)
@@ -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.
                 """
             ),
         ),

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:

pants-plugins/uses_services/scripts/is_redis_running.py

@@ -39,9 +39,9 @@ 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
-    coord_url = args.get(1, "redis://127.0.0.1:6379")
+    # 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)
     exit_code = 0 if is_running else 1

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",
@@ -250,9 +251,12 @@ extra_env_vars = [
   # Use these to override RabbitMQ connection details
   "ST2_MESSAGING__URL",
   "ST2_MESSAGING__PREFIX", # Tests will modify this to be "{prefix}{ST2TESTS_PARALLEL_SLOT}"
-  # 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

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

st2tests/requirements.txt

@@ -14,4 +14,4 @@ psutil==6.1.0
 pyrabbit
 rednose
 unittest2
-webtest
+webtest==3.0.1

st2tests/st2tests/config.py

@@ -174,7 +174,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")