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

use COPY --chown to stage repo (docker 17.09) #164

Closed
minrk opened this issue Dec 7, 2017 · 14 comments · Fixed by #969
Closed

use COPY --chown to stage repo (docker 17.09) #164

minrk opened this issue Dec 7, 2017 · 14 comments · Fixed by #969
Labels
enhancement help wanted needs: PR This issue is ready for a pull request from a community member.
Milestone

Comments

@minrk
Copy link
Member

minrk commented Dec 7, 2017

Right now, we copy and chown the repo in two layers, which doubles the size the repo takes up in the env. This used to be required. Docker 17.09 adds support for COPY --chown to do it in one layer. So we can do this when it becomes okay to require docker 17.09 as the base version.

@mukundans91
Copy link
Contributor

Can we add the functionality now with a check done to see if the underlying docker version is >= 17.09, if so use COPY --chown else use the older form of the template?

@minrk
Copy link
Member Author

minrk commented Feb 9, 2018

Good idea! We can indeed do that.

@willingc
Copy link
Contributor

Adding this to the next release as this seems pretty straightforward.

@willingc willingc added this to the 0.6 milestone Mar 13, 2018
@choldgraf choldgraf modified the milestones: 0.6, v0.7 Nov 6, 2018
@choldgraf
Copy link
Member

I'm moving this to milestone v0.7, since we've already released v0.6 and this issue doesn't seem to be in it!

@betatim
Copy link
Member

betatim commented Nov 6, 2018

What does it mean if something is added to a milestone?

Should we make it to mean that we will wait to release the next version until this is done or do we take it as an indication that it would be a good fit in the next release (it isn't too early to work on this) but release will happen if no one works on it?

@choldgraf
Copy link
Member

in other projects that I work on, the milestone issues are blocking on a release, though often people decide to punt them to the next release once it gets closer to deadline time

@betatim
Copy link
Member

betatim commented Nov 6, 2018

I think we should use it like that too (if it is in a milestone then the release waits till it is done). Maybe we can ease into it by only adding things to a milestone a few days before a release as a kind of todo list/making sure we don't forget something. With the current state of contributors I worry that if we add something to a milestone that isn't security or usability critical (and hence has a high likelihood of getting worked on) we will end up punting a lot or slow down our releases even more :-/

@Xarthisius
Copy link
Contributor

Just as a note: an implementation of COPY --chown that expands variables in options was merged only two weeks ago (see moby/moby#35018 (comment)). Now it's going to be possible to actually do:

COPY --chown=${NB_USER}:${NB_USER} src/ ${REPO_DIR}

@yuvipanda yuvipanda added the needs: PR This issue is ready for a pull request from a community member. label May 21, 2019
@yuvipanda
Copy link
Collaborator

Me and @jhamman found out that version of docker used by GKE does not support this yet, so the default install of BinderHub can't use it (since it uses the same docker as the underlying cluster)

@minrk
Copy link
Member Author

minrk commented Jul 16, 2019

in other projects that I work on, the milestone issues are blocking on a release, though often people decide to punt them to the next release once it gets closer to deadline time

This is how we've usually used them in Jupyter projects. It's not that they must block a release, but rather that the the decision to not include a given feature/fix is explicitly rather than simply forgetting, which can happen with longstanding issues. The idea is: we're getting close to release, go through issues tagged with the milestone and ask "Is this really a blocker that we need to tackle, or should we let it slip?" This has worked well for me, but I'm not sure how clear it is or how comfortable other team members might be in deciding what gets punted vs should stick around as a blocker. It works best when we have thorough milestone/tagging coverage of issues, which we have had in the past on some Jupyter repos, but I'm not sure we have now.

@choldgraf
Copy link
Member

I'm fine on this as a system - I think the bottleneck will be people having the bandwidth to stay on top of the milestones/tags/etc for a given repository. Any system isn't going to work if we don't have person-time reliably going into maintaining it. :-/ perhaps this is something the "JupyterHub fellow" could help with?

@willingc
Copy link
Contributor

We have a "JupyterHub fellow"???

@davidrpugh
Copy link
Contributor

I was just able to upgrade my Ubuntu 16.04 LTS docker to 19.03 which supports copy-as-user with variable substitution. Hopefully GKE will update the supported Docker version soon!

@choldgraf
Copy link
Member

@willingc not yet but maybe (hopefully?) sometime soon :-) jupyterhub/team-compass#165

Was just trying to brainstorm things that this person could do if they began existing!

yuvipanda added a commit to yuvipanda/repo2docker that referenced this issue Mar 4, 2020
COPY directive always copied files into the image owned
by root. Since we wanted this to be owned by non-root users,
we needed a RUN chown command, which added a fresh layer to
the docker image for no good reason.

This fixes that, but requires docker 19.03 to work.

Ideally, we can either:

1. Pass NB_USER to the template, so we can expand that with
   jinja2 instead of as an env. This makes it work with earlier
   versions of docker
2. Dynamically determine which version of docker we are running,
   so we can conditionally use features. Me no likey

Fixes jupyterhub#164
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted needs: PR This issue is ready for a pull request from a community member.
Projects
None yet
8 participants