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

jenkins/cloud: Stop binding /srv, use workspace #211

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

Just use $WORKSPACE for the images - it'll be naturally namespaced
by our job and lifecycled with it.

Note I tried really really hard to also drop --privileged but
the problem is the usage of bwrap drags us into unprivileged
nested containers.

One downside is we'll be rsyncing in the images on each job
invocation, but I'll work on a fix for that later.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 7, 2018
@cgwalters
Copy link
Member Author

See bwrap issue 284 for where I got on the unprivileged bits.

Just use $WORKSPACE for the images - it'll be naturally namespaced
by our job and lifecycled with it.

Note I tried *really* really hard to also drop `--privileged` but
the problem is the usage of bwrap drags us into unprivileged
nested containers.

One downside is we'll be rsyncing in the images on each job
invocation, but I'll work on a fix for that later.
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 8, 2018
@cgwalters
Copy link
Member Author

Rebased 🏄‍♂️

@jlebon
Copy link
Member

jlebon commented Aug 8, 2018

One downside is we'll be rsyncing in the images on each job
invocation, but I'll work on a fix for that later.

Yeah, that will mean a massive amount of data to rsync on each run. May I suggest until we fix that, we drop the workspace cleanup here?

@cgwalters
Copy link
Member Author

cgwalters commented Aug 8, 2018

One approach we could take would be to do pruning (and s3 sync) as a separate task. So we drop the --delete, and this job only writes a new directory and swaps the latest symlink, which wouldn't require syncing in anything.

Does that sound OK?

@jlebon
Copy link
Member

jlebon commented Aug 8, 2018

That makes sense, though... is it possible to do the symlink swap without having to rsync the parent directory? I guess we could revert to scp for that.

@jlebon jlebon mentioned this pull request Aug 9, 2018
@openshift-bot
Copy link

@cgwalters: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2018
jlebon added a commit to jlebon/os that referenced this pull request Sep 26, 2018
We shouldn't `rsync` to the shared `/srv` location. Just sync directly
to our `WORKSPACE` to avoid races.

See also openshift#211.
jlebon added a commit to jlebon/os that referenced this pull request Sep 27, 2018
We shouldn't `rsync` to the shared `/srv` location. Just sync directly
to our `WORKSPACE` to avoid races.

See also openshift#211.
@cgwalters cgwalters closed this Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants