fix: use update instead of patch to set pod's role (#8865)

apecloud · Feb 6, 2025 · 24892e7 · 24892e7
1 parent 19cf09b
commit 24892e7
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 9 deletions.
diff --git a/pkg/controller/instanceset/pod_role_event_handler.go b/pkg/controller/instanceset/pod_role_event_handler.go
@@ -246,20 +246,20 @@ func updatePodRoleLabel(cli client.Client, reqCtx intctrlutil.RequestCtx,
 	roleName = strings.ToLower(roleName)
 
 	// update pod role label
-	patch := client.MergeFrom(pod.DeepCopy())
+	newPod := pod.DeepCopy()
 	role, ok := roleMap[roleName]
 	switch ok {
 	case true:
-		pod.Labels[RoleLabelKey] = role.Name
+		newPod.Labels[RoleLabelKey] = role.Name
 	case false:
-		delete(pod.Labels, RoleLabelKey)
+		delete(newPod.Labels, RoleLabelKey)
 	}
 
-	if pod.Annotations == nil {
-		pod.Annotations = map[string]string{}
+	if newPod.Annotations == nil {
+		newPod.Annotations = map[string]string{}
 	}
-	pod.Annotations[constant.LastRoleSnapshotVersionAnnotationKey] = version
-	return cli.Patch(ctx, pod, patch, inDataContext())
+	newPod.Annotations[constant.LastRoleSnapshotVersionAnnotationKey] = version
+	return cli.Update(ctx, newPod, inDataContext())
 }
 
 func inDataContext() *multicluster.ClientOption {

diff --git a/pkg/controller/instanceset/pod_role_event_handler_test.go b/pkg/controller/instanceset/pod_role_event_handler_test.go
@@ -46,6 +46,7 @@ var _ = Describe("pod role label event handler test", func() {
 				Log: logger,
 			}
 			pod := builder.NewPodBuilder(namespace, getPodName(name, 0)).SetUID(uid).GetObject()
+			pod.ResourceVersion = "1"
 			objectRef := corev1.ObjectReference{
 				APIVersion: "v1",
 				Kind:       "Pod",
@@ -90,8 +91,8 @@ var _ = Describe("pod role label event handler test", func() {
 					return nil
 				}).Times(1)
 			k8sMock.EXPECT().
-				Patch(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
-				DoAndReturn(func(_ context.Context, pd *corev1.Pod, patch client.Patch, _ ...client.PatchOption) error {
+				Update(gomock.Any(), gomock.Any(), gomock.Any()).
+				DoAndReturn(func(_ context.Context, pd *corev1.Pod, _ ...client.UpdateOption) error {
 					Expect(pd).ShouldNot(BeNil())
 					Expect(pd.Labels).ShouldNot(BeNil())
 					Expect(pd.Labels[RoleLabelKey]).Should(Equal(role.Name))
@@ -123,6 +124,49 @@ var _ = Describe("pod role label event handler test", func() {
 					return nil
 				}).Times(1)
 			Expect(handler.Handle(cli, reqCtx, nil, event)).Should(Succeed())
+
+			By("read a stale pod")
+			message = fmt.Sprintf("Readiness probe failed: error: health rpc failed: rpc error: code = Unknown desc = {\"event\":\"Success\",\"originalRole\":\"\",\"role\":\"%s\"}", role.Name)
+			event = builder.NewEventBuilder(namespace, "foo").
+				SetInvolvedObject(objectRef).
+				SetReason(checkRoleOperation).
+				SetMessage(message).
+				GetObject()
+
+			k8sMock.EXPECT().
+				Get(gomock.Any(), gomock.Any(), &corev1.Pod{}, gomock.Any()).
+				DoAndReturn(func(_ context.Context, objKey client.ObjectKey, p *corev1.Pod, _ ...client.GetOption) error {
+					p.Namespace = objKey.Namespace
+					p.ResourceVersion = "0"
+					p.Name = objKey.Name
+					p.UID = pod.UID
+					p.Labels = map[string]string{
+						constant.AppInstanceLabelKey: name,
+						WorkloadsInstanceLabelKey:    name,
+					}
+					return nil
+				}).Times(1)
+			k8sMock.EXPECT().
+				Get(gomock.Any(), gomock.Any(), &workloads.InstanceSet{}, gomock.Any()).
+				DoAndReturn(func(_ context.Context, objKey client.ObjectKey, its *workloads.InstanceSet, _ ...client.GetOption) error {
+					its.Namespace = objKey.Namespace
+					its.Name = objKey.Name
+					its.Spec.Roles = []workloads.ReplicaRole{role}
+					return nil
+				}).Times(1)
+			updateErr := fmt.Errorf("the object has been modified; please apply your changes to the latest version and try again")
+			k8sMock.EXPECT().
+				Update(gomock.Any(), gomock.Any(), gomock.Any()).
+				DoAndReturn(func(_ context.Context, pd *corev1.Pod, _ ...client.UpdateOption) error {
+					Expect(pd).ShouldNot(BeNil())
+					Expect(pd.Labels).ShouldNot(BeNil())
+					Expect(pd.Labels[RoleLabelKey]).Should(Equal(role.Name))
+					if pd.ResourceVersion <= pod.ResourceVersion {
+						return updateErr
+					}
+					return nil
+				}).Times(1)
+			Expect(handler.Handle(cli, reqCtx, nil, event)).Should(Equal(updateErr))
 		})
 	})