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

Add support for overriding st2.conf settings via env vars #6277

Merged
merged 10 commits into from
Nov 16, 2024

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Nov 13, 2024

This PR's commits were extracted from #6273 where I'm working on getting pants+pytest to run integration tests.

We have upgraded to a version of oslo_config that supports overriding conf vars via environment variables. That support is enabled by default, but it hardcodes the env var prefix OS_. Since oslo_config is designed for use within OpenStack, the OpenStack devs did not have a reason to make that prefix configurable. Using OS_ does not make sense for the StackStorm project, so I changed the prefix to ST2_ in this subclass:

class St2EnvironmentConfigurationSource(EnvironmentConfigurationSource):
@staticmethod
def get_name(group_name, option_name):
group_name = group_name or "DEFAULT"
return "ST2_{}__{}".format(group_name.upper(), option_name.upper())

To use this class, we have to update the cfg.CONF global to use our subclass instead of the default. This is done with a helper method:

def use_st2_env_vars(conf: cfg.ConfigOpts) -> None:
# Override oslo_config's 'OS_' env var prefix with 'ST2_'.
conf._env_driver = St2EnvironmentConfigurationSource()

This helper needs to be called just before we tell oslo to parse the config (by calling cfg.CONF(...)), which happens in various parse_args() functions throughout the ST2 codebase, including the primary one here:

def parse_args(args=None, ignore_errors=False):
use_st2_env_vars(cfg.CONF)
register_opts(ignore_errors=ignore_errors)
cfg.CONF(
args=args,
version=VERSION_STRING,
default_config_files=[DEFAULT_CONFIG_FILE_PATH],
)

Finally, I made use of this feature to pass settings to Integration test subprocesses. When running the tests via pants+pytest, we have to modify things like the db-name to use a slot number so that tests can run safely in parallel without clobbering other tests' data. That slot number only gets injected in test code (in st2tests.config), so we needed a way to override the conf file on the fly with the updated settings for the test. For example, this test now injects various env vars to ensure the subprocess uses the conf values that the test expects:

env = env or os.environ.copy()
env.update(st2tests.config.db_opts_as_env_vars())
env.update(st2tests.config.mq_opts_as_env_vars())
env.update(st2tests.config.coord_opts_as_env_vars())
process = subprocess.Popen(
cmd,
env=env,

These env var helpers are defined in st2tests.config to make it as easy as possible for tests to inject the relevant env vars.

Bonus: This will make configuration for docker and kubernetes much more straightforward. We have had requests to make secret vars in particular settable via env vars in addition to the conf file. So, this makes that possible.

Integration tests need to start subprocesses that use the
same database as the test. This matters when running under
pantsbuild, because pants runs several instances of pytest
in parallel. We configured pants to pass an env var to
disambiguate parallel test runs: ST2TESTS_PARALLEL_SLOT.
Then, the db name gets suffixed with this slot number at
runtime. But, when an integration test runs production
code in a subprocess, the production code does not use--
and should not use--any ST2TESTS_* vars, meaning the
subprocess ends up using what is configured in the conf
file instead of the parallel-safe test db.

Thanks to a new-ish oslo_config feature, we can now
update config via env variables. So, make use of that
in integration tests to override the conf-file provided
values with test-provided values.
Integration tests need to start subprocesses that use the
same redis as the test. This matters when running under
pantsbuild, because pants runs several instances of pytest
in parallel. This PR prepares to add support, to disambiguate
test runs--similar to the database logic--using the env var:
ST2TESTS_PARALLEL_SLOT.
In any case, when an integration test runs production
code in a subprocess, the production code does not use--
and should not use--any ST2TESTS_* vars, meaning the
subprocess ends up using what is configured in the conf
file instead of the (planned) parallel-safe coordinator.

Thanks to a new-ish oslo_config feature, we can now
update config via env variables. So, make use of that
in integration tests to override the conf-file provided
values with test-provided values.
This is needed so that vars are initialized before
using them to configure the itest subprocesses
via env vars.
Integration tests need to start subprocesses that use the
same exchanges as the test. This matters when running under
pantsbuild, because pants runs several instances of pytest
in parallel. This PR prepares to add support, to disambiguate
test runs--similar to the database logic--using the env var:
ST2TESTS_PARALLEL_SLOT.
In any case, when an integration test runs production
code in a subprocess, the production code does not use--
and should not use--any ST2TESTS_* vars, meaning the
subprocess ends up using what is configured in the conf
file instead of the (planned) parallel-safe coordinator.

Thanks to a new-ish oslo_config feature, we can now
update config via env variables. So, make use of that
in integration tests to override the conf-file provided
values with test-provided values.
@cognifloyd cognifloyd added this to the 3.9.0 milestone Nov 13, 2024
@cognifloyd cognifloyd self-assigned this Nov 13, 2024
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Nov 13, 2024
@cognifloyd cognifloyd merged commit f39c755 into master Nov 16, 2024
45 checks passed
@cognifloyd cognifloyd deleted the pants-itests-oslo_config-env-vars branch November 16, 2024 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants