fix-inplace-update-with-only-metadata-changed

go.mod

@@ -23,25 +23,26 @@ require (
 	k8s.io/kubernetes v0.0.0-00010101000000-000000000000
 	k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
 	kusionstack.io/kube-api v0.7.4-0.20250909095208-496f60eea9b5
-	kusionstack.io/kube-utils v0.2.1-0.20250908100835-947051067cbe
+	kusionstack.io/kube-utils v0.2.1-0.20251031115408-8f00ba110277
 	kusionstack.io/resourceconsist v0.0.1
 	sigs.k8s.io/controller-runtime v0.17.3
 )
 
 require (
+	github.com/Azure/go-autorest/autorest v0.11.18 // indirect
+	github.com/Azure/go-autorest/autorest/adal v0.9.13 // indirect
+	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
+	github.com/Azure/go-autorest/logger v0.2.1 // indirect
+	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
 	github.com/docker/distribution v2.8.2+incompatible // indirect
+	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
 	github.com/samber/lo v1.47.0 // indirect
 )
 
 require (
 	cloud.google.com/go v0.65.0 // indirect
 	github.com/Azure/go-autorest v14.2.0+incompatible // indirect
-	github.com/Azure/go-autorest/autorest v0.11.18 // indirect
-	github.com/Azure/go-autorest/autorest/adal v0.9.13 // indirect
-	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
-	github.com/Azure/go-autorest/logger v0.2.1 // indirect
-	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
 	github.com/alibabacloud-go/alibabacloud-gateway-spi v0.0.4 // indirect
 	github.com/alibabacloud-go/darabonba-openapi/v2 v2.0.4 // indirect
 	github.com/alibabacloud-go/debug v0.0.0-20190504072949-9472017b5c68 // indirect
@@ -63,7 +64,6 @@ require (
 	github.com/go-logr/zapr v1.2.4 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
-	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/googleapis/gnostic v0.5.5 // indirect

go.sum

@@ -1153,8 +1153,8 @@ k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 h1:M3sRQVHv7vB20Xc2ybTt7ODCeFj6J
 k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 kusionstack.io/kube-api v0.7.4-0.20250909095208-496f60eea9b5 h1:/jbKYMeXiYnuxyJQs72MoL6vxVoY15FX/7tCKWljscY=
 kusionstack.io/kube-api v0.7.4-0.20250909095208-496f60eea9b5/go.mod h1:e1jtrQH2LK5fD2nTyfIXG6nYrYbU8VXShRxTRwVPaLk=
-kusionstack.io/kube-utils v0.2.1-0.20250908100835-947051067cbe h1:vLIXXJCL3ryDj4KZzUMlXUENhPcv0cfF77PNwjrttcU=
-kusionstack.io/kube-utils v0.2.1-0.20250908100835-947051067cbe/go.mod h1:pThYvlUgm58SXyax5nxHRln6EW7l6sqkNmlUqa/6OR0=
+kusionstack.io/kube-utils v0.2.1-0.20251031115408-8f00ba110277 h1:jsaRBLVOfkpAgPz1JA0Cp+fs7tqi+9Uccu0JJlCeaO0=
+kusionstack.io/kube-utils v0.2.1-0.20251031115408-8f00ba110277/go.mod h1:KEHTfo1Y8SWMODnckF6daO2cSIW0FJ8fkk8PBA5O2GU=
 kusionstack.io/resourceconsist v0.0.1 h1:+k/jriq5Ld7fQUYfWSMGynz/FesHtl3Rk2fmQPjBe0g=
 kusionstack.io/resourceconsist v0.0.1/go.mod h1:816xS/fY6EOUbPFjXIWW/TGs8/YE46qP4ElKeIiwFdU=
 modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=

pkg/controllers/collaset/collaset_controller_test.go

@@ -70,6 +70,109 @@ var (
 )
 
 var _ = Describe("collaset controller", func() {
+	It("inplaceUpdate with only metadata changed", func() {
+		testcase := "test-inplace-update-with-only-metadata-changed"
+		Expect(createNamespace(c, testcase)).Should(BeNil())
+
+		cs := &appsv1alpha1.CollaSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: testcase,
+				Name:      "foo",
+			},
+			Spec: appsv1alpha1.CollaSetSpec{
+				Replicas: int32Pointer(2),
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "foo",
+					},
+				},
+				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: map[string]string{
+							"app": "foo",
+						},
+					},
+					Spec: corev1.PodSpec{
+						Containers: []corev1.Container{
+							{
+								Name:  "foo",
+								Image: "nginx:v1",
+							},
+						},
+					},
+				},
+				UpdateStrategy: appsv1alpha1.UpdateStrategy{
+					OperationDelaySeconds: int32Pointer(1),
+					PodUpdatePolicy:       appsv1alpha1.CollaSetInPlaceIfPossiblePodUpdateStrategyType,
+				},
+			},
+		}
+		Expect(c.Create(context.TODO(), cs)).Should(BeNil())
+
+		podList := &corev1.PodList{}
+		Eventually(func() bool {
+			Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
+			return len(podList.Items) == 2
+		}, 5*time.Second, 1*time.Second).Should(BeTrue())
+		Expect(c.Get(context.TODO(), types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}, cs)).Should(BeNil())
+		Expect(expectedStatusReplicas(c, cs, 0, 0, 0, 2, 2, 0, 0, 0)).Should(BeNil())
+
+		observedGeneration := cs.Status.ObservedGeneration
+		// update CollaSet template metadata
+		Expect(updateCollaSetWithRetry(c, cs.Namespace, cs.Name, func(cls *appsv1alpha1.CollaSet) bool {
+			cls.Spec.Template.Annotations = map[string]string{
+				"test": "v1",
+			}
+			return true
+		})).Should(BeNil())
+		Eventually(func() bool {
+			Expect(c.Get(context.TODO(), types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}, cs)).Should(BeNil())
+			return cs.Status.ObservedGeneration != observedGeneration
+		}, 5*time.Second, 1*time.Second).Should(BeTrue())
+
+		// allow origin pod to update
+		Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
+		for i := range podList.Items {
+			pod := &podList.Items[i]
+			Expect(updatePodWithRetry(c, pod.Namespace, pod.Name, func(pod *corev1.Pod) bool {
+				labelOperate := fmt.Sprintf("%s/%s", appsv1alpha1.PodOperateLabelPrefix, collasetutils.UpdateOpsLifecycleAdapter.GetID())
+				pod.Labels[labelOperate] = fmt.Sprintf("%d", time.Now().UnixNano())
+				return true
+			})).Should(BeNil())
+		}
+
+		// waiting for update finished
+		Eventually(func() error {
+			return expectedStatusReplicas(c, cs, 0, 0, 0, 2, 2, 2, 0, 0)
+		}, 10*time.Second, 1*time.Second).Should(BeNil())
+
+		// mock container status
+		Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
+		for i := range podList.Items {
+			Expect(updatePodStatusWithRetry(c, podList.Items[i].Namespace, podList.Items[i].Name, func(pod *corev1.Pod) bool {
+				pod.Status.ContainerStatuses = []corev1.ContainerStatus{
+					{
+						Name:    "foo",
+						ImageID: "nginx:v1",
+					},
+				}
+				return true
+			})).Should(BeNil())
+		}
+
+		// check pods update are finished
+		Eventually(func() bool {
+			Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
+			for i := range podList.Items {
+				pod := &podList.Items[i]
+				if podopslifecycle.IsDuringOps(collasetutils.UpdateOpsLifecycleAdapter, pod) {
+					return false
+				}
+			}
+			return true
+		}, 10*time.Second, 1*time.Second).Should(BeTrue())
+	})
+
 	It("replace pod with update", func() {
 		for _, updateStrategy := range []appsv1alpha1.PodUpdateStrategyType{appsv1alpha1.CollaSetRecreatePodUpdateStrategyType, appsv1alpha1.CollaSetInPlaceIfPossiblePodUpdateStrategyType, appsv1alpha1.CollaSetReplacePodUpdateStrategyType} {
 			testcase := fmt.Sprintf("test-replace-pod-with-%s-update", strings.ToLower(string(updateStrategy)))

pkg/controllers/collaset/synccontrol/update.go

@@ -591,10 +591,9 @@ func (u *inPlaceIfPossibleUpdater) FulfillPodUpdatedInfo(_ context.Context, _ *a
 		return err
 	}
 
+	var podStatus *PodStatus
 	if podUpdateInfo.OnlyMetadataChanged {
-		if podUpdateInfo.UpdatedPod.Annotations != nil {
-			delete(podUpdateInfo.UpdatedPod.Annotations, appsv1alpha1.LastPodStatusAnnotationKey)
-		}
+		podStatus = &PodStatus{ContainerStates: nil}
 	} else {
 		containerCurrentStatusMapping := map[string]*corev1.ContainerStatus{}
 		for i := range podUpdateInfo.Status.ContainerStatuses {
@@ -605,7 +604,7 @@ func (u *inPlaceIfPossibleUpdater) FulfillPodUpdatedInfo(_ context.Context, _ *a
 			}
 		}
 
-		podStatus := &PodStatus{ContainerStates: map[string]*ContainerStatus{}}
+		podStatus = &PodStatus{ContainerStates: map[string]*ContainerStatus{}}
 		for _, container := range podUpdateInfo.UpdatedPod.Spec.Containers {
 			podStatus.ContainerStates[container.Name] = &ContainerStatus{
 				// store image of each container in updated Pod
@@ -620,17 +619,17 @@ func (u *inPlaceIfPossibleUpdater) FulfillPodUpdatedInfo(_ context.Context, _ *a
 			// store image ID of each container in current Pod
 			podStatus.ContainerStates[container.Name].LastImageID = containerCurrentStatus.ImageID
 		}
+	}
 
-		podStatusStr, err := json.Marshal(podStatus)
-		if err != nil {
-			return err
-		}
+	podStatusStr, err := json.Marshal(podStatus)
+	if err != nil {
+		return err
+	}
 
-		if podUpdateInfo.UpdatedPod.Annotations == nil {
-			podUpdateInfo.UpdatedPod.Annotations = map[string]string{}
-		}
-		podUpdateInfo.UpdatedPod.Annotations[appsv1alpha1.LastPodStatusAnnotationKey] = string(podStatusStr)
+	if podUpdateInfo.UpdatedPod.Annotations == nil {
+		podUpdateInfo.UpdatedPod.Annotations = map[string]string{}
 	}
+	podUpdateInfo.UpdatedPod.Annotations[appsv1alpha1.LastPodStatusAnnotationKey] = string(podStatusStr)
 	return nil
 }
 
@@ -709,8 +708,8 @@ func (u *inPlaceIfPossibleUpdater) diffPod(currentPod, updatedPod *corev1.Pod) (
 }
 
 func (u *inPlaceIfPossibleUpdater) GetPodUpdateFinishStatus(_ context.Context, podUpdateInfo *PodUpdateInfo) (finished bool, msg string, err error) {
-	if podUpdateInfo.PodDecorationChanged {
-		return false, "add on not updated", nil
+	if !podUpdateInfo.IsUpdatedRevision || podUpdateInfo.PodDecorationChanged {
+		return false, "pod is not updated or add on not updated", nil
 	}
 
 	if podUpdateInfo.Status.ContainerStatuses == nil {
@@ -738,7 +737,7 @@ func (u *inPlaceIfPossibleUpdater) GetPodUpdateFinishStatus(_ context.Context, p
 	}
 
 	if podLastState.ContainerStates == nil {
-		return true, "empty last container state recorded", nil
+		return true, "pod is updated with only metadata changed", nil
 	}
 
 	imageMapping := map[string]string{}