Configure redis coordination backend for orquesta integration tests #5206
Conversation
pull-request-size
bot
added
the
size/M
Kami
force-pushed
the
gha_integration_tests_redis
branch
from
09bb629 to
9fb22ed
Compare
see if it helps with race conditions.
Kami
force-pushed
the
gha_integration_tests_redis
branch
from
9fb22ed to
78fb275
Compare
|
Is this duplicate of #5052? |
|
@armab Thanks - I missed that one. In short, yeah, it's a "kinda of a duplicate. My PR just configures it for tests, whereas that one also configures is for local development (and as such, also requires changes / updates to st2vagrantde, etc.). For the goal is to just make those integration tests more stable and fail less often due to the race conditions. So in short, technically we could also merge this PR earlier and then sync it up once #5052 is ready. |
cognifloyd
left a comment
cognifloyd
left a comment
There was a problem hiding this comment.
I experimented with redis here:
https://github.com/st2sandbox/st2/blob/gha-service/.github/workflows/test-with-services.yml#L143-L203
I was looking for how to reconfigure it on the fly.
maybe that would be helpful
| --health-cmd "redis-cli ping" | ||
| --health-interval 10s | ||
| --health-timeout 5s | ||
| --health-retries 5 |
There was a problem hiding this comment.
I would add --name redis so that it is easier to target in docker commands like:
docker exec redis echo command to run
There was a problem hiding this comment.
Yeah, for now I just added it as a service container, but once I have some more time and once I tackle GHA workflow refactoring into multiple jobs, I will re-organize it so we only spin it up for integration tests.
At this point I don't think we get tons if we move it to a task and conditionally spin it up there since we need to refactor GHA workflow sooner or later since it's a bit messy and instead of those various ifs, we should utilize multiple jobs...
There was a problem hiding this comment.
And done - added the container name (will push it once the build is done).
up to make troubleshooting easier.
|
First test run with coordination backend passed, but this doesn't really mean anything. I will change the code to run it in a loop a couple of time to see if there are any failures. |
pull-request-size
bot
added
size/L
pull-request-size
bot
added
size/M
|
OK so after 6 runs it didn't fail so that's a good sign. If there are no objections, I will go ahead and merge it. It doesn't mean it fixes the problem for 100%, but we will see over time over more test runs. |
st2common/st2common/service_setup.py
Outdated
|
|
||
| LOG.info("Using logging config: %s", logging_config_path) | ||
|
|
||
| LOG.info("Using coordination url: %s", cfg.CONF.coordination.url) |
There was a problem hiding this comment.
Would we need to obfuscate the url if there is a password in it?
There was a problem hiding this comment.
@cognifloyd Good catch, indeed - will add tests for it and obfuscate it (or may just log the driver name).
pull-request-size
bot
removed
the
size/M
pull-request-size
bot
added
the
size/L
This pull request configures redis coordination backend for integration tests to see if this will help with various race conditions which cause test failures we see quite often with orquesta integration tests.