fix: Integration testing leaked processes on DualCluster

tests/integration/commontestutils/skrcontextimpl/dual_cluster.go

@@ -10,27 +10,30 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 
+	"fmt"
 	"github.com/kyma-project/lifecycle-manager/internal/event"
 	"github.com/kyma-project/lifecycle-manager/internal/remote"
 )
 
 var (
 	ErrEmptyRestConfig  = errors.New("rest.Config is nil")
-	errSkrEnvNotStarted = errors.New("SKR envtest environment not started")
+	ErrSkrEnvNotStarted = errors.New("SKR envtest environment not started")
 )
 
 type DualClusterFactory struct {
 	clients sync.Map
 	scheme  *machineryruntime.Scheme
 	event   event.Event
 	skrEnv  *envtest.Environment
+	skrEnvs sync.Map
 }
 
 func NewDualClusterFactory(scheme *machineryruntime.Scheme, event event.Event) *DualClusterFactory {
 	return &DualClusterFactory{
 		clients: sync.Map{},
 		scheme:  scheme,
 		event:   event,
+		skrEnvs: sync.Map{},
 	}
 }
 
@@ -40,46 +43,63 @@ func (f *DualClusterFactory) Init(_ context.Context, kyma types.NamespacedName)
 		return nil
 	}
 
-	f.skrEnv = &envtest.Environment{
+	skrEnv := &envtest.Environment{
 		ErrorIfCRDPathMissing: true,
 		// Scheme: scheme,
 	}
-	cfg, err := f.GetSkrEnv().Start()
+
+	// Start the envtest and record the returned cfg
+	cfg, err := skrEnv.Start()
 	if err != nil {
 		return err
 	}
 	if cfg == nil {
+		// cleanup fast - if start returned nil cfg
+		_ = skrEnv.Stop()
 		return ErrEmptyRestConfig
 	}
 
 	var authUser *envtest.AuthenticatedUser
-	authUser, err = f.GetSkrEnv().AddUser(envtest.User{
+	authUser, err = skrEnv.AddUser(envtest.User{
 		Name:   "skr-admin-account",
 		Groups: []string{"system:masters"},
 	}, cfg)
 	if err != nil {
+		_ = skrEnv.Stop()
 		return err
 	}
 
 	skrClient, err := client.New(authUser.Config(), client.Options{Scheme: f.scheme})
+	if err != nil {
+		_ = skrEnv.Stop()
+		return err
+	}
 	newClient := remote.NewClientWithConfig(skrClient, authUser.Config())
 	f.clients.Store(kyma.Name, newClient)
 
+	f.skrEnv = skrEnv
+	// track this envtest so Stop() can stop all started envs
+	f.skrEnvs.Store(kyma.Name, skrEnv)
+
 	return err
 }
 
 func (f *DualClusterFactory) Get(kyma types.NamespacedName) (*remote.SkrContext, error) {
 	value, ok := f.clients.Load(kyma.Name)
 	if !ok {
-		return nil, errSkrEnvNotStarted
+		return nil, ErrSkrEnvNotStarted
 	}
 	skrClient, ok := value.(*remote.ConfigAndClient)
 	if !ok {
-		return nil, errSkrEnvNotStarted
+		return nil, ErrSkrEnvNotStarted
 	}
 	return remote.NewSkrContext(skrClient, f.event), nil
 }
 
+func (f *DualClusterFactory) StoreEnv(name string, env interface{}) {
+	f.skrEnvs.Store(name, env)
+}
+
 func (f *DualClusterFactory) InvalidateCache(_ types.NamespacedName) {
 	// no-op
 }
@@ -89,8 +109,37 @@ func (f *DualClusterFactory) GetSkrEnv() *envtest.Environment {
 }
 
 func (f *DualClusterFactory) Stop() error {
-	if f.skrEnv == nil {
+	var errs []error
+
+	f.skrEnvs.Range(func(key, value interface{}) bool {
+		name := ""
+		if ks, ok := key.(string); ok {
+			name = ks
+		}
+
+		if env, ok := value.(*envtest.Environment); ok && env != nil {
+			if err := env.Stop(); err != nil {
+				if name != "" {
+					errs = append(errs, fmt.Errorf("stop envtest %q: %w", name, err))
+				} else {
+					errs = append(errs, fmt.Errorf("stop envtest (unknown key): %w", err))
+				}
+			}
+		}
+
+		// remove entries so we don't double-stop later
+		f.skrEnvs.Delete(key)
+		if name != "" {
+			f.clients.Delete(name)
+		}
+		return true
+	})
+
+	// Clear skrEnv
+	f.skrEnv = nil
+
+	if len(errs) == 0 {
 		return nil
 	}
-	return f.skrEnv.Stop()
+	return fmt.Errorf("errors stopping envtests: %w", errors.Join(errs...))
 }

tests/integration/commontestutils/skrcontextimpl/dual_cluster_test.go

@@ -0,0 +1,117 @@
+package skrcontextimpl_test
+
+import (
+	"context"
+	"errors"
+	"runtime"
+	"testing"
+
+	machineryruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+
+	testskrcontext "github.com/kyma-project/lifecycle-manager/tests/integration/commontestutils/skrcontextimpl"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func newFactory() *testskrcontext.DualClusterFactory {
+	scheme := machineryruntime.NewScheme()
+	return testskrcontext.NewDualClusterFactory(scheme, nil)
+}
+
+func Test_GetBeforeInit(t *testing.T) {
+	dualFactory := newFactory()
+	_, err := dualFactory.Get(types.NamespacedName{Name: "kymaUninitialized"})
+	require.Error(t, err)
+	assert.ErrorIs(t, err, testskrcontext.ErrSkrEnvNotStarted)
+}
+
+func Test_InitAndGet(t *testing.T) {
+	dualFactory := newFactory()
+	kyma := types.NamespacedName{Name: "kymaInit"}
+	require.NoError(t, dualFactory.Init(context.Background(), kyma))
+	skrCtx, err := dualFactory.Get(kyma)
+	require.NoError(t, err)
+	require.NotNil(t, skrCtx)
+	require.NotNil(t, dualFactory.GetSkrEnv())
+}
+
+func Test_InitTwiceSameKyma(t *testing.T) {
+	dualFactory := newFactory()
+	kyma := types.NamespacedName{Name: "kymaSame"}
+	require.NoError(t, dualFactory.Init(context.Background(), kyma))
+	envFirst := dualFactory.GetSkrEnv()
+	require.NotNil(t, envFirst)
+	require.NoError(t, dualFactory.Init(context.Background(), kyma))
+	envSecond := dualFactory.GetSkrEnv()
+	assert.Equal(t, envFirst, envSecond)
+}
+
+func Test_MultipleKymasAndStop(t *testing.T) {
+	dualFactory := newFactory()
+	kymaPrimary := types.NamespacedName{Name: "kymaPrimary"}
+	kymaSecondary := types.NamespacedName{Name: "kymaSecondary"}
+
+	for _, k := range []types.NamespacedName{kymaPrimary, kymaSecondary} {
+		require.NoError(t, dualFactory.Init(context.Background(), k))
+		_, err := dualFactory.Get(k)
+		require.NoError(t, err)
+	}
+
+	require.NoError(t, dualFactory.Stop())
+	assert.Nil(t, dualFactory.GetSkrEnv())
+
+	_, err := dualFactory.Get(kymaPrimary)
+	assert.Error(t, err)
+}
+
+func Test_StopIdempotent(t *testing.T) {
+	dualFactory := newFactory()
+	kyma := types.NamespacedName{Name: "kymaLifecycle"}
+	require.NoError(t, dualFactory.Init(context.Background(), kyma))
+	require.NoError(t, dualFactory.Stop())
+	require.NoError(t, dualFactory.Stop())
+}
+
+func Test_NoLeakedProcesses(t *testing.T) {
+	dualFactory := newFactory()
+	kyma := types.NamespacedName{Name: "kymaGoroutineCheck"}
+
+	before := runtime.NumGoroutine()
+	require.NoError(t, dualFactory.Init(context.Background(), kyma))
+	require.NoError(t, dualFactory.Stop())
+
+	runtime.GC()
+	after := runtime.NumGoroutine()
+	assert.LessOrEqual(t, after, before+2)
+}
+
+type fakeEnv struct {
+	name       string
+	stopCalled bool
+	stopErr    error
+}
+
+func (fenv *fakeEnv) Stop() error {
+	fenv.stopCalled = true
+	return fenv.stopErr
+}
+
+func Test_StopAggregatesErrors(t *testing.T) {
+	dualFactory := newFactory()
+
+	envPrimary := &fakeEnv{name: "primary-env", stopErr: errors.New("primary stop failure")}
+	envSecondary := &fakeEnv{name: "secondary-env", stopErr: errors.New("secondary stop failure")}
+	envTertiary := &fakeEnv{name: "tertiary-env"}
+
+	dualFactory.StoreEnv("primary-env", envPrimary)
+	dualFactory.StoreEnv("secondary-env", envSecondary)
+	dualFactory.StoreEnv("tertiary-env", envTertiary)
+
+	err := dualFactory.Stop()
+	require.Error(t, err)
+	msg := err.Error()
+	assert.Contains(t, msg, "primary stop failure")
+	assert.Contains(t, msg, "secondary stop failure")
+	assert.Nil(t, dualFactory.GetSkrEnv())
+}