Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: SidecarSet injection supports Partial strategy #1856

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions apis/apps/v1alpha1/sidecarset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand All @@ -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
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions config/crd/bases/apps.kruise.io_sidecarsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 0 additions & 16 deletions pkg/controller/sidecarset/sidecarset_pod_event_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
14 changes: 14 additions & 0 deletions pkg/util/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,20 @@
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) {
furykerry marked this conversation as resolved.
Show resolved Hide resolved
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
}

Check warning on line 240 in pkg/util/tools.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/tools.go#L239-L240

Added lines #L239 - L240 were not covered by tests
return v / 100, nil
}

func EqualIgnoreHash(template1, template2 *corev1.PodTemplateSpec) bool {
t1Copy := template1.DeepCopy()
t2Copy := template2.DeepCopy()
Expand Down
36 changes: 35 additions & 1 deletion pkg/util/tools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package util

import (
"fmt"
"math"
"sync"
"testing"

Expand Down Expand Up @@ -323,7 +324,7 @@ func TestGetScaledValueFromIntOrPercent(t *testing.T) {
expectVal int
}{
{
input: intstr.FromInt(123),
input: intstr.FromInt32(123),
expectErr: false,
expectVal: 123,
},
Expand Down Expand Up @@ -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)
}
})
}
}
61 changes: 44 additions & 17 deletions pkg/webhook/pod/mutating/sidecarset.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"fmt"
"math/rand"
"sort"
"strings"

Expand All @@ -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"
Expand Down Expand Up @@ -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
}
}

Expand Down
Loading
Loading