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

feat: Add ability to run esrallyd inside docker [ES-9146] #1885

Merged
merged 14 commits into from
Nov 12, 2024

Conversation

favilo
Copy link
Contributor

@favilo favilo commented Oct 23, 2024

This waits for child processes to exit before exiting itself. This is necessary, because docker -d will just terminate the container once the main process ends.

Also adding ability to log json files.

@favilo favilo changed the title feat: Add ability to run esrallyd inside docker feat: Add ability to run esrallyd inside docker [ES-9146] Oct 23, 2024
@favilo favilo marked this pull request as ready for review November 5, 2024 20:08
@favilo favilo requested review from a team and removed request for a team November 5, 2024 20:08
@@ -84,6 +84,8 @@ dependencies = [
"zstandard==0.21.0",
# License: Python Software Foundation License
"typing-extensions==4.12.2",
# License: BSD-2-Clause license
"python-json-logger==2.0.7",
Copy link
Contributor Author

@favilo favilo Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used directly, but it allows us to specify JSON logs in our logging.json configuration. Useful if you are using docker, and can't add dependencies in your virtual environment.

See change in configuration.rst for example of how to do this.

Copy link

@dpifke-elastic dpifke-elastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one (maybe dumb) question and a doc update.

esrally/utils/process.py Outdated Show resolved Hide resolved
esrally/utils/process.py Outdated Show resolved Hide resolved
Running the docker image with a mounted `/rally/.rally` directory that
already contains the configuration file didn't work. So I had to change
how we parse the logging config and rally.ini files.

Now we dealy the parsing of the environment variables to read time, to
make running from docker and outside of docker compatible with each
other.
will allow us to actually run the thespian children inside docker.
It'll be easier to wait for the child processes from python
when RALLY_RUNNING_IN_DOCKER is set. And we can just set the docker
command to include `--daemon` to make sure that we don't block on the
command running.
Introduce `process.current_pid()` helper function.
@favilo favilo merged commit ac680ed into elastic:master Nov 12, 2024
17 checks passed
@favilo favilo deleted the esrallyd_docker branch November 12, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants