From e2827b1b2ef0ed49d8956b2be39a3b03536e759b Mon Sep 17 00:00:00 2001 From: zach593 Date: Sun, 3 Nov 2024 15:36:46 +0800 Subject: [PATCH] Add index for Work to speed up the processing of resource binding related controllers Signed-off-by: zach593 --- .../app/controllermanager.go | 4 ++ .../binding/binding_controller_test.go | 12 +++- ...luster_resource_binding_controller_test.go | 11 +++- .../status/crb_status_controller_test.go | 16 ++++- .../status/rb_status_controller_test.go | 16 ++++- pkg/util/helper/binding_test.go | 26 +++++++- pkg/util/helper/index.go | 40 +++++++++++++ pkg/util/helper/index_test.go | 60 +++++++++++++++++++ pkg/util/helper/work.go | 15 +++-- pkg/util/helper/workstatus_test.go | 10 ++++ test/e2e/clusterresourcebinding_test.go | 7 ++- 11 files changed, 200 insertions(+), 17 deletions(-) create mode 100644 pkg/util/helper/index.go create mode 100644 pkg/util/helper/index_test.go diff --git a/cmd/controller-manager/app/controllermanager.go b/cmd/controller-manager/app/controllermanager.go index 3429981480b4..9441db5de153 100644 --- 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. diff --git a/pkg/controllers/binding/binding_controller_test.go b/pkg/controllers/binding/binding_controller_test.go index 70d813d98d17..6e75e1ed7122 100644 --- a/pkg/controllers/binding/binding_controller_test.go +++ b/pkg/controllers/binding/binding_controller_test.go @@ -37,11 +37,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" + workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1" workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" testing2 "github.com/karmada-io/karmada/pkg/search/proxy/testing" "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager" "github.com/karmada-io/karmada/pkg/util/gclient" + utilhelper "github.com/karmada-io/karmada/pkg/util/helper" testing3 "github.com/karmada-io/karmada/pkg/util/testing" "github.com/karmada-io/karmada/test/helper" ) @@ -50,10 +52,16 @@ import ( // Currently support kind: Pod,Node. If you want support more kind, pls add it. // rs is nil means use default RestMapper, see: github.com/karmada-io/karmada/pkg/search/proxy/testing/constant.go func makeFakeRBCByResource(rs *workv1alpha2.ObjectReference) (*ResourceBindingController, error) { + c := fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ResourceBindingPermanentIDLabel, + utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel), + ).Build() + tempDyClient := fakedynamic.NewSimpleDynamicClient(scheme.Scheme) if rs == nil { return &ResourceBindingController{ - Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), + Client: c, RESTMapper: testing2.RestMapper, InformerManager: genericmanager.NewSingleClusterInformerManager(tempDyClient, 0, nil), DynamicClient: tempDyClient, @@ -83,7 +91,7 @@ func makeFakeRBCByResource(rs *workv1alpha2.ObjectReference) (*ResourceBindingCo } return &ResourceBindingController{ - Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), + Client: c, RESTMapper: helper.NewGroupRESTMapper(rs.Kind, meta.RESTScopeNamespace), InformerManager: testing3.NewSingleClusterInformerManagerByRS(src, obj), DynamicClient: tempDyClient, diff --git a/pkg/controllers/binding/cluster_resource_binding_controller_test.go b/pkg/controllers/binding/cluster_resource_binding_controller_test.go index 22572b4935d4..6d82ed82a70c 100644 --- a/pkg/controllers/binding/cluster_resource_binding_controller_test.go +++ b/pkg/controllers/binding/cluster_resource_binding_controller_test.go @@ -37,20 +37,27 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" + workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1" workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" testing2 "github.com/karmada-io/karmada/pkg/search/proxy/testing" "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager" "github.com/karmada-io/karmada/pkg/util/gclient" + utilhelper "github.com/karmada-io/karmada/pkg/util/helper" testing3 "github.com/karmada-io/karmada/pkg/util/testing" "github.com/karmada-io/karmada/test/helper" ) func makeFakeCRBCByResource(rs *workv1alpha2.ObjectReference) (*ClusterResourceBindingController, error) { + c := fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ClusterResourceBindingPermanentIDLabel, + utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel), + ).Build() tempDyClient := fakedynamic.NewSimpleDynamicClient(scheme.Scheme) if rs == nil { return &ClusterResourceBindingController{ - Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), + Client: c, RESTMapper: testing2.RestMapper, InformerManager: genericmanager.NewSingleClusterInformerManager(tempDyClient, 0, nil), DynamicClient: tempDyClient, @@ -77,7 +84,7 @@ func makeFakeCRBCByResource(rs *workv1alpha2.ObjectReference) (*ClusterResourceB } return &ClusterResourceBindingController{ - Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), + Client: c, RESTMapper: helper.NewGroupRESTMapper(rs.Kind, meta.RESTScopeNamespace), InformerManager: testing3.NewSingleClusterInformerManagerByRS(src, obj), DynamicClient: tempDyClient, diff --git a/pkg/controllers/status/crb_status_controller_test.go b/pkg/controllers/status/crb_status_controller_test.go index 1bf0b33dfa4e..b27589970ac0 100644 --- a/pkg/controllers/status/crb_status_controller_test.go +++ b/pkg/controllers/status/crb_status_controller_test.go @@ -33,10 +33,12 @@ import ( controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" + workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1" workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" "github.com/karmada-io/karmada/pkg/resourceinterpreter/default/native" "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager" "github.com/karmada-io/karmada/pkg/util/gclient" + utilhelper "github.com/karmada-io/karmada/pkg/util/helper" ) func generateCRBStatusController() *CRBStatusController { @@ -50,7 +52,11 @@ func generateCRBStatusController() *CRBStatusController { m.WaitForCacheSync() c := &CRBStatusController{ - Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), + Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ClusterResourceBindingPermanentIDLabel, + utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel), + ).Build(), DynamicClient: dynamicClient, InformerManager: m, RESTMapper: func() meta.RESTMapper { @@ -130,7 +136,9 @@ 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).Build() + c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(tt.binding).WithStatusSubresource(tt.binding). + WithIndex(&workv1alpha1.Work{}, workv1alpha2.ClusterResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel)). + Build() } res, err := c.Reconcile(context.Background(), req) @@ -200,7 +208,9 @@ func TestCRBStatusController_syncBindingStatus(t *testing.T) { } if tt.resourceExistInClient { - c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(binding).WithStatusSubresource(binding).Build() + c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(binding).WithStatusSubresource(binding). + WithIndex(&workv1alpha1.Work{}, workv1alpha2.ClusterResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel)). + Build() } err := c.syncBindingStatus(context.Background(), binding) diff --git a/pkg/controllers/status/rb_status_controller_test.go b/pkg/controllers/status/rb_status_controller_test.go index 537a07e8560e..d632b0a6185b 100644 --- a/pkg/controllers/status/rb_status_controller_test.go +++ b/pkg/controllers/status/rb_status_controller_test.go @@ -33,11 +33,13 @@ import ( controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" + workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1" workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" "github.com/karmada-io/karmada/pkg/resourceinterpreter" "github.com/karmada-io/karmada/pkg/resourceinterpreter/default/native" "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager" "github.com/karmada-io/karmada/pkg/util/gclient" + utilhelper "github.com/karmada-io/karmada/pkg/util/helper" ) func generateRBStatusController() *RBStatusController { @@ -51,7 +53,11 @@ func generateRBStatusController() *RBStatusController { m.WaitForCacheSync() c := &RBStatusController{ - Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), + Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ClusterResourceBindingPermanentIDLabel, + utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel), + ).Build(), DynamicClient: dynamicClient, InformerManager: m, RESTMapper: func() meta.RESTMapper { @@ -136,7 +142,9 @@ 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).Build() + c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(tt.binding).WithStatusSubresource(tt.binding). + WithIndex(&workv1alpha1.Work{}, workv1alpha2.ResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel)). + Build() } res, err := c.Reconcile(context.Background(), req) @@ -209,7 +217,9 @@ func TestRBStatusController_syncBindingStatus(t *testing.T) { } if tt.resourceExistInClient { - c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(binding).WithStatusSubresource(binding).Build() + c.Client = fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(binding).WithStatusSubresource(binding). + WithIndex(&workv1alpha1.Work{}, workv1alpha2.ResourceBindingPermanentIDLabel, utilhelper.IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel)). + Build() } err := c.syncBindingStatus(context.Background(), binding) diff --git a/pkg/util/helper/binding_test.go b/pkg/util/helper/binding_test.go index 4114aa9eff3c..3db7f62de5eb 100644 --- a/pkg/util/helper/binding_test.go +++ b/pkg/util/helper/binding_test.go @@ -433,6 +433,10 @@ func TestFindOrphanWorks(t *testing.T) { }, }, }, + ).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ResourceBindingPermanentIDLabel, + IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel), ).Build(), bindingNamespace: "default", bindingName: "binding", @@ -478,6 +482,10 @@ func TestFindOrphanWorks(t *testing.T) { }, }, }, + ).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ResourceBindingPermanentIDLabel, + IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel), ).Build(), bindingNamespace: "default", bindingName: "binding", @@ -530,6 +538,10 @@ func TestFindOrphanWorks(t *testing.T) { }, }, }, + ).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ClusterResourceBindingPermanentIDLabel, + IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel), ).Build(), bindingNamespace: "", bindingName: "binding", @@ -987,7 +999,11 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) { { name: "work is not found", args: args{ - c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), + c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ResourceBindingPermanentIDLabel, + IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel), + ).Build(), namespace: "default", name: "foo", bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c", @@ -1011,6 +1027,10 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) { }, }, }, + ).WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ResourceBindingPermanentIDLabel, + IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel), ).Build(), namespace: "default", name: "foo", @@ -1034,6 +1054,10 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) { }, }, }, + ).WithIndex( + &workv1alpha1.Work{}, + 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 new file mode 100644 index 000000000000..3d3a39facb96 --- /dev/null +++ b/pkg/util/helper/index.go @@ -0,0 +1,40 @@ +package helper + +import ( + "context" + + "k8s.io/klog/v2" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1" + workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" +) + +// IndexWork creates index for Work. +func IndexWork(ctx context.Context, mgr ctrl.Manager) error { + err := mgr.GetFieldIndexer().IndexField(ctx, &workv1alpha1.Work{}, 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, + IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel)) + if err != nil { + klog.Errorf("failed to create index for work, err: %v", err) + return err + } + return nil +} + +// 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 { + val, ok := obj.GetLabels()[key] + if !ok { + return nil + } + return []string{val} + } +} diff --git a/pkg/util/helper/index_test.go b/pkg/util/helper/index_test.go new file mode 100644 index 000000000000..1838e26d75a8 --- /dev/null +++ b/pkg/util/helper/index_test.go @@ -0,0 +1,60 @@ +package helper + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestIndexerFuncBasedOnLabel(t *testing.T) { + type args struct { + key string + obj client.Object + } + tests := []struct { + name string + args args + want []string + }{ + { + name: "cache hit", + args: args{ + key: "a", + obj: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "a", + }, + }, + }, + }, + want: []string{"a"}, + }, + { + name: "cache missed", + args: args{ + key: "b", + obj: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "a", + }, + }, + }, + }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fn := IndexerFuncBasedOnLabel(tt.args.key) + if !assert.NotNil(t, fn) { + t.FailNow() + } + assert.Equalf(t, tt.want, fn(tt.args.obj), "IndexerFuncBasedOnLabel(%v)", tt.args.key) + }) + } +} diff --git a/pkg/util/helper/work.go b/pkg/util/helper/work.go index 438a7309e8eb..987a3dc8e709 100644 --- a/pkg/util/helper/work.go +++ b/pkg/util/helper/work.go @@ -23,6 +23,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -117,15 +118,19 @@ func GetWorksByLabelsSet(ctx context.Context, c client.Client, ls labels.Set) (* } // GetWorksByBindingID gets WorkList by matching same binding's permanent id. +// Caller should ensure Work is indexed by binding's permanent id. func GetWorksByBindingID(ctx context.Context, c client.Client, bindingID string, namespaced bool) (*workv1alpha1.WorkList, error) { - var ls labels.Set + var key string if namespaced { - ls = labels.Set{workv1alpha2.ResourceBindingPermanentIDLabel: bindingID} + key = workv1alpha2.ResourceBindingPermanentIDLabel } else { - ls = labels.Set{workv1alpha2.ClusterResourceBindingPermanentIDLabel: bindingID} + key = workv1alpha2.ClusterResourceBindingPermanentIDLabel } - - return GetWorksByLabelsSet(ctx, c, ls) + workList := &workv1alpha1.WorkList{} + listOpt := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector(key, bindingID), + } + return workList, c.List(ctx, workList, listOpt) } // GenEventRef returns the event reference. sets the UID(.spec.uid) that might be missing for fire events. diff --git a/pkg/util/helper/workstatus_test.go b/pkg/util/helper/workstatus_test.go index ced58ffe8db0..557d75694817 100644 --- a/pkg/util/helper/workstatus_test.go +++ b/pkg/util/helper/workstatus_test.go @@ -199,6 +199,11 @@ func TestAggregateResourceBindingWorkStatus(t *testing.T) { c := fake.NewClientBuilder(). WithScheme(scheme). + WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ResourceBindingPermanentIDLabel, + IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel), + ). WithObjects(objects...). WithStatusSubresource(tt.binding). Build() @@ -391,6 +396,11 @@ func TestAggregateClusterResourceBindingWorkStatus(t *testing.T) { c := fake.NewClientBuilder(). WithScheme(scheme). + WithIndex( + &workv1alpha1.Work{}, + workv1alpha2.ClusterResourceBindingPermanentIDLabel, + IndexerFuncBasedOnLabel(workv1alpha2.ClusterResourceBindingPermanentIDLabel), + ). WithObjects(objects...). WithStatusSubresource(tt.binding). Build() diff --git a/test/e2e/clusterresourcebinding_test.go b/test/e2e/clusterresourcebinding_test.go index 13885ff2cbaa..2937d45fea61 100644 --- a/test/e2e/clusterresourcebinding_test.go +++ b/test/e2e/clusterresourcebinding_test.go @@ -22,6 +22,7 @@ import ( "github.com/onsi/ginkgo/v2" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/rand" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" @@ -80,7 +81,11 @@ var _ = ginkgo.Describe("ClusterResourceBinding test", func() { ginkgo.It("creates work with permanent ID label", func() { framework.WaitClusterResourceBindingFitWith(karmadaClient, bindingName, func(clusterResourceBinding *workv1alpha2.ClusterResourceBinding) bool { - workList, err := helper.GetWorksByBindingID(context.Background(), controlPlaneClient, clusterResourceBinding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel], false) + workList, err := helper.GetWorksByLabelsSet(context.Background(), controlPlaneClient, + labels.Set{ + workv1alpha2.ClusterResourceBindingPermanentIDLabel: clusterResourceBinding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel], + }, + ) if err != nil { return false }