fix: skip revision injection when matched UpdateStrategy.Selector

Signed-off-by: kim <[email protected]>

pkg/webhook/pod/mutating/sidecarset.go

@@ -33,6 +33,7 @@ import (
 	apps "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -199,6 +200,18 @@ func (h *PodCreateHandler) getSuitableRevisionSidecarSet(sidecarSet *appsv1alpha
 		// TODO: support 'PartitionBased' policy to inject old/new revision according to Partition
 		switch sidecarSet.Spec.InjectionStrategy.Revision.Policy {
 		case "", appsv1alpha1.AlwaysSidecarSetInjectRevisionPolicy:
+			// if pod matched the UpdateStrategy.Selector, skip inject custom revision
+			strategy := sidecarSet.Spec.UpdateStrategy
+			if !strategy.Paused && strategy.Selector != nil {
+				selector, err := util.ValidatedLabelSelectorAsSelector(strategy.Selector)
+				if err != nil {
+					return nil, err
+				}
+
+				if selector.Matches(labels.Set(newPod.Labels)) {
+					return h.getSpecificHistorySidecarSet(sidecarSet, nil)
+				}
+			}
 			return h.getSpecificHistorySidecarSet(sidecarSet, revisionInfo)
 		}
 

pkg/webhook/pod/mutating/sidecarset_test.go

@@ -567,43 +567,97 @@ func TestInjectionStrategyRevision(t *testing.T) {
 
 	raw, _ := json.Marshal(spec)
 	revisionID := fmt.Sprintf("%s-12345", sidecarSet1.Name)
+	injectedRevision := &apps.ControllerRevision{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: webhookutil.GetNamespace(),
+			Name:      revisionID,
+			Labels: map[string]string{
+				appsv1alpha1.SidecarSetCustomVersionLabel: revisionID,
+			},
+		},
+		Data: runtime.RawExtension{
+			Raw: raw,
+		},
+	}
+
 	sidecarSetIn := sidecarSet1.DeepCopy()
 	sidecarSetIn.Spec.InjectionStrategy.Revision = &appsv1alpha1.SidecarSetInjectRevision{
 		CustomVersion: &revisionID,
 		Policy:        appsv1alpha1.AlwaysSidecarSetInjectRevisionPolicy,
 	}
-	historyInjection := []client.Object{
-		sidecarSetIn,
-		&apps.ControllerRevision{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: webhookutil.GetNamespace(),
-				Name:      revisionID,
-				Labels: map[string]string{
-					appsv1alpha1.SidecarSetCustomVersionLabel: revisionID,
-				},
+
+	testInjectionStrategyRevision(t, sidecarSetIn, injectedRevision)
+}
+
+func testInjectionStrategyRevision(t *testing.T, sidecarSet *appsv1alpha1.SidecarSet, injectedRevision client.Object) {
+	cases := []struct {
+		name                 string
+		getSidecarSets       func() *appsv1alpha1.SidecarSet
+		expectInitContainers int
+		expectContainers     int
+	}{
+		{
+			name: "inject CustomVersion by default",
+			getSidecarSets: func() *appsv1alpha1.SidecarSet {
+				return sidecarSet.DeepCopy()
 			},
-			Data: runtime.RawExtension{
-				Raw: raw,
+			expectInitContainers: 2,
+			expectContainers:     2,
+		},
+		{
+			name: "inject latest version when pod matches UpdateStrategy.Selector",
+			getSidecarSets: func() *appsv1alpha1.SidecarSet {
+				s := sidecarSet.DeepCopy()
+				s.Spec.UpdateStrategy.Selector = &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "suxing-test",
+					},
+				}
+				return s
 			},
+			expectInitContainers: 3,
+			expectContainers:     3,
 		},
-	}
-	testInjectionStrategyRevision(t, historyInjection)
-}
+		{
+			name: "inject CustomVersion when .UpdateStrategy.Paused set to true",
+			getSidecarSets: func() *appsv1alpha1.SidecarSet {
+				s := sidecarSet.DeepCopy()
 
-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()
-	podHandler := &PodCreateHandler{Decoder: decoder, Client: client}
-	req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "")
-	_, err := podHandler.sidecarsetMutatingPod(context.Background(), req, podOut)
-	if err != nil {
-		t.Fatalf("failed to mutating pod, err: %v", err)
+				s.Spec.UpdateStrategy.Paused = true
+				s.Spec.UpdateStrategy.Selector = &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "suxing-test",
+					},
+				}
+				return s
+			},
+			expectInitContainers: 2,
+			expectContainers:     2,
+		},
 	}
 
-	if len(podIn.Spec.Containers)+len(podIn.Spec.InitContainers)+2 != len(podOut.Spec.Containers)+len(podOut.Spec.InitContainers) {
-		t.Fatalf("expect %v containers but got %v", len(podIn.Spec.Containers)+2, len(podOut.Spec.Containers))
+	for _, c := range cases {
+		podIn := pod1.DeepCopy()
+		podOut := podIn.DeepCopy()
+
+		revisions := []client.Object{c.getSidecarSets(), injectedRevision}
+
+		decoder, _ := admission.NewDecoder(scheme.Scheme)
+		client := fake.NewClientBuilder().WithObjects(revisions...).Build()
+		podHandler := &PodCreateHandler{Decoder: decoder, Client: client}
+		req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "")
+
+		_, err := podHandler.sidecarsetMutatingPod(context.Background(), req, podOut)
+		if err != nil {
+			t.Fatalf("inject sidecar into pod failed, err: %v", err)
+		}
+
+		if len(podOut.Spec.InitContainers) != c.expectInitContainers {
+			t.Fatalf("expect %v initContainers but got %v", c.expectInitContainers, len(podOut.Spec.InitContainers))
+		}
+		if len(podOut.Spec.Containers) != c.expectContainers {
+			t.Fatalf("expect %v containers but got %v", c.expectContainers, len(podOut.Spec.Containers))
+		}
 	}
 }