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

Dockerfile fix #431

Closed
wants to merge 6 commits into from
Closed

Dockerfile fix #431

wants to merge 6 commits into from

Conversation

Hainish
Copy link
Contributor

@Hainish Hainish commented May 19, 2015

Add support for docker. This is mainly for development, as it is yet unclear to me how production support might work, given that Let's Encrypt operates on active webserver config files. For development, I have Docker more or less mimic the way Vagrant works - install all the requirements in the build phase of the image, then mount the host repo to the guest for data sharing.

@kuba
Copy link
Contributor

kuba commented May 19, 2015

(Non-working) duplicate of #386.

I've spent considerable amount of time on getting well documented and properly working Dockerfile in #386 building up on the abandoned #297 and asked you for a review around 2 weeks ago... ;) Could you please have a look and suggest fixes over at #386 rather than going with a fresh new stuff? Thanks!

@Hainish
Copy link
Contributor Author

Hainish commented May 19, 2015

What isn't working about it? Please be more specific. The Dockerfile you provided in #386 inherits my own in #297, which only attempts to run the client in production. Creating a production Dockerfile is of limited value without evaluating how it would be used in conjunction with other Docker images in production (e.g. nginx, apache). The Dockerfile and docker-compose.yml in #386 supply the immediate advantage of a containerized development environment, no long Virtualbox build process necessary.

@kuba
Copy link
Contributor

kuba commented May 19, 2015

Sorry, by saying "non-working" I mostly meant that Travis build has failed.

However, IMHO, virtualenv is still necessary as otherwise we mix with distro packages and it's only a matter of time when we run into unexpected problems. I also tried to explain it in #430 (comment). I think that it would be reasonable to get rid of virtualenv in those builds only if we use apt-get install letsencrypt, but for now it we don't have the debs yet. Moreover, mimicking virtualenv setup in docker gives as a quick way to test whether our recommended instructions really work for non-Docker setup.

Production docker file does not have to be necessarily used with other Docker images, in the "dockerland infrastructure". I find it as an attractive channel of letsencrypt binary distribution (see the quick start section from #386): all you have to do is docker run quay.io/letsencrypt/lets-encrypt-preview auth to get certificates using standalone plugin.

Also, my tests show that WORKDIR in Docker is bugged and in fact all consecutive ADD or COPY commands throw stuff at / and not /opt. Using debian:jessie produces smaller images.

I think like all this should be discussed under #386. What's wrong with it?

@kuba
Copy link
Contributor

kuba commented May 19, 2015

Ah, BTW #386 works for both production and dev using exactly the same setup. Package is being installed in editable mode (c.f. comment in the Dockerfile near pip install -e, users can just edit files in /opt/letsencrypt/src and changes appear instantaneously). In fact it provides the same experience as to that the developers have without using Docker. python setup.py install as used in this PR doesn't allow you that. Maybe what's missing from #386 is the fact that you should mount /opt/ as volume if you want to edit files without having to open up the container.

@Hainish
Copy link
Contributor Author

Hainish commented May 19, 2015

One can issue standalone certificates, but this workflow is not as standard as nginx or apache config file modification. Additionally, standalone certificate issuance will be troublesome when it comes time for renewal if only port 443 is allowed, and your webserver is already running on that port. This is why I find it much more desirable to have the letsencrypt docker instance modify the configs for an exposed nginx or apache instance. Making sure this works with the webserver of choice should be the next step in our docker buildout. Again, probably not a priority until after launch.

I'm not sure why you're having trouble with WORKDIR, I haven't had any of the problems you listed.

I chose ubuntu:trusty simply because it is more likely developers will already have it than debian:jessie. From their respective dockerhub pages, here's the number of times each has been downloaded:

Ubuntu: 4,107,590
Debian: 346,724

You're right: the base image of jessie is significantly smaller: 125.1 MB as compared to trusty's 188.3. However, the letsencrypt build on top of jessie is 355.8 MB, compared to trusty's 373.9 MB: not a great improvement. Additionally, if you already have ubuntu, that means letsencrypt's layers compose of 185.6 MB, as compared to jessie, which amounds to 230.7. And you're much more likely to have the ubuntu base image as a docker user.

@Hainish
Copy link
Contributor Author

Hainish commented May 19, 2015

PR is passing in Travis currently.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.33% when pulling 6a129e3 on Hainish:dockerfile-fix into 1ada2ca on letsencrypt:master.

@Hainish Hainish closed this May 20, 2015
@Hainish Hainish deleted the dockerfile-fix branch May 21, 2015 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants