-
Notifications
You must be signed in to change notification settings - Fork 324
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
Permanently enable installed collections #108
Conversation
@rhcarvalho so this is basically hardcoding the env variables that scl enable otherwise sets? @hhorak can you or someone from the SCL team take a look at this? Is there anything missing from this that scl enable handles? I know we also had fears at one point about "yum" using the wrong python version if we do this, but @rhcarvalho has tested yum and it appears to tolerate python3. |
Not that it runs on Python 3. The image for Python 3.3 and Python 3.4 have both those versions and the system version, 2.7.x.
BTW In the "scl-enabled" image, $ docker run --rm --entrypoint=python2 scl-enabled-python-34-centos7 --version
Python 2.7.5 |
@praiskup will something like work for postgresql/mysql SCL? |
@rhcarvalho i experimented with something similar couple months ago... one caveat of doing this is that some collections require another collections enabled :-) for example nodejs also require v8.... ruby require nodejs, etc... we can hardcode this into Dockerfiles, but it might get messy. Generally I like this idea, but I think SCL folks are more familiar with SCL tooling and all pros/cons of doing this. |
The tool to generate this patch takes a diff of a clean environment and an environment where all collections installed in an image are enabled. So this is not something to worry about.
That's why I'm only proposing this because we have software to generate it for any Docker image you can get. I have generated similar patches for all images we distribute, including CentOS and RHEL variants.
From one hand, our existing hacks put the user in an all-scl-enabled environment when using a shell ( What our hack today doesn't cover is life outside of a shell and extending the image, what this PR would hopefully help with. |
The assumptions here are:
|
I've been thinking about this issue long time and it seems to be the only working solution to have the SCL enabled properly everywhere, anytime. I've asked other maintainers to look at it as well, whether there are not some other hidden issues.
|
@hhorak As one can see in the example there, |
LD_LIBRARY_PATH=/opt/rh/python27/root/usr/lib64 \ | ||
PATH=/opt/rh/python27/root/usr/bin:$PATH \ | ||
XDG_DATA_DIRS=/opt/rh/python27/root/usr/share \ | ||
PKG_CONFIG_PATH=/opt/rh/python27/root/usr/lib64/pkgconfig |
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.
We might want to "fine-tune" certain list variables to include a recursive reference to its previous value, even if it was previously undefined. Those wouldn't be possible to auto-detect.
For example:
-MANPATH=/opt/rh/python27/root/usr/share/man:
+MANPATH=/opt/rh/python27/root/usr/share/man:$MANPATH
# oddly, scl enable does leave a trailing ':' there!
-LD_LIBRARY_PATH=/opt/rh/python27/root/usr/lib64
+LD_LIBRARY_PATH=/opt/rh/python27/root/usr/lib64:$LD_LIBRARY_PATH
-XDG_DATA_DIRS=/opt/rh/python27/root/usr/share
+XDG_DATA_DIRS=/opt/rh/python27/root/usr/share:$XDG_DATA_DIRS
-PKG_CONFIG_PATH=/opt/rh/python27/root/usr/lib64/pkgconfig
+PKG_CONFIG_PATH=/opt/rh/python27/root/usr/lib64/pkgconfig:$PKG_CONFIG_PATH
The reason to do that is if we ever try to set a value for one of those variables before this opaque ENV instruction, or, harder to find out, a parent layer does set that variable, we'd be inadvertently overwriting the previous value.
To minimize human intervention, we can have a whitelist of variables that we want to give this special treatment, and have envdiff
do the work for us.
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.
yeah i think we should definitely do that.
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.
For RPMs security team pointed out that enable scriptlet needs to avoid exporting path with empty path segment (usually a trailing ':'). It's done by appending envar only if it is already defined, e.g. export LD_LIBRARY_PATH=/opt/rh/ruby200/root/usr/lib64${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}
IMO we should do the same in the case of docker images too.
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.
Thanks @lukaszachy. That sounds like a good idea. I'll see if Docker supports the syntax, I'm not sure about that.
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.
Nice nice! It works 🎉 🎊
I did it for PATH:
$ grep \$\{PATH Dockerfile
PATH=/opt/rh/rh-python34/root/usr/bin${PATH:+:${PATH}} \
$ docker build -t python-34-smart-path .
...
$ docker inspect --type=image python-34-smart-path | grep \"PATH=
"PATH=/opt/rh/rh-python34/root/usr/bin:/opt/app-root/src/.local/bin/:/opt/app-root/src/bin:/opt/app-root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"PATH=/opt/rh/rh-python34/root/usr/bin:/opt/app-root/src/.local/bin/:/opt/app-root/src/bin:/opt/app-root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
There is a conceptual problem. The "scl enable" can do more things than just setting environment variables to same fixed value. The enable script is for each collection is an arbitrary shell script. I have no idea if there is an collections that does more, but I know that we cannot exclude it for the future. What about nfs-mountable collections? Don't they touch file system on "scl enable"? Another issue is mixing system scripts with collection interpreters. yum has /usr/bin/python shell bang. But have you checked all the other system scripts (rpm -q --whatrequires /usr/bin/env)? What will be the procedure for executing commands out of collection environment? |
That's why we need the feedback from SCL specialists here :) Last December I was brainstorming with @hhorak and it seemed at the time that if we had a way to set the proper environment variables in a Dockerfile we'd be heading into a "solution", or at least something better than the current state of affairs. Of course maintaining the environment instruction manually would be a no go, but having a tool perhaps could possibly work?
True thing. How does this affects us for the Docker images we ship and plan to ship?
I have no idea either, we need SCL experts to validate this.
No, I haven't. Help would be appreciated.
AFAIK, there is no such a thing as What is the procedure now? We're already in the situation that you exec into a container to a Bash shell and you have all collections enabled (via BASH_ENV, ENV and PROMPT_COMMAND hacks). There is no way out of that. I would love to get help to improve the user experience of our images. |
@ppisar what is the use-case where you need disable particular collection and get the system version of something inside a container? We can provide Docker images that does not have any SCL installed (just use the system version of the thing). So in my mind, if you need something like that, you will have to craft the image or layer up and existing image, unset all the envs and execute your command. |
On Fri, Apr 01, 2016 at 04:24:49AM -0700, Rodolfo Carvalho wrote:
python-libs-2.7.5-34.el7.x86_64 Now you need automake and python experts to evaluate if the two packages will And it still does not cover executions from programs by execvp(3) or system(3).
I'd like to quote Docker documentation:
I think you are trying to bend Docker for something it goes against its |
On Fri, Apr 01, 2016 at 04:58:31AM -0700, Michal Fojtik wrote:
Executing system tools that expects system versions? Don't you remember all |
So if I use an official Docker image for Python (or any other stack), based on Ubuntu or Alpine, I can choose, I can Do we want every user to do:
? Can't we do better than that? The line gets much uglier if we'd be talking about the Ruby image with its many collections. And what is the point of having a MySQL image that I cannot run I think that's terrible user experience that move users away from our images and we need to fix it. |
PR testing has started. |
1 similar comment
PR testing has started. |
Well, there's this alternative that's arguably a bit more user friendly:
However, I agree it would improve the user experience if we could get rid of this boiler-plate. |
I have no time to push this forward, closing. |
Forget the past:
Introducing a world where users don't have to care about enabling SCL collections to run their software:
Think
oc exec my-python-powered-pod ./manage.py migrate
, without having to wrap the call in a Bash shell...With a similar patch applied to all images, one day we can drop the hacks with
BASH_ENV
,ENV
andPROMPT_COMMAND
and enjoy permanent, baked-in, enabled collections in Docker images.The future is also bright for users trying to build images based on our images: