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

Fix DOCKER_HOST default to include unix:// path prefix #260

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Zortaniac
Copy link
Contributor

The default for the docker_host() function does not include the unix:// path prefix. This causes issues when used in exec resources.

@nicholasjackson
Copy link
Contributor

Hey @Zortaniac, thanks for this; this never had unix:// prefixed to it because we always used this function to mount the default docker sock into a container. The name needs to be more accurate from the intention; you are, however, correct, and this should be updated. I will create a separate PR that combines your changes with a new function for getting the docker sock.

I have also fixed all the tests for the project, which was long overdue from when we moved to the new HCL config package. I will update the tests for you.

@Zortaniac
Copy link
Contributor Author

Hey @nicholasjackson, the problem I see with your approach is, that you derive the value from the environment DOCKER_HOST. The default (if unset) for DOCKER_HOST is unix:///var/run/docker.sock, so you have to expect the unix:// prefix anyway. The same problem occurs for environments with remote docker servers using tcp://.
For now I will just set DOCKER_HOST explicitly if not set, but would be great to get this working to be able to pass the environment variable to exec resources.

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