Skip to content

test: Fix guest_env_test.go #21801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 27, 2025
Merged

test: Fix guest_env_test.go #21801

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bf746b
test: Fix guest_env_test.go
nirs Oct 25, 2025
5bd82cb
test: Fix iso_test on arm64
nirs Oct 26, 2025
3dc60e2
test: Speed up iso test with --no-kubernetes
nirs Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions test/integration/guest_env_test.go → test/integration/iso_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//go:build iso
//go:build integration

/*
Copyright 2016 The Kubernetes Authors All rights reserved.
Expand All @@ -22,35 +22,52 @@ import (
"context"
"fmt"
"os/exec"
"strings"
"runtime"
"testing"

"k8s.io/minikube/pkg/minikube/vmpath"
)

// TestGuestEnvironment verifies files and packages installed inside minikube ISO/Base image
func TestGuestEnvironment(t *testing.T) {
// TestISOImage verifies files and packages installed inside minikube ISO/Base image
func TestISOImage(t *testing.T) {
Copy link
Member

@medyagh medyagh Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for finding out this test never ran !!!

How about we make this a SubTest of No-Kubernetes Test ? so we spin up one No-Kuberentes VM and dont spin up any more new VMs

Copy link
Contributor Author

@nirs nirs Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using --no-kubernetes makes sense. I'll check this out.

Copy link
Contributor Author

@nirs nirs Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should make this a sub test of TestNoKubernetes.

The test is complicated enough as is:

	// Serial tests
	t.Run("serial", func(t *testing.T) {
		tests := []struct {
			name      string
			validator validateFunc
		}{
			{"StartNoK8sWithVersion", validateStartNoK8sWithVersion},
			{"StartWithK8s", validateStartWithK8S},
			{"StartWithStopK8s", validateStartWithStopK8s},
			{"Start", validateStartNoK8S},
			{"VerifyK8sNotRunning", validateK8SNotRunning},
			{"ProfileList", validateProfileListNoK8S},
			{"Stop", validateStopNoK8S},
			{"StartNoArgs", validateStartNoArgs},
			{"VerifyK8sNotRunningSecond", validateK8SNotRunning},
		}

		for _, tc := range tests {
			tc := tc

			if ctx.Err() == context.DeadlineExceeded {
				t.Fatalf("Unable to run more tests (deadline exceeded)")
			}

			t.Run(tc.name, func(t *testing.T) {
				tc.validator(ctx, t, profile)
				if t.Failed() && *postMortemLogs {
					PostMortemLogs(t, profile)
				}
			})
		}
	})

Adding the test as sub test will make it impossible to run the test separately like we can do with the current PR. It may not run the test if a previous unrelated test failed, and it will make it harder to maintain the tests.

What we can as should do is to start the cluster with --no-kubernetes in the iso test since we test the iso contents, and kubernetes is not required of this. This will make the test much quicker locally.

Copy link
Contributor Author

@nirs nirs Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using --no-kubernetes now.

if !VMDriver() {
t.Skip("This test requires a VM driver")
}

MaybeParallel(t)

profile := UniqueProfileName("guest")
ctx, cancel := context.WithTimeout(context.Background(), Minutes(15))
defer CleanupWithLogs(t, profile, cancel)

t.Run("Setup", func(t *testing.T) {
args := append([]string{"start", "-p", profile, "--install-addons=false", "--memory=3072", "--wait=false", "--disable-metrics=true"}, StartArgs()...)
args := append([]string{"start", "-p", profile, "--no-kubernetes"}, StartArgs()...)
rr, err := Run(t, exec.CommandContext(ctx, Target(), args...))
if err != nil {
t.Errorf("failed to start minikube: args %q: %v", rr.Command(), err)
}

if strings.Contains(rr.Stderr.String(), "kubelet.housekeeping-interval=5m") {
t.Error("--disable-metrics=true is not working, housekeeping interval not increased")
}
})

// Run as a group so that our defer doesn't happen as tests are runnings
t.Run("Binaries", func(t *testing.T) {
for _, pkg := range []string{"git", "rsync", "curl", "wget", "socat", "iptables", "VBoxControl", "VBoxService", "crictl", "podman", "docker"} {
binaries := []string{
Copy link
Member

@medyagh medyagh Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional but we can check for other things too, we put a File that has the minikube version in there

example:

$ cat /version.json
{"iso_version": "v1.37.0-1758198818-20370", "kicbase_version": "v0.0.48", "minikube_version": "v1.37.0", "commit": "a4f96d0469d67330691be52a99ff1f91e31ba77f"}

Copy link
Contributor Author

@nirs nirs Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! open #21815 as good first issue.

"crictl",
"curl",
"docker",
"git",
"iptables",
"podman",
"rsync",
"socat",
"wget",
}

// virtualbox is not available in the arm64 iso.
if runtime.GOARCH == "amd64" {
binaries = append(binaries, "VBoxControl", "VBoxService")
}

for _, pkg := range binaries {
pkg := pkg
t.Run(pkg, func(t *testing.T) {
t.Parallel()
Expand Down
Loading