Enable Integration tests in Windows CI #3280

arixmkii · 2025-02-25T21:58:52Z

My try to address Windows CI. This enables integration tests for Windows. My test run could be found here https://github.com/arixmkii/qcw/actions/runs/13531374510/job/37814107225

It is pretty unstable on my local machine, so, I have no idea if this will be flaky or not. With WSL2 tooling tests felt bullet proof, but Git bash and tooling is not as stable on my local machine.

The idea is that it will download locally the last known good portable Git for Windows and will use that tooling. Approach is not really suitable for production purposes, but pinning deps for tests only is on the edge of the acceptable.

Let's at least see if this breaks some other stuff.

.github/workflows/test.yml

AkihiroSuda · 2025-02-26T00:28:45Z

hack/test-templates.sh

@@ -1,6 +1,8 @@
 #!/usr/bin/env bash
 set -eu -o pipefail

+export MSYS2_ARG_CONV_EXCL='*'


Needs a comment line for explanation

AkihiroSuda · 2025-02-26T00:30:43Z

hack/test-templates.sh

@@ -234,32 +258,37 @@ nginx_image="ghcr.io/stargz-containers/nginx:1.19-alpine-org"
 alpine_image="ghcr.io/containerd/alpine:3.14.0"

 if [[ -n ${CHECKS["container-engine"]} ]]; then
+	sudo=""
+	if [[ ${NAME} == "wsl2" ]]; then


This should check the vmType, not the instance name

Do you have a snippet what is the recommended way to get vmType from inside the shell script.

Also, I'm thinking that this might be extracted to a variable "rootfulContainerEngine" and set it per template. I genuinely believe that it is possible to fix WSL2 template to support user level nerdctl instead. Or there is a possibility that such WSL2 template could appear.

WDYT?

vmType

limactl ls --json "${NAME}" | jq -r .vmType

rootfulContainerEngine

No need to introduce a new variable:

docker info --format json | jq -r '.SecurityOptions | if index("name=rootless") then "rootless" else "rootful" end'

For now I opted for vmType check. Check for rootless will be added when it is really needed (when WSL2 will be able to run nerdctl as non-root).

AkihiroSuda · 2025-02-26T00:31:14Z

hack/test-templates.sh

@@ -13,6 +15,9 @@ fi
 FILE="$1"
 NAME="$(basename -s .yaml "$FILE")"

+HOME_SRC=${HOME_SRC:-$HOME}
+HOME_DST=${HOME_DST:-$HOME}


Needs a comment line

I'm also thinking of renaming this into

HOME_HOST HOME_GUEST

And adding comment that on some platforms they might differ. Will this be fine?

Renamed and description added with a comment

AkihiroSuda · 2025-02-26T00:34:01Z

pkg/osutil/user.go

@@ -141,7 +141,7 @@ func LimaUser(limaVersion string, warn bool) *user.User {
 				warnings = append(warnings, warning)
 				limaUser.Gid = formatUidGid(gid)
 			}
-			home, err := call([]string{"cygpath", limaUser.HomeDir})
+			home, err := call([]string{"realpath", "~"})


Does this depend on MSYS2?

Eventually this should use pure Windows API that doesn't depend on MSYS2, Cygwin, or whatever else similar.

The deps should be documented in https://lima-vm.io/docs/config/vmtype/#known-issues-1

I will check this out and update documentation.

Does this depend on MSYS2?

realpath is part of the same core/minimal msys2 package as cygpath (and is bundled with Git). But I will rework it back to cygpath + .linux suffix. This is needed to prevent nuking home dir, when used with QEMU

AkihiroSuda · 2025-02-26T23:59:48Z

hack/test-templates.sh

 	mkdir -p "${tmpconfig}"
 	defer "rm -rf \"$tmpconfig\""
 	tmpfile="${tmpconfig}/${NAME}.yaml"
 	cp "$FILE" "${tmpfile}"
 	FILE="${tmpfile}"
 	INFO "Setup port forwarding rules for testing in \"${FILE}\""
 	"${scriptdir}/test-port-forwarding.pl" "${FILE}"
-	limactl validate "$FILE"
+	if [ "$(uname -o)" = "Msys" ]; then
+		limactl validate "$(cygpath -w "$FILE")"


Maybe you should set $FILE_HOST as in HOME_HOST

Your comment lead to another discovery. I didn't think of it before. It would be more correct to always pass OS dependent path format into limactl and use unix like for tooling. So, I added FILE_HOST, where the platform specific representation of FILE resides and used it through the whole shell file, where appropriate.

AkihiroSuda · 2025-02-27T00:00:34Z

hack/test-templates.sh

@@ -234,32 +261,37 @@ nginx_image="ghcr.io/stargz-containers/nginx:1.19-alpine-org"
 alpine_image="ghcr.io/containerd/alpine:3.14.0"

 if [[ -n ${CHECKS["container-engine"]} ]]; then
+	sudo=""
+	if [[ "$(limactl ls --json "${NAME}" | jq -r .vmType)" == "wsl2" ]]; then
+		sudo="sudo"


Needs a comment to explain the reason

AkihiroSuda · 2025-02-27T00:01:28Z

hack/test-templates.sh

@@ -284,6 +316,9 @@ if [[ -n ${CHECKS["port-forwards"]} ]]; then
 	if [ "${NAME}" = "opensuse" ]; then
 		limactl shell "$NAME" sudo zypper in -y netcat-openbsd
 	fi
+	if [ "${NAME}" == "wsl2" ]; then


Probably this should check limactl shell "$NAME" command -v dnf

Yes, this will be better. There is low probability to still face incompatibility with dnf distro, but with a different name of netcat package, but this could be fixed if it ever pops out.

Another minor problem that it will result in a duplicate run for "fedora" case. It would result in noop install, but just is not 100% clean

AkihiroSuda · 2025-02-27T00:02:27Z

hack/test-templates.sh

@@ -306,6 +341,9 @@ if [[ -n ${CHECKS["port-forwards"]} ]]; then
 				rm nerdctl-full.tgz
 				sudo="sudo"
 			fi
+			if [[ ${NAME} == "wsl2" ]]; then


Better to check vmType?

Agreed. Added.

AkihiroSuda · 2025-02-27T00:02:57Z

hack/test-templates.sh

@@ -360,7 +398,8 @@ if [[ -n ${CHECKS["restart"]} ]]; then
 	fi

 	INFO "Stopping \"$NAME\""
-	limactl stop "$NAME"
+	# TODO https://github.com/lima-vm/lima/issues/3221
+	limactl stop "$NAME" || [ "$(uname -o)" = "Msys" ]


The result of $(uname -o) can be cached in the top of the script

Implemented

Signed-off-by: Arthur Sengileyev <[email protected]>

AkihiroSuda

Thanks

arixmkii force-pushed the fix-windows-ci branch 2 times, most recently from 34afea9 to 2c17d3f Compare February 25, 2025 22:15

AkihiroSuda reviewed Feb 26, 2025

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

AkihiroSuda reviewed Feb 26, 2025

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

AkihiroSuda reviewed Feb 26, 2025

View reviewed changes

AkihiroSuda added the platform/Windows label Feb 26, 2025

AkihiroSuda added this to the v1.1 (tentative) milestone Feb 26, 2025

arixmkii force-pushed the fix-windows-ci branch from 2c17d3f to 30ce30d Compare February 26, 2025 21:42

arixmkii changed the title ~~Fix Windows CI~~ Enable Integration tests in Windows CI Feb 26, 2025

AkihiroSuda reviewed Feb 26, 2025

View reviewed changes

AkihiroSuda reviewed Feb 27, 2025

View reviewed changes

arixmkii force-pushed the fix-windows-ci branch from 30ce30d to a9f2a01 Compare February 28, 2025 19:57

Enable Integration tests in Windows CI

22d86cc

Signed-off-by: Arthur Sengileyev <[email protected]>

arixmkii force-pushed the fix-windows-ci branch from a9f2a01 to 22d86cc Compare February 28, 2025 20:07

AkihiroSuda approved these changes Mar 1, 2025

View reviewed changes

AkihiroSuda requested a review from a team March 1, 2025 17:11

AkihiroSuda merged commit bfac818 into lima-vm:master Mar 3, 2025
31 checks passed

AkihiroSuda modified the milestones: v1.1 (tentative), v1.1.0, or perhaps v1.0.7 Mar 4, 2025

arixmkii deleted the fix-windows-ci branch March 6, 2025 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Integration tests in Windows CI #3280

Enable Integration tests in Windows CI #3280

arixmkii commented Feb 25, 2025 •

edited

Loading

AkihiroSuda Feb 26, 2025 •

edited

Loading

arixmkii Feb 26, 2025

AkihiroSuda Feb 26, 2025

arixmkii Feb 26, 2025

AkihiroSuda Feb 26, 2025

arixmkii Feb 26, 2025

AkihiroSuda Feb 26, 2025

arixmkii Feb 26, 2025

AkihiroSuda Feb 26, 2025

arixmkii Feb 26, 2025

AkihiroSuda Feb 26, 2025

AkihiroSuda Feb 26, 2025

arixmkii Feb 26, 2025 •

edited

Loading

arixmkii Feb 26, 2025 •

edited

Loading

AkihiroSuda Feb 26, 2025

arixmkii Feb 28, 2025

AkihiroSuda Feb 27, 2025

arixmkii Feb 28, 2025

AkihiroSuda Feb 27, 2025

arixmkii Feb 28, 2025

arixmkii Feb 28, 2025

AkihiroSuda Feb 27, 2025

arixmkii Feb 28, 2025

AkihiroSuda Feb 27, 2025

arixmkii Feb 28, 2025

AkihiroSuda left a comment

Enable Integration tests in Windows CI #3280

Enable Integration tests in Windows CI #3280

Conversation

arixmkii commented Feb 25, 2025 • edited Loading

AkihiroSuda Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arixmkii Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

arixmkii Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda left a comment

Choose a reason for hiding this comment

arixmkii commented Feb 25, 2025 •

edited

Loading

AkihiroSuda Feb 26, 2025 •

edited

Loading

arixmkii Feb 26, 2025 •

edited

Loading

arixmkii Feb 26, 2025 •

edited

Loading