-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Config generation at runtime + buildtime #529
Conversation
See SeleniumHQ#502 Generation at build time = default configuration file Generation at run time = allow to pass environment variables
@@ -1,6 +1,7 @@ | |||
#!/bin/bash | |||
|
|||
source /opt/bin/functions.sh | |||
/opt/bin/generate_config > /opt/selenium/config.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NodeBase
doesn't have a generate_config
this would fail, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before the commit 0dd193a (Moving config generation in docker files) the line was here.
I believe that it's because NodeBase is not runned directly, it's used in other images that have generate_config.
For instance for Chrome.
In NodeBase Dockerfile.txt, the entry point is set :
CMD ["/opt/bin/entry_point.sh"]
It calls entry_point.sh
which calls for generate_config
.
In NodeChrome Dockerfile.txt, the generate_config
file is copyied inside the image, before the entrypoint is called :
COPY generate_config /opt/bin/generate_config
So when the image is launched, the entrypoint is called, and it can generate the config because generate_config is in the image.
@@ -3,6 +3,7 @@ | |||
# IMPORTANT: Change this file only in directory NodeDebug! | |||
|
|||
source /opt/bin/functions.sh | |||
/opt/bin/generate_config > /opt/selenium/config.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would these lines replace the RUN /opt/bin/generate_config > /opt/selenium/config.json
lines? or would they be supplemental
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they should both exist.
The RUN directive would create a default configuration file, and the generate_config from the entry_point.sh
will allow to customize environment variables.
This is actually done for the hub : Hub/Dockerfile#L39 & Hub/entry_point.sh#L6
…node containers at runtime When changes were introduced to generate default config during build time with SeleniumHQ#502 and then with SeleniumHQ#529, it broke the ability to pass in environment variables to the node containers at runtime. This fix aims to provide the ability to do both i.e. continue to pass environment variables at runtime and generate a default config at build time for the node.
Hi,
As @junhui pointed out, my previous pull request (#502) broke the ability to pass environment variables for config generation.
I believe a fix could be to generate config at build time (it creates a default config file), and at runtime. As it is actually done with the Hub : Hub/Dockerfile#L39 and Hub/entry_point.sh#L6. See issue 502 last comment.
X
in the preceding checkbox, I verify that I have signed the Contributor License Agreement