Restore DynamicSSHAddress functionality for WSL2

[afbjorklund]

Restores the functionality:
commit df5e35a

That was reverted (again):
commit 2076ff1

func (l *LimaWslDriver) SSHAddress(_ context.Context) (string, error) {
	return "127.0.0.1", nil
}

It was first removed in 1.0.7:
commit ebaa100

---

• Add fallback for busybox hostname with wsl2 #3842

• Use loopback for SSH connection in WSL2 #3299

[unsuman]

It seems like InspectStatus is never called, and it expects the new SSHAddress to do this instead?

driver.InspectStatus() is called in:

lima/pkg/store/instance.go

Line 114 in b9b9aa6

inspectStatus(ctx, instDir, inst, y)

This broke the drivers, because they now don't get the dynamic ssh address even after running...

Drivers? It is only applicable to wsl2 afaik.

[afbjorklund]

Drivers? It is only applicable to wsl2 afaik.

I was referring to the three container drivers

[afbjorklund]

The driver starts out with the address from SSHAdress, later when it is running it is replaced in InspectStatus

It works the same in the other container drivers, they each implement two helper methods in order to do this:

func (l *LimaWslDriver) InspectStatus(ctx context.Context, inst *limatype.Instance) string {
        status, err := getWslStatus(inst.Name)
        if err != nil {
                inst.Status = limatype.StatusBroken
                inst.Errors = append(inst.Errors, err)
        } else {
                inst.Status = status
        }

        inst.SSHLocalPort = 22

        if inst.Status == limatype.StatusRunning {
                sshAddr, err := getSSHAddress(ctx, inst.Name)
                if err == nil {
                        inst.SSHAddress = sshAddr
                } else {
                        inst.Errors = append(inst.Errors, err)
                }
        }

        return inst.Status
}

Ideally we should probably not have the VM type in the method name like that, in the VM implementation...

func getWslStatus(instName string) (string, error) {
func getSSHAddress(ctx context.Context, instName string) (string, error) {

func getAcStatus(ctx context.Context, instName string) (string, error) {
func getSSHAddress(ctx context.Context, instName string) (string, error) {

func getDcStatus(ctx context.Context, instName string) (string, error) {
func getSSHAddress(ctx context.Context, instName string) (string, error) {

We also seem to have lost all documentation of the WSL2 methods in the "refactoring", that is not ideal.

[AkihiroSuda]

Why not implement the new logic in l.SSHAddress ?

[afbjorklund]

Because SSHAddress does not check the Status, that is currently in InspectStatus

If we make XXXStatus part of the API instead, we could move the logic to store ?

[afbjorklund]

This happened when we added the "no-op" functions, that other drivers currently use.

func (l *LimaVzDriver) SSHAddress(_ context.Context) (string, error) {
        return "127.0.0.1", nil
}

func (l *LimaVzDriver) InspectStatus(_ context.Context, _ *limatype.Instance) string {
        return ""
}

[afbjorklund]

Normally SSHAddress is never called, unless a specific driver feature is enabled.

        // WSL instance SSH address isn't known until after VM start
        if a.driver.Info().Features.DynamicSSHAddress {
                sshAddr, err := a.driver.SSHAddress(ctx)
                if err != nil {
                        return err
                }
                a.instSSHAddress = sshAddr
        }

And InspectStatus is not implemented, which means it will check the PID files.

func inspectStatus(ctx context.Context, instDir string, inst *limatype.Instance, y *limatype.LimaYAML) {
        status, err := driverutil.InspectStatus(ctx, inst)
        if err != nil {
                inst.Status = limatype.StatusBroken
                inst.Errors = append(inst.Errors, fmt.Errorf("failed to inspect status: %w", err))
                return
        }

        if status == "" {
                inspectStatusWithPIDFiles(instDir, inst, y)
                return
        }

        inst.Status = status
}

[afbjorklund]

Note that the logic is not new, it was just moved from pkg/store (in 1.x) to the drivers (2.x).

https://github.com/lima-vm/lima/blob/release/1.2/pkg/store/instance_windows.go

[unsuman]

Thanks, LGTM!

[afbjorklund]

@unsuman this old behaviour (from before, in pkg/store) is also a bit weird.

It starts out with "port 0" like any other driver, then go to "22" when running.
But connecting to 127.0.0.1:22 is probably never what you want, anyway...

Will follow up with a fix to that, when I know where the functions end up

[jandubois]

I think this command will always return 127.0.1.1. It is documented at https://learn.microsoft.com/en-us/windows/wsl/networking#accessing-a-wsl-2-distribution-from-your-local-area-network-lan

I can confirm that this is still true with the latest WSL2:

C:\Users\suse>wsl -d Ubuntu hostname -i
127.0.1.1

C:\Users\suse>wsl -d rancher-desktop hostname -i
127.0.1.1

So I don't know if this fallback will ever help; maybe it should be removed?