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

Support for separate nginx and docker-gen containers #20

Closed
wants to merge 5 commits into from
Closed

Support for separate nginx and docker-gen containers #20

wants to merge 5 commits into from

Conversation

nilroy
Copy link

@nilroy nilroy commented Feb 4, 2016

This PR addresses the case where docker-gen and nginx are in separate containers

@nilroy
Copy link
Author

nilroy commented Feb 5, 2016

@JrCs --> We are using separate docker-gen and nginx containers for some security reasons. This PR addresses our case and at the same time it works with the bundled nginx-proxy also.

@JrCs
Copy link
Collaborator

JrCs commented Feb 6, 2016

Thanks nilroy, i will merge it as soon as possible

@JrCs
Copy link
Collaborator

JrCs commented Feb 6, 2016

Will close #12

@mpatton125
Copy link

So the LE container has /usr/local/bin/docker-gen (I wasn't aware), then this is even easier and better than my currently (mostly) working implementation. Get the LE container to update the default.conf via the same template and just restart nginx. Very simple and great! Am testing it now locally (mainly because I can't wait). Thanks!

@mpatton125
Copy link

Actually... I note that the nginx.tmpl file that comes with docker-gen 0.5.0 is very basic and doesn't cater at all to SSL. I am currently using my own version of that file which is an amalgamation of the one from the current nginx-proxy and the newer network specific parts of the docker-gen 0.5.0 template...

What are you guys using for this?

@JrCs
Copy link
Collaborator

JrCs commented Feb 7, 2016

@nilroy the LE container have the docker-gen command so we doesn't need another docker-gen container, but only the right nginx.tmpl file

@nilroy
Copy link
Author

nilroy commented Feb 8, 2016

@JrCs Yes you are right...I am currently testing the stuf without the docker-gen container. However I could not use docker-gen 0.5.0 as it does not exited out after generating the nginx files nginx-proxy/docker-gen#159 .I mean docker-gen version 0.5.0 can't be used in LE container.Probably there are some options that I am not aware of. Also the readme needs to be changed a bit. And do you have a plan to keep the nginx.tmpl file as part of this repo?

@nilroy
Copy link
Author

nilroy commented Feb 8, 2016

@falcon15500 I am using nginx.tmpl from nginx-proxy stuff. Also have you encountered rate limits for letsencrypt?

@nilroy
Copy link
Author

nilroy commented Feb 8, 2016

@JrCs I tested. It works without docker-gen container only when we use SSL and there is nothing wrong in it. The LE container is designed for SSL. Now I have adifferent scenario. In my environment we need to use mixed type of containers where some will use SSL and some not. I would still use the setup that contain both docker-gen and the LE container. I would leave a note in the README about it. So that people can choose whatever that fits them. What do you say?

@mpatton125
Copy link

I have the mixed scenario working using the files from this pull request along with my own modified template (and docker-gen 0.5.0). This container handles the ssl stuff and the docker-gen container handles the rest.

They both reference the same template.

In regards to the issue with docker-gen 0.5.0 not exiting, I encountered this only when calling the docker-gen command remotely from the LE container (via that api exec call). However it seems to be working okay by itself.

I did raise it as an issue in the docker-gen github.

@nilroy
Copy link
Author

nilroy commented Feb 8, 2016

@falcon15500 I have encountered the issue(docker-gen not exiting) even If ran it on the docker-gen container. But let them solve it anyways. In my environment also we are using the same nginx.tmpl file for docker-gen and LE container. Now I have another problem. I have encountered rate limit from ACME.

@JrCs
Copy link
Collaborator

JrCs commented Feb 8, 2016

The problem is nginx.tmpl. I don’t want to have a specific version in my container.
This container was solely created to generate letsencrypt certificates, not to manage nginx proxy configuration(s).

@nilroy
Copy link
Author

nilroy commented Feb 8, 2016

@JrCs then lets not maintain nginx.tmpl here. But can we merge this PR?

@JrCs
Copy link
Collaborator

JrCs commented Feb 8, 2016

@nilroy yes but i need some time to review/test it.

@nilroy
Copy link
Author

nilroy commented Feb 9, 2016

@JrCs No hurry. 👍

@mpatton125
Copy link

Problem I currently have, is that the version of docker-gen included within this LE container isn't new enough to accept the docker networking changes with the newer nginx.tmpl file I am using...

Any chance you can upgrade the docker-gen binary?

@nilroy
Copy link
Author

nilroy commented Feb 12, 2016

I can test if that works.

@mpatton125
Copy link

Using docker-gen 0.5.0 definitely doesn't work due to the already reported "non-exit" bug. I tried the binary from 0.4.3 as well, but it's not updated with the new networking stuff so I can't use it.

Hopefully the 0.5.0 issue will be sorted soon, as that is all that I need to have this all working seemlessly.

@nilroy
Copy link
Author

nilroy commented Feb 13, 2016

@falcon15500. I tested docker-gen 0.5.0 in LE container and it works. It is exiting properly while it is run locally inside the LE container.

@nilroy
Copy link
Author

nilroy commented Feb 13, 2016

@JrCs I am updating this PR

@mpatton125
Copy link

@nilroy Definitely doesn't exit for me, even when run manually. I need to CTRL C to kill it. Tried it this morning.

@nilroy
Copy link
Author

nilroy commented Feb 13, 2016

@falcon15500 Sorry... Its not exiting. I was wrongly testing. Its generated the configuration file but did not reloaded nginx. My bad

@mpatton125
Copy link

Okay, jwilder has just put through a commit that fixes the "not exiting" problem in docker-gen 0.5.0 and the new version is 0.6.0. I have just tested it and it is working fine.

So now my nginx (official), docker-gen (0.6.0) and an LE companion container (modified with this pull request, my own modified nginx.tmpl and injected with the new docker-gen 0.6.0 executable) are working perfectly to grab/renew my SAN certificate under docker 1.10.1 (with the new networking changes).

Thanks everyone! :) 👍

@nilroy
Copy link
Author

nilroy commented Feb 15, 2016

@JrCs Should I bump the docker-gen version here?

@JrCs
Copy link
Collaborator

JrCs commented Feb 16, 2016

@Nitroy not needed it's already in the master branch.

@nilroy
Copy link
Author

nilroy commented Feb 16, 2016

@JrCs cool. I shall wait for this PR to be merged and the latest LE container available publicly. Till then I am using my locally built one.

@JrCs
Copy link
Collaborator

JrCs commented Feb 16, 2016

@Nitroy i'm currently thinking how to mix your PR and some enhancement i have.

@nilroy
Copy link
Author

nilroy commented Feb 16, 2016

@JrCs Well I think its better to do that in a separate PR

@JrCs
Copy link
Collaborator

JrCs commented Feb 16, 2016

@falcon15500 What modifications have you done in nginx.tmpl.

@mpatton125
Copy link

I am completely new to templating, but here is my nginx.tmpl - it's an amalgamation of the template inside your LE container and the one from the newer docker-gen (0.5.0 at the time), with it's support for the newer docker networking.

nginx.tmpl.txt

@JrCs
Copy link
Collaborator

JrCs commented Feb 16, 2016

@nilroy and @falcon15500 can you try my dev branch (image dev created also) ?

@mpatton125
Copy link

@JrCs @nilroy I have tested out your DEV branch and it seems to be working fine. I started fresh and it generated my 10+ domains worth under the 1 SAN certificate, sent a HUP to the nginx-gen which regenerated the nginx default.conf via my nginx.tmpl - all working. Good job!

@JrCs
Copy link
Collaborator

JrCs commented Feb 17, 2016

Thanks @falcon15500, need some little enhancement and il will merge into the master branch.

@JrCs
Copy link
Collaborator

JrCs commented Feb 19, 2016

Close by 0d6d105

@JrCs JrCs closed this Feb 19, 2016
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.

None yet

3 participants