Feature: SidecarSet injection supports Partial strategy (#1856)

* SidecarSet injection supports TODO strategy Signed-off-by: AiRanthem <[email protected]> * passing test Signed-off-by: AiRanthem <[email protected]> * rename the policy Signed-off-by: AiRanthem <[email protected]> * Partition and Selector cannot be used together Signed-off-by: AiRanthem <[email protected]> * Partition and Selector cannot be used together Signed-off-by: AiRanthem <[email protected]> --------- Signed-off-by: AiRanthem <[email protected]>
openkruise · Dec 25, 2024 · 2cdb760 · 2cdb760
1 parent aeb7f19
commit 2cdb760
Show file tree

Hide file tree

Showing 10 changed files with 311 additions and 132 deletions.
diff --git a/apis/apps/v1alpha1/sidecarset_types.go b/apis/apps/v1alpha1/sidecarset_types.go
@@ -216,7 +216,8 @@ type SidecarSetInjectRevision struct {
 	// + optional
 	RevisionName *string `json:"revisionName,omitempty"`
 	// Policy describes the behavior of revision injection.
-	// Defaults to Always.
+	// +kubebuilder:validation:Enum=Always;Partial;
+	// +kubebuilder:default=Always
 	Policy SidecarSetInjectRevisionPolicy `json:"policy,omitempty"`
 }
 
@@ -226,9 +227,15 @@ const (
 	// AlwaysSidecarSetInjectRevisionPolicy means the SidecarSet will always inject
 	// the specific revision to Pods when pod creating, except matching UpdateStrategy.Selector.
 	AlwaysSidecarSetInjectRevisionPolicy SidecarSetInjectRevisionPolicy = "Always"
-	// PartitionBasedSidecarSetInjectRevisionPolicy means the SidecarSet will inject the
-	// specific or the latest revision according to Partition.
-	//PartitionBasedSidecarSetInjectRevisionPolicy SidecarSetInjectRevisionPolicy = "PartitionBased"
+
+	// PartialSidecarSetInjectRevisionPolicy means the SidecarSet will inject the specific or the latest revision according to UpdateStrategy.
+	//
+	// If UpdateStrategy.Pause is not true, only when a newly created Pod is **not** selected by the Selector explicitly
+	// configured in `UpdateStrategy` will it be injected with the specified version of the Sidecar.
+	// Under all other conditions, newly created Pods have a probability of being injected with the latest Sidecar,
+	// where the probability is `1 - UpdateStrategy.Partition`.
+	// If `Partition` is not a percentage or is not configured, its value is considered to be 0%.
+	PartialSidecarSetInjectRevisionPolicy SidecarSetInjectRevisionPolicy = "Partial"
 )
 
 // SidecarSetUpdateStrategy indicates the strategy that the SidecarSet
@@ -251,8 +258,6 @@ type SidecarSetUpdateStrategy struct {
 	// injected into newly created Pods by a SidecarSet configured with an injectionStrategy.
 	// In most cases, all newly created Pods are injected with the specified Sidecar version as configured in injectionStrategy.revision,
 	// which is consistent with previous versions.
-	// Now, if updateStrategy.Selector is  also configured and the updateStrategy.paused field is set to false,
-	// then Pods matching the selector will be injected with the latest version of the Sidecar container.
 	Selector *metav1.LabelSelector `json:"selector,omitempty"`
 
 	// Partition is the desired number of pods in old revisions. It means when partition

diff --git a/config/crd/bases/apps.kruise.io_sidecarsets.yaml b/config/crd/bases/apps.kruise.io_sidecarsets.yaml
@@ -258,9 +258,11 @@ spec:
                           history SidecarSet to inject specific version of the sidecar to pods.
                         type: string
                       policy:
-                        description: |-
-                          Policy describes the behavior of revision injection.
-                          Defaults to Always.
+                        default: Always
+                        description: Policy describes the behavior of revision injection.
+                        enum:
+                        - Always
+                        - Partial
                         type: string
                       revisionName:
                         description: RevisionName corresponds to a specific ControllerRevision
@@ -534,8 +536,6 @@ spec:
                       injected into newly created Pods by a SidecarSet configured with an injectionStrategy.
                       In most cases, all newly created Pods are injected with the specified Sidecar version as configured in injectionStrategy.revision,
                       which is consistent with previous versions.
-                      Now, if updateStrategy.Selector is  also configured and the updateStrategy.paused field is set to false,
-                      then Pods matching the selector will be injected with the latest version of the Sidecar container.
                     properties:
                       matchExpressions:
                         description: matchExpressions is a list of label selector

diff --git a/pkg/controller/sidecarset/sidecarset_pod_event_handler.go b/pkg/controller/sidecarset/sidecarset_pod_event_handler.go
@@ -140,22 +140,6 @@ func (p *enqueueRequestForPod) getPodMatchedSidecarSets(pod *corev1.Pod) ([]*app
 		return matchedSidecarSets, nil
 	}
 
-	// This code will trigger an invalid reconcile, where the Pod matches the sidecarSet selector but does not inject the sidecar container.
-	// Comment out this code to reduce some invalid reconcile.
-	/*sidecarSets := appsv1alpha1.SidecarSetList{}
-	if err := p.reader.List(context.TODO(), &sidecarSets); err != nil {
-		return nil, err
-	}
-
-	for _, sidecarSet := range sidecarSets.Items {
-		matched, err := sidecarcontrol.PodMatchedSidecarSet(pod, sidecarSet)
-		if err != nil {
-			return nil, err
-		}
-		if matched {
-			matchedSidecarSets = append(matchedSidecarSets, &sidecarSet)
-		}
-	}*/
 	return matchedSidecarSets, nil
 }
 

diff --git a/pkg/util/tools.go b/pkg/util/tools.go
@@ -228,6 +228,20 @@ func GetScaledValueFromIntOrPercent(intOrPercent *intstrutil.IntOrString, total
 	return 0, fmt.Errorf("invalid type: neither int nor percentage")
 }
 
+// ParsePercentageAsFloat64 parses a string as a percentage and returns the value as a float64.
+func ParsePercentageAsFloat64(s string) (float64, error) {
+	if strings.HasSuffix(s, "%") {
+		s = strings.TrimSuffix(s, "%")
+	} else {
+		return 0, fmt.Errorf("invalid type: string is not a percentage")
+	}
+	v, err := strconv.ParseFloat(s, 64)
+	if err != nil {
+		return 0, err
+	}
+	return v / 100, nil
+}
+
 func EqualIgnoreHash(template1, template2 *corev1.PodTemplateSpec) bool {
 	t1Copy := template1.DeepCopy()
 	t2Copy := template2.DeepCopy()

diff --git a/pkg/util/tools_test.go b/pkg/util/tools_test.go
@@ -19,6 +19,7 @@ package util
 
 import (
 	"fmt"
+	"math"
 	"sync"
 	"testing"
 
@@ -323,7 +324,7 @@ func TestGetScaledValueFromIntOrPercent(t *testing.T) {
 		expectVal int
 	}{
 		{
-			input:     intstr.FromInt(123),
+			input:     intstr.FromInt32(123),
 			expectErr: false,
 			expectVal: 123,
 		},
@@ -410,3 +411,36 @@ func TestGetScaledValueFromIntOrPercent(t *testing.T) {
 		}
 	}
 }
+
+func TestParsePercentageAsFloat64(t *testing.T) {
+	testCases := []struct {
+		input    string
+		expected float64
+		err      error
+	}{
+		{"100%", 1.0, nil},
+		{"50%", 0.5, nil},
+		{"25%", 0.25, nil},
+		{"10%", 0.10, nil},
+		{"1%", 0.01, nil},
+		{"0%", 0.00, nil},
+		{"invalid", 0, fmt.Errorf("invalid type: string is not a percentage")},
+		{"100", 0, fmt.Errorf("invalid type: string is not a percentage")},
+		{"", 0, fmt.Errorf("invalid type: string is not a percentage")},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.input, func(t *testing.T) {
+			got, err := ParsePercentageAsFloat64(tc.input)
+			if err != nil && tc.err == nil {
+				t.Errorf("expected no error, but got: %v", err)
+			}
+			if err == nil && tc.err != nil {
+				t.Errorf("expected error: %v, but got none", tc.err)
+			}
+			if math.Abs(got-tc.expected) > 1e-9 {
+				t.Errorf("expected %v, but got %v", tc.expected, got)
+			}
+		})
+	}
+}
diff --git a/pkg/webhook/pod/mutating/sidecarset.go b/pkg/webhook/pod/mutating/sidecarset.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math/rand"
 	"sort"
 	"strings"
 
@@ -30,6 +31,7 @@ import (
 	"github.com/openkruise/kruise/pkg/util/fieldindex"
 	"github.com/openkruise/kruise/pkg/util/history"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/intstr"
 
 	admissionv1 "k8s.io/api/admission/v1"
 	apps "k8s.io/api/apps/v1"
@@ -206,28 +208,53 @@ func (h *PodCreateHandler) getSuitableRevisionSidecarSet(sidecarSet *appsv1alpha
 			return sidecarSet.DeepCopy(), nil
 		}
 
-		// On pod creation, if a new pod matches the SidecarSet update strategy selector,
-		// the latest revision rather than that specified in the sidecarset.spec.injectionStrategy will be injected.
-		if updateStrategy := sidecarSet.Spec.UpdateStrategy; !updateStrategy.Paused && updateStrategy.Selector != nil {
-			selector, err := util.ValidatedLabelSelectorAsSelector(updateStrategy.Selector)
-			if err != nil {
-				klog.ErrorS(err, "Failed to parse SidecarSet update strategy selector", "name", sidecarSet.Name)
-				return nil, err
-			}
-			if selector.Matches(labels.Set(newPod.Labels)) {
-				klog.InfoS("New pod matches SidecarSet update strategy selector, latest revision will be injected",
-					"namespace", newPod.Namespace, "podName", newPod.Name, "sidecarSet", sidecarSet.Name)
-				return sidecarSet.DeepCopy(), nil
-			}
+		specificHistory, err := h.getSpecificHistorySidecarSet(sidecarSet, revisionInfo)
+		if err != nil {
+			return nil, err
+		}
+
+		if sidecarSet.Spec.UpdateStrategy.Paused {
+			klog.V(3).InfoS("sidecarset upgrade is paused, will inject specified revision", "sidecarSet", klog.KObj(sidecarSet))
+			return specificHistory, nil
 		}
 
-		// TODO: support 'PartitionBased' policy to inject old/new revision according to Partition
 		switch sidecarSet.Spec.InjectionStrategy.Revision.Policy {
-		case "", appsv1alpha1.AlwaysSidecarSetInjectRevisionPolicy:
-			return h.getSpecificHistorySidecarSet(sidecarSet, revisionInfo)
+		case appsv1alpha1.PartialSidecarSetInjectRevisionPolicy:
+			if updateStrategy := sidecarSet.Spec.UpdateStrategy; updateStrategy.Selector != nil {
+				selector, err := util.ValidatedLabelSelectorAsSelector(updateStrategy.Selector)
+				if err != nil {
+					klog.ErrorS(err, "Failed to parse SidecarSet update strategy selector", "sidecarSet", klog.KObj(sidecarSet))
+					return nil, err
+				}
+				if !selector.Matches(labels.Set(newPod.Labels)) {
+					// Only the Pods that are not selected by the selector will definitely be injected with the specified version of the Sidecar.
+					klog.V(3).InfoS("New pod is not updated, specified revision will be injected",
+						"pod", klog.KObj(newPod), "sidecarSet", klog.KObj(sidecarSet), "revisionInfo", revisionInfo)
+					return specificHistory, nil
+				}
+			}
+			klog.V(3).InfoS("New pod is updated, which has a probability to be injected with the latest sidecar",
+				"pod", klog.KObj(newPod), "sidecarSet", klog.KObj(sidecarSet), "partition", sidecarSet.Spec.UpdateStrategy.Partition)
+			return h.selectRevisionRandomly(specificHistory, sidecarSet.DeepCopy(), sidecarSet.Spec.UpdateStrategy.Partition)
+		default: // Always strategy
+			return specificHistory, nil
 		}
+	}
+}
 
-		return h.getSpecificHistorySidecarSet(sidecarSet, revisionInfo)
+// selectRevisionRandomly selects 'old' according to the probabilities specified by the partition.
+func (h *PodCreateHandler) selectRevisionRandomly(old, new *appsv1alpha1.SidecarSet, partition *intstr.IntOrString) (*appsv1alpha1.SidecarSet, error) {
+	if partition == nil || partition.Type == intstr.Int {
+		return new, nil
+	}
+	probability, err := util.ParsePercentageAsFloat64(partition.StrVal)
+	if err != nil {
+		return nil, err
+	}
+	if rand.Float64() <= probability {
+		return old, nil
+	} else {
+		return new, nil
 	}
 }