Improve support for WSL with Docker Desktop and Podman Desktop #2875
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The existing --auto-wsl-workaround allowed to run ig in a docker container running in the docker-desktop WSL distribution. This patch adds support for running ig in any WSL distribution and interact with Docker Desktop with the WSL integration. Additionally, the same is done for Podman Desktop. Summary of supported WSL configurations: | ig running on | container runtime | Support | |-----------------------------------------------|---------------------------------------------------|---------| | WSL distribution (without container) | docker installed natively in the WSL distribution | ✅ | | WSL distribution (without container) | podman installed natively in the WSL distribution | ✅ | | WSL distribution (without container) | Docker Desktop with WSL integration | ✅ | | WSL distribution (without container) | Podman Desktop (podman-machine-default) | ✅ | | Docker Desktop container with WSL integration | Docker Desktop with WSL integration | ✅ | | Podman Desktop container | Podman Desktop (podman-machine-default) | ✅ | | - | Docker Desktop with Hyper-V backend | ❌ | Signed-off-by: Alban Crequy <[email protected]>
eiffel-fl
reviewed
May 27, 2024
Comment on lines
+284
to
+291
| dockerExePath, _ := exec.LookPath("docker") | ||
| if dockerExePath != "" { | ||
| dockerLinkPath, _ := os.Readlink(dockerExePath) | ||
| if strings.HasPrefix(dockerLinkPath, "/mnt/wsl/docker-desktop/cli-tools/") { | ||
| return true | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
Suggested change
| dockerExePath, _ := exec.LookPath("docker") | |
| if dockerExePath != "" { | |
| dockerLinkPath, _ := os.Readlink(dockerExePath) | |
| if strings.HasPrefix(dockerLinkPath, "/mnt/wsl/docker-desktop/cli-tools/") { | |
| return true | |
| } | |
| } | |
| return false | |
| dockerExePath, _ := exec.LookPath("docker") | |
| if dockerExePath == "" { | |
| return false | |
| } | |
| dockerLinkPath, _ := os.Readlink(dockerExePath) | |
| return strings.HasPrefix(dockerLinkPath, "/mnt/wsl/docker-desktop/cli-tools/") |
There was a problem hiding this comment.
I am also wondering if you should catch the error.
Comment on lines
+295
to
+301
| podmanExePath, _ := exec.LookPath("podman") | ||
| if podmanExePath != "" { | ||
| if _, err := os.Stat("/mnt/wsl/podman-sockets/podman-machine-default/podman-root.sock"); err == nil { | ||
| return true | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
Suggested change
| podmanExePath, _ := exec.LookPath("podman") | |
| if podmanExePath != "" { | |
| if _, err := os.Stat("/mnt/wsl/podman-sockets/podman-machine-default/podman-root.sock"); err == nil { | |
| return true | |
| } | |
| } | |
| return false | |
| podmanExePath, _ := exec.LookPath("podman") | |
| if podmanExePath == "" { | |
| return false | |
| } | |
| _, err := os.Stat("/mnt/wsl/podman-sockets/podman-machine-default/podman-root.sock") | |
| return err == nil |
There was a problem hiding this comment.
Same comment with regard to catching the errors.
|
|
||
| // getPid1Environ returns the environment variables of the init process in the | ||
| // given procfs path. | ||
| func getPid1Environ(procPath string) map[string]string { |
There was a problem hiding this comment.
I would rather use Environment instead of Environ.
Comment on lines
+347
to
+349
| wslDistroName := pid1Environ["WSL2_DISTRO_NAME"] | ||
| if wslDistroName == "" { | ||
| wslDistroName = pid1Environ["WSL_DISTRO_NAME"] |
There was a problem hiding this comment.
Can you please check the map access with ok?
| return nil | ||
| } | ||
|
|
||
| // We can use the workaround based on wsl.exe |
| return nil | ||
| } | ||
|
|
||
| if isPodmanDesktopWSL2() { |
There was a problem hiding this comment.
You can factorize code between the isPodmand and isDocker cases.
|
Also:
|
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The existing --auto-wsl-workaround allowed to run ig in a docker container running in the docker-desktop WSL distribution.
This patch adds support for running ig in any WSL distribution and interact with Docker Desktop with the WSL integration.
Additionally, the same is done for Podman Desktop.
Summary of supported WSL configurations:
The different Linux distributions run in different pidns and mntns under the same Linux kernel. They share a bind mount, typically in
/mnt/wsl. Andwsl.execan be used in a WSL distribution to execute a command in another WSL distribution.