bugfix (CollaSet): correctly check Pod update finished (#43)

fix(CollaSet): correctly check Pod update finished
KusionStack · Aug 21, 2023 · 89c4df9 · 89c4df9
1 parent 9bf0be1
commit 89c4df9
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 15 deletions.
diff --git a/pkg/controllers/collaset/synccontrol/sync_control.go b/pkg/controllers/collaset/synccontrol/sync_control.go
@@ -460,11 +460,11 @@ func (sc *RealSyncControl) Update(set *appsv1alpha1.CollaSet, podWrapers []*coll
 	}
 
 	// try to finish all Pods'PodOpsLifecycle if its update is finished.
-	succCount, err = controllerutils.SlowStartBatch(len(podWrapers), controllerutils.SlowStartInitialBatchSize, false, func(i int, _ error) error {
-		podInfo := podWrapers[i]
+	succCount, err = controllerutils.SlowStartBatch(len(podUpdateInfos), controllerutils.SlowStartInitialBatchSize, false, func(i int, _ error) error {
+		podInfo := podUpdateInfos[i]
 
 		// check Pod update is finished or not
-		finished, msg, err := updater.GetPodUpdateFinishStatus(podInfo.Pod)
+		finished, msg, err := updater.GetPodUpdateFinishStatus(podInfo)
 		if err != nil {
 			return fmt.Errorf("fail to get pod %s/%s update finished: %s", podInfo.Namespace, podInfo.Name, err)
 		}

diff --git a/pkg/controllers/collaset/synccontrol/update.go b/pkg/controllers/collaset/synccontrol/update.go
@@ -139,7 +139,7 @@ func (o orderByDefault) Less(i, j int) bool {
 
 type PodUpdater interface {
 	AnalyseAndGetUpdatedPod(cls *appsv1alpha1.CollaSet, revision *appsv1.ControllerRevision, podUpdateInfo *PodUpdateInfo) (inPlaceUpdateSupport bool, onlyMetadataChanged bool, updatedPod *corev1.Pod, err error)
-	GetPodUpdateFinishStatus(pod *corev1.Pod) (bool, string, error)
+	GetPodUpdateFinishStatus(podUpdateInfo *PodUpdateInfo) (bool, string, error)
 }
 
 func newPodUpdater(cls *appsv1alpha1.CollaSet) PodUpdater {
@@ -263,25 +263,29 @@ func (u *InPlaceIfPossibleUpdater) diffPod(currentPod, updatedPod *corev1.Pod) (
 	return true, false
 }
 
-func (u *InPlaceIfPossibleUpdater) GetPodUpdateFinishStatus(pod *corev1.Pod) (finished bool, msg string, err error) {
-	if pod.Status.ContainerStatuses == nil {
+func (u *InPlaceIfPossibleUpdater) GetPodUpdateFinishStatus(podUpdateInfo *PodUpdateInfo) (finished bool, msg string, err error) {
+	if !podUpdateInfo.IsUpdatedRevision {
+		return false, "not updated revision", nil
+	}
+
+	if podUpdateInfo.Status.ContainerStatuses == nil {
 		return false, "no container status", nil
 	}
 
-	if pod.Spec.Containers == nil {
+	if podUpdateInfo.Spec.Containers == nil {
 		return false, "no container spec", nil
 	}
 
-	if len(pod.Spec.Containers) != len(pod.Status.ContainerStatuses) {
+	if len(podUpdateInfo.Spec.Containers) != len(podUpdateInfo.Status.ContainerStatuses) {
 		return false, "container status number does not match", nil
 	}
 
-	if pod.Annotations == nil {
+	if podUpdateInfo.Annotations == nil {
 		return true, "no annotations for last container status", nil
 	}
 
 	podLastState := &PodStatus{}
-	if lastStateJson, exist := pod.Annotations[appsv1alpha1.LastPodStatusAnnotationKey]; !exist {
+	if lastStateJson, exist := podUpdateInfo.Annotations[appsv1alpha1.LastPodStatusAnnotationKey]; !exist {
 		return true, "no pod last state annotation", nil
 	} else if err := json.Unmarshal([]byte(lastStateJson), podLastState); err != nil {
 		msg := fmt.Sprintf("malformat pod last state annotation [%s]: %s", lastStateJson, err)
@@ -293,12 +297,12 @@ func (u *InPlaceIfPossibleUpdater) GetPodUpdateFinishStatus(pod *corev1.Pod) (fi
 	}
 
 	imageMapping := map[string]string{}
-	for _, containerSpec := range pod.Spec.Containers {
+	for _, containerSpec := range podUpdateInfo.Spec.Containers {
 		imageMapping[containerSpec.Name] = containerSpec.Image
 	}
 
 	imageIdMapping := map[string]string{}
-	for _, containerStatus := range pod.Status.ContainerStatuses {
+	for _, containerStatus := range podUpdateInfo.Status.ContainerStatuses {
 		imageIdMapping[containerStatus.Name] = containerStatus.ImageID
 	}
 
@@ -335,7 +339,7 @@ func (u *InPlaceOnlyPodUpdater) AnalyseAndGetUpdatedPod(_ *appsv1alpha1.CollaSet
 	return
 }
 
-func (u *InPlaceOnlyPodUpdater) GetPodUpdateFinishStatus(_ *corev1.Pod) (finished bool, msg string, err error) {
+func (u *InPlaceOnlyPodUpdater) GetPodUpdateFinishStatus(_ *PodUpdateInfo) (finished bool, msg string, err error) {
 	return
 }
 
@@ -346,7 +350,7 @@ func (u *RecreatePodUpdater) AnalyseAndGetUpdatedPod(_ *appsv1alpha1.CollaSet, _
 	return false, false, nil, nil
 }
 
-func (u *RecreatePodUpdater) GetPodUpdateFinishStatus(_ *corev1.Pod) (finished bool, msg string, err error) {
+func (u *RecreatePodUpdater) GetPodUpdateFinishStatus(podUpdateInfo *PodUpdateInfo) (finished bool, msg string, err error) {
 	// Recreate policy alway treat Pod as update finished
-	return true, "", nil
+	return podUpdateInfo.IsUpdatedRevision, "", nil
 }