From 981083db3366b4274f8d0ed3509989e13ddd84df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=94=A6=E5=8D=97=E8=B7=AF=E4=B9=8B=E8=8A=B1?= Date: Wed, 3 Apr 2024 21:44:13 +0200 Subject: [PATCH 1/3] test: allow running functional tests for different k8s versions --- site/content/en/docs/contrib/tests.en.md | 3 +++ test/integration/functional_test.go | 28 +++++++++++++++++++----- test/integration/main_test.go | 9 ++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/site/content/en/docs/contrib/tests.en.md b/site/content/en/docs/contrib/tests.en.md index cb7fb58a4def..3c017cc416f5 100644 --- a/site/content/en/docs/contrib/tests.en.md +++ b/site/content/en/docs/contrib/tests.en.md @@ -111,6 +111,9 @@ asserts that there are no unexpected errors displayed in minikube command output ## TestFunctional are functionality tests which can safely share a profile in parallel +## TestFunctionalNewestKubernetes +are functionality run functional tests using NewestKubernetesVersion + #### validateNodeLabels checks if minikube cluster is created with correct kubernetes's node label diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index fd50dadc4d07..f01d3c058752 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -42,6 +42,7 @@ import ( "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/config" + "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/detect" "k8s.io/minikube/pkg/minikube/localpath" "k8s.io/minikube/pkg/minikube/reason" @@ -72,9 +73,25 @@ var runCorpProxy = detect.GithubActionRunner() && runtime.GOOS == "linux" && !ar // TestFunctional are functionality tests which can safely share a profile in parallel func TestFunctional(t *testing.T) { + testFunctional(t, "") +} + +// TestFunctionalNewestKubernetes are functionality run functional tests using NewestKubernetesVersion +func TestFunctionalNewestKubernetes(t *testing.T) { + if strings.Contains(*startArgs, "--kubernetes-version") { + t.Skip() + } + k8sVersionString := constants.NewestKubernetesVersion + t.Run("Version"+k8sVersionString, func(t *testing.T) { + testFunctional(t, k8sVersionString) + }) +} + +func testFunctional(t *testing.T, k8sVersion string) { profile := UniqueProfileName("functional") - ctx, cancel := context.WithTimeout(context.Background(), Minutes(40)) + ctx := context.WithValue(context.Background(), "k8sVersion", k8sVersion) + ctx, cancel := context.WithTimeout(ctx, Minutes(40)) defer func() { if !*cleanup { return @@ -86,7 +103,6 @@ func TestFunctional(t *testing.T) { Cleanup(t, profile, cancel) }() - // Serial tests t.Run("serial", func(t *testing.T) { tests := []struct { @@ -965,7 +981,7 @@ func validateDryRun(ctx context.Context, t *testing.T, profile string) { // docs: Run `minikube start --dry-run --memory 250MB` // Too little memory! - startArgs := append([]string{"start", "-p", profile, "--dry-run", "--memory", "250MB", "--alsologtostderr"}, StartArgs()...) + startArgs := append([]string{"start", "-p", profile, "--dry-run", "--memory", "250MB", "--alsologtostderr"}, StartArgsWithContext(ctx)...) c := exec.CommandContext(mctx, Target(), startArgs...) rr, err := Run(t, c) @@ -982,7 +998,7 @@ func validateDryRun(ctx context.Context, t *testing.T, profile string) { dctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() // docs: Run `minikube start --dry-run` - startArgs = append([]string{"start", "-p", profile, "--dry-run", "--alsologtostderr", "-v=1"}, StartArgs()...) + startArgs = append([]string{"start", "-p", profile, "--dry-run", "--alsologtostderr", "-v=1"}, StartArgsWithContext(ctx)...) c = exec.CommandContext(dctx, Target(), startArgs...) rr, err = Run(t, c) // docs: Make sure the command doesn't raise any error @@ -1007,7 +1023,7 @@ func validateInternationalLanguage(ctx context.Context, t *testing.T, profile st defer cancel() // Too little memory! - startArgs := append([]string{"start", "-p", profile, "--dry-run", "--memory", "250MB", "--alsologtostderr"}, StartArgs()...) + startArgs := append([]string{"start", "-p", profile, "--dry-run", "--memory", "250MB", "--alsologtostderr"}, StartArgsWithContext(ctx)...) c := exec.CommandContext(mctx, Target(), startArgs...) // docs: Set environment variable `LC_ALL=fr` to enable minikube translation to French c.Env = append(os.Environ(), "LC_ALL=fr") @@ -2221,7 +2237,7 @@ func startMinikubeWithProxy(ctx context.Context, t *testing.T, profile string, p memoryFlag = "--memory=6000" } // passing --api-server-port so later verify it didn't change in soft start. - startArgs := append([]string{"start", "-p", profile, memoryFlag, fmt.Sprintf("--apiserver-port=%d", apiPortTest), "--wait=all"}, StartArgs()...) + startArgs := append([]string{"start", "-p", profile, memoryFlag, fmt.Sprintf("--apiserver-port=%d", apiPortTest), "--wait=all"}, StartArgsWithContext(ctx)...) c := exec.CommandContext(ctx, Target(), startArgs...) env := os.Environ() env = append(env, fmt.Sprintf("%s=%s", proxyEnv, addr)) diff --git a/test/integration/main_test.go b/test/integration/main_test.go index 051c302b2951..7024983d80e3 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -119,6 +119,15 @@ func StartArgs() []string { return strings.Split(*startArgs, " ") } +func StartArgsWithContext(ctx context.Context) []string { + res := strings.Split(*startArgs, " ") + value := ctx.Value("k8sVersion") + if value != nil && value != "" { + res = append(res, fmt.Sprintf("--kubernetes-version=%s", value)) + } + return res +} + // Target returns where the minikube binary can be found func Target() string { return *binaryPath From b1396398f85b77bf6809147cc1ad67986c29ddb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=94=A6=E5=8D=97=E8=B7=AF=E4=B9=8B=E8=8A=B1?= Date: Thu, 4 Jul 2024 00:31:01 +0200 Subject: [PATCH 2/3] fix: fix incomplete cleanup in tunnel.json and zombie process in functional test --- pkg/minikube/tunnel/registry.go | 5 ++++- test/integration/functional_test.go | 4 ++-- test/integration/functional_test_tunnel_test.go | 5 +++++ test/integration/main_test.go | 4 +++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/minikube/tunnel/registry.go b/pkg/minikube/tunnel/registry.go index 2bf3ec03be64..dea5890343ad 100644 --- a/pkg/minikube/tunnel/registry.go +++ b/pkg/minikube/tunnel/registry.go @@ -87,7 +87,10 @@ func (r *persistentRegistry) Register(tunnel *ID) (rerr error) { alreadyExists := false for i, t := range tunnels { - if t.Route.Equal(tunnel.Route) { + // it is allowed for multiple minikube clusters to have multiple tunnels simultaneously + // it is possible that an old tunnel from an old profile has duplicated route information + // so we need to check both machine name and route information + if tunnel.MachineName == t.MachineName && t.Route.Equal(tunnel.Route) { isRunning, err := checkIfRunning(t.Pid) if err != nil { return fmt.Errorf("error checking whether conflicting tunnel (%v) is running: %s", t, err) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index f01d3c058752..5b47e6803230 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -78,7 +78,7 @@ func TestFunctional(t *testing.T) { // TestFunctionalNewestKubernetes are functionality run functional tests using NewestKubernetesVersion func TestFunctionalNewestKubernetes(t *testing.T) { - if strings.Contains(*startArgs, "--kubernetes-version") { + if strings.Contains(*startArgs, "--kubernetes-version") || constants.NewestKubernetesVersion == constants.DefaultKubernetesVersion { t.Skip() } k8sVersionString := constants.NewestKubernetesVersion @@ -90,7 +90,7 @@ func TestFunctionalNewestKubernetes(t *testing.T) { func testFunctional(t *testing.T, k8sVersion string) { profile := UniqueProfileName("functional") - ctx := context.WithValue(context.Background(), "k8sVersion", k8sVersion) + ctx := context.WithValue(context.Background(), ContextKey("k8sVersion"), k8sVersion) ctx, cancel := context.WithTimeout(ctx, Minutes(40)) defer func() { if !*cleanup { diff --git a/test/integration/functional_test_tunnel_test.go b/test/integration/functional_test_tunnel_test.go index b7f85d251a68..b41753d216ac 100644 --- a/test/integration/functional_test_tunnel_test.go +++ b/test/integration/functional_test_tunnel_test.go @@ -432,4 +432,9 @@ func validateTunnelDelete(_ context.Context, t *testing.T, _ string) { checkRoutePassword(t) // Stop tunnel tunnelSession.Stop(t) + // prevent the child process from becoming a defunct zombie process + if err := tunnelSession.cmd.Wait(); err != nil { + t.Logf("failed to stop process: %v", err) + return + } } diff --git a/test/integration/main_test.go b/test/integration/main_test.go index 7024983d80e3..4b850a111770 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -119,9 +119,11 @@ func StartArgs() []string { return strings.Split(*startArgs, " ") } +type ContextKey string + func StartArgsWithContext(ctx context.Context) []string { res := strings.Split(*startArgs, " ") - value := ctx.Value("k8sVersion") + value := ctx.Value(ContextKey("k8sVersion")) if value != nil && value != "" { res = append(res, fmt.Sprintf("--kubernetes-version=%s", value)) } From 7b31a438c27698dfa0c9057939b9e5874246495d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=8C=A6=E5=8D=97=E8=B7=AF=E4=B9=8B=E8=8A=B1?= <46831212+ComradeProgrammer@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:02:21 +0100 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Steven Powell <44844360+spowelljr@users.noreply.github.com> --- pkg/minikube/tunnel/registry.go | 7 ++++--- site/content/en/docs/contrib/tests.en.md | 3 ++- test/integration/functional_test.go | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/minikube/tunnel/registry.go b/pkg/minikube/tunnel/registry.go index dea5890343ad..9894884f7744 100644 --- a/pkg/minikube/tunnel/registry.go +++ b/pkg/minikube/tunnel/registry.go @@ -87,9 +87,10 @@ func (r *persistentRegistry) Register(tunnel *ID) (rerr error) { alreadyExists := false for i, t := range tunnels { - // it is allowed for multiple minikube clusters to have multiple tunnels simultaneously - // it is possible that an old tunnel from an old profile has duplicated route information - // so we need to check both machine name and route information + // It is allowed for multiple minikube clusters to have multiple + // tunnels simultaneously. It is possible that an old tunnel + // from an old profile has duplicated route information so we + // need to check both machine name and route information. if tunnel.MachineName == t.MachineName && t.Route.Equal(tunnel.Route) { isRunning, err := checkIfRunning(t.Pid) if err != nil { diff --git a/site/content/en/docs/contrib/tests.en.md b/site/content/en/docs/contrib/tests.en.md index 3c017cc416f5..0d51d745b8da 100644 --- a/site/content/en/docs/contrib/tests.en.md +++ b/site/content/en/docs/contrib/tests.en.md @@ -112,7 +112,8 @@ asserts that there are no unexpected errors displayed in minikube command output are functionality tests which can safely share a profile in parallel ## TestFunctionalNewestKubernetes -are functionality run functional tests using NewestKubernetesVersion +are functionality run functional tests using +NewestKubernetesVersion #### validateNodeLabels checks if minikube cluster is created with correct kubernetes's node label diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 5b47e6803230..1793206ee5a3 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -76,7 +76,8 @@ func TestFunctional(t *testing.T) { testFunctional(t, "") } -// TestFunctionalNewestKubernetes are functionality run functional tests using NewestKubernetesVersion +// TestFunctionalNewestKubernetes are functionality run functional tests using +// NewestKubernetesVersion func TestFunctionalNewestKubernetes(t *testing.T) { if strings.Contains(*startArgs, "--kubernetes-version") || constants.NewestKubernetesVersion == constants.DefaultKubernetesVersion { t.Skip()