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

Docker Compose support #203

Closed
wants to merge 5 commits into from
Closed

Docker Compose support #203

wants to merge 5 commits into from

Conversation

ekkis
Copy link

@ekkis ekkis commented Apr 26, 2017

  • added docker-compose.yml file
  • included jwilder’s generator template for sharing with docker-gen

- added docker-compose.yml file
- included jwilder’s generator template for sharing with docker-gen
as per
https://github.com/JrCs/docker-letsencrypt-nginx-proxy-companion/issues/
201 I had the reference to the nginx container wrong for notifications
@giordy
Copy link

giordy commented May 3, 2017

Hi @ekkis, I tested your PR, I think there is a small bug.
You should change
- nginx-gen-tmpl:/etc/docker-gen/templates/nginx.tmpl
into
- nginx-gen-tmpl:/etc/docker-gen/templates

@ekkis
Copy link
Author

ekkis commented May 3, 2017

yes, you're correct. I had already fixed that (in fact, it needs .../templates:rw since it's responsible for writing) but haven't synced it with github because I need to address the issue of the network name. I haven't had time but in the next day or so I will test that and resubmit

@giordy
Copy link

giordy commented May 3, 2017

One more note: I think that using docker compose might still be not that straight forward, since there seems to be a bug in the custom vhost management: #193
I stumbled upon it while working on this project, where I tried to reuse your PR, and I'm not sure how to go around it.
If you are familiar with the problem or have an idea for a solution it might be a good chance to include it.

@ekkis
Copy link
Author

ekkis commented May 4, 2017

@giordy, that issue is using version 2 of the docker-compose. I guess the first question to ask would be whether it breaks with v3 (which I'm using)

@ekkis
Copy link
Author

ekkis commented May 4, 2017

ok, I've made all the changes and the PR is ready to be accepted. please let me know if there's anything else I need to do

@ekkis
Copy link
Author

ekkis commented May 4, 2017

incidentally, the PR includes the changes to the Dockerfile discussed here: #201 which make it unnecessary to download the template

@giordy
Copy link

giordy commented May 4, 2017

@ekkis I confirm I experienced it with version 3 of the docker compose file

@JrCs
Copy link
Collaborator

JrCs commented May 4, 2017

Hi @ekkis, thanks for your PR but i don't want to add files specific to the manner that the container are start (in this PR docker-compose).
This project contain only the container and the basic methods to start it (using docker run only command). There so many different manners/environment to start the container that i don't want to support.
So if you want to propose a manner to start the container with docker compose v3, the best is to create your own repository with the examples/files that are needed to work.
Then i can create a link on the README page to your repository like i already done for other people.

@ekkis
Copy link
Author

ekkis commented May 4, 2017

one more change: I've added name support for the real interfaces as per #200

@ekkis
Copy link
Author

ekkis commented May 4, 2017

@JrCs: yes, I can understand your point. someone else I was discussing this with is using saltstack to start the containers. so this really belongs as a separate project. I will create one and get back to you with it. as a matter of fact, I'll reach out to that person to perhaps do the same for saltstack

@ekkis ekkis closed this May 4, 2017
@ekkis
Copy link
Author

ekkis commented May 4, 2017

@JrCs: I've created a separate project for the docker-compose file but realised that the changes to your Dockerfile actually belong in your project (my project won't have a container). can I modify this PR then for that functionality? it allows a user not to have any extra steps (I wasted a lot of time figuring out I was missing that stuff)

@ekkis ekkis reopened this May 4, 2017
@ekkis
Copy link
Author

ekkis commented May 4, 2017

here's the new project: https://github.com/ekkis/nginx-proxy-LE-docker-compose I've modified your README to point to this and I've reached out to the other guy about SaltStack (hopefully he'll put it together). so this PR is ready for acceptance

@ekkis
Copy link
Author

ekkis commented May 6, 2017

incidentally, I've added a warning to my package (https://github.com/ekkis/nginx-proxy-LE-docker-compose#warning) that depending on whether this PR is approved, there may be some manual work involved for the user. I will remove that if you merge this PR

@dmitrym0
Copy link

Hmm, I realize that I added a duplicate request #208. Mine appears to be much shorter. What am I missing?

@Azuka
Copy link

Azuka commented May 26, 2017

@ekkis, thanks, this was a great lifesaver. I wanted to say that I modified your docker-compose slightly and was able to get it working without the added changes to the container: I just needed to mount the nginx.tmpl directly (well, in this case the path to the nginx.tmpl)

version: "3"
services:
  nginx-proxy:
    image: nginx
    container_name: nginx-proxy
    ports:
      - "80:80"
      - "443:443"
    volumes:
      - nginx-conf:/etc/nginx/conf.d
      - nginx-vhost:/etc/nginx/vhost.d
      - nginx-html:/usr/share/nginx/html
      - nginx-ssl:/etc/nginx/certs:ro
    labels:
      com.github.jrcs.letsencrypt_nginx_proxy_companion.nginx_proxy: "true"
    networks:
      - nginx-proxy

  nginx-gen:
    image: jwilder/docker-gen
    container_name: nginx-gen
    command: -notify-sighup nginx-proxy -wait 5s:30s -watch /etc/docker-gen/templates/nginx.tmpl /etc/nginx/conf.d/default.conf
    volumes:
      - /var/run/docker.sock:/tmp/docker.sock:ro
      - nginx-conf:/etc/nginx/conf.d
      - nginx-vhost:/etc/nginx/vhost.d
      - nginx-html:/usr/share/nginx/html
      - ./nginx:/etc/docker-gen/templates:rw
      - nginx-ssl:/etc/nginx/certs:ro
    links:
      - nginx-proxy
    networks:
      - nginx-proxy

  nginx-ssl:
    image: jrcs/letsencrypt-nginx-proxy-companion
    container_name: nginx-ssl
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock:ro
      - nginx-conf:/etc/nginx/conf.d
      - nginx-vhost:/etc/nginx/vhost.d
      - nginx-html:/usr/share/nginx/html
      - ./nginx:/etc/docker-gen/templates:ro
      - nginx-ssl:/etc/nginx/certs:rw
    environment:
      - ACME_CA_URI
      - "NGINX_DOCKER_GEN_CONTAINER=nginx-gen"
    links:
      - nginx-proxy
      - nginx-gen
    networks:
      - nginx-proxy
  api:
    restart: always
    image: $IMAGE_NAME:$VERSION
    restart: always
    env_file:
      - ./.env
      - ./version.env
    environment:
      - API_PORT=7000
      - VIRTUAL_HOST=<my-host-name>
      - VIRTUAL_NETWORK=nginx-proxy
      - VIRTUAL_PORT=7000
      - LETSENCRYPT_HOST=<my-host-name>
      - LETSENCRYPT_EMAIL=<my-email>
    networks:
      - nginx-proxy
    volumes:
      - /var/run/docker.sock:/tmp/docker.sock:ro

volumes:
  nginx-conf:
  nginx-vhost:
  nginx-html:
  nginx-ssl:

networks:
  nginx-proxy:
    external: true

Please let me know if this works for you as well.

@ekkis
Copy link
Author

ekkis commented May 26, 2017

@Azuka, I can see that you're anchoring the volume to your file system, presumably so you can download the template and make it available to the system. that part will work automatically if @JrCs approves this PR.

as for the api target, what is it? why do I need it? I don't understand what problem it solves

@Azuka
Copy link

Azuka commented May 26, 2017

@ekkis, that's my own service that needs the nginx proxy with letsencrypt. I have them as separate files, but I combined them above just as an example.

@JrCs
Copy link
Collaborator

JrCs commented May 28, 2017

@ekkis sorry for this but like i said in #202 i don't want to include the nginx.tmpl file into my letsencrypt container

@JrCs JrCs closed this May 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants