Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion conf/st2.dev.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 23 additions & 11 deletions pants-plugins/uses_services/redis_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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()

Expand Down Expand Up @@ -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.
"""
),
Expand All @@ -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.
"""
),
),
Expand Down
13 changes: 10 additions & 3 deletions pants-plugins/uses_services/redis_rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions pants-plugins/uses_services/scripts/is_redis_running.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion pants.toml
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,20 @@ 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",
# "ST2_DATABASE__DB_NAME", # Tests override this with: "st2-test{ST2TESTS_PARALLEL_SLOT}"
"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
Expand Down
4 changes: 3 additions & 1 deletion st2tests/st2tests/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
16 changes: 16 additions & 0 deletions tools/launchdev.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -131,6 +134,16 @@ function init()
if [ -z "$ST2_CONF" ]; then
ST2_CONF=${ST2_REPO}/conf/st2.dev.conf
fi
# 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="${ST2TESTS_SYSTEM_USER}"
ST2VARS+=("ST2_SYSTEM_USER__USER")
fi
if [ -n "${ST2TESTS_REDIS_HOST}" ] && [ -n "${ST2TESTS_REDIS_PORT}"]; then
export ST2_COORDINATION__URL="redis://${ST2TESTS_REDIS_HOST}:${ST2TESTS_REDIS_PORT}?namespace=_st2_dev"
ST2VARS+=("ST2_COORDINATION__URL")
fi

ST2_CONF=$(readlink -f ${ST2_CONF})
echo -n "Using st2 config file: "; iecho "$ST2_CONF"
Expand Down Expand Up @@ -237,6 +250,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.
Expand Down
Loading