feat: Mount to correct DOCKER_HOST#959
Conversation
|
Thanks a lot for the PR, I proposed a suggestion that I think will make the code slightly cleaner and more resilient to weird DOCKER_HOST values. |
Co-authored-by: David Barroso <dbarrosop@dravetech.com>
There was a problem hiding this comment.
Pull Request Overview
This PR updates the traefik service configuration to mount the correct Docker socket based on the DOCKER_HOST environment variable.
- Introduces logic to check and parse the DOCKER_HOST variable
- Updates the volume source in docker-compose configuration accordingly
| return nil, fmt.Errorf("failed to parse DOCKER_HOST: %w", err) | ||
| } | ||
| if u.Scheme != "unix" { | ||
| return nil, fmt.Errorf("unsupported scheme %s in DOCKER_HOST, only unix supported", protocol) |
There was a problem hiding this comment.
The error message uses the variable 'protocol', which is undefined. It should use 'u.Scheme' instead.
| return nil, fmt.Errorf("unsupported scheme %s in DOCKER_HOST, only unix supported", protocol) | |
| return nil, fmt.Errorf("unsupported scheme %s in DOCKER_HOST, only unix supported", u.Scheme) |
|
Thanks, unfortunately this is broken. Please, make sure it works and let me know so I can test/review again. |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the traefik service configuration to mount the Docker socket dynamically based on the DOCKER_HOST environment variable rather than relying on a hardcoded path.
- Added support to parse DOCKER_HOST and mount the corresponding UNIX socket.
- Introduced an error handling mechanism for non-unix socket schemes.
Comments suppressed due to low confidence (1)
dockercompose/compose.go:212
- [nitpick] Consider renaming the variable 'path' to 'socketPath' for improved clarity regarding its purpose in representing the Docker socket path.
path := "/var/run/docker.sock"
|
The CI is failing due to lack of permissions to read secrets, I tested things by hand and they work so merging. |
|
Oh, and thanks for this, of course :D |
|
@dbarrosop how would I go about using the nhost cli with rootless podman with the above changes? I saw you resolved #727 with this change, that's why I ask. |
|
Unfortunately, I don't know, this was a community contribution. I have no means of reproducing the problem so I am unsure what the problem is. I would suggest checking the comment that lead to this PR and following up there if the latest cli doesn't work for you. Thanks! |
|
Why do you then complete a pull request, and link an issue as completed when you have no idea what it's about or if it resolves it? The #727 issue mentions that nhost up doesn't work with podman installed instead of docker, and it still doesn't. Why would you not have the means to reproduce this, or think this has now been solved then?! |
|
Please, refer to my previous comment. Thanks! |
|
@unmade-spangle @christensenjairus I made this PR, if you are still having issues after following these instructions #727 (comment). Then reopen #727 and post what problems you are having. This patch works for me and my team but we are all using Arch Linux based systems. If your installation of Podman is different your mileage may vary. |
Description
Problem
Currently the
docker.sockpath is hardcoded when generating the volumes for the traefik container in thedocker-compose.ymlfile generation.Solution
Checks the
DOCKER_HOSTenvironment variable for the active socket location, if it doesn't exist the CLI defaults to/var/run/docker.sock(no change), if it does exist and is aunix:socket, then mount the appropriate file. If it does exist and is not aunix:socket, throw an error.Notes
This helps resolve #727 because Podman relies on the
DOCKER_HOSTenvironment variable when using the rootless socket.