updated based on review comments

Signed-off-by: zach593 <zach_li@outlook.com>
karmada-io · Nov 5, 2024 · e5e7408 · e5e7408
1 parent 20af47d
commit e5e7408
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 28 deletions.
diff --git a/cmd/controller-manager/app/controllermanager.go b/cmd/controller-manager/app/controllermanager.go
@@ -190,6 +190,10 @@ func Run(ctx context.Context, opts *options.Options) error {
 	crtlmetrics.Registry.MustRegister(metrics.ResourceCollectors()...)
 	crtlmetrics.Registry.MustRegister(metrics.PoolCollectors()...)
 
+	if err := helper.IndexWork(ctx, controllerManager); err != nil {
+		klog.Fatalf("Failed to index Work: %v", err)
+	}
+
 	setupControllers(controllerManager, opts, ctx.Done())
 
 	// blocks until the context is done.
@@ -741,10 +745,6 @@ func setupControllers(mgr controllerruntime.Manager, opts *options.Options, stop
 	sharedFactory.Start(stopChan)
 	sharedFactory.WaitForCacheSync(stopChan)
 
-	if err := helper.IndexWork(context.TODO(), mgr); err != nil {
-		klog.Fatalf("Failed to index Work: %v", err)
-	}
-
 	resourceInterpreter := resourceinterpreter.NewResourceInterpreter(controlPlaneInformerManager, serviceLister)
 	if err := mgr.Add(resourceInterpreter); err != nil {
 		klog.Fatalf("Failed to setup custom resource interpreter: %v", err)

diff --git a/pkg/controllers/binding/binding_controller_test.go b/pkg/controllers/binding/binding_controller_test.go
@@ -55,7 +55,7 @@ func makeFakeRBCByResource(rs *workv1alpha2.ObjectReference) (*ResourceBindingCo
 	c := fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex(
 		&workv1alpha1.Work{},
 		workv1alpha2.ResourceBindingPermanentIDLabel,
-		utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
+		utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel),
 	).Build()
 
 	tempDyClient := fakedynamic.NewSimpleDynamicClient(scheme.Scheme)

diff --git a/pkg/controllers/binding/cluster_resource_binding_controller_test.go b/pkg/controllers/binding/cluster_resource_binding_controller_test.go
@@ -52,7 +52,7 @@ func makeFakeCRBCByResource(rs *workv1alpha2.ObjectReference) (*ClusterResourceB
 	c := fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex(
 		&workv1alpha1.Work{},
 		workv1alpha2.ClusterResourceBindingPermanentIDLabel,
-		utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
+		utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
 	).Build()
 	tempDyClient := fakedynamic.NewSimpleDynamicClient(scheme.Scheme)
 	if rs == nil {

diff --git a/pkg/controllers/status/crb_status_controller_test.go b/pkg/controllers/status/crb_status_controller_test.go
@@ -55,7 +55,7 @@ func generateCRBStatusController() *CRBStatusController {
 		Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex(
 			&workv1alpha1.Work{},
 			workv1alpha2.ClusterResourceBindingPermanentIDLabel,
-			utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
+			utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
 		).Build(),
 		DynamicClient:   dynamicClient,
 		InformerManager: m,
@@ -137,7 +137,7 @@ func TestCRBStatusController_Reconcile(t *testing.T) {
 			// Prepare binding and create it in client
 			if tt.binding != nil {
 				c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(tt.binding).WithStatusSubresource(tt.binding).
-					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ClusterResourceBindingPermanentIDLabel, utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel)).
+					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ClusterResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel)).
 					Build()
 			}
 
@@ -209,7 +209,7 @@ func TestCRBStatusController_syncBindingStatus(t *testing.T) {
 
 			if tt.resourceExistInClient {
 				c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(binding).WithStatusSubresource(binding).
-					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ClusterResourceBindingPermanentIDLabel, utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel)).
+					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ClusterResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel)).
 					Build()
 			}
 

diff --git a/pkg/controllers/status/rb_status_controller_test.go b/pkg/controllers/status/rb_status_controller_test.go
@@ -56,7 +56,7 @@ func generateRBStatusController() *RBStatusController {
 		Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex(
 			&workv1alpha1.Work{},
 			workv1alpha2.ClusterResourceBindingPermanentIDLabel,
-			utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
+			utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
 		).Build(),
 		DynamicClient:   dynamicClient,
 		InformerManager: m,
@@ -143,7 +143,7 @@ func TestRBStatusController_Reconcile(t *testing.T) {
 			// Prepare binding and create it in client
 			if tt.binding != nil {
 				c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(tt.binding).WithStatusSubresource(tt.binding).
-					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ResourceBindingPermanentIDLabel, utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel)).
+					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel)).
 					Build()
 			}
 
@@ -218,7 +218,7 @@ func TestRBStatusController_syncBindingStatus(t *testing.T) {
 
 			if tt.resourceExistInClient {
 				c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(binding).WithStatusSubresource(binding).
-					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ResourceBindingPermanentIDLabel, utilhelper.GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel)).
+					WithIndex(&workv1alpha1.Work{}, workv1alpha2.ResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel)).
 					Build()
 			}
 

diff --git a/pkg/util/helper/binding_test.go b/pkg/util/helper/binding_test.go
@@ -436,7 +436,7 @@ func TestFindOrphanWorks(t *testing.T) {
 				).WithIndex(
 					&workv1alpha1.Work{},
 					workv1alpha2.ResourceBindingPermanentIDLabel,
-					GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
+					IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel),
 				).Build(),
 				bindingNamespace: "default",
 				bindingName:      "binding",
@@ -485,7 +485,7 @@ func TestFindOrphanWorks(t *testing.T) {
 				).WithIndex(
 					&workv1alpha1.Work{},
 					workv1alpha2.ResourceBindingPermanentIDLabel,
-					GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
+					IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel),
 				).Build(),
 				bindingNamespace: "default",
 				bindingName:      "binding",
@@ -541,7 +541,7 @@ func TestFindOrphanWorks(t *testing.T) {
 				).WithIndex(
 					&workv1alpha1.Work{},
 					workv1alpha2.ClusterResourceBindingPermanentIDLabel,
-					GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
+					IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
 				).Build(),
 				bindingNamespace: "",
 				bindingName:      "binding",
@@ -1002,7 +1002,7 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) {
 				c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex(
 					&workv1alpha1.Work{},
 					workv1alpha2.ResourceBindingPermanentIDLabel,
-					GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
+					IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel),
 				).Build(),
 				namespace: "default",
 				name:      "foo",
@@ -1030,7 +1030,7 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) {
 				).WithIndex(
 					&workv1alpha1.Work{},
 					workv1alpha2.ResourceBindingPermanentIDLabel,
-					GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
+					IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel),
 				).Build(),
 				namespace: "default",
 				name:      "foo",
@@ -1057,7 +1057,7 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) {
 				).WithIndex(
 					&workv1alpha1.Work{},
 					workv1alpha2.ClusterResourceBindingPermanentIDLabel,
-					GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
+					IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
 				).Build(),
 				name:      "foo",
 				bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c",

diff --git a/pkg/util/helper/index.go b/pkg/util/helper/index.go
@@ -14,27 +14,27 @@ import (
 // IndexWork creates index for Work.
 func IndexWork(ctx context.Context, mgr ctrl.Manager) error {
 	err := mgr.GetFieldIndexer().IndexField(ctx, &workv1alpha1.Work{}, workv1alpha2.ResourceBindingPermanentIDLabel,
-		GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel))
+		IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel))
 	if err != nil {
 		klog.Errorf("failed to create index for work, err: %v", err)
 		return err
 	}
 	err = mgr.GetFieldIndexer().IndexField(ctx, &workv1alpha1.Work{}, workv1alpha2.ClusterResourceBindingPermanentIDLabel,
-		GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel))
+		IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel))
 	if err != nil {
 		klog.Errorf("failed to create index for work, err: %v", err)
 		return err
 	}
 	return nil
 }
 
-// GenOneLabelEqualIndexerFunc returns an IndexerFunc used to index resource with the given key as label key.
-func GenOneLabelEqualIndexerFunc(key string) client.IndexerFunc {
+// IndexerFuncBasedOnLabel returns an IndexerFunc used to index resource with the given key as label key.
+func IndexerFuncBasedOnLabel(key string) client.IndexerFunc {
 	return func(obj client.Object) []string {
-		refKey := obj.GetLabels()[key]
-		if refKey == "" {
+		val, ok := obj.GetLabels()[key]
+		if !ok {
 			return nil
 		}
-		return []string{refKey}
+		return []string{val}
 	}
 }
diff --git a/pkg/util/helper/index_test.go b/pkg/util/helper/index_test.go
@@ -9,7 +9,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func TestGenOneLabelEqualIndexerFunc(t *testing.T) {
+func TestIndexerFuncBasedOnLabel(t *testing.T) {
 	type args struct {
 		key string
 		obj client.Object
@@ -50,9 +50,9 @@ func TestGenOneLabelEqualIndexerFunc(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			fn := GenOneLabelEqualIndexerFunc(tt.args.key)
+			fn := IndexerFuncBasedOnLabel(tt.args.key)
 			assert.NotNil(t, fn)
-			assert.Equalf(t, tt.want, fn(tt.args.obj), "GenOneLabelEqualIndexerFunc(%v)", tt.args.key)
+			assert.Equalf(t, tt.want, fn(tt.args.obj), "IndexerFuncBasedOnLabel(%v)", tt.args.key)
 		})
 	}
 }