From 87421d95084486448f85f776529a688a852d902c Mon Sep 17 00:00:00 2001 From: Mario Loriedo Date: Mon, 10 Mar 2025 17:56:35 +0100 Subject: [PATCH 1/2] Fix WSL installation check on Windows Fixes #25234 Signed-off-by: Mario Loriedo --- Makefile | 2 +- pkg/machine/wsl/machine.go | 12 +-- pkg/machine/wsl/wutil/wutil.go | 91 ++++++++++++----- pkg/machine/wsl/wutil/wutil_test.go | 145 ++++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 31 deletions(-) create mode 100644 pkg/machine/wsl/wutil/wutil_test.go diff --git a/Makefile b/Makefile index 36be6988ee..44706ee8fe 100644 --- a/Makefile +++ b/Makefile @@ -652,7 +652,7 @@ localunit: test/goecho/goecho test/version/version UNIT=1 $(GINKGO) \ -r \ $(TESTFLAGS) \ - --skip-package test/e2e,pkg/bindings,hack,pkg/machine/e2e \ + --skip-package test/e2e,pkg/bindings,hack,pkg/machine/e2e,pkg/machine/wsl \ --cover \ --covermode atomic \ --coverprofile coverprofile \ diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 3355f6b2b5..73650c18ea 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -317,12 +317,12 @@ func checkAndInstallWSL(reExec bool) (bool, error) { admin := HasAdminRights() - if !IsWSLFeatureEnabled() { + if !wutil.IsWSLFeatureEnabled() { return false, attemptFeatureInstall(reExec, admin) } skip := false - if reExec && !admin { + if !reExec && !admin { fmt.Println("Launching WSL Kernel Install...") if err := launchElevate(wslInstallKernel); err != nil { return false, err @@ -363,11 +363,11 @@ func attemptFeatureInstall(reExec, admin bool) error { message += "NOTE: A system reboot will be required as part of this process. " + "If you prefer, you may abort now, and perform a manual installation using the \"wsl --install\" command." - if reExec && MessageBox(message, "Podman Machine", false) != 1 { + if !reExec && MessageBox(message, "Podman Machine", false) != 1 { return errors.New("the WSL installation aborted") } - if reExec && !admin { + if !reExec && !admin { return launchElevate("install the Windows WSL Features") } @@ -622,10 +622,6 @@ func obtainGlobalConfigLock() (*fileLock, error) { return lockFile(filepath.Join(lockDir, "podman-config.lck")) } -func IsWSLFeatureEnabled() bool { - return wutil.SilentExec(wutil.FindWSL(), "--set-default-version", "2") == nil -} - func isWSLRunning(dist string) (bool, error) { return wslCheckExists(dist, true) } diff --git a/pkg/machine/wsl/wutil/wutil.go b/pkg/machine/wsl/wutil/wutil.go index bc07ffa2a9..49bc638ac4 100644 --- a/pkg/machine/wsl/wutil/wutil.go +++ b/pkg/machine/wsl/wutil/wutil.go @@ -19,16 +19,26 @@ import ( ) var ( - once sync.Once - wslPath string + onceFind, onceStatus sync.Once + wslPath string + status wslStatus + wslNotInstalledMessages = []string{"kernel file is not found", "The Windows Subsystem for Linux is not installed"} + vmpDisabledMessages = []string{"enable the Virtual Machine Platform Windows feature", "Enable \"Virtual Machine Platform\""} + wslDisabledMessages = []string{"enable the \"Windows Subsystem for Linux\" optional component"} ) +type wslStatus struct { + installed bool + vmpFeatureEnabled bool + wslFeatureEnabled bool +} + func FindWSL() string { // At the time of this writing, a defect appeared in the OS preinstalled WSL executable // where it no longer reliably locates the preferred Windows App Store variant. // // Manually discover (and cache) the wsl.exe location to bypass the problem - once.Do(func() { + onceFind.Do(func() { var locs []string // Prefer Windows App Store version @@ -87,24 +97,44 @@ func SilentExecCmd(command string, args ...string) *exec.Cmd { return cmd } +func parseWSLStatus() wslStatus { + onceStatus.Do(func() { + status = wslStatus{ + installed: false, + vmpFeatureEnabled: false, + wslFeatureEnabled: false, + } + cmd := SilentExecCmd(FindWSL(), "--status") + out, err := cmd.StdoutPipe() + cmd.Stderr = nil + if err != nil { + return + } + if err = cmd.Start(); err != nil { + return + } + + status = matchOutputLine(out) + + if err := cmd.Wait(); err != nil { + return + } + }) + + return status +} + func IsWSLInstalled() bool { - cmd := SilentExecCmd(FindWSL(), "--status") - out, err := cmd.StdoutPipe() - cmd.Stderr = nil - if err != nil { + status := parseWSLStatus() + return status.installed && status.vmpFeatureEnabled +} + +func IsWSLFeatureEnabled() bool { + if SilentExec(FindWSL(), "--set-default-version", "2") != nil { return false } - if err = cmd.Start(); err != nil { - return false - } - - kernelNotFound := matchOutputLine(out, "kernel file is not found") - - if err := cmd.Wait(); err != nil { - return false - } - - return !kernelNotFound + status := parseWSLStatus() + return status.vmpFeatureEnabled } func IsWSLStoreVersionInstalled() bool { @@ -118,13 +148,30 @@ func IsWSLStoreVersionInstalled() bool { return true } -func matchOutputLine(output io.ReadCloser, match string) bool { +func matchOutputLine(output io.ReadCloser) wslStatus { + status := wslStatus{ + installed: true, + vmpFeatureEnabled: true, + wslFeatureEnabled: true, + } scanner := bufio.NewScanner(transform.NewReader(output, unicode.UTF16(unicode.LittleEndian, unicode.UseBOM).NewDecoder())) for scanner.Scan() { line := scanner.Text() - if strings.Contains(line, match) { - return true + for _, match := range wslNotInstalledMessages { + if strings.Contains(line, match) { + status.installed = false + } + } + for _, match := range vmpDisabledMessages { + if strings.Contains(line, match) { + status.vmpFeatureEnabled = false + } + } + for _, match := range wslDisabledMessages { + if strings.Contains(line, match) { + status.wslFeatureEnabled = false + } } } - return false + return status } diff --git a/pkg/machine/wsl/wutil/wutil_test.go b/pkg/machine/wsl/wutil/wutil_test.go new file mode 100644 index 0000000000..4b7bd6af47 --- /dev/null +++ b/pkg/machine/wsl/wutil/wutil_test.go @@ -0,0 +1,145 @@ +//go:build windows + +package wutil + +import ( + "github.com/stretchr/testify/assert" + "golang.org/x/text/encoding/unicode" + "io" + "strings" + "testing" +) + +const ( + WSL1InstalledWithWSLAndVMPEnabled = `Default Version: 1` + WSL2InstalledWithWSLAndVMPEnabled = `Default Version: 2` + WSL1NotInstalled = `Default Version: 1 + +The Windows Subsystem for Linux kernel can be manually updated with 'wsl --update', but automatic updates cannot occur due to your system settings. +To receive automatic kernel updates, please enable the Windows Update setting: 'Receive updates for other Microsoft products when you update Windows'. +For more information please visit https://aka.ms/wsl2kernel. + +The WSL 2 kernel file is not found. To update or restore the kernel please run 'wsl --update'.` + WSL2NotInstalled = `The Windows Subsystem for Linux is not installed. You can install by running 'wsl.exe --install'. +For more information please visit https://aka.ms/wslinstall` + WSL2InstalledWithWSLDisabled = `Default Version: 2 +WSL1 is not supported with your current machine configuration. +Please enable the "Windows Subsystem for Linux" optional component to use WSL1.` + WSL2InstalledWithVMPDisabled = `Default Version: 2 +WSL2 is not supported with your current machine configuration. +Please enable the "Virtual Machine Platform" optional component and ensure virtualization is enabled in the BIOS. +Enable "Virtual Machine Platform" by running: wsl.exe --install --no-distribution +For information please visit https://aka.ms/enablevirtualization` + WSL2InstalledWithWSLAndVMPDisabled = `Default Version: 2 +WSL1 is not supported with your current machine configuration. +Please enable the "Windows Subsystem for Linux" optional component to use WSL1. +WSL2 is not supported with your current machine configuration. +Please enable the "Virtual Machine Platform" optional component and ensure virtualization is enabled in the BIOS. +Enable "Virtual Machine Platform" by running: wsl.exe --install --no-distribution +For information please visit https://aka.ms/enablevirtualization` + WSL1InstalledWithVMPDisabled = `Default Version: 1 +Please enable the Virtual Machine Platform Windows feature and ensure virtualization is enabled in the BIOS. +For information please visit https://aka.ms/enablevirtualization` + WSL1InstalledWithWSLDisabled = `Default Version: 1 +WSL1 is not supported with your current machine configuration. +Please enable the "Windows Subsystem for Linux" optional component to use WSL1.` +) + +func TestMatchOutputLine(t *testing.T) { + tests := []struct { + winVariant string + statusOutput string + want wslStatus + }{ + { + "WSL1 configured and both Virtual Machine Platform enabled and Windows Subsystem for Linux are enabled", + WSL1InstalledWithWSLAndVMPEnabled, + wslStatus{ + installed: true, + vmpFeatureEnabled: true, + wslFeatureEnabled: true, + }, + }, + { + "WSL2 configured and both Virtual Machine Platform enabled and Windows Subsystem for Linux enabled", + WSL2InstalledWithWSLAndVMPEnabled, + wslStatus{ + installed: true, + vmpFeatureEnabled: true, + wslFeatureEnabled: true, + }, + }, + { + "WSL not installed (was previously configured as version 1)", + WSL1NotInstalled, + wslStatus{ + installed: false, + vmpFeatureEnabled: true, + wslFeatureEnabled: true, + }, + }, + { + "WSL not installed (was previously configured as version 2)", + WSL2NotInstalled, + wslStatus{ + installed: false, + vmpFeatureEnabled: true, + wslFeatureEnabled: true, + }, + }, + { + "WSL2 configured and Virtual Machine Platform is enabled but Windows Subsystem for Linux is disabled", + WSL2InstalledWithWSLDisabled, + wslStatus{ + installed: true, + vmpFeatureEnabled: true, + wslFeatureEnabled: false, + }, + }, + { + "WSL2 configured and Virtual Machine Platform is disabled but Windows Subsystem for Linux is enabled", + WSL2InstalledWithVMPDisabled, + wslStatus{ + installed: true, + vmpFeatureEnabled: false, + wslFeatureEnabled: true, + }, + }, + { + "WSL2 configured and both Virtual Machine Platform and Windows Subsystem for Linux are disabled", + WSL2InstalledWithWSLAndVMPDisabled, + wslStatus{ + installed: true, + vmpFeatureEnabled: false, + wslFeatureEnabled: false, + }, + }, + { + "WSL1 configured and Virtual Machine Platform is disabled but Windows Subsystem for Linux is enabled", + WSL1InstalledWithVMPDisabled, + wslStatus{ + installed: true, + vmpFeatureEnabled: false, + wslFeatureEnabled: true, + }, + }, + { + "WSL1 configured and Virtual Machine Platform is enabled but Windows Subsystem for Linux is disabled", + WSL1InstalledWithWSLDisabled, + wslStatus{ + installed: true, + vmpFeatureEnabled: true, + wslFeatureEnabled: false, + }, + }, + } + for _, tt := range tests { + t.Run(tt.winVariant, func(t *testing.T) { + encoder := unicode.UTF16(unicode.LittleEndian, unicode.UseBOM).NewEncoder() + encodedOutput, err := encoder.String(tt.statusOutput) + assert.Nil(t, err) + reader := io.NopCloser(strings.NewReader(encodedOutput)) + assert.Equal(t, tt.want, matchOutputLine(reader)) + }) + } +} From af29bb5b6ed0c779d4f2a89186578d3666849458 Mon Sep 17 00:00:00 2001 From: Mario Loriedo Date: Mon, 10 Mar 2025 18:11:12 +0100 Subject: [PATCH 2/2] Update CI to run Windows unit tests Add a new target in winmake.ps1 to run unit tests and use use it in a new cirrus task. Fix machine_windows_test.go to make it work in CI machine. Add the `!windows` tag on tests files that fail on Windows. Signed-off-by: Mario Loriedo --- .cirrus.yml | 43 ++++++++++++++++++++++++-- cmd/podman/parse/net_test.go | 2 ++ contrib/cirrus/win-unit-main.ps1 | 13 ++++++++ pkg/auth/auth_test.go | 2 ++ pkg/machine/machine_windows_test.go | 47 +++++++++++++++++++++++++---- pkg/util/utils_test.go | 4 +++ winmake.ps1 | 20 +++++++++--- 7 files changed, 118 insertions(+), 13 deletions(-) create mode 100644 contrib/cirrus/win-unit-main.ps1 diff --git a/.cirrus.yml b/.cirrus.yml index 00046172e3..d62e8d3e67 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -558,6 +558,44 @@ unit_test_task: always: *logs_artifacts +unit_test_windows_task: + name: "Unit tests on Windows" + alias: unit_test_windows + # Docs: ./contrib/cirrus/CIModes.md (Cirrus Task contexts and runtime modes) + # only when: - main rules (see doc above); or + # - unit test files are changed (contains a false positves such as test/e2e/ + # but that should not be an issue, it only runs when it doesn't have to) + # - actual source code changed + only_if: >- + $CIRRUS_PR == '' || + $CIRRUS_CHANGE_TITLE =~ '.*CI:ALL.*' || + changesInclude('.cirrus.yml', 'Makefile', 'contrib/cirrus/**', 'vendor/**', 'test/tools/**', 'test/registries*.conf', 'hack/**', 'version/rawversion/*') || + changesInclude('winmake.ps1') || + changesInclude('**/*_test.go') || + (changesInclude('**/*.go', '**/*.c', '**/*.h') && !changesIncludeOnly('test/**', 'pkg/machine/e2e/**')) + # Special case, we do not run macos/windows builds on rhel branches. + # Thus the machine task should not be run too, while we use only_if + # everywhere to do so here it would mean we would need duplicate the + # full big only_if condition which is more difficult to maintain so + # use the skip here. + skip: &skip_rhel_release | + $CIRRUS_BRANCH =~ 'v[0-9\.]+-rhel' || + $CIRRUS_BASE_BRANCH =~ 'v[0-9\.]+-rhel' + depends_on: *build + ec2_instance: *windows + timeout_in: 20m + env: + <<: *winenv + TEST_FLAVOR: unit + clone_script: *winclone + main_script: ".\\repo\\contrib\\cirrus\\win-unit-main.ps1" + always: + # Required for `contrib/cirrus/logformatter` to work properly + html_artifacts: + path: ./*.html + type: text/html + + apiv2_test_task: name: "APIv2 test on $DISTRO_NV ($PRIV_NAME)" alias: apiv2_test @@ -760,9 +798,7 @@ podman_machine_windows_task: # everywhere to do so here it would mean we would need duplicate the # full big only_if condition which is more difficult to maintain so # use the skip here. - skip: &skip_rhel_release | - $CIRRUS_BRANCH =~ 'v[0-9\.]+-rhel' || - $CIRRUS_BASE_BRANCH =~ 'v[0-9\.]+-rhel' + skip: *skip_rhel_release depends_on: *build ec2_instance: <<: *windows @@ -1060,6 +1096,7 @@ success_task: - win_installer - docker-py_test - unit_test + - unit_test_windows - apiv2_test - compose_test - local_integration_test diff --git a/cmd/podman/parse/net_test.go b/cmd/podman/parse/net_test.go index 2242173268..f71dc6ba1f 100644 --- a/cmd/podman/parse/net_test.go +++ b/cmd/podman/parse/net_test.go @@ -1,3 +1,5 @@ +//go:build !windows + // most of these validate and parse functions have been taken from projectatomic/docker // and modified for cri-o package parse diff --git a/contrib/cirrus/win-unit-main.ps1 b/contrib/cirrus/win-unit-main.ps1 new file mode 100644 index 0000000000..b945f4a155 --- /dev/null +++ b/contrib/cirrus/win-unit-main.ps1 @@ -0,0 +1,13 @@ +#!/usr/bin/env powershell + +. $PSScriptRoot\win-lib.ps1 + +if ($Env:CI -eq "true") { + Push-Location "$ENV:CIRRUS_WORKING_DIR\repo" +} else { + Push-Location $PSScriptRoot\..\.. +} + +Run-Command ".\winmake.ps1 localunit" + +Pop-Location diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index 37cf817917..ccb205c18e 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package auth import ( diff --git a/pkg/machine/machine_windows_test.go b/pkg/machine/machine_windows_test.go index 48f3eafd53..5255a7502b 100644 --- a/pkg/machine/machine_windows_test.go +++ b/pkg/machine/machine_windows_test.go @@ -3,15 +3,41 @@ package machine import ( + "fmt" "os" "os/exec" "path/filepath" "testing" + "golang.org/x/sys/windows" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// shortPathToLongPath converts a Windows short path (C:\PROGRA~1) to its +// long path equivalent (C:\Program Files). It returns an error if shortPath +// doesn't exist. +func shortPathToLongPath(shortPath string) (string, error) { + shortPathPtr, err := windows.UTF16PtrFromString(shortPath) + if err != nil { + return "", err + } + len, err := windows.GetLongPathName(shortPathPtr, nil, 0) + if err != nil { + return "", err + } + if len == 0 { + return "", fmt.Errorf("failed to get buffer size for path: %s", shortPath) + } + longPathPtr := &(make([]uint16, len)[0]) + _, err = windows.GetLongPathName(shortPathPtr, longPathPtr, len) + if err != nil { + return "", err + } + return windows.UTF16PtrToString(longPathPtr), nil +} + // CreateNewItemWithPowerShell creates a new item using PowerShell. // It's an helper to easily create junctions on Windows (as well as other file types). // It constructs a PowerShell command to create a new item at the specified path with the given item type. @@ -23,15 +49,21 @@ import ( // - itemType: The type of the item to be created (e.g., "File", "SymbolicLink", "Junction"). // - target: The target for the new item, if applicable. func CreateNewItemWithPowerShell(t *testing.T, path string, itemType string, target string) { - var pwshCmd string + var pwshCmd, pwshPath string + // Look for Powershell 7 first as it allow Symlink creation for non-admins too + pwshPath, err := exec.LookPath("pwsh.exe") + if err != nil { + // Use Powershell 5 that is always present + pwshPath = "powershell.exe" + } pwshCmd = "New-Item -Path " + path + " -ItemType " + itemType if target != "" { pwshCmd += " -Target " + target } - cmd := exec.Command("pwsh", "-Command", pwshCmd) + cmd := exec.Command(pwshPath, "-Command", pwshCmd) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - err := cmd.Run() + err = cmd.Run() require.NoError(t, err) } @@ -45,13 +77,16 @@ func CreateNewItemWithPowerShell(t *testing.T, path string, itemType string, tar // with filepath.EvalSymlink(). func TestEvalSymlinksOrClean(t *testing.T) { // Create a temporary directory to store the normal file - normalFileDir := t.TempDir() + normalFileDir, err := shortPathToLongPath(t.TempDir()) + require.NoError(t, err) // Create a temporary directory to store the (hard/sym)link files - linkFilesDir := t.TempDir() + linkFilesDir, err := shortPathToLongPath(t.TempDir()) + require.NoError(t, err) // Create a temporary directory where the mount point will be created - mountPointDir := t.TempDir() + mountPointDir, err := shortPathToLongPath(t.TempDir()) + require.NoError(t, err) // Create a normal file normalFile := filepath.Join(normalFileDir, "testFile") diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go index 37795d3111..b582b51ecc 100644 --- a/pkg/util/utils_test.go +++ b/pkg/util/utils_test.go @@ -3,6 +3,7 @@ package util import ( "fmt" "math" + "runtime" "sort" "testing" "time" @@ -827,6 +828,9 @@ func TestProcessOptions(t *testing.T) { } func TestGetRootlessPauseProcessPidPath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Not implemented on Windows") + } dir, err := GetRootlessPauseProcessPidPath() assert.NoError(t, err) assert.NotEqual(t, dir, "libpod/tmp/pause.pid") diff --git a/winmake.ps1 b/winmake.ps1 index 2491b2cb1f..3b37c8a2d7 100644 --- a/winmake.ps1 +++ b/winmake.ps1 @@ -44,6 +44,14 @@ function Make-Clean{ } } +function Local-Unit { + Build-Ginkgo + $skippackages="hack,internal\domain\infra\abi,internal\domain\infra\tunnel,libpod\lock\shm,pkg\api\handlers\libpod,pkg\api\handlers\utils,pkg\bindings," + $skippackages+="pkg\domain\infra\abi,pkg\emulation,pkg\machine\apple,pkg\machine\applehv,pkg\machine\e2e,pkg\machine\libkrun," + $skippackages+="pkg\machine\provider,pkg\machine\proxyenv,pkg\machine\qemu,pkg\specgen\generate,pkg\systemd,test\e2e,test\utils" + Run-Command "./test/tools/build/ginkgo.exe -vv -r --tags `"$remotetags`" --timeout=15m --trace --no-color --skip-package `"$skippackages`"" +} + function Local-Machine { param ( [string]$files @@ -53,7 +61,7 @@ function Local-Machine { $files = " --focus-file $files " } - Run-Command "./test/tools/build/ginkgo.exe -vv --tags `"$remotetags`" -timeout=90m --trace --no-color $files pkg/machine/e2e/." + Run-Command "./test/tools/build/ginkgo.exe -vv --tags `"$remotetags`" --timeout=90m --trace --no-color $files pkg/machine/e2e/." } # Expect starting directory to be /podman @@ -219,9 +227,7 @@ function Build-Ginkgo{ return } Write-Host "Building Ginkgo" - Push-Location ./test/tools - Run-Command "go build -o build/ginkgo.exe ./vendor/github.com/onsi/ginkgo/v2/ginkgo" - Pop-Location + Run-Command "go build -o ./test/tools/build/ginkgo.exe ./vendor/github.com/onsi/ginkgo/v2/ginkgo" } function Git-Commit{ @@ -287,6 +293,9 @@ switch ($target) { {$_ -in '', 'podman-remote', 'podman'} { Podman-Remote } + 'localunit' { + Local-Unit + } 'localmachine' { if ($args.Count -gt 1) { $files = $args[1] @@ -331,6 +340,9 @@ switch ($target) { Write-Host "Example: Build podman-remote " Write-Host " .\winmake podman-remote" Write-Host + Write-Host "Example: Run all unit tests " + Write-Host " .\winmake localunit" + Write-Host Write-Host "Example: Run all machine tests " Write-Host " .\winmake localmachine" Write-Host