Skip to content

Commit

Permalink
sidecarset support k8s 1.28 sidecarContainers (#1613)
Browse files Browse the repository at this point in the history
Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com>
  • Loading branch information
zmberg committed May 10, 2024
1 parent b969432 commit 1bc8d85
Show file tree
Hide file tree
Showing 10 changed files with 504 additions and 58 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ test/e2e/generated/bindata.go
.vscode

.DS_Store

vendor/
15 changes: 14 additions & 1 deletion pkg/control/sidecarcontrol/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ func SidecarSetHashWithoutImage(sidecarSet *appsv1alpha1.SidecarSet) (string, er
for i := range ss.Spec.Containers {
ss.Spec.Containers[i].Image = ""
}
for i := range ss.Spec.InitContainers {
ss.Spec.InitContainers[i].Image = ""
}
encoded, err := encodeSidecarSet(ss)
if err != nil {
return "", err
Expand All @@ -53,7 +56,17 @@ func SidecarSetHashWithoutImage(sidecarSet *appsv1alpha1.SidecarSet) (string, er
func encodeSidecarSet(sidecarSet *appsv1alpha1.SidecarSet) (string, error) {
// json.Marshal sorts the keys in a stable order in the encoding
m := map[string]interface{}{"containers": sidecarSet.Spec.Containers}
//m["initContainers"] = sidecarSet.Spec.InitContainers
// when k8s 1.28, if initContainer restartPolicy = Always, indicates it is sidecar container, so the hash needs to contain it.
initContainer := make([]appsv1alpha1.SidecarContainer, 0)
for i := range sidecarSet.Spec.InitContainers {
container := &sidecarSet.Spec.InitContainers[i]
if IsSidecarContainer(container.Container) {
initContainer = append(initContainer, *container)
}
}
if len(initContainer) > 0 {
m["initContainers"] = sidecarSet.Spec.InitContainers
}
data, err := json.Marshal(m)
if err != nil {
return "", err
Expand Down
240 changes: 186 additions & 54 deletions pkg/control/sidecarcontrol/hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,78 +8,210 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var always = corev1.ContainerRestartPolicyAlways

func TestSidecarSetHash(t *testing.T) {
sidecarSet := &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sidecar-set",
cases := []struct {
name string
getSidecarSet func() *appsv1alpha1.SidecarSet
expectHash string
}{
{
name: "containers",
getSidecarSet: func() *appsv1alpha1.SidecarSet {
return &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sidecar-set",
},
Spec: appsv1alpha1.SidecarSetSpec{
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
},
},
},
},
}
},
expectHash: "w26c4x8fz245642fdv499b464248f974xddx4x55z5dw55bc6x66464fxz77dc78",
},
Spec: appsv1alpha1.SidecarSetSpec{
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
{
name: "containers and initContainers",
getSidecarSet: func() *appsv1alpha1.SidecarSet {
return &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sidecar-set",
},
Spec: appsv1alpha1.SidecarSetSpec{
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
},
},
},
InitContainers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
},
},
},
},
},
}
},
expectHash: "w26c4x8fz245642fdv499b464248f974xddx4x55z5dw55bc6x66464fxz77dc78",
},
{
name: "containers and initContainers with restartPolicy=Always",
getSidecarSet: func() *appsv1alpha1.SidecarSet {
return &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sidecar-set",
},
Spec: appsv1alpha1.SidecarSetSpec{
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
},
},
},
InitContainers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
RestartPolicy: &always,
},
},
},
},
}
},
expectHash: "4xwx4d4844vd4v9x79wb4xbxf4xb29475cc4446v8cz2c2f2f5c5bw448vd42z8w",
},
}

hash, err := SidecarSetHash(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

if hash == "" {
t.Fatalf("Expected non-empty hash")
}

// Change sidecar set and expect different hash
sidecarSet.Spec.Containers[0].Image = "new-image"
newHash, err := SidecarSetHash(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
for _, cs := range cases {
t.Run(cs.name, func(t *testing.T) {
sidecarSet := cs.getSidecarSet()
hash1, err := SidecarSetHash(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
} else if hash1 == "" {
t.Fatalf("Expected non-empty hash")
}
if cs.expectHash != hash1 {
t.Fatalf("expect(%s), but get(%s)", cs.expectHash, hash1)
}

if newHash == hash {
t.Fatalf("Expected different hashes for different SidecarSets")
// Change sidecar set and expect different hash
sidecarSet.Spec.Containers[0].Image = "new-image"
newHash, err := SidecarSetHash(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
} else if newHash == hash1 {
t.Fatalf("Expected different hashes for different SidecarSets")
}
})
}
}

func TestSidecarSetHashWithoutImage(t *testing.T) {
sidecarSet := &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sidecar-set",
cases := []struct {
name string
getSidecarSet func() *appsv1alpha1.SidecarSet
expectHash string
}{
{
name: "containers and initContainers",
getSidecarSet: func() *appsv1alpha1.SidecarSet {
return &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sidecar-set",
},
Spec: appsv1alpha1.SidecarSetSpec{
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
},
},
},
InitContainers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
},
},
},
},
}
},
expectHash: "8wzddb4dvv9c6x8zdc77z4z75987424f457dfv6724ddw6zbdx467wz5x24fc759",
},
Spec: appsv1alpha1.SidecarSetSpec{
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
{
name: "containers and initContainers with restartPolicy=Always",
getSidecarSet: func() *appsv1alpha1.SidecarSet {
return &appsv1alpha1.SidecarSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sidecar-set",
},
Spec: appsv1alpha1.SidecarSetSpec{
Containers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
},
},
},
InitContainers: []appsv1alpha1.SidecarContainer{
{
Container: corev1.Container{
Name: "container1",
Image: "test-image",
RestartPolicy: &always,
},
},
},
},
},
}
},
expectHash: "5725fw8bwbx249bw57v5892c847dzf48bww9zb7c86xb95264fdz26654847b2c8",
},
}

hash, err := SidecarSetHashWithoutImage(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

if hash == "" {
t.Fatalf("Expected non-empty hash")
}

// Change sidecar set image and expect same hash
sidecarSet.Spec.Containers[0].Image = "new-image"
newHash, err := SidecarSetHashWithoutImage(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
for _, cs := range cases {
t.Run(cs.name, func(t *testing.T) {
sidecarSet := cs.getSidecarSet()
hash1, err := SidecarSetHashWithoutImage(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
} else if hash1 == "" {
t.Fatalf("Expected non-empty hash")
}
if cs.expectHash != hash1 {
t.Fatalf("expect(%s), but get(%s)", cs.expectHash, hash1)
}

if newHash != hash {
t.Fatalf("Expected same hashes for SidecarSets with different images")
// Change sidecar set and expect different hash
sidecarSet.Spec.Containers[0].Image = "new-image"
sidecarSet.Spec.InitContainers[0].Image = "new-image"
newHash, err := SidecarSetHashWithoutImage(sidecarSet)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
} else if newHash != hash1 {
t.Fatalf("Expected same hashes for different SidecarSets")
}
})
}
}
11 changes: 11 additions & 0 deletions pkg/control/sidecarcontrol/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,9 @@ func UpdatePodSidecarSetHash(pod *corev1.Pod, sidecarSet *appsv1alpha1.SidecarSe
for _, sidecar := range sidecarSet.Spec.Containers {
sidecarList.Insert(sidecar.Name)
}
for _, sidecar := range sidecarSet.Spec.InitContainers {
sidecarList.Insert(sidecar.Name)
}

sidecarSetHash[sidecarSet.Name] = SidecarSetUpgradeSpec{
UpdateTimestamp: metav1.Now(),
Expand Down Expand Up @@ -564,3 +567,11 @@ func matchRegKey(key string, regs []*regexp.Regexp) bool {
}
return false
}

// IsSidecarContainer check whether initContainer is sidecar container in k8s 1.28.
func IsSidecarContainer(container corev1.Container) bool {
if container.RestartPolicy != nil && *container.RestartPolicy == corev1.ContainerRestartPolicyAlways {
return true
}
return false
}
16 changes: 14 additions & 2 deletions pkg/webhook/pod/mutating/sidecarset.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func (h *PodCreateHandler) sidecarsetMutatingPod(ctx context.Context, req admiss
sort.SliceStable(sidecarInitContainers, func(i, j int) bool {
return sidecarInitContainers[i].Name < sidecarInitContainers[j].Name
})
// TODO, implement PodInjectPolicy for initContainers
for _, initContainer := range sidecarInitContainers {
pod.Spec.InitContainers = append(pod.Spec.InitContainers, initContainer.Container)
}
Expand Down Expand Up @@ -368,11 +369,13 @@ func buildSidecars(isUpdated bool, pod *corev1.Pod, oldPod *corev1.Pod, matchedS
}

isInjecting := false
sidecarList := sets.NewString()
//process initContainers
//only when created pod, inject initContainer and pullSecrets
if !isUpdated {
for i := range sidecarSet.Spec.InitContainers {
initContainer := &sidecarSet.Spec.InitContainers[i]
sidecarList.Insert(initContainer.Name)
// volumeMounts that injected into sidecar container
// when volumeMounts SubPathExpr contains expansions, then need copy container EnvVars(injectEnvs)
injectedMounts, injectedEnvs := sidecarcontrol.GetInjectedVolumeMountsAndEnvs(control, initContainer, pod)
Expand All @@ -393,13 +396,22 @@ func buildSidecars(isUpdated bool, pod *corev1.Pod, oldPod *corev1.Pod, matchedS
// merged Env from sidecar.Env and transfer envs
initContainer.Env = util.MergeEnvVar(initContainer.Env, transferEnvs)
isInjecting = true
sidecarInitContainers = append(sidecarInitContainers, initContainer)

// when sidecar container UpgradeStrategy is HotUpgrade
if sidecarcontrol.IsSidecarContainer(initContainer.Container) && sidecarcontrol.IsHotUpgradeContainer(initContainer) {
hotContainers, annotations := injectHotUpgradeContainers(hotUpgradeWorkInfo, initContainer)
sidecarInitContainers = append(sidecarInitContainers, hotContainers...)
for k, v := range annotations {
injectedAnnotations[k] = v
}
} else {
sidecarInitContainers = append(sidecarInitContainers, initContainer)
}
}
//process imagePullSecrets
sidecarSecrets = append(sidecarSecrets, sidecarSet.Spec.ImagePullSecrets...)
}

sidecarList := sets.NewString()
//process containers
for i := range sidecarSet.Spec.Containers {
sidecarContainer := &sidecarSet.Spec.Containers[i]
Expand Down
Loading

0 comments on commit 1bc8d85

Please sign in to comment.