Skip to content

Commit

Permalink
tools/events: retry on AlreadyExist for Series
Browse files Browse the repository at this point in the history
When attempting to record a new Event and a new Serie on the apiserver
at the same time, the patch of the Serie might happen before the Event
is actually created. In that case, we handle the error and try to create
the Event. But the Event might be created during that period of time and
it is treated as an error today. So in order to handle that scenario, we
need to retry when a Create call for a Serie results in an AlreadyExist
error.

Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>
  • Loading branch information
dgrisonnet committed Jul 17, 2023
1 parent 952f38a commit 92c7042
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,14 @@ func recordEvent(sink EventSink, event *eventsv1.Event) (*eventsv1.Event, bool)
return nil, false
case *errors.StatusError:
if errors.IsAlreadyExists(err) {
// If we tried to create an Event from an EventSerie, it means that
// the original Patch request failed because the Event we were
// trying to patch didn't exist. If the creation failed because the
// Event now exists, it is safe to retry. This occurs when a new
// Event is emitted twice in a very short period of time.
if isEventSeries {
return nil, true
}
klog.V(5).Infof("Server rejected event '%#v': '%v' (will not retry!)", event, err)
} else {
klog.Errorf("Server rejected event '%#v': '%v' (will not retry!)", event, err)
Expand Down
103 changes: 103 additions & 0 deletions staging/src/k8s.io/client-go/tools/events/event_broadcaster_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
Copyright 2022 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package events

import (
"context"
"reflect"
"testing"

eventsv1 "k8s.io/api/events/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/client-go/kubernetes/fake"
)

func TestRecordEventToSink(t *testing.T) {
nonIsomorphicEvent := eventsv1.Event{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: metav1.NamespaceDefault,
},
Series: nil,
}

isomorphicEvent := *nonIsomorphicEvent.DeepCopy()
isomorphicEvent.Series = &eventsv1.EventSeries{Count: 2}

testCases := []struct {
name string
eventsToRecord []eventsv1.Event
expectedRecordedEvent eventsv1.Event
}{
{
name: "record one Event",
eventsToRecord: []eventsv1.Event{
nonIsomorphicEvent,
},
expectedRecordedEvent: nonIsomorphicEvent,
},
{
name: "record one Event followed by an isomorphic one",
eventsToRecord: []eventsv1.Event{
nonIsomorphicEvent,
isomorphicEvent,
},
expectedRecordedEvent: isomorphicEvent,
},
{
name: "record one isomorphic Event before the original",
eventsToRecord: []eventsv1.Event{
isomorphicEvent,
nonIsomorphicEvent,
},
expectedRecordedEvent: isomorphicEvent,
},
{
name: "record one isomorphic Event without one already existing",
eventsToRecord: []eventsv1.Event{
isomorphicEvent,
},
expectedRecordedEvent: isomorphicEvent,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
kubeClient := fake.NewSimpleClientset()
eventSink := &EventSinkImpl{Interface: kubeClient.EventsV1()}

for _, ev := range tc.eventsToRecord {
recordEvent(eventSink, &ev)
}

recordedEvents, err := kubeClient.EventsV1().Events(metav1.NamespaceDefault).List(context.TODO(), metav1.ListOptions{})
if err != nil {
t.Errorf("expected to be able to list Events from fake client")
}

if len(recordedEvents.Items) != 1 {
t.Errorf("expected one Event to be recorded, found: %d", len(recordedEvents.Items))
}

recordedEvent := recordedEvents.Items[0]
if !reflect.DeepEqual(recordedEvent, tc.expectedRecordedEvent) {
t.Errorf("expected to have recorded Event: %#+v, got: %#+v\n diff: %s", tc.expectedRecordedEvent, recordedEvent, diff.ObjectReflectDiff(tc.expectedRecordedEvent, recordedEvent))
}
})
}
}

0 comments on commit 92c7042

Please sign in to comment.