From 8bb89648de6c4944239c03a9a4a22f18e9616193 Mon Sep 17 00:00:00 2001 From: ls-2018 Date: Mon, 8 Apr 2024 11:52:46 +0800 Subject: [PATCH] feature: Optimizing Pod SidecarSet webhook performance (#1547) Signed-off-by: acejilam --- .../sidecarset/sidecarset_processor.go | 14 ++- pkg/util/fieldindex/register.go | 50 ++++++++-- pkg/util/fieldindex/register_test.go | 96 +++++++++++++++++++ pkg/webhook/pod/mutating/sidecarset.go | 18 +++- .../mutating/sidecarset_hotupgrade_test.go | 5 +- pkg/webhook/pod/mutating/sidecarset_test.go | 49 +++++++--- 6 files changed, 206 insertions(+), 26 deletions(-) create mode 100644 pkg/util/fieldindex/register_test.go diff --git a/pkg/controller/sidecarset/sidecarset_processor.go b/pkg/controller/sidecarset/sidecarset_processor.go index 3aa3592870..3307376248 100644 --- a/pkg/controller/sidecarset/sidecarset_processor.go +++ b/pkg/controller/sidecarset/sidecarset_processor.go @@ -30,6 +30,7 @@ import ( controlutil "github.com/openkruise/kruise/pkg/controller/util" "github.com/openkruise/kruise/pkg/util" utilclient "github.com/openkruise/kruise/pkg/util/client" + "github.com/openkruise/kruise/pkg/util/fieldindex" historyutil "github.com/openkruise/kruise/pkg/util/history" webhookutil "github.com/openkruise/kruise/pkg/webhook/util" podutil "k8s.io/kubernetes/pkg/api/v1/pod" @@ -225,14 +226,23 @@ func (p *Processor) updatePodSidecarAndHash(control sidecarcontrol.SidecarContro func (p *Processor) listMatchedSidecarSets(pod *corev1.Pod) string { sidecarSetList := &appsv1alpha1.SidecarSetList{} - if err := p.Client.List(context.TODO(), sidecarSetList); err != nil { + sidecarSetList2 := &appsv1alpha1.SidecarSetList{} + podNamespace := pod.Namespace + if podNamespace == "" { + podNamespace = "default" + } + if err := p.Client.List(context.TODO(), sidecarSetList, client.MatchingFields{fieldindex.IndexNameForSidecarSetNamespace: podNamespace}, utilclient.DisableDeepCopy); err != nil { + klog.Errorf("List SidecarSets failed: %s", err.Error()) + return "" + } + if err := p.Client.List(context.TODO(), sidecarSetList2, client.MatchingFields{fieldindex.IndexNameForSidecarSetNamespace: fieldindex.IndexValueSidecarSetClusterScope}, utilclient.DisableDeepCopy); err != nil { klog.Errorf("List SidecarSets failed: %s", err.Error()) return "" } //matched SidecarSet.Name list sidecarSetNames := make([]string, 0) - for _, sidecarSet := range sidecarSetList.Items { + for _, sidecarSet := range append(sidecarSetList.Items, sidecarSetList2.Items...) { if matched, _ := sidecarcontrol.PodMatchedSidecarSet(p.Client, pod, &sidecarSet); matched { sidecarSetNames = append(sidecarSetNames, sidecarSet.Name) } diff --git a/pkg/util/fieldindex/register.go b/pkg/util/fieldindex/register.go index 52bf7c9f84..39ea11b3d9 100644 --- a/pkg/util/fieldindex/register.go +++ b/pkg/util/fieldindex/register.go @@ -20,21 +20,24 @@ import ( "context" "sync" - "sigs.k8s.io/controller-runtime/pkg/client" - appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" utildiscovery "github.com/openkruise/kruise/pkg/util/discovery" + batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - IndexNameForPodNodeName = "spec.nodeName" - IndexNameForOwnerRefUID = "ownerRefUID" - IndexNameForController = ".metadata.controller" - IndexNameForIsActive = "isActive" + IndexNameForPodNodeName = "spec.nodeName" + IndexNameForOwnerRefUID = "ownerRefUID" + IndexNameForController = ".metadata.controller" + IndexNameForIsActive = "isActive" + IndexNameForSidecarSetNamespace = "namespace" + IndexValueSidecarSetClusterScope = "clusterScope" + LabelMetadataName = v1.LabelMetadataName ) var ( @@ -87,6 +90,12 @@ func RegisterFieldIndexes(c cache.Cache) error { return } } + // sidecar spec namespaces + if utildiscovery.DiscoverObject(&appsv1alpha1.SidecarSet{}) { + if err = indexSidecarSet(c); err != nil { + return + } + } }) return err } @@ -152,3 +161,32 @@ func indexImagePullJobActive(c cache.Cache) error { return []string{isActive} }) } + +func IndexSidecarSet(rawObj client.Object) []string { + obj := rawObj.(*appsv1alpha1.SidecarSet) + if obj == nil { + return nil + } + if obj.Spec.Namespace != "" { + return []string{obj.Spec.Namespace} + } + if obj.Spec.NamespaceSelector != nil { + if obj.Spec.NamespaceSelector.MatchLabels != nil { + if v, ok := obj.Spec.NamespaceSelector.MatchLabels[LabelMetadataName]; ok { + return []string{v} + } + } + for _, item := range obj.Spec.NamespaceSelector.MatchExpressions { + if item.Key == LabelMetadataName && item.Operator == metav1.LabelSelectorOpIn { + return item.Values + } + } + } + return []string{IndexValueSidecarSetClusterScope} +} + +func indexSidecarSet(c cache.Cache) error { + return c.IndexField(context.TODO(), &appsv1alpha1.SidecarSet{}, IndexNameForSidecarSetNamespace, func(rawObj client.Object) []string { + return IndexSidecarSet(rawObj) + }) +} diff --git a/pkg/util/fieldindex/register_test.go b/pkg/util/fieldindex/register_test.go new file mode 100644 index 0000000000..f9496a9b5a --- /dev/null +++ b/pkg/util/fieldindex/register_test.go @@ -0,0 +1,96 @@ +package fieldindex + +import ( + "fmt" + "reflect" + "testing" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestIndexSidecarSet(t *testing.T) { + type args struct { + workload *appsv1alpha1.SidecarSet + } + tests := []struct { + name string + args args + want []string + }{ + { + name: "nil obj", + args: args{ + workload: nil, + }, + want: nil, + }, + { + name: "namespace is specified in SidecarSet", + args: args{ + workload: &appsv1alpha1.SidecarSet{ + Spec: appsv1alpha1.SidecarSetSpec{ + Namespace: "default", + }, + }, + }, + want: []string{"default"}, + }, + { + name: fmt.Sprintf("namespaceSelector is specified in SidecarSet and exists labels: %s", LabelMetadataName), + args: args{ + workload: &appsv1alpha1.SidecarSet{ + Spec: appsv1alpha1.SidecarSetSpec{ + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + LabelMetadataName: "default", + }, + MatchExpressions: nil, + }, + }, + }, + }, + want: []string{"default"}, + }, + { + name: fmt.Sprintf("namespaceSelector is specified in SidecarSet and exists labels: %s", LabelMetadataName), + args: args{ + workload: &appsv1alpha1.SidecarSet{ + Spec: appsv1alpha1.SidecarSetSpec{ + NamespaceSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: LabelMetadataName, + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "default", + }, + }, + }, + }, + }, + }, + }, + want: []string{"default"}, + }, + { + name: "namespace and namespaceSelector not specified", + args: args{ + workload: &appsv1alpha1.SidecarSet{ + Spec: appsv1alpha1.SidecarSetSpec{ + NamespaceSelector: &metav1.LabelSelector{}, + }, + }, + }, + want: []string{IndexValueSidecarSetClusterScope}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IndexSidecarSet(tt.args.workload) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("IndexSidecarSet() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/webhook/pod/mutating/sidecarset.go b/pkg/webhook/pod/mutating/sidecarset.go index 2d0a1a3dd0..3399e7f544 100644 --- a/pkg/webhook/pod/mutating/sidecarset.go +++ b/pkg/webhook/pod/mutating/sidecarset.go @@ -27,6 +27,7 @@ import ( "github.com/openkruise/kruise/pkg/control/sidecarcontrol" "github.com/openkruise/kruise/pkg/util" utilclient "github.com/openkruise/kruise/pkg/util/client" + "github.com/openkruise/kruise/pkg/util/fieldindex" "github.com/openkruise/kruise/pkg/util/history" admissionv1 "k8s.io/api/admission/v1" @@ -35,6 +36,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) @@ -64,13 +66,21 @@ func (h *PodCreateHandler) sidecarsetMutatingPod(ctx context.Context, req admiss } // DisableDeepCopy:true, indicates must be deep copy before update sidecarSet objection - sidecarsetList := &appsv1alpha1.SidecarSetList{} - if err = h.Client.List(ctx, sidecarsetList, utilclient.DisableDeepCopy); err != nil { + + sidecarSetList := &appsv1alpha1.SidecarSetList{} + sidecarSetList2 := &appsv1alpha1.SidecarSetList{} + podNamespace := pod.Namespace + if podNamespace == "" { + podNamespace = "default" + } + if err := h.Client.List(ctx, sidecarSetList, client.MatchingFields{fieldindex.IndexNameForSidecarSetNamespace: podNamespace}, utilclient.DisableDeepCopy); err != nil { + return false, err + } + if err := h.Client.List(ctx, sidecarSetList2, client.MatchingFields{fieldindex.IndexNameForSidecarSetNamespace: fieldindex.IndexValueSidecarSetClusterScope}, utilclient.DisableDeepCopy); err != nil { return false, err } - matchedSidecarSets := make([]sidecarcontrol.SidecarControl, 0) - for _, sidecarSet := range sidecarsetList.Items { + for _, sidecarSet := range append(sidecarSetList.Items, sidecarSetList2.Items...) { if sidecarSet.Spec.InjectionStrategy.Paused { continue } diff --git a/pkg/webhook/pod/mutating/sidecarset_hotupgrade_test.go b/pkg/webhook/pod/mutating/sidecarset_hotupgrade_test.go index fc01d62bff..707028f25f 100644 --- a/pkg/webhook/pod/mutating/sidecarset_hotupgrade_test.go +++ b/pkg/webhook/pod/mutating/sidecarset_hotupgrade_test.go @@ -22,6 +22,7 @@ import ( appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" "github.com/openkruise/kruise/pkg/control/sidecarcontrol" + "github.com/openkruise/kruise/pkg/util/fieldindex" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/runtime" @@ -41,7 +42,9 @@ func TestInjectHotUpgradeSidecar(t *testing.T) { func testInjectHotUpgradeSidecar(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarSet) { podIn := pod1.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podOut := podIn.DeepCopy() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") diff --git a/pkg/webhook/pod/mutating/sidecarset_test.go b/pkg/webhook/pod/mutating/sidecarset_test.go index 0326f40112..3057dd167e 100644 --- a/pkg/webhook/pod/mutating/sidecarset_test.go +++ b/pkg/webhook/pod/mutating/sidecarset_test.go @@ -24,12 +24,11 @@ import ( "path/filepath" "testing" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "github.com/openkruise/kruise/apis" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" "github.com/openkruise/kruise/pkg/control/sidecarcontrol" "github.com/openkruise/kruise/pkg/util" + "github.com/openkruise/kruise/pkg/util/fieldindex" webhookutil "github.com/openkruise/kruise/pkg/webhook/util" admissionv1 "k8s.io/api/admission/v1" @@ -37,6 +36,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -424,7 +424,9 @@ func testPodHasNoMatchedSidecarSet(t *testing.T, sidecarSetIn *appsv1alpha1.Side podIn.Labels["app"] = "doesnt-match" podOut := podIn.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") _, _ = podHandler.sidecarsetMutatingPod(context.Background(), req, podOut) @@ -466,7 +468,9 @@ func doMergeSidecarSecretsTest(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarS podIn.Spec.ImagePullSecrets = podImagePullSecrets podOut := podIn.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") _, _ = podHandler.sidecarsetMutatingPod(context.Background(), req, podOut) @@ -487,7 +491,9 @@ func testInjectionStrategyPaused(t *testing.T, sidecarIn *appsv1alpha1.SidecarSe sidecarPaused := sidecarIn sidecarPaused.Spec.InjectionStrategy.Paused = true decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarPaused).Build() + client := fake.NewClientBuilder().WithObjects(sidecarPaused).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") _, _ = podHandler.sidecarsetMutatingPod(context.Background(), req, podOut) @@ -525,7 +531,10 @@ func TestInjectMetadata(t *testing.T) { }, } decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(demo1, demo2).Build() + client := fake.NewClientBuilder().WithObjects(demo1, demo2).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() + podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") podHandler.sidecarsetMutatingPod(context.Background(), req, podIn) @@ -594,7 +603,9 @@ func testInjectionStrategyRevision(t *testing.T, env []client.Object) { podIn := pod1.DeepCopy() podOut := podIn.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(env...).Build() + client := fake.NewClientBuilder().WithObjects(env...).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") _, err := podHandler.sidecarsetMutatingPod(context.Background(), req, podOut) @@ -615,7 +626,9 @@ func TestSidecarSetPodInjectPolicy(t *testing.T) { func testSidecarSetPodInjectPolicy(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarSet) { podIn := pod1.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podOut := podIn.DeepCopy() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") @@ -692,7 +705,9 @@ func testSidecarVolumesAppend(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarSe podIn := pod1.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podOut := podIn.DeepCopy() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") @@ -845,7 +860,9 @@ func testPodVolumeMountsAppend(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarS t.Run(cs.name, func(t *testing.T) { podIn := cs.getPod() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(cs.getSidecarSets()).Build() + client := fake.NewClientBuilder().WithObjects(cs.getSidecarSets()).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podOut := podIn.DeepCopy() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") @@ -889,7 +906,9 @@ func TestSidecarSetTransferEnv(t *testing.T) { func testSidecarSetTransferEnv(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarSet) { podIn := pod1.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podOut := podIn.DeepCopy() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") @@ -923,7 +942,9 @@ func testSidecarSetHashInject(t *testing.T, sidecarSetIn1 *appsv1alpha1.SidecarS sidecarSetIn3 := sidecarSet3.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn1, sidecarSetIn2, sidecarSetIn3).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn1, sidecarSetIn2, sidecarSetIn3).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podOut := podIn.DeepCopy() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") @@ -954,7 +975,9 @@ func TestSidecarSetNameInject(t *testing.T) { func testSidecarSetNameInject(t *testing.T, sidecarSetIn1, sidecarSetIn3 *appsv1alpha1.SidecarSet) { podIn := pod1.DeepCopy() decoder, _ := admission.NewDecoder(scheme.Scheme) - client := fake.NewClientBuilder().WithObjects(sidecarSetIn1, sidecarSetIn3).Build() + client := fake.NewClientBuilder().WithObjects(sidecarSetIn1, sidecarSetIn3).WithIndex( + &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, + ).Build() podOut := podIn.DeepCopy() podHandler := &PodCreateHandler{Decoder: decoder, Client: client} req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "")