From f4e30a897e647d349f469ee264825c35234874d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A2=D0=BE=D0=B2=D0=B0=D1=80=D0=B8=D1=89=20=D0=BF=D1=80?= =?UTF-8?q?=D0=BE=D0=B3=D1=80=D0=B0=D0=BC=D0=BC=D0=B8=D1=81=D1=82?= <2962928213@qq.com> Date: Wed, 17 Apr 2024 08:51:03 +0800 Subject: [PATCH] fix: fix incomplete cleanup in tunnel.json and zombie process in functional test --- pkg/minikube/tunnel/registry.go | 10 ++++++++-- test/integration/functional_test.go | 4 ++-- test/integration/functional_test_tunnel_test.go | 5 +++++ test/integration/main_test.go | 4 +++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/pkg/minikube/tunnel/registry.go b/pkg/minikube/tunnel/registry.go index 2bf3ec03be64..876d47a9236b 100644 --- a/pkg/minikube/tunnel/registry.go +++ b/pkg/minikube/tunnel/registry.go @@ -61,7 +61,10 @@ func (r *persistentRegistry) IsAlreadyDefinedAndRunning(tunnel *ID) (*ID, error) } for _, t := range tunnels { - if t.Route.Equal(tunnel.Route) { + // it is possible that different tunnel can be started for different tunnel + // so we also need to check whether the machine name(profile name) + // when we want to identify duplicated tunnels + if tunnel.MachineName == t.MachineName && t.Route.Equal(tunnel.Route) { isRunning, err := checkIfRunning(t.Pid) if err != nil { return nil, fmt.Errorf("error checking whether conflicting tunnel (%v) is running: %s", t, err) @@ -87,7 +90,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)) }