Skip to content

Conversation

buchdag
Copy link
Member

@buchdag buchdag commented May 19, 2018

This is a fix for #375

This PR move the self ID retrieval to a reusable function, instead of a one time operation that stores the ID in an environment variable at container startup.

The /app/force_renew script being used through docker exec, it can't access this environment variable and therefore fails to check for a running nginx container (#321) when using the --volumes-from method.

Copy link
Collaborator

@JrCs JrCs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not better to assign container_id in a variable instead of calling the function multiple times ?

@buchdag
Copy link
Member Author

buchdag commented May 19, 2018

It is but docker exec can't read this variable because it does not go through the entrypoint script and the current CONTAINER_ID variable scope is limited to this same script.

A better approach might be to save and export the result of get_self_id in the entrypoint script and do something like ${CONTAINER_ID:-$(get_self_id)} when retrieving the nginx container ID through --volumes-from. Sounds okay to you ?

Fix nginx-proxy#375
This enable the /app/force_renew script to get the LE companion
container ID, which is required to check if the nginx container
is running (nginx-proxy#321) when using the --volumes-from method.
@buchdag buchdag merged commit aca144c into nginx-proxy:master May 20, 2018
@buchdag buchdag deleted the fix-375 branch May 26, 2018 15:15
bingozb pushed a commit to bingozb/docker-letsencrypt-nginx-proxy-companion that referenced this pull request Dec 4, 2019
Fix nginx-proxy#375
This enable the /app/force_renew script to get the LE companion
container ID, which is required to check if the nginx container
is running (nginx-proxy#321) when using the --volumes-from method.
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.

2 participants