-
-
Notifications
You must be signed in to change notification settings - Fork 746
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
requested review from
winem,
nzlosh,
rush-skills,
guzzijones,
amanda11 and
a team
November 13, 2024 01:47
pull-request-size
bot
added
the
size/L
PR that changes 100-499 lines. Requires some effort to review.
label
Nov 13, 2024
nzlosh
approved these changes
Nov 13, 2024
winem
approved these changes
Nov 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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. UsingOS_
does not make sense for the StackStorm project, so I changed the prefix toST2_
in this subclass:st2/st2common/st2common/config.py
Lines 904 to 908 in 4e8794c
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:st2/st2common/st2common/config.py
Lines 911 to 913 in 4e8794c
This helper needs to be called just before we tell oslo to parse the config (by calling
cfg.CONF(...)
), which happens in variousparse_args()
functions throughout the ST2 codebase, including the primary one here:st2/st2common/st2common/config.py
Lines 916 to 923 in 4e8794c
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:st2/st2common/tests/integration/test_service_setup_log_level_filtering.py
Lines 232 to 238 in 4e8794c
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.