-
Notifications
You must be signed in to change notification settings - Fork 824
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
Get nginx container id from labelled container #181
Conversation
Not tested 100% yet, when I test it in docker compose v3 I'll post a comment. Feel free to comment on this PR in the meantime. |
@emmetog It's fully functional and completely compatible with docker compose v3. Thanks for your PR 👍 |
README.md
Outdated
@@ -30,14 +30,15 @@ $ docker run -d -p 80:80 -p 443:443 \ | |||
-v /etc/nginx/vhost.d \ | |||
-v /usr/share/nginx/html \ | |||
-v /var/run/docker.sock:/tmp/docker.sock:ro \ | |||
--label jrcs.nginx_letsencrypt_companion.is_proxy=true \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should use a fully qualified domain name, i.e. com.github.jrcs.letsencrypt_nginx_proxy_companion.nginx_proxy=true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it will better to use a fully qualified domain name.
@emmetog can you make the changes ?
@@ -30,14 +30,15 @@ $ docker run -d -p 80:80 -p 443:443 \ | |||
-v /etc/nginx/vhost.d \ | |||
-v /usr/share/nginx/html \ | |||
-v /var/run/docker.sock:/tmp/docker.sock:ro \ | |||
--label com.github.jrcs.letsencrypt_nginx_proxy_companion.nginx_proxy=true \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need for =true
. Just a plain label is enough. It doesn't have to be a key-value pair.
Fixes #170