Added tests for namespace controller

Signed-off-by: Anuj Agrawal <anujagrawal380@gmail.com>

pkg/controllers/namespace/namespace_sync_controller_test.go

@@ -21,22 +21,19 @@ import (
 	"regexp"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/klog/v2"
 	controllerruntime "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
@@ -94,11 +91,14 @@ func TestController_Reconcile(t *testing.T) {
 	_ = workv1alpha1.Install(scheme)
 
 	tests := []struct {
-		name           string
+		name              string
-		namespace      *corev1.Namespace
+		namespace         *corev1.Namespace
-		clusters       []clusterv1alpha1.Cluster
+		clusters          []clusterv1alpha1.Cluster
-		expectedResult controllerruntime.Result
+		expectedResult    controllerruntime.Result
-		expectedError  bool
+		expectedError     bool
+		expectWorkCreated bool
+		namespaceNotFound bool
+		namespaceDeleting bool
 	}{
 		{
 			name: "Namespace should be synced",
@@ -114,8 +114,9 @@ func TestController_Reconcile(t *testing.T) {
 					},
 				},
 			},
-			expectedResult: controllerruntime.Result{},
+			expectedResult:    controllerruntime.Result{},
-			expectedError:  false,
+			expectedError:     false,
+			expectWorkCreated: true,
 		},
 		{
 			name: "Namespace should not be synced",
@@ -124,8 +125,9 @@ func TestController_Reconcile(t *testing.T) {
 					Name: "kube-system",
 				},
 			},
-			expectedResult: controllerruntime.Result{},
+			expectedResult:    controllerruntime.Result{},
-			expectedError:  false,
+			expectedError:     false,
+			expectWorkCreated: false,
 		},
 		{
 			name: "Namespace with skip auto propagation label",
@@ -140,11 +142,51 @@ func TestController_Reconcile(t *testing.T) {
 			expectedResult: controllerruntime.Result{},
 			expectedError:  false,
 		},
+		{
+			name: "Namespace should not be synced - kube-public",
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "kube-public",
+				},
+			},
+			expectedResult:    controllerruntime.Result{},
+			expectedError:     false,
+			expectWorkCreated: false,
+		},
+		{
+			name:              "Namespace not found",
+			namespace:         &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "non-existent"}},
+			namespaceNotFound: true,
+			expectedResult:    controllerruntime.Result{},
+			expectedError:     false,
+			expectWorkCreated: false,
+		},
+		{
+			name: "Namespace is being deleted",
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "deleting-namespace",
+					DeletionTimestamp: &metav1.Time{Time: time.Now()},
+					Finalizers:        []string{"some-finalizer"},
+				},
+			},
+			namespaceDeleting: true,
+			expectedResult:    controllerruntime.Result{},
+			expectedError:     false,
+			expectWorkCreated: false,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.namespace).WithLists(&clusterv1alpha1.ClusterList{Items: tt.clusters}).Build()
+			fakeClientBuilder := fake.NewClientBuilder().WithScheme(scheme)
+
+			if !tt.namespaceNotFound {
+				fakeClientBuilder = fakeClientBuilder.WithObjects(tt.namespace)
+			}
+
+			fakeClientBuilder = fakeClientBuilder.WithLists(&clusterv1alpha1.ClusterList{Items: tt.clusters})
+			fakeClient := fakeClientBuilder.Build()
 			fakeRecorder := record.NewFakeRecorder(100)
 
 			c := &Controller{
@@ -163,7 +205,21 @@ func TestController_Reconcile(t *testing.T) {
 			assert.Equal(t, tt.expectedResult, result)
 			assert.Equal(t, tt.expectedError, err != nil)
 
-			if tt.name == "Namespace should be synced" {
+			if tt.namespaceNotFound {
+				ns := &corev1.Namespace{}
+				err := fakeClient.Get(context.Background(), types.NamespacedName{Name: tt.namespace.Name}, ns)
+				assert.True(t, errors.IsNotFound(err))
+			} else if tt.namespaceDeleting {
+				ns := &corev1.Namespace{}
+				err := fakeClient.Get(context.Background(), types.NamespacedName{Name: tt.namespace.Name}, ns)
+				assert.NoError(t, err)
+				assert.NotNil(t, ns.DeletionTimestamp)
+			}
+
+			if tt.expectWorkCreated {
+				if len(tt.clusters) == 0 {
+					t.Fatal("Test case expects work to be created but no clusters are defined")
+				}
 				work := &workv1alpha1.Work{}
 				err = fakeClient.Get(context.Background(), types.NamespacedName{
 					Namespace: names.GenerateExecutionSpaceName(tt.clusters[0].Name),
@@ -171,6 +227,14 @@ func TestController_Reconcile(t *testing.T) {
 				}, work)
 				assert.NoError(t, err)
 				assert.NotNil(t, work)
+			} else if len(tt.clusters) > 0 {
+				work := &workv1alpha1.Work{}
+				err = fakeClient.Get(context.Background(), types.NamespacedName{
+					Namespace: names.GenerateExecutionSpaceName(tt.clusters[0].Name),
+					Name:      names.GenerateWorkName(tt.namespace.Kind, tt.namespace.Name, tt.namespace.Namespace),
+				}, work)
+				assert.Error(t, err)
+				assert.True(t, errors.IsNotFound(err))
 			}
 		})
 	}
@@ -202,51 +266,6 @@ func TestController_buildWorks(t *testing.T) {
 		},
 	}
 
-	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(namespace, &clusters[0], &clusters[1]).Build()
-	fakeRecorder := record.NewFakeRecorder(100)
-
-	c := &Controller{
-		Client:          fakeClient,
-		EventRecorder:   fakeRecorder,
-		OverrideManager: overridemanager.New(fakeClient, fakeRecorder),
-	}
-
-	err := c.buildWorks(context.Background(), namespace, clusters)
-	assert.NoError(t, err)
-
-	for _, cluster := range clusters {
-		work := &workv1alpha1.Work{}
-		err = fakeClient.Get(context.Background(), types.NamespacedName{
-			Namespace: names.GenerateExecutionSpaceName(cluster.Name),
-			Name:      names.GenerateWorkName(namespace.Kind, namespace.Name, namespace.Namespace),
-		}, work)
-		assert.NoError(t, err)
-		assert.NotNil(t, work)
-		assert.Equal(t, namespace.Name, work.OwnerReferences[0].Name)
-	}
-}
-
-func TestController_buildWorksWithOverridePolicy(t *testing.T) {
-	scheme := runtime.NewScheme()
-	_ = corev1.AddToScheme(scheme)
-	_ = clusterv1alpha1.Install(scheme)
-	_ = workv1alpha1.Install(scheme)
-	_ = policyv1alpha1.Install(scheme)
-
-	namespace := &corev1.Namespace{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "test-namespace",
-		},
-	}
-
-	clusters := []clusterv1alpha1.Cluster{
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "cluster1",
-			},
-		},
-	}
-
 	overridePolicy := &policyv1alpha1.ClusterOverridePolicy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-override",
@@ -280,226 +299,124 @@ func TestController_buildWorksWithOverridePolicy(t *testing.T) {
 		},
 	}
 
-	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(namespace, &clusters[0], overridePolicy).Build()
+	tests := []struct {
-	fakeRecorder := record.NewFakeRecorder(100)
+		name         string
-
+		withOverride bool
-	c := &Controller{
+	}{
-		Client:          fakeClient,
+		{
-		EventRecorder:   fakeRecorder,
+			name:         "Build works without override",
-		OverrideManager: overridemanager.New(fakeClient, fakeRecorder),
+			withOverride: false,
-	}
+		},
-
+		{
-	err := c.buildWorks(context.Background(), namespace, clusters)
+			name:         "Build works with override",
-	assert.NoError(t, err)
+			withOverride: true,
-
-	work := &workv1alpha1.Work{}
-	err = fakeClient.Get(context.Background(), types.NamespacedName{
-		Namespace: names.GenerateExecutionSpaceName(clusters[0].Name),
-		Name:      names.GenerateWorkName(namespace.Kind, namespace.Name, namespace.Namespace),
-	}, work)
-	assert.NoError(t, err)
-	assert.NotNil(t, work)
-
-	t.Logf("Work found: %+v", work)
-	t.Logf("Work annotations: %v", work.Annotations)
-	t.Logf("Work spec: %+v", work.Spec)
-
-	assert.NotNil(t, work)
-}
-
-func TestController_SetupWithManager(t *testing.T) {
-	scheme := runtime.NewScheme()
-	err := corev1.AddToScheme(scheme)
-	assert.NoError(t, err)
-	err = clusterv1alpha1.Install(scheme)
-	assert.NoError(t, err)
-	err = workv1alpha1.Install(scheme)
-	assert.NoError(t, err)
-	err = policyv1alpha1.Install(scheme)
-	assert.NoError(t, err)
-
-	cfg := &rest.Config{Host: "http://localhost:8080"}
-	mgr, err := controllerruntime.NewManager(cfg, controllerruntime.Options{
-		Scheme: scheme,
-	})
-	assert.NoError(t, err)
-
-	c := &Controller{
-		Client:          mgr.GetClient(),
-		EventRecorder:   mgr.GetEventRecorderFor("test-controller"),
-		OverrideManager: overridemanager.New(mgr.GetClient(), mgr.GetEventRecorderFor("test-controller")),
-	}
-
-	err = c.SetupWithManager(mgr)
-	assert.NoError(t, err)
-
-	assert.NotNil(t, c.Client, "Controller's Client should not be nil")
-	assert.NotNil(t, c.EventRecorder, "Controller's EventRecorder should not be nil")
-	assert.NotNil(t, c.OverrideManager, "Controller's OverrideManager should not be nil")
-}
-
-func TestController_SetupWithManagerConcurrent(t *testing.T) {
-	scheme := runtime.NewScheme()
-	err := corev1.AddToScheme(scheme)
-	assert.NoError(t, err)
-	err = clusterv1alpha1.Install(scheme)
-	assert.NoError(t, err)
-	err = workv1alpha1.Install(scheme)
-	assert.NoError(t, err)
-	err = policyv1alpha1.Install(scheme)
-	assert.NoError(t, err)
-
-	cfg := &rest.Config{Host: "http://localhost:8080"}
-	mgr, err := controllerruntime.NewManager(cfg, controllerruntime.Options{
-		Scheme: scheme,
-	})
-	assert.NoError(t, err)
-
-	var wg sync.WaitGroup
-	for i := 0; i < 10; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			c := &Controller{
-				Client:          mgr.GetClient(),
-				EventRecorder:   mgr.GetEventRecorderFor("test-controller"),
-				OverrideManager: overridemanager.New(mgr.GetClient(), mgr.GetEventRecorderFor("test-controller")),
-			}
-			err := c.SetupWithManager(mgr)
-			assert.NoError(t, err)
-		}()
-	}
-	wg.Wait()
-}
-
-func TestController_SetupWithManagerError(t *testing.T) {
-	scheme := runtime.NewScheme()
-	// Schemes are not added to intentionaly trigger an error
-
-	cfg := &rest.Config{Host: "http://localhost:8080"}
-	mgr, err := controllerruntime.NewManager(cfg, controllerruntime.Options{
-		Scheme: scheme,
-	})
-	assert.NoError(t, err)
-
-	c := &Controller{
-		Client:          mgr.GetClient(),
-		EventRecorder:   mgr.GetEventRecorderFor("test-controller"),
-		OverrideManager: overridemanager.New(mgr.GetClient(), mgr.GetEventRecorderFor("test-controller")),
-	}
-
-	err = c.SetupWithManager(mgr)
-	assert.Error(t, err, "SetupWithManager should return an error when schemes are not properly set up")
-}
-
-func TestClusterNamespaceFn(t *testing.T) {
-	scheme := runtime.NewScheme()
-	_ = corev1.AddToScheme(scheme)
-	namespace1 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}}
-	namespace2 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "namespace2"}}
-
-	fakeClient := fake.NewClientBuilder().
-		WithScheme(scheme).
-		WithObjects(namespace1, namespace2).
-		Build()
-
-	c := &Controller{Client: fakeClient}
-
-	clusterNamespaceFn := handler.MapFunc(
-		func(ctx context.Context, _ client.Object) []reconcile.Request {
-			var requests []reconcile.Request
-			namespaceList := &corev1.NamespaceList{}
-			if err := c.Client.List(ctx, namespaceList); err != nil {
-				klog.Errorf("Failed to list namespace, error: %v", err)
-				return nil
-			}
-
-			for _, namespace := range namespaceList.Items {
-				requests = append(requests, reconcile.Request{NamespacedName: types.NamespacedName{
-					Name: namespace.Name,
-				}})
-			}
-			return requests
-		})
-
-	requests := clusterNamespaceFn(context.Background(), nil)
-	assert.Len(t, requests, 2)
-	assert.Contains(t, requests, reconcile.Request{NamespacedName: types.NamespacedName{Name: "namespace1"}})
-	assert.Contains(t, requests, reconcile.Request{NamespacedName: types.NamespacedName{Name: "namespace2"}})
-}
-
-func TestClusterOverridePolicyNamespaceFn(t *testing.T) {
-	scheme := runtime.NewScheme()
-	_ = corev1.AddToScheme(scheme)
-	_ = policyv1alpha1.Install(scheme)
-
-	cop := &policyv1alpha1.ClusterOverridePolicy{
-		ObjectMeta: metav1.ObjectMeta{Name: "test-policy"},
-		Spec: policyv1alpha1.OverrideSpec{
-			ResourceSelectors: []policyv1alpha1.ResourceSelector{
-				{APIVersion: "v1", Kind: "Namespace", Name: "test-namespace"},
-			},
 		},
 	}
 
-	namespace1 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeClientBuilder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(namespace, &clusters[0], &clusters[1])
+			if tt.withOverride {
+				fakeClientBuilder = fakeClientBuilder.WithObjects(overridePolicy)
+			}
+			fakeClient := fakeClientBuilder.Build()
+			fakeRecorder := record.NewFakeRecorder(100)
 
-	fakeClient := fake.NewClientBuilder().
+			c := &Controller{
-		WithScheme(scheme).
+				Client:          fakeClient,
-		WithObjects(namespace1, cop).
+				EventRecorder:   fakeRecorder,
-		Build()
+				OverrideManager: overridemanager.New(fakeClient, fakeRecorder),
-
-	c := &Controller{Client: fakeClient}
-
-	clusterOverridePolicyNamespaceFn := handler.MapFunc(
-		func(ctx context.Context, obj client.Object) []reconcile.Request {
-			var requests []reconcile.Request
-			cop, ok := obj.(*policyv1alpha1.ClusterOverridePolicy)
-			if !ok {
-				return requests
 			}
 
-			selectedNamespaces := sets.NewString()
+			err := c.buildWorks(context.Background(), namespace, clusters)
-			containsAllNamespace := false
+			assert.NoError(t, err)
-			for _, rs := range cop.Spec.ResourceSelectors {
-				if rs.APIVersion != "v1" || rs.Kind != "Namespace" {
-					continue
-				}
 
-				if rs.Name == "" {
+			for _, cluster := range clusters {
-					containsAllNamespace = true
+				work := &workv1alpha1.Work{}
-					break
+				err = fakeClient.Get(context.Background(), types.NamespacedName{
-				}
+					Namespace: names.GenerateExecutionSpaceName(cluster.Name),
+					Name:      names.GenerateWorkName(namespace.Kind, namespace.Name, namespace.Namespace),
+				}, work)
+				assert.NoError(t, err)
+				assert.NotNil(t, work)
+				assert.Equal(t, namespace.Name, work.OwnerReferences[0].Name)
 
-				selectedNamespaces.Insert(rs.Name)
+				assert.NotEmpty(t, work.Spec.Workload.Manifests)
 			}
-
-			if containsAllNamespace {
-				namespaceList := &corev1.NamespaceList{}
-				if err := c.Client.List(ctx, namespaceList); err != nil {
-					klog.Errorf("Failed to list namespace, error: %v", err)
-					return nil
-				}
-
-				for _, namespace := range namespaceList.Items {
-					requests = append(requests, reconcile.Request{NamespacedName: types.NamespacedName{
-						Name: namespace.Name,
-					}})
-				}
-
-				return requests
-			}
-
-			for _, ns := range selectedNamespaces.UnsortedList() {
-				requests = append(requests, reconcile.Request{NamespacedName: types.NamespacedName{
-					Name: ns,
-				}})
-			}
-
-			return requests
 		})
-
+	}
-	requests := clusterOverridePolicyNamespaceFn(context.Background(), cop)
+}
-	assert.Len(t, requests, 1)
+
-	assert.Contains(t, requests, reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-namespace"}})
+func TestController_SetupWithManager(t *testing.T) {
+	tests := []struct {
+		name           string
+		setupScheme    func(*runtime.Scheme)
+		concurrentRuns int
+		expectError    bool
+	}{
+		{
+			name: "Successful setup",
+			setupScheme: func(scheme *runtime.Scheme) {
+				_ = corev1.AddToScheme(scheme)
+				_ = clusterv1alpha1.Install(scheme)
+				_ = workv1alpha1.Install(scheme)
+				_ = policyv1alpha1.Install(scheme)
+			},
+			concurrentRuns: 1,
+			expectError:    false,
+		},
+		{
+			name: "Concurrent setup",
+			setupScheme: func(scheme *runtime.Scheme) {
+				_ = corev1.AddToScheme(scheme)
+				_ = clusterv1alpha1.Install(scheme)
+				_ = workv1alpha1.Install(scheme)
+				_ = policyv1alpha1.Install(scheme)
+			},
+			concurrentRuns: 10,
+			expectError:    false,
+		},
+		{
+			name:           "Setup with error",
+			setupScheme:    func(_ *runtime.Scheme) {}, // Intentionally empty to trigger error
+			concurrentRuns: 1,
+			expectError:    true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			scheme := runtime.NewScheme()
+			tt.setupScheme(scheme)
+
+			cfg := &rest.Config{Host: "http://localhost:8080"}
+			mgr, err := controllerruntime.NewManager(cfg, controllerruntime.Options{
+				Scheme: scheme,
+			})
+			assert.NoError(t, err)
+
+			var wg sync.WaitGroup
+			for i := 0; i < tt.concurrentRuns; i++ {
+				wg.Add(1)
+				go func() {
+					defer wg.Done()
+					c := &Controller{
+						Client:          mgr.GetClient(),
+						EventRecorder:   mgr.GetEventRecorderFor("test-controller"),
+						OverrideManager: overridemanager.New(mgr.GetClient(), mgr.GetEventRecorderFor("test-controller")),
+					}
+					err := c.SetupWithManager(mgr)
+					if tt.expectError {
+						assert.Error(t, err)
+					} else {
+						assert.NoError(t, err)
+						assert.NotNil(t, c.Client, "Controller's Client should not be nil")
+						assert.NotNil(t, c.EventRecorder, "Controller's EventRecorder should not be nil")
+						assert.NotNil(t, c.OverrideManager, "Controller's OverrideManager should not be nil")
+					}
+				}()
+			}
+			wg.Wait()
+		})
+	}
 }