Skip to content

Commit eb1beba

Browse files
rossigeerossigee
committed
Consolidate EventsRecorder into APIRecorder and document annotation drop
The duplicate EventsRecorder type was identical to the updated APIRecorder; consolidate to a single type. Document clearly that the events.k8s.io API has no AnnotatedEventf equivalent, so annotations accumulated via WithAnnotations are stored but cannot be forwarded to the API server. Also fix the FilterFn godoc (returning true prevents recording), update the stale godoc URL to point at the new events package, and tighten tests to use a single mockKubeRecorder that satisfies events.EventRecorder.
1 parent f8541dc commit eb1beba

2 files changed

Lines changed: 68 additions & 112 deletions

File tree

‎pkg/event/event.go‎

Lines changed: 19 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ import (
2727
// A Type of event.
2828
type Type string
2929

30-
// Event types. See below for valid types.
31-
// https://godoc.org/k8s.io/client-go/tools/record#EventRecorder
30+
// Event types.
31+
// https://pkg.go.dev/k8s.io/client-go/tools/events#EventRecorder
3232
const (
3333
TypeNormal Type = "Normal"
3434
TypeWarning Type = "Warning"
@@ -77,28 +77,26 @@ type Recorder interface {
7777
WithAnnotations(keysAndValues ...string) Recorder
7878
}
7979

80+
// FilterFn is a function used to filter events. Returning true prevents the
81+
// event from being recorded.
82+
type FilterFn func(obj runtime.Object, e Event) bool
83+
8084
// An APIRecorder records Kubernetes events to an API server using the
81-
// events.k8s.io API.
85+
// events.k8s.io/v1 API introduced in Kubernetes 1.19.
86+
//
87+
// Note: the events.EventRecorder interface does not support per-event
88+
// annotations (unlike the deprecated record.EventRecorder.AnnotatedEventf).
89+
// Annotations accumulated via WithAnnotations are stored but cannot be
90+
// forwarded to the API server. Callers that relied on annotation propagation
91+
// should encode that metadata into the event message instead.
8292
type APIRecorder struct {
8393
kube events.EventRecorder
84-
annotations map[string]string
94+
annotations map[string]string // stored but not forwarded; see type doc.
8595
filterFns []FilterFn
8696
}
8797

88-
// An EventsRecorder records Kubernetes events to an API server using the
89-
// new events.k8s.io API.
90-
type EventsRecorder struct {
91-
kube events.EventRecorder
92-
annotations map[string]string
93-
filterFns []FilterFn
94-
}
95-
96-
// FilterFn is a function used to filter events.
97-
// It should return true when events should not be sent.
98-
type FilterFn func(obj runtime.Object, e Event) bool
99-
10098
// NewAPIRecorder returns an APIRecorder that records Kubernetes events to an
101-
// APIServer using the supplied EventRecorder.
99+
// API server using the supplied EventRecorder.
102100
func NewAPIRecorder(r events.EventRecorder, fns ...FilterFn) *APIRecorder {
103101
return &APIRecorder{kube: r, annotations: map[string]string{}, filterFns: fns}
104102
}
@@ -115,7 +113,10 @@ func (r *APIRecorder) Event(obj runtime.Object, e Event) {
115113
}
116114

117115
// WithAnnotations returns a new *APIRecorder that includes the supplied
118-
// annotations with all recorded events.
116+
// annotations. Note: the events.k8s.io API does not support per-event
117+
// annotations, so accumulated annotations are stored but not propagated to
118+
// the API server. Encode annotation data in the event message if it must
119+
// appear in the emitted event.
119120
func (r *APIRecorder) WithAnnotations(keysAndValues ...string) Recorder {
120121
ar := NewAPIRecorder(r.kube, r.filterFns...)
121122
maps.Copy(ar.annotations, r.annotations)
@@ -125,34 +126,6 @@ func (r *APIRecorder) WithAnnotations(keysAndValues ...string) Recorder {
125126
return ar
126127
}
127128

128-
// NewEventsRecorder returns an EventsRecorder that records Kubernetes events to an
129-
// APIServer using the new events.k8s.io API.
130-
func NewEventsRecorder(r events.EventRecorder, fns ...FilterFn) *EventsRecorder {
131-
return &EventsRecorder{kube: r, annotations: map[string]string{}, filterFns: fns}
132-
}
133-
134-
// Event records the supplied event using the new events API.
135-
func (r *EventsRecorder) Event(obj runtime.Object, e Event) {
136-
for _, filter := range r.filterFns {
137-
if filter(obj, e) {
138-
return
139-
}
140-
}
141-
142-
r.kube.Eventf(obj, nil, string(e.Type), string(e.Reason), "", "%s", e.Message)
143-
}
144-
145-
// WithAnnotations returns a new *EventsRecorder that includes the supplied
146-
// annotations with all recorded events.
147-
func (r *EventsRecorder) WithAnnotations(keysAndValues ...string) Recorder {
148-
ar := NewEventsRecorder(r.kube, r.filterFns...)
149-
maps.Copy(ar.annotations, r.annotations)
150-
151-
sliceMap(keysAndValues, ar.annotations)
152-
153-
return ar
154-
}
155-
156129
func sliceMap(from []string, to map[string]string) {
157130
for i := 0; i+1 < len(from); i += 2 {
158131
k, v := from[i], from[i+1]

‎pkg/event/event_test.go‎

Lines changed: 49 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -24,36 +24,26 @@ import (
2424
"k8s.io/apimachinery/pkg/runtime/schema"
2525
)
2626

27-
type mockRecordRecorder struct {
27+
// mockKubeRecorder satisfies events.EventRecorder.
28+
type mockKubeRecorder struct {
2829
events []mockEvent
2930
}
3031

3132
type mockEvent struct {
3233
obj runtime.Object
33-
annots map[string]string
3434
typeStr string
3535
reason string
3636
msg string
3737
}
3838

39-
func (m *mockRecordRecorder) Event(obj runtime.Object, eventtype, reason, message string) {
40-
m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, msg: message})
41-
}
42-
43-
func (m *mockRecordRecorder) Eventf(obj runtime.Object, related runtime.Object, eventtype, reason, action, note string, args ...interface{}) {
44-
m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, msg: args[0].(string)})
45-
}
46-
47-
func (m *mockRecordRecorder) AnnotatedEventf(obj runtime.Object, annots map[string]string, typeStr, reason, msg string, args ...interface{}) {
48-
m.events = append(m.events, mockEvent{obj: obj, annots: annots, typeStr: typeStr, reason: reason, msg: args[0].(string)})
49-
}
50-
51-
type mockEventsRecorder struct {
52-
events []mockEvent
53-
}
54-
55-
func (m *mockEventsRecorder) Eventf(obj runtime.Object, related runtime.Object, eventtype, reason, action, note string, args ...interface{}) {
56-
m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, msg: args[0].(string)})
39+
func (m *mockKubeRecorder) Eventf(obj runtime.Object, _ runtime.Object, eventtype, reason, _, note string, args ...interface{}) {
40+
msg := note
41+
if len(args) > 0 {
42+
if s, ok := args[0].(string); ok {
43+
msg = s
44+
}
45+
}
46+
m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, msg: msg})
5747
}
5848

5949
type mockObj struct{}
@@ -128,52 +118,15 @@ func TestSliceMap(t *testing.T) {
128118
}
129119
}
130120

131-
func TestAPIRecorderWithAnnotationsFilterFns(t *testing.T) {
132-
filterCalled := false
133-
filter := func(obj runtime.Object, e Event) bool {
134-
filterCalled = true
135-
return false
136-
}
137-
138-
mr := &mockRecordRecorder{}
139-
rec := NewAPIRecorder(mr, filter)
140-
_ = rec.WithAnnotations("key", "val")
141-
142-
rec.Event(&mockObj{}, Normal("test", "msg"))
143-
144-
if !filterCalled {
145-
t.Error("filter function was not preserved after WithAnnotations")
146-
}
147-
}
148-
149-
func TestEventsRecorderWithAnnotationsFilterFns(t *testing.T) {
150-
filterCalled := false
151-
filter := func(obj runtime.Object, e Event) bool {
152-
filterCalled = true
153-
return false
154-
}
155-
156-
mr := &mockEventsRecorder{}
157-
rec := NewEventsRecorder(mr, filter)
158-
_ = rec.WithAnnotations("key", "val")
159-
160-
rec.Event(&mockObj{}, Normal("test", "msg"))
161-
162-
if !filterCalled {
163-
t.Error("filter function was not preserved after WithAnnotations")
164-
}
165-
}
166-
167-
func TestEventsRecorderEvent(t *testing.T) {
168-
mr := &mockEventsRecorder{}
169-
rec := NewEventsRecorder(mr)
121+
func TestAPIRecorderEvent(t *testing.T) {
122+
mr := &mockKubeRecorder{}
123+
rec := NewAPIRecorder(mr)
170124

171125
rec.Event(&mockObj{}, Normal("testReason", "test message"))
172126

173127
if len(mr.events) != 1 {
174128
t.Fatalf("expected 1 event, got %d", len(mr.events))
175129
}
176-
177130
if mr.events[0].reason != "testReason" {
178131
t.Errorf("expected reason 'testReason', got %q", mr.events[0].reason)
179132
}
@@ -185,16 +138,46 @@ func TestEventsRecorderEvent(t *testing.T) {
185138
}
186139
}
187140

188-
func TestEventsRecorderFilter(t *testing.T) {
189-
mr := &mockEventsRecorder{}
190-
filter := func(obj runtime.Object, e Event) bool {
191-
return true
192-
}
193-
rec := NewEventsRecorder(mr, filter)
141+
func TestAPIRecorderFilter(t *testing.T) {
142+
mr := &mockKubeRecorder{}
143+
filter := func(_ runtime.Object, _ Event) bool { return true }
144+
rec := NewAPIRecorder(mr, filter)
194145

195146
rec.Event(&mockObj{}, Normal("testReason", "test message"))
196147

197148
if len(mr.events) != 0 {
198149
t.Errorf("expected event to be filtered, got %d events", len(mr.events))
199150
}
200151
}
152+
153+
func TestAPIRecorderWithAnnotationsPreservesFilterFns(t *testing.T) {
154+
filterCalled := false
155+
filter := func(_ runtime.Object, _ Event) bool {
156+
filterCalled = true
157+
return false
158+
}
159+
160+
mr := &mockKubeRecorder{}
161+
rec := NewAPIRecorder(mr, filter)
162+
derived := rec.WithAnnotations("key", "val")
163+
164+
derived.Event(&mockObj{}, Normal("test", "msg"))
165+
166+
if !filterCalled {
167+
t.Error("filter function was not preserved after WithAnnotations")
168+
}
169+
}
170+
171+
func TestAPIRecorderWithAnnotationsPreservesExistingAnnotations(t *testing.T) {
172+
mr := &mockKubeRecorder{}
173+
rec := NewAPIRecorder(mr)
174+
r1 := rec.WithAnnotations("k1", "v1").(*APIRecorder)
175+
r2 := r1.WithAnnotations("k2", "v2").(*APIRecorder)
176+
177+
if r2.annotations["k1"] != "v1" {
178+
t.Errorf("expected k1=v1 to be preserved, got %q", r2.annotations["k1"])
179+
}
180+
if r2.annotations["k2"] != "v2" {
181+
t.Errorf("expected k2=v2 to be set, got %q", r2.annotations["k2"])
182+
}
183+
}

0 commit comments

Comments
 (0)