sidecarset container upgrade type should be consistent with pod (#850)

Signed-off-by: veophi <vec.g.sun@gmail.com>

apis/apps/v1alpha1/sidecarset_types.go

@@ -31,7 +31,7 @@ type SidecarSetSpec struct {
 	// otherwise, match pods in all namespaces(in cluster)
 	Namespace string `json:"namespace,omitempty"`
 
-	// Containers is the list of init containers to be injected into the selected pod
+	// InitContainers is the list of init containers to be injected into the selected pod
 	// We will inject those containers by their name in ascending order
 	// We only inject init containers when a new pod is created, it does not apply to any existing pod
 	InitContainers []SidecarContainer `json:"initContainers,omitempty"`

config/crd/bases/apps.kruise.io_sidecarsets.yaml

@@ -144,7 +144,7 @@ spec:
                   type: object
                 type: array
               initContainers:
-                description: Containers is the list of init containers to be injected
+                description: InitContainers is the list of init containers to be injected
                   into the selected pod We will inject those containers by their name
                   in ascending order We only inject init containers when a new pod
                   is created, it does not apply to any existing pod

pkg/control/sidecarcontrol/util.go

@@ -213,6 +213,35 @@ func GetPodsSortFunc(pods []*corev1.Pod, waitUpdateIndexes []int) func(i, j int)
 	}
 }
 
+func IsPodInjectedSidecarSet(pod *corev1.Pod, sidecarSet *appsv1alpha1.SidecarSet) bool {
+	sidecarSetNameStr, ok := pod.Annotations[SidecarSetListAnnotation]
+	if !ok || len(sidecarSetNameStr) == 0 {
+		return false
+	}
+	sidecarSetNames := sets.NewString(strings.Split(sidecarSetNameStr, ",")...)
+	return sidecarSetNames.Has(sidecarSet.Name)
+}
+
+func IsPodConsistentWithSidecarSet(pod *corev1.Pod, sidecarSet *appsv1alpha1.SidecarSet) bool {
+	for i := range sidecarSet.Spec.Containers {
+		container := &sidecarSet.Spec.Containers[i]
+		switch container.UpgradeStrategy.UpgradeType {
+		case appsv1alpha1.SidecarContainerHotUpgrade:
+			_, exist := GetPodHotUpgradeInfoInAnnotations(pod)[container.Name]
+			if !exist || util.GetContainer(fmt.Sprintf("%v-1", container.Name), pod) == nil ||
+				util.GetContainer(fmt.Sprintf("%v-2", container.Name), pod) == nil {
+				return false
+			}
+		default:
+			if util.GetContainer(container.Name, pod) == nil {
+				return false
+			}
+		}
+
+	}
+	return true
+}
+
 func IsInjectedSidecarContainerInPod(container *corev1.Container) bool {
 	return util.GetContainerEnvValue(container, SidecarEnvKey) == "true"
 }

pkg/controller/sidecarset/sidecarset_hotupgrade_test.go

@@ -68,6 +68,7 @@ var (
 	podHotUpgrade = &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
+				sidecarcontrol.SidecarSetListAnnotation: `test-sidecarset`,
 				//hash
 				sidecarcontrol.SidecarSetHashAnnotation: `{"test-sidecarset":{"hash":"aaa","sidecarList":["test-sidecar"]}}`,
 				//111111111

pkg/controller/sidecarset/sidecarset_pod_event_handler.go

@@ -115,7 +115,9 @@ func (p *enqueueRequestForPod) getPodMatchedSidecarSets(pod *corev1.Pod) ([]*app
 				}
 				return nil, err
 			}
-			matchedSidecarSets = append(matchedSidecarSets, sidecarSet)
+			if sidecarcontrol.IsPodConsistentWithSidecarSet(pod, sidecarSet) {
+				matchedSidecarSets = append(matchedSidecarSets, sidecarSet)
+			}
 		}
 		return matchedSidecarSets, nil
 	}

pkg/controller/sidecarset/sidecarset_processor.go

@@ -259,7 +259,8 @@ func (p *Processor) getMatchingPods(s *appsv1alpha1.SidecarSet) ([]*corev1.Pod,
 	// 3. never be injected sidecar container
 	var filteredPods []*corev1.Pod
 	for _, pod := range selectedPods {
-		if sidecarcontrol.IsActivePod(pod) && isPodInjectedSidecar(s, pod) {
+		if sidecarcontrol.IsActivePod(pod) && sidecarcontrol.IsPodInjectedSidecarSet(pod, s) &&
+			sidecarcontrol.IsPodConsistentWithSidecarSet(pod, s) {
 			filteredPods = append(filteredPods, pod)
 		}
 	}
@@ -315,12 +316,6 @@ func calculateStatus(control sidecarcontrol.SidecarControl, pods []*corev1.Pod)
 	}
 }
 
-// whether this pod has been injected sidecar container based on the sidecarSet
-func isPodInjectedSidecar(sidecarSet *appsv1alpha1.SidecarSet, pod *corev1.Pod) bool {
-	// if pod annotations contain sidecarset hash, then indicates the pod has been injected in sidecar container
-	return sidecarcontrol.GetPodSidecarSetRevision(sidecarSet.Name, pod) != ""
-}
-
 func isSidecarSetNotUpdate(s *appsv1alpha1.SidecarSet) bool {
 	if s.Spec.UpdateStrategy.Type == appsv1alpha1.NotUpdateSidecarSetStrategyType {
 		klog.V(3).Infof("sidecarSet spreading RollingUpdate config type, name: %s, type: %s", s.Name, s.Spec.UpdateStrategy.Type)

pkg/controller/sidecarset/sidecarset_processor_test.go

@@ -272,6 +272,7 @@ func TestCanUpgradePods(t *testing.T) {
 	pods := factoryPodsCommon(100, 0, sidecarSet)
 	exps := expectations.NewUpdateExpectations(sidecarcontrol.RevisionAdapterImpl)
 	for i := range pods {
+		pods[i].Annotations[sidecarcontrol.SidecarSetListAnnotation] = `test-sidecarset`
 		if i < 50 {
 			pods[i].Annotations[sidecarcontrol.SidecarSetHashWithoutImageAnnotation] = `{"test-sidecarset":{"hash":"without-aaa"}}`
 		} else {

pkg/webhook/pod/mutating/sidecarset.go

@@ -79,6 +79,12 @@ func (h *PodCreateHandler) sidecarsetMutatingPod(ctx context.Context, req admiss
 		} else if !matched {
 			continue
 		}
+		// if the sidecarSet has been injected to the pod,
+		// check whether the pod is consistent with the sidecarSet.
+		if sidecarcontrol.IsPodInjectedSidecarSet(pod, &sidecarSet) &&
+			!sidecarcontrol.IsPodConsistentWithSidecarSet(pod, &sidecarSet) {
+			continue
+		}
 		// check whether sidecarSet is active
 		// when sidecarSet is not active, it will not perform injections and upgrades process.
 		control := sidecarcontrol.New(sidecarSet.DeepCopy())

test/e2e/framework/sidecarset_utils.go

@@ -153,7 +153,7 @@ func (s *SidecarSetTester) UpdateSidecarSet(sidecarSet *appsv1alpha1.SidecarSet)
 		if updateErr == nil {
 			return nil
 		}
-		sidecarSetClone, _ = s.kc.AppsV1alpha1().SidecarSets().Get(context.TODO(), sidecarSetClone.Name, metav1.GetOptions{})
+		sidecarSetClone, _ = s.kc.AppsV1alpha1().SidecarSets().Get(context.TODO(), sidecarSet.Name, metav1.GetOptions{})
 		return updateErr
 	})
 	gomega.Expect(err).NotTo(gomega.HaveOccurred())
@@ -170,7 +170,7 @@ func (s *SidecarSetTester) UpdatePod(pod *corev1.Pod) {
 		if updateErr == nil {
 			return nil
 		}
-		podClone, _ = s.c.CoreV1().Pods(podClone.Namespace).Update(context.TODO(), podClone, metav1.UpdateOptions{})
+		podClone, _ = s.c.CoreV1().Pods(pod.Namespace).Get(context.TODO(), pod.Name, metav1.GetOptions{})
 		return updateErr
 	})
 	gomega.Expect(err).NotTo(gomega.HaveOccurred())