Skip to content

Commit

Permalink
Add index for Work to speed up the processing of resource binding rel…
Browse files Browse the repository at this point in the history
…ated controllers

Signed-off-by: zach593 <zach_li@outlook.com>
  • Loading branch information
zach593 committed Nov 4, 2024
1 parent e90ff63 commit 20af47d
Show file tree
Hide file tree
Showing 12 changed files with 228 additions and 17 deletions.
4 changes: 4 additions & 0 deletions cmd/controller-manager/app/controllermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,10 @@ 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)
Expand Down
12 changes: 10 additions & 2 deletions pkg/controllers/binding/binding_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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.GenOneLabelEqualIndexerFunc(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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.GenOneLabelEqualIndexerFunc(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,
Expand All @@ -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,
Expand Down
16 changes: 13 additions & 3 deletions pkg/controllers/status/crb_status_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
).Build(),
DynamicClient: dynamicClient,
InformerManager: m,
RESTMapper: func() meta.RESTMapper {
Expand Down Expand Up @@ -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.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel)).
Build()
}

res, err := c.Reconcile(context.Background(), req)
Expand Down Expand Up @@ -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.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel)).
Build()
}

err := c.syncBindingStatus(context.Background(), binding)
Expand Down
16 changes: 13 additions & 3 deletions pkg/controllers/status/rb_status_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
).Build(),
DynamicClient: dynamicClient,
InformerManager: m,
RESTMapper: func() meta.RESTMapper {
Expand Down Expand Up @@ -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.GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel)).
Build()
}

res, err := c.Reconcile(context.Background(), req)
Expand Down Expand Up @@ -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.GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel)).
Build()
}

err := c.syncBindingStatus(context.Background(), binding)
Expand Down
26 changes: 25 additions & 1 deletion pkg/util/helper/binding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,10 @@ func TestFindOrphanWorks(t *testing.T) {
},
},
},
).WithIndex(
&workv1alpha1.Work{},
workv1alpha2.ResourceBindingPermanentIDLabel,
GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
).Build(),
bindingNamespace: "default",
bindingName: "binding",
Expand Down Expand Up @@ -478,6 +482,10 @@ func TestFindOrphanWorks(t *testing.T) {
},
},
},
).WithIndex(
&workv1alpha1.Work{},
workv1alpha2.ResourceBindingPermanentIDLabel,
GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
).Build(),
bindingNamespace: "default",
bindingName: "binding",
Expand Down Expand Up @@ -530,6 +538,10 @@ func TestFindOrphanWorks(t *testing.T) {
},
},
},
).WithIndex(
&workv1alpha1.Work{},
workv1alpha2.ClusterResourceBindingPermanentIDLabel,
GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
).Build(),
bindingNamespace: "",
bindingName: "binding",
Expand Down Expand Up @@ -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,
GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
).Build(),
namespace: "default",
name: "foo",
bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c",
Expand All @@ -1011,6 +1027,10 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) {
},
},
},
).WithIndex(
&workv1alpha1.Work{},
workv1alpha2.ResourceBindingPermanentIDLabel,
GenOneLabelEqualIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
).Build(),
namespace: "default",
name: "foo",
Expand All @@ -1034,6 +1054,10 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) {
},
},
},
).WithIndex(
&workv1alpha1.Work{},
workv1alpha2.ClusterResourceBindingPermanentIDLabel,
GenOneLabelEqualIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
).Build(),
name: "foo",
bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c",
Expand Down
40 changes: 40 additions & 0 deletions pkg/util/helper/index.go
Original file line number Diff line number Diff line change
@@ -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,
GenOneLabelEqualIndexerFunc(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))
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 {
return func(obj client.Object) []string {
refKey := obj.GetLabels()[key]
if refKey == "" {
return nil
}
return []string{refKey}
}
}
58 changes: 58 additions & 0 deletions pkg/util/helper/index_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
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 TestGenOneLabelEqualIndexerFunc(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 := GenOneLabelEqualIndexerFunc(tt.args.key)
assert.NotNil(t, fn)
assert.Equalf(t, tt.want, fn(tt.args.obj), "GenOneLabelEqualIndexerFunc(%v)", tt.args.key)
})
}
}
15 changes: 10 additions & 5 deletions pkg/util/helper/work.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/clusterresourcebinding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ 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.GetWorksByBindingID(context.Background(), controllerManager.GetClient(), clusterResourceBinding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel], false)
if err != nil {
return false
}
Expand Down
Loading

0 comments on commit 20af47d

Please sign in to comment.