Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions pkg/controller/instanceset/pod_role_event_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
xuriwuyun marked this conversation as resolved.
Show resolved Hide resolved
newPod.Annotations[constant.LastRoleSnapshotVersionAnnotationKey] = version
return cli.Update(ctx, newPod, inDataContext())
}

func inDataContext() *multicluster.ClientOption {
Expand Down
42 changes: 40 additions & 2 deletions pkg/controller/instanceset/pod_role_event_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,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))
Expand Down Expand Up @@ -123,6 +123,44 @@ 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")
xuriwuyun marked this conversation as resolved.
Show resolved Hide resolved
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.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))
return updateErr
}).Times(1)
Expect(handler.Handle(cli, reqCtx, nil, event)).Should(Equal(updateErr))
})
})

Expand Down
Loading