sidecarSet initContainer support InjectPolicy

Signed-off-by: liheng.zms <[email protected]>

apis/apps/defaults/v1alpha1.go

@@ -37,11 +37,11 @@ func SetDefaultsSidecarSet(obj *v1alpha1.SidecarSet) {
 	setSidecarSetUpdateStrategy(&obj.Spec.UpdateStrategy)
 
 	for i := range obj.Spec.InitContainers {
-		setSidecarDefaultContainer(&obj.Spec.InitContainers[i])
+		setDefaultSidecarContainer(&obj.Spec.InitContainers[i], v1alpha1.AfterAppContainerType)
 	}
 
 	for i := range obj.Spec.Containers {
-		setDefaultSidecarContainer(&obj.Spec.Containers[i])
+		setDefaultSidecarContainer(&obj.Spec.Containers[i], v1alpha1.BeforeAppContainerType)
 	}
 
 	//default setting volumes
@@ -74,9 +74,9 @@ func SetDefaultRevisionHistoryLimit(revisionHistoryLimit **int32) {
 	}
 }
 
-func setDefaultSidecarContainer(sidecarContainer *v1alpha1.SidecarContainer) {
+func setDefaultSidecarContainer(sidecarContainer *v1alpha1.SidecarContainer, injectPolicy v1alpha1.PodInjectPolicyType) {
 	if sidecarContainer.PodInjectPolicy == "" {
-		sidecarContainer.PodInjectPolicy = v1alpha1.BeforeAppContainerType
+		sidecarContainer.PodInjectPolicy = injectPolicy
 	}
 	if sidecarContainer.UpgradeStrategy.UpgradeType == "" {
 		sidecarContainer.UpgradeStrategy.UpgradeType = v1alpha1.SidecarContainerColdUpgrade
@@ -85,7 +85,7 @@ func setDefaultSidecarContainer(sidecarContainer *v1alpha1.SidecarContainer) {
 		sidecarContainer.ShareVolumePolicy.Type = v1alpha1.ShareVolumePolicyDisabled
 	}
 
-	setSidecarDefaultContainer(sidecarContainer)
+	setDefaultContainer(sidecarContainer)
 }
 
 func setSidecarSetUpdateStrategy(strategy *v1alpha1.SidecarSetUpdateStrategy) {
@@ -102,7 +102,7 @@ func setSidecarSetUpdateStrategy(strategy *v1alpha1.SidecarSetUpdateStrategy) {
 	}
 }
 
-func setSidecarDefaultContainer(sidecarContainer *v1alpha1.SidecarContainer) {
+func setDefaultContainer(sidecarContainer *v1alpha1.SidecarContainer) {
 	container := &sidecarContainer.Container
 	v1.SetDefaults_Container(container)
 	for i := range container.Ports {

pkg/webhook/pod/mutating/sidecarset.go

@@ -152,10 +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)
-	}
+	pod.Spec.InitContainers = mergeSidecarContainers(pod.Spec.InitContainers, sidecarInitContainers)
 	// 2. inject containers
 	pod.Spec.Containers = mergeSidecarContainers(pod.Spec.Containers, sidecarContainers)
 	// 3. inject volumes
@@ -287,7 +284,7 @@ func mergeSidecarContainers(origins []corev1.Container, injected []*appsv1alpha1
 		case appsv1alpha1.AfterAppContainerType:
 			afterAppContainers = append(afterAppContainers, sidecar.Container)
 		default:
-			beforeAppContainers = append(beforeAppContainers, sidecar.Container)
+			afterAppContainers = append(afterAppContainers, sidecar.Container)
 		}
 	}
 	origins = append(beforeAppContainers, origins...)

pkg/webhook/pod/mutating/sidecarset_test.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"sort"
 	"testing"
 
 	"github.com/openkruise/kruise/apis"
@@ -946,6 +947,7 @@ func TestSidecarSetTransferEnv(t *testing.T) {
 
 func testSidecarSetTransferEnv(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarSet) {
 	podIn := pod1.DeepCopy()
+	sidecarSetIn.Spec.InitContainers[0].PodInjectPolicy = ""
 	decoder := admission.NewDecoder(scheme.Scheme)
 	client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex(
 		&appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet,
@@ -958,7 +960,7 @@ func testSidecarSetTransferEnv(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarS
 		t.Fatalf("inject sidecar into pod failed, err: %v", err)
 	}
 	if len(podOut.Spec.InitContainers[1].Env) != 2 {
-		t.Fatalf("expect 2 envs but got %v", len(podOut.Spec.InitContainers[0].Env))
+		t.Fatalf("expect 2 envs but got %v", len(podOut.Spec.InitContainers[1].Env))
 	}
 	if podOut.Spec.InitContainers[1].Env[1].Value != "world2" {
 		t.Fatalf("expect env with value 'world2' but got %v", podOut.Spec.Containers[0].Env[1].Value)
@@ -1129,6 +1131,17 @@ func TestMergeSidecarContainers(t *testing.T) {
 			expectContainerLen: 4,
 			expectedContainers: []string{"new-sidecar-1", "sidecar-1", "app-container", "sidecar-2"},
 		},
+		{
+			name: "origin no sidecar, sidecar have new containers",
+			getOrigins: func() []corev1.Container {
+				return nil
+			},
+			getInjected: func() []*appsv1alpha1.SidecarContainer {
+				return sidecarContainers
+			},
+			expectContainerLen: 3,
+			expectedContainers: []string{"new-sidecar-1", "sidecar-1", "sidecar-2"},
+		},
 	}
 
 	for _, cs := range cases {
@@ -1255,6 +1268,134 @@ func TestInjectInitContainer(t *testing.T) {
 	}
 }
 
+func TestInjectInitContainerSort(t *testing.T) {
+	cases := []struct {
+		name               string
+		getOrigins         func() []corev1.Container
+		getInjected        func() []*appsv1alpha1.SidecarContainer
+		expectContainerLen int
+		expectedContainers []string
+	}{
+		{
+			name: "origins nil, inject containers(a, b, c)",
+			getOrigins: func() []corev1.Container {
+				return nil
+			},
+			getInjected: func() []*appsv1alpha1.SidecarContainer {
+				return []*appsv1alpha1.SidecarContainer{
+					{
+						Container: corev1.Container{
+							Name: "a-init",
+						},
+					},
+					{
+						Container: corev1.Container{
+							Name: "c-init",
+						},
+					},
+					{
+						Container: corev1.Container{
+							Name: "b-init",
+						},
+					},
+				}
+			},
+			expectContainerLen: 3,
+			expectedContainers: []string{"a-init", "b-init", "c-init"},
+		},
+		{
+			name: "origin init, inject containers(a, b, c)",
+			getOrigins: func() []corev1.Container {
+				return []corev1.Container{
+					{
+						Name: "app1-init",
+					},
+					{
+						Name: "app2-init",
+					},
+				}
+			},
+			getInjected: func() []*appsv1alpha1.SidecarContainer {
+				return []*appsv1alpha1.SidecarContainer{
+					{
+						Container: corev1.Container{
+							Name: "a-init",
+						},
+					},
+					{
+						Container: corev1.Container{
+							Name: "c-init",
+						},
+					},
+					{
+						Container: corev1.Container{
+							Name: "b-init",
+						},
+					},
+				}
+			},
+			expectContainerLen: 5,
+			expectedContainers: []string{"app1-init", "app2-init", "a-init", "b-init", "c-init"},
+		},
+		{
+			name: "origins nil, inject containers(a, b, c)-2",
+			getOrigins: func() []corev1.Container {
+				return []corev1.Container{
+					{
+						Name: "app1-init",
+					},
+					{
+						Name: "app2-init",
+					},
+				}
+			},
+			getInjected: func() []*appsv1alpha1.SidecarContainer {
+				return []*appsv1alpha1.SidecarContainer{
+					{
+						Container: corev1.Container{
+							Name: "a-init",
+						},
+						PodInjectPolicy: appsv1alpha1.AfterAppContainerType,
+					},
+					{
+						Container: corev1.Container{
+							Name: "c-init",
+						},
+						PodInjectPolicy: appsv1alpha1.BeforeAppContainerType,
+					},
+					{
+						Container: corev1.Container{
+							Name: "b-init",
+						},
+						PodInjectPolicy: appsv1alpha1.BeforeAppContainerType,
+					},
+				}
+			},
+			expectContainerLen: 5,
+			expectedContainers: []string{"b-init", "c-init", "app1-init", "app2-init", "a-init"},
+		},
+	}
+
+	for _, cs := range cases {
+		t.Run(cs.name, func(t *testing.T) {
+			origins := cs.getOrigins()
+			injected := cs.getInjected()
+			sort.SliceStable(injected, func(i, j int) bool {
+				return injected[i].Name < injected[j].Name
+			})
+			finals := mergeSidecarContainers(origins, injected)
+			if len(finals) != cs.expectContainerLen {
+				t.Fatalf("expect %d containers but got %v", cs.expectContainerLen, len(finals))
+			}
+			for index, cName := range cs.expectedContainers {
+				if finals[index].Name != cName {
+					t.Fatalf("expect index(%d) container(%s) but got %s", index, cName, finals[index].Name)
+				}
+			}
+		})
+	}
+}
+
 func newAdmission(op admissionv1.Operation, object, oldObject runtime.RawExtension, subResource string) admission.Request {
 	return admission.Request{
 		AdmissionRequest: newAdmissionRequest(op, object, oldObject, subResource),

pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go

@@ -90,6 +90,9 @@ func (h *SidecarSetCreateUpdateHandler) validateSidecarSet(obj *appsv1alpha1.Sid
 	if older != nil {
 		allErrs = append(allErrs, validateSidecarContainerConflict(obj.Spec.Containers, older.Spec.Containers, field.NewPath("spec.containers"))...)
 	}
+	if len(allErrs) != 0 {
+		return allErrs
+	}
 	// iterate across all containers in other sidecarsets to avoid duplication of name
 	sidecarSets := &appsv1alpha1.SidecarSetList{}
 	if err := h.Client.List(context.TODO(), sidecarSets, &client.ListOptions{}); err != nil {

test/e2e/apps/sidecarset.go

@@ -1328,6 +1328,8 @@ var _ = SIGDescribe("SidecarSet", func() {
 			sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1]
 			ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name))
 			sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn)
+			gomega.Expect(sidecarSetIn.Spec.InitContainers[0].PodInjectPolicy).To(gomega.Equal(appsv1alpha1.AfterAppContainerType))
+			gomega.Expect(sidecarSetIn.Spec.Containers[0].PodInjectPolicy).To(gomega.Equal(appsv1alpha1.BeforeAppContainerType))
 			time.Sleep(time.Second)
 
 			// create deployment