feat: add WSL2 driver #1721

pendo324 · 2023-08-08T00:15:08Z

This PR users WSL2 to add a new VM Driver for Windows. This addresses the longstanding GitHub issue asking for Windows support (#391).

The traditional thinking (as seen in #909) is to make QEMU work on Windows, and the addition of a WSL2 driver should not inhibit that effort, just provide another option (I'd also like to see a direct HyperV option 👀 ).

Considerations:

The implementation in this PR currently does not forward the VM's SSH address to 127.0.0.1:<Instance.SSHLocalPort>, although this is possible through the use of netsh (I couldn't get ssh port forwarding to work). Currently, the PR just uses the WSL2 instance's IP address and port 22. I can add some netsh calling code to get this working relatively easily, I just wanted to know if that's required
The implementation requires an ssh binary on Windows, which isn't common. My recommendation is to have users install Git for Windows (specifically the MinGit release), which comes with some other helpful utilities like cygpath. Bundling this with Lima or just requiring users to run winget install -e --id Git.MinGit (winget is now built into Windows) and adding it to path are both possible. The version of OpenSSH installable as a Windows optional feature is pretty old. Open to suggestions incase I missed something.
The implementation uses WSL2's built-in network sharing and ssh forwarding of ports. It should technically be possible to use gvisor or similar, but I think that can be future work
The implementation keeps the WSL2 instance running until stopped by executing an extremely long sleep command. Open to suggestions here, as I'm sure there are better ways to do this
There might be some final debug code left in this diff. I will remove it in revisions over the coming days
There are a lot of tests left to be added, which I'll be working in the coming days

cmd/limactl/delete.go

pkg/store/instance.go

pkg/ioutilx/ioutilx.go

afbjorklund · 2023-08-08T05:51:06Z

@pendo324 : thank you for your contribution, it would be have been good to open an issue for discussion about the feature itself and keep the PR for the implementation - but that can be done afterwards too, and reference this PR

Running Lima in a container is slightly different from all the other drivers, so it might need some thought (Lima itself has some similaries to WSL2). I agree that a more traditional HyperV driver would also be a nice addition, like "VZ"...

cmd/lima-guestagent/daemon_linux.go

AkihiroSuda · 2023-08-08T06:17:10Z

cmd/limactl/delete.go

@@ -53,9 +55,16 @@ func deleteInstance(inst *store.Instance, force bool) error {

 	stopInstanceForcibly(inst)

+	if inst.VMType == limayaml.WSL {
+		if _, err := executil.RunUTF16leCommand([]string{"wsl", "--unregister", inst.DistroName}); err != nil {


Can we just limit it to ASCII?

examples/experimental/wsl.yaml

pkg/driverutil/instance.go

pendo324 · 2023-08-10T00:05:52Z

@pendo324 : thank you for your contribution, it would be have been good to open an issue for discussion about the feature itself and keep the PR for the implementation - but that can be done afterwards too, and reference this PR

Running Lima in a container is slightly different from all the other drivers, so it might need some thought (Lima itself has some similaries to WSL2). I agree that a more traditional HyperV driver would also be a nice addition, like "VZ"...

I'll create an issue to track Windows support in general, and weigh the different options (WSL2, HyperV, qemu, etc.) as I do think there are reasons you might want to use one over the other. I wanted to come to the project with some code instead of just an issue, but it ballooned into a lot of code before I knew it 😅

examples/default.yaml

examples/experimental/wsl.yaml

pkg/cidata/cidata.TEMPLATE.d/boot.sh

pkg/downloader/downloader_unix.go

pkg/downloader/downloader_windows.go

pkg/httpclientutil/httpclientutil_others.go

pendo324 · 2023-08-23T00:44:17Z

Addressed comments and rebased. Also added documentation. Would be good to merge #1726 first since it's just extracted from @afbjorklund PR. I can just squash it into this PR if that's preferable though.

jandubois · 2023-08-23T00:57:01Z

I guess the most common reason to prefer WSL2 over bare Hyper-V would be to share files using 9p?

Another one is that WSL2 is supported in Windows Home whereas Hyper-V requires at least Windows Pro. This is purely a licensing issue as WSL2 is running on top of Hyper-V.

pendo324 · 2023-08-28T23:30:20Z

Not sure why the tests are failing, but it doesn't seem related. Should I keep retrying @AkihiroSuda?

AkihiroSuda · 2023-08-29T14:19:49Z

Not sure why the tests are failing, but it doesn't seem related. Should I keep retrying @AkihiroSuda?

CI for Colima seems consistently failing after restarting several times. Maybe a regression in this PR?

pendo324 · 2023-08-30T23:58:56Z

Not sure why the tests are failing, but it doesn't seem related. Should I keep retrying @AkihiroSuda?

CI for Colima seems consistently failing after restarting several times. Maybe a regression in this PR?

Found the bug (typo) and fixed it (tests passed locally). Should be good to go after CI passes

AkihiroSuda · 2023-09-03T15:10:02Z

docs/vmtype.md

+> **Warning**
+> "wsl2" mode is experimental
+
+| :zap: Requirement | Lima >= 0.14, macOS >= 13.0 |


Seems a copy paste error

Thanks, fixed in latest revision

AkihiroSuda · 2023-09-03T15:11:07Z

Thanks, almost looks good, but needs rebase

Signed-off-by: Justin Alvarez <alvajus@amazon.com>

pendo324 · 2023-09-05T18:16:03Z

Thanks, almost looks good, but needs rebase

Thanks. Just rebased and addressed your new comment

AkihiroSuda

Thanks

pendo324 · 2023-09-05T19:40:03Z

Looks like the tests failed/timed out on TestGetPorts/nodePort_serivce. Trying to repro locally

pendo324 · 2023-09-06T15:44:19Z

Couldn't repro test error locally:

$ go test -timeout 30s -v -run ^TestGetPorts github.com/lima-vm/lima/pkg/guestagent/kubernetesservice

=== RUN   TestGetPorts
=== RUN   TestGetPorts/nodePort_serivce
=== RUN   TestGetPorts/loadBalancer_service
=== RUN   TestGetPorts/clusterIP_service
--- PASS: TestGetPorts (0.00s)
    --- PASS: TestGetPorts/nodePort_serivce (0.00s)
    --- PASS: TestGetPorts/loadBalancer_service (0.00s)
    --- PASS: TestGetPorts/clusterIP_service (0.00s)
PASS
ok      github.com/lima-vm/lima/pkg/guestagent/kubernetesservice        0.440s

Is it possible to re-run the step?

AkihiroSuda · 2023-09-07T02:38:09Z

Restarted CI, all green

AkihiroSuda · 2023-09-07T04:03:23Z

PTAL:

CI: test WSL2 #1781

afbjorklund reviewed Aug 8, 2023

View reviewed changes

cmd/limactl/delete.go Outdated Show resolved Hide resolved

afbjorklund reviewed Aug 8, 2023

View reviewed changes

pkg/store/instance.go Outdated Show resolved Hide resolved

afbjorklund reviewed Aug 8, 2023

View reviewed changes

pkg/ioutilx/ioutilx.go Outdated Show resolved Hide resolved

AkihiroSuda added the platform/Windows label Aug 8, 2023

AkihiroSuda reviewed Aug 8, 2023

View reviewed changes

cmd/lima-guestagent/daemon_linux.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Aug 8, 2023

View reviewed changes

cmd/lima-guestagent/daemon_linux.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Aug 8, 2023

View reviewed changes

examples/experimental/wsl.yaml Outdated Show resolved Hide resolved

AkihiroSuda reviewed Aug 8, 2023

View reviewed changes

pkg/driverutil/instance.go Outdated Show resolved Hide resolved

pendo324 mentioned this pull request Aug 10, 2023

feat: add Register/Unregister to Driver interface #1726

Merged

pendo324 force-pushed the wsl2 branch 5 times, most recently from bdc225f to 4101c78 Compare August 15, 2023 23:47

pendo324 requested review from AkihiroSuda and afbjorklund August 16, 2023 00:02

pendo324 marked this pull request as ready for review August 16, 2023 00:02