diff --git a/pkg/controllers/namespace/namespace_sync_controller_test.go b/pkg/controllers/namespace/namespace_sync_controller_test.go index 50f86fe5a..7bc0624f2 100644 --- a/pkg/controllers/namespace/namespace_sync_controller_test.go +++ b/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 - namespace *corev1.Namespace - clusters []clusterv1alpha1.Cluster - expectedResult controllerruntime.Result - expectedError bool + name string + namespace *corev1.Namespace + clusters []clusterv1alpha1.Cluster + expectedResult controllerruntime.Result + 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{}, - expectedError: false, + expectedResult: controllerruntime.Result{}, + 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{}, - expectedError: false, + expectedResult: controllerruntime.Result{}, + 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() - 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) - - 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"}, - }, + tests := []struct { + name string + withOverride bool + }{ + { + name: "Build works without override", + withOverride: false, + }, + { + name: "Build works with override", + withOverride: true, }, } - 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(). - WithScheme(scheme). - WithObjects(namespace1, cop). - Build() - - 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 + c := &Controller{ + Client: fakeClient, + EventRecorder: fakeRecorder, + OverrideManager: overridemanager.New(fakeClient, fakeRecorder), } - selectedNamespaces := sets.NewString() - containsAllNamespace := false - for _, rs := range cop.Spec.ResourceSelectors { - if rs.APIVersion != "v1" || rs.Kind != "Namespace" { - continue - } + err := c.buildWorks(context.Background(), namespace, clusters) + assert.NoError(t, err) - if rs.Name == "" { - containsAllNamespace = true - break - } + 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) - 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() + }) + } }