pipeline-utils: add registry_login() and prep_container_storage()
#309
Conversation
openshift-ci-robot
added
the
size/S
pipeline-utils.groovy
Outdated
| // re-implementation of some functionality from scripts/pull-mount-oscontainer | ||
| // sets up local container storage so the pipeline can pull container images | ||
| // from inside another container | ||
| def prep_container_storage(hostStorage, localStorage) { |
There was a problem hiding this comment.
Hmm...maybe I chose the wrong names here...maybe they should be oldStorage and newStorage?
There was a problem hiding this comment.
How about using hostStorage and containerStorage? That may be a bit clearer.
There was a problem hiding this comment.
I think hostStorage was a mistake on my part.
As far as I can tell, the host container storage isn't ever mounted into the assembler container (as of now), so we are really just deleting the "typical" location of the container storage in the container.
For example,scripts/pull-mount-oscontainer deletes /var/lib/containers in its container, then bind mounts something like $WORKSPACE/containers to /var/lib/containers. https://github.com/openshift/os/blob/master/scripts/pull-mount-oscontainer#L21-L23)
I think something more accurate would be oldContainerStorage and newContainerStorage
What do you think?
There was a problem hiding this comment.
I actually discovered that podman/buildah support a --root argument and have been using that in coreos/coreos-assembler#90
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
I'd like to improve this slightly to verify the proposed mount point isn't an overlay filesystem |
This adds a simple function to login to a container registry using username, password, and registry name as args.
miabbott
force-pushed
the
extra_util_functions
branch
from
829caa6 to
ed1d95e
Compare
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
| @@ -139,6 +139,27 @@ def sh_capture(cmd) { | |||
| return sh(returnStdout: true, script: cmd).trim() | |||
| } | |||
|
|
|||
| def registry_login(username, password, registry) { | |||
| sh "podman login -u '${username}' -p '${password}' ${registry}" | |||
There was a problem hiding this comment.
Note: I believe this will populate ${XDG_RUNTIME_DIR}/containers/auth.json (EG: /run/user/1000/containers/auth.json). Let's make sure this file doesn't get shown via the output or links.
There was a problem hiding this comment.
That should be outside the $WORKSPACE, so the JSON shouldn't get archived anywhere. And there's no output from the podman login except Login Succeeded!
For example, here -
os/scripts/pull-mount-oscontainer
Line 10 in 1143fc9
...looks like this from the pipeline logs
+ set +x
podman login -u unused -p <password> registry.svc.ci.openshift.org
Login Succeeded!
pipeline-utils.groovy
Outdated
| echo 'Must supply non-overlay location' | ||
| exit 1 | ||
| fi | ||
| rm -rf \$container_storage && mkdir -p \$container_storage |
There was a problem hiding this comment.
Minor nit, \${container_storage} is more consistent
There was a problem hiding this comment.
That will work for shell variables?
There was a problem hiding this comment.
Yes, e.g. there's a lot of uses of it here: https://github.com/openshift/os/blob/master/Jenkinsfile.cloud#L218-L239
This re-implements some of what is contained in `scripts/pull-mount-oscontainer` to allow parts of the pipeline to pull container images from inside another container.
miabbott
force-pushed
the
extra_util_functions
branch
from
ed1d95e to
e32f240
Compare
|
@yuqi-zhang pushed a new commit with your suggested changes |
|
/lgtm |
This adds two utility functions for logging into a container registry and setting up container storage inside a container. The latter allows us to pull container images from inside the pipeline container.
Split out from #308