Skip to content

Commit

Permalink
fix: optimize record role change event time accuracy (apecloud#5127)
Browse files Browse the repository at this point in the history
  • Loading branch information
Y-Rookie authored Sep 14, 2023
1 parent 31619f0 commit ade244b
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 12 deletions.
2 changes: 1 addition & 1 deletion controllers/apps/components/consensus_set_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func updateConsensusSetRoleLabel(cli client.Client,
if pod.Annotations == nil {
pod.Annotations = map[string]string{}
}
pod.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey] = event.LastTimestamp.Time.Format(time.RFC3339)
pod.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey] = event.EventTime.Time.Format(time.RFC3339Nano)
return cli.Patch(ctx, pod, patch)
}

Expand Down
2 changes: 1 addition & 1 deletion controllers/apps/components/replication_set_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func updateObjRoleChangedInfo[T generics.Object, PT generics.PObject[T]](
if pObj.GetAnnotations() == nil {
pObj.SetAnnotations(map[string]string{})
}
pObj.GetAnnotations()[constant.LastRoleChangedEventTimestampAnnotationKey] = event.LastTimestamp.Time.Format(time.RFC3339)
pObj.GetAnnotations()[constant.LastRoleChangedEventTimestampAnnotationKey] = event.EventTime.Time.Format(time.RFC3339Nano)
if err := cli.Patch(ctx, pObj, patch); err != nil {
return err
}
Expand Down
14 changes: 9 additions & 5 deletions controllers/k8score/event_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ var _ = Describe("Event Controller", func() {
SetType(corev1.EventTypeNormal).
SetFirstTimestamp(metav1.NewTime(initLastTS)).
SetLastTimestamp(metav1.NewTime(initLastTS)).
SetEventTime(metav1.NewMicroTime(initLastTS)).
SetReportingController("lorry").
SetReportingInstance(podName).
SetAction("mock-create-event-action").
GetObject()
}

Expand Down Expand Up @@ -155,7 +159,7 @@ var _ = Describe("Event Controller", func() {
g.Expect(p).ShouldNot(BeNil())
g.Expect(p.Labels).ShouldNot(BeNil())
g.Expect(p.Labels[constant.RoleLabelKey]).Should(Equal(role))
g.Expect(p.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey]).Should(Equal(sndEvent.LastTimestamp.Time.Format(time.RFC3339)))
g.Expect(p.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey]).Should(Equal(sndEvent.EventTime.Time.Format(time.RFC3339Nano)))
})).Should(Succeed())

Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(sndEvent), func(g Gomega, e *corev1.Event) {
Expand All @@ -170,7 +174,7 @@ var _ = Describe("Event Controller", func() {
By("send role changed event with beforeLastTS earlier than pod last role changes event timestamp annotation should not be update successfully")
role = "follower"
sndInvalidEvent := createRoleChangedEvent(podName, role, uid)
sndInvalidEvent.LastTimestamp = metav1.NewTime(beforeLastTS)
sndInvalidEvent.EventTime = metav1.NewMicroTime(beforeLastTS)
Expect(testCtx.CreateObj(ctx, sndInvalidEvent)).Should(Succeed())
Eventually(func() string {
event := &corev1.Event{}
Expand All @@ -186,13 +190,13 @@ var _ = Describe("Event Controller", func() {
g.Expect(p).ShouldNot(BeNil())
g.Expect(p.Labels).ShouldNot(BeNil())
g.Expect(p.Labels[constant.RoleLabelKey]).ShouldNot(Equal(role))
g.Expect(p.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey]).ShouldNot(Equal(sndInvalidEvent.LastTimestamp.Time.Format(time.RFC3339)))
g.Expect(p.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey]).ShouldNot(Equal(sndInvalidEvent.EventTime.Time.Format(time.RFC3339Nano)))
})).Should(Succeed())

By("send role changed event with afterLastTS later than pod last role changes event timestamp annotation should be update successfully")
role = "follower"
sndValidEvent := createRoleChangedEvent(podName, role, uid)
sndValidEvent.LastTimestamp = metav1.NewTime(afterLastTS)
sndValidEvent.EventTime = metav1.NewMicroTime(afterLastTS)
Expect(testCtx.CreateObj(ctx, sndValidEvent)).Should(Succeed())
Eventually(func() string {
event := &corev1.Event{}
Expand All @@ -208,7 +212,7 @@ var _ = Describe("Event Controller", func() {
g.Expect(p).ShouldNot(BeNil())
g.Expect(p.Labels).ShouldNot(BeNil())
g.Expect(p.Labels[constant.RoleLabelKey]).Should(Equal(role))
g.Expect(p.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey]).Should(Equal(sndValidEvent.LastTimestamp.Time.Format(time.RFC3339)))
g.Expect(p.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey]).Should(Equal(sndValidEvent.EventTime.Time.Format(time.RFC3339Nano)))
})).Should(Succeed())
})
})
Expand Down
10 changes: 5 additions & 5 deletions controllers/k8score/role_change_event_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,19 @@ func handleRoleChangedEvent(cli client.Client, reqCtx intctrlutil.RequestCtx, re
return role, nil
}

// compare the lastTimestamp of the current event object with the lastTimestamp of the last recorded in the pod annotation,
// if the current event's lastTimestamp is earlier than the recorded lastTimestamp in the pod annotation,
// compare the EventTime of the current event object with the lastTimestamp of the last recorded in the pod annotation,
// if the current event's EventTime is earlier than the recorded lastTimestamp in the pod annotation,
// it indicates that the current event has arrived out of order and is expired, so it should not be processed.
lastTimestampStr, ok := pod.Annotations[constant.LastRoleChangedEventTimestampAnnotationKey]
if ok {
lastTimestamp, err := time.Parse(time.RFC3339, lastTimestampStr)
lastTimestamp, err := time.Parse(time.RFC3339Nano, lastTimestampStr)
if err != nil {
reqCtx.Log.Info("failed to parse last role changed event timestamp from pod annotation", "pod", pod.Name, "error", err.Error())
return role, err
}
eventLastTS := event.LastTimestamp.Time
eventLastTS := event.EventTime.Time
if !eventLastTS.After(lastTimestamp) {
reqCtx.Log.Info("event's lastTimestamp is earlier than the recorded lastTimestamp in the pod annotation, it should not be processed.", "event uid", event.UID, "pod", pod.Name, "role", role, "originalRole", message.OriginalRole, "event lastTimestamp", event.LastTimestamp.Time.String(), "annotation lastTimestamp", lastTimestampStr)
reqCtx.Log.Info("event's EventTime is earlier than the recorded lastTimestamp in the pod annotation, it should not be processed.", "event uid", event.UID, "pod", pod.Name, "role", role, "originalRole", message.OriginalRole, "event EventTime", event.EventTime.Time.String(), "annotation lastTimestamp", lastTimestampStr)
return role, nil
}
}
Expand Down
20 changes: 20 additions & 0 deletions internal/controller/builder/builder_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,23 @@ func (builder *EventBuilder) SetLastTimestamp(lastTimestamp metav1.Time) *EventB
builder.get().LastTimestamp = lastTimestamp
return builder
}

func (builder *EventBuilder) SetEventTime(eventTime metav1.MicroTime) *EventBuilder {
builder.get().EventTime = eventTime
return builder
}

func (builder *EventBuilder) SetReportingController(reportingController string) *EventBuilder {
builder.get().ReportingController = reportingController
return builder
}

func (builder *EventBuilder) SetReportingInstance(reportingInstance string) *EventBuilder {
builder.get().ReportingInstance = reportingInstance
return builder
}

func (builder *EventBuilder) SetAction(action string) *EventBuilder {
builder.get().Action = action
return builder
}

0 comments on commit ade244b

Please sign in to comment.