Skip to content

Commit

Permalink
On pod creation, if a new pod matches the SidecarSet update strategy …
Browse files Browse the repository at this point in the history
…selector, the latest revision rather than that specified in the sidecarset.spec.injectionStrategy will be injected. (#1689)

Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
Co-authored-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
  • Loading branch information
AiRanthem and AiRanthem committed Sep 10, 2024
1 parent be1a79e commit 2d992bf
Show file tree
Hide file tree
Showing 5 changed files with 274 additions and 6 deletions.
10 changes: 8 additions & 2 deletions apis/apps/v1alpha1/sidecarset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,17 @@ type SidecarSetUpdateStrategy struct {
Type SidecarSetUpdateStrategyType `json:"type,omitempty"`

// Paused indicates that the SidecarSet is paused to update the injected pods,
// but it don't affect the webhook inject sidecar container into the newly created pods.
// default is false
// For the impact on the injection behavior for newly created Pods, please refer to the comments of Selector.
Paused bool `json:"paused,omitempty"`

// If selector is not nil, this upgrade will only update the selected pods.
//
// Starting from Kruise 1.8.0, the updateStrategy.Selector affects the version of the Sidecar container
// 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
15 changes: 11 additions & 4 deletions config/crd/bases/apps.kruise.io_sidecarsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,7 @@ spec:
paused:
description: |-
Paused indicates that the SidecarSet is paused to update the injected pods,
but it don't affect the webhook inject sidecar container into the newly created pods.
default is false
For the impact on the injection behavior for newly created Pods, please refer to the comments of Selector.
type: boolean
priorityStrategy:
description: |-
Expand Down Expand Up @@ -527,8 +526,16 @@ spec:
type: object
type: array
selector:
description: If selector is not nil, this upgrade will only update
the selected pods.
description: |-
If selector is not nil, this upgrade will only update the selected pods.
Starting from Kruise 1.8.0, the updateStrategy.Selector affects the version of the Sidecar container
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: 16 additions & 0 deletions pkg/webhook/pod/mutating/sidecarset.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
utilclient "github.com/openkruise/kruise/pkg/util/client"
"github.com/openkruise/kruise/pkg/util/fieldindex"
"github.com/openkruise/kruise/pkg/util/history"
"k8s.io/apimachinery/pkg/labels"

admissionv1 "k8s.io/api/admission/v1"
apps "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -205,6 +206,21 @@ 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
}
}

// TODO: support 'PartitionBased' policy to inject old/new revision according to Partition
switch sidecarSet.Spec.InjectionStrategy.Revision.Policy {
case "", appsv1alpha1.AlwaysSidecarSetInjectRevisionPolicy:
Expand Down
128 changes: 128 additions & 0 deletions pkg/webhook/pod/mutating/sidecarset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,134 @@ func TestSidecarSetPodInjectPolicy(t *testing.T) {
testSidecarSetPodInjectPolicy(t, sidecarSetIn)
}

func TestCanarySidecarSetInjection(t *testing.T) {
podStable := pod1.DeepCopy()
podCanary := podStable.DeepCopy()
podCanary.Labels["canary"] = "true"

stableImage := "sidecar-image:stable"
canaryImage := "sidecar-image:canary"
sidecarSetName := "sidecarset1"

// a canary sidecarset contains both updatestrategy.selector and injectionstrategy.revision
revisionID := fmt.Sprintf("%s-12345", sidecarSetName)
sidecarSetCanary := &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: sidecarSetName,
},
Spec: appsv1alpha1.SidecarSetSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "demo",
},
},
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "sidecar",
Image: canaryImage,
},
},
},
UpdateStrategy: appsv1alpha1.SidecarSetUpdateStrategy{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"canary": "true"},
},
},
InjectionStrategy: appsv1alpha1.SidecarSetInjectionStrategy{
Revision: &appsv1alpha1.SidecarSetInjectRevision{
CustomVersion: &revisionID,
Policy: appsv1alpha1.AlwaysSidecarSetInjectRevisionPolicy,
},
},
},
}

specRaw, _ := json.Marshal(map[string]interface{}{
"spec": map[string]interface{}{
"$patch": "replace",
"containers": []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "sidecar",
Image: stableImage,
},
},
},
},
})

revision := &apps.ControllerRevision{
ObjectMeta: metav1.ObjectMeta{
Namespace: webhookutil.GetNamespace(),
Name: revisionID,
Labels: map[string]string{
sidecarcontrol.SidecarSetKindName: sidecarSet1.GetName(),
appsv1alpha1.SidecarSetCustomVersionLabel: revisionID,
},
},
Data: runtime.RawExtension{
Raw: specRaw,
},
}

stablePod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod-stable",
Labels: map[string]string{
"app": "demo",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "app",
Image: "demo",
},
},
},
}

canaryPod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod-canary",
Labels: map[string]string{
"app": "demo",
"canary": "true",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "app",
Image: "demo",
},
},
},
}

decoder := admission.NewDecoder(scheme.Scheme)
testClient := fake.NewClientBuilder().WithObjects(revision, sidecarSetCanary).WithIndex(
&appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet,
).Build()
podHandler := &PodCreateHandler{Decoder: decoder, Client: testClient}
req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "")
if _, err := podHandler.sidecarsetMutatingPod(context.Background(), req, stablePod); err != nil {
t.Fatalf("failed to mutating pod, err: %v", err)
}
if _, err := podHandler.sidecarsetMutatingPod(context.Background(), req, canaryPod); err != nil {
t.Fatalf("failed to mutating pod, err: %v", err)
}
if len(stablePod.Spec.Containers) != 2 || stablePod.Spec.Containers[1].Image != stableImage {
t.Fatalf("inject stable sidecar failed")
}
fmt.Printf("stable sidecar image: %s\n", stablePod.Spec.Containers[1].Image)
if len(canaryPod.Spec.Containers) != 2 || canaryPod.Spec.Containers[1].Image != canaryImage {
t.Fatalf("inject canary sidecar failed")
}
fmt.Printf("canary sidecar image: %s\n", canaryPod.Spec.Containers[1].Image)
}

func testSidecarSetPodInjectPolicy(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarSet) {
podIn := pod1.DeepCopy()
decoder := admission.NewDecoder(scheme.Scheme)
Expand Down
111 changes: 111 additions & 0 deletions test/e2e/apps/sidecarset.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/controller/history"
utilpointer "k8s.io/utils/pointer"
"k8s.io/utils/ptr"
)

var _ = SIGDescribe("SidecarSet", func() {
Expand Down Expand Up @@ -1585,5 +1586,115 @@ var _ = SIGDescribe("SidecarSet", func() {
gomega.Expect(hash[sidecarSetIn.Name].SidecarSetControllerRevision).To(gomega.Equal(list[5].Name))
ginkgo.By("sidecarSet InjectionStrategy.Revision check done")
})

framework.ConformanceIt("sidecarSet inject pod sidecar during canary upgrades", func() {
// create sidecarSet
nginxName := func(tag string) string {
return fmt.Sprintf("nginx:%s", tag)
}
stableTag, canaryTag := "1.20.1", "1.21.1"
sidecarSetIn := tester.NewBaseSidecarSet(ns)
sidecarSetIn.Labels = map[string]string{
appsv1alpha1.SidecarSetCustomVersionLabel: "0",
}
sidecarSetIn.SetName("e2e-test-for-canary-upgrade")
sidecarSetIn.Spec.Containers[0].Image = nginxName(stableTag)
sidecarSetIn.Spec.UpdateStrategy.Type = appsv1alpha1.RollingUpdateSidecarSetStrategyType
ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name))
sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn)
time.Sleep(time.Second)

// create deployment
deploymentStable, deploymentCanary := tester.NewBaseDeployment(ns), tester.NewBaseDeployment(ns)
deploymentStable.Name += "-stable"
deploymentStable.Spec.Replicas = ptr.To(int32(1))
deploymentStable.Spec.Template.Labels["version"] = "stable"
deploymentStable.Spec.Template.Spec.Containers[0].ImagePullPolicy = corev1.PullIfNotPresent
deploymentStable.Spec.Template.Spec.TerminationGracePeriodSeconds = ptr.To(int64(0))
ginkgo.By(fmt.Sprintf("Creating Deployment(%s.%s)", deploymentStable.Namespace, deploymentStable.Name))
deploymentCanary.Name += "-canary"
deploymentCanary.Spec.Replicas = ptr.To(int32(1))
deploymentCanary.Spec.Template.Labels["version"] = "canary"
deploymentCanary.Spec.Template.Spec.Containers[0].ImagePullPolicy = corev1.PullIfNotPresent
deploymentCanary.Spec.Template.Spec.TerminationGracePeriodSeconds = ptr.To(int64(0))
ginkgo.By(fmt.Sprintf("Creating Deployment(%s.%s)", deploymentCanary.Namespace, deploymentCanary.Name))
tester.CreateDeployment(deploymentStable)
tester.CreateDeployment(deploymentCanary)
tester.WaitForDeploymentRunning(deploymentStable)
tester.WaitForDeploymentRunning(deploymentCanary)

calculateSidecarImages := func(pods []*corev1.Pod) (stableNum, canaryNum int) {
for _, pod := range pods {
for _, container := range pod.Spec.Containers {
if container.Image == nginxName(stableTag) {
stableNum++
}
if container.Image == nginxName(canaryTag) {
canaryNum++
}
}
}
return
}
var stableNum, canaryNum int

// check sidecar original revisions
podStable, err := tester.GetSelectorPods(ns, &metav1.LabelSelector{MatchLabels: deploymentStable.Spec.Template.Labels})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
gomega.Expect(podStable).To(gomega.HaveLen(1))
stableNum, canaryNum = calculateSidecarImages(podStable)
gomega.Expect(stableNum).To(gomega.Equal(1))
gomega.Expect(canaryNum).To(gomega.Equal(0))
podCanary, err := tester.GetSelectorPods(ns, &metav1.LabelSelector{MatchLabels: deploymentCanary.Spec.Template.Labels})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
gomega.Expect(podCanary).To(gomega.HaveLen(1))
stableNum, canaryNum = calculateSidecarImages(podCanary)
gomega.Expect(stableNum).To(gomega.Equal(1))
gomega.Expect(canaryNum).To(gomega.Equal(0))
ginkgo.By(fmt.Sprintf("All pods are injected with a stable sidecar"))

// canary update sidecarSet
sidecarSetIn.Spec.Containers[0].Image = nginxName(canaryTag)
sidecarSetIn.Spec.Containers[0].Env = []corev1.EnvVar{{Name: "SOME_ENV", Value: "SOME_VAL"}} // make in-place upgrade impossible
sidecarSetIn.Spec.UpdateStrategy.Selector = &metav1.LabelSelector{
MatchLabels: map[string]string{"version": "canary"},
}
sidecarSetIn.Spec.InjectionStrategy.Revision = &appsv1alpha1.SidecarSetInjectRevision{CustomVersion: ptr.To("0")}
sidecarSetIn.Labels = map[string]string{
appsv1alpha1.SidecarSetCustomVersionLabel: "1",
}
tester.UpdateSidecarSet(sidecarSetIn)
expect := &appsv1alpha1.SidecarSetStatus{
MatchedPods: 2,
UpdatedPods: 0,
UpdatedReadyPods: 0,
ReadyPods: 2,
}
tester.WaitForSidecarSetUpgradeComplete(sidecarSetIn, expect)
ginkgo.By(fmt.Sprintf("Sidecarset Updated"))

// scale up deployments
deploymentStable.Spec.Replicas = ptr.To(int32(2))
deploymentCanary.Spec.Replicas = ptr.To(int32(2))
tester.UpdateDeployment(deploymentStable)
tester.UpdateDeployment(deploymentCanary)
tester.WaitForDeploymentRunning(deploymentStable)
tester.WaitForDeploymentRunning(deploymentCanary)

// check sidecar versions
podStable, err = tester.GetSelectorPods(ns, &metav1.LabelSelector{MatchLabels: deploymentStable.Spec.Template.Labels})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
gomega.Expect(podStable).To(gomega.HaveLen(2))
stableNum, canaryNum = calculateSidecarImages(podStable)
gomega.Expect(stableNum).To(gomega.Equal(2))
gomega.Expect(canaryNum).To(gomega.Equal(0))
podCanary, err = tester.GetSelectorPods(ns, &metav1.LabelSelector{MatchLabels: deploymentCanary.Spec.Template.Labels})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
gomega.Expect(podCanary).To(gomega.HaveLen(2))
stableNum, canaryNum = calculateSidecarImages(podCanary)
gomega.Expect(stableNum).To(gomega.Equal(1))
gomega.Expect(canaryNum).To(gomega.Equal(1))
ginkgo.By(fmt.Sprintf("All pods are injected with a suitable sidecar"))
})
})
})

0 comments on commit 2d992bf

Please sign in to comment.