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 <[email protected]>
  • Loading branch information
zach593 committed Nov 19, 2024
1 parent ae65de4 commit e2827b1
Show file tree
Hide file tree
Showing 11 changed files with 200 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 @@ -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.
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.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,
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.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,
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.IndexerFuncBasedOnLabel(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.IndexerFuncBasedOnLabel(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.IndexerFuncBasedOnLabel(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.IndexerFuncBasedOnLabel(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.IndexerFuncBasedOnLabel(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.IndexerFuncBasedOnLabel(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,
IndexerFuncBasedOnLabel(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,
IndexerFuncBasedOnLabel(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,
IndexerFuncBasedOnLabel(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,
IndexerFuncBasedOnLabel(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,
IndexerFuncBasedOnLabel(workv1alpha2.ResourceBindingPermanentIDLabel),
).Build(),
namespace: "default",
name: "foo",
Expand All @@ -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",
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,
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}
}
}
60 changes: 60 additions & 0 deletions pkg/util/helper/index_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
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
Loading

0 comments on commit e2827b1

Please sign in to comment.