-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Allow a mounted repository to be injected into yum #9947
Conversation
68cee92
to
e173c2c
Compare
This adds Also, added the symlink to the overrides repo, so adding the new repo to |
[test] |
Addresses part of #8571, still need to determine whether we need the "magic" solution for the CI machines or whether we just support additional flags to the builders. |
There is no reason not to
Allows files to be bind mounted into the running container that will not be part of the final build.
e173c2c
to
0047dfb
Compare
@stevekuznetsov this is the PR for adding the mount. We'll want to
|
@smarterclayton and what's the use case for arbitrary args given how difficult it is to do properly? Is it not alright to have a log-level and over-ride location as two named args for the make target? |
I don't want the complexity of those args to surface in Make or the test environment. |
The fragility and complexity of trying to and failing to handle the "any args" case doesn't seem preferable to me ... |
I don't see how it's failing. |
Show me how this will fail |
@@ -17,7 +17,12 @@ Build a Dockerfile into a single layer | |||
|
|||
.PP | |||
Builds the provided directory with a Dockerfile into a single layered image. | |||
Requires that you have a working connection to a Docker engine. | |||
Requires that you have a working connection to a Docker engine. You may mount |
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.
it's kinda bizarre to me that this is part of the oc tooling when it's not actually interacting with openshift at all?
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.
Just so we have a vehicle for it to be testable. Eventually I think both this and s2i commands would be under the CLI.
Anything you pass through with whitespace is going to be split and not make it to the script the way you wanted. Not only do I see there being merit to a global LOGLEVEL variable in our Makefile for other jobs that run OpenShift server or client stuff, but also we already have job-specific variables for test jobs. REPO_OVERRIDE would be the only job-specific variable that this would introduce... I don't know... it just seems like Makefile has been set up explicitly not to handle this case and also explicitly to handle well-defined, named, variables. Working around it in a fragile way reeks of the "Bash hammer" that I really don't like us using. |
One failure condition, for example -- what if the path to the override file has a space in it? |
sharedMount = v.Mountpoint | ||
opts.HostConfig = &docker.HostConfig{ | ||
Binds: []string{sharedMount + ":/tmp/__temporarymount"}, | ||
} |
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.
sorry but i'm seriously struggling to follow what is going on here... it seems like you're creating a docker volume, then setting up a bindmount for that (empty) volume.
Then you copy the source paths the user specified into that volume? Except when you do the copies, you are copying to /tmp/_temporarymount, which i would have thought is the path inside the container, not the path on the host that represents the volume. So i don't see how that copy actually works (but i haven't dug into what e.Copy is actually doing).
Then you bindmount all those copied directories, again seemingly mounting from the target path of the bindmount, not the source(host) path, so the container is bindmounting from within itself?
I would have expected something much more straightforward (setup the host paths, mount them into the container, then delete the host paths before committing the image(I guess))
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 want to do this without being local to the machine, so I need a volume. If you try to bind mount something before it exists, it gets bind mounted as a dir. I don't want the bind mounts to be writable (otherwise we have a quota problem), so I need a two phase mount + copy, then remount.
I'll do
This does not have to support arbitrary input. |
So Joe Shmoe has: And your house of cards comes tumbling down. I really don't get why you need an arbitrary number of flags -- do you expect this to grow? If we want to do one thing, and that one thing is to enable the over-ride flag to be set, pass the one thing you need (override location on host) in as the one envar, defined in the Makefile and everything. |
Joe Schmoe does:
which works for me. |
Also I may have to specify multiple mounts in the future (which has the same array problem). |
$ my_args="--flag='long value'"
$ for arg in ${my_args}; do echo "${arg}"; done
--flag='long
value' |
fair enough |
0047dfb
to
e3f9c2e
Compare
Allows a set of arguments to be passed to the image builder
e3f9c2e
to
5fb8206
Compare
[test] |
[merge] so we can fix this. Labeled dockerbuild as experimental.
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6690/) (Image: devenv-rhel7_4643) |
Evaluated for origin merge up to 5fb8206 |
[test] |
2 similar comments
[test] |
[test] |
[test] On Thu, Jul 21, 2016 at 1:05 AM, OpenShift Bot [email protected]
|
@smarterclayton post what flake happened on each failed job before you re-trigger tests so the poor sod that investigates them later has logs to look through |
[test] #9929 On Thu, Jul 21, 2016 at 10:50 AM, OpenShift Bot [email protected]
|
[test] database sync bug and extended docker flake |
Cluster resource quota flake [test] |
[test] e2e failure
|
Evaluated for origin test up to 5fb8206 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6690/) |
OMG. On Thu, Jul 21, 2016 at 10:26 PM, OpenShift Bot [email protected]
|
[merge] |
Mounting a yum repository file at /var/run/secrets/overrides.repo will
allow any image based on this to use those repositories.