diff --git a/pkg/event/event.go b/pkg/event/event.go index c1630e6b9..f5957a550 100644 --- a/pkg/event/event.go +++ b/pkg/event/event.go @@ -18,17 +18,15 @@ limitations under the License. package event import ( - "maps" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/record" + "k8s.io/client-go/tools/events" ) // A Type of event. type Type string -// Event types. See below for valid types. -// https://godoc.org/k8s.io/client-go/tools/record#EventRecorder +// Event types. +// https://pkg.go.dev/k8s.io/client-go/tools/events#EventRecorder const ( TypeNormal Type = "Normal" TypeWarning Type = "Warning" @@ -39,59 +37,54 @@ type Reason string // An Event relating to a Crossplane resource. type Event struct { - Type Type - Reason Reason - Message string - Annotations map[string]string + Type Type + Reason Reason + Message string } // Normal returns a normal, informational event. -func Normal(r Reason, message string, keysAndValues ...string) Event { - e := Event{ - Type: TypeNormal, - Reason: r, - Message: message, - Annotations: map[string]string{}, +func Normal(r Reason, message string) Event { + return Event{ + Type: TypeNormal, + Reason: r, + Message: message, } - sliceMap(keysAndValues, e.Annotations) - - return e } // Warning returns a warning event, typically due to an error. -func Warning(r Reason, err error, keysAndValues ...string) Event { - e := Event{ - Type: TypeWarning, - Reason: r, - Message: err.Error(), - Annotations: map[string]string{}, +func Warning(r Reason, err error) Event { + return Event{ + Type: TypeWarning, + Reason: r, + Message: err.Error(), } - sliceMap(keysAndValues, e.Annotations) - - return e } // A Recorder records Kubernetes events. type Recorder interface { Event(obj runtime.Object, e Event) - WithAnnotations(keysAndValues ...string) Recorder } -// An APIRecorder records Kubernetes events to an API server. +// FilterFn is a function used to filter events. Returning true prevents the +// event from being recorded. +type FilterFn func(obj runtime.Object, e Event) bool + +// An APIRecorder records Kubernetes events to an API server using the +// events.k8s.io/v1 API introduced in Kubernetes 1.19. +// +// Note: the events.k8s.io API does not support per-event annotations (unlike +// the deprecated record.EventRecorder.AnnotatedEventf). Callers that previously +// relied on annotation propagation should encode that metadata into the event +// message instead. type APIRecorder struct { - kube record.EventRecorder - annotations map[string]string - filterFns []FilterFn + kube events.EventRecorder + filterFns []FilterFn } -// FilterFn is a function used to filter events. -// It should return false when events should not be sent. -type FilterFn func(obj runtime.Object, e Event) bool - // NewAPIRecorder returns an APIRecorder that records Kubernetes events to an -// APIServer using the supplied EventRecorder. -func NewAPIRecorder(r record.EventRecorder, fns ...FilterFn) *APIRecorder { - return &APIRecorder{kube: r, annotations: map[string]string{}, filterFns: fns} +// API server using the supplied EventRecorder. +func NewAPIRecorder(r events.EventRecorder, fns ...FilterFn) *APIRecorder { + return &APIRecorder{kube: r, filterFns: fns} } // Event records the supplied event. @@ -102,25 +95,7 @@ func (r *APIRecorder) Event(obj runtime.Object, e Event) { } } - r.kube.AnnotatedEventf(obj, r.annotations, string(e.Type), string(e.Reason), "%s", e.Message) -} - -// WithAnnotations returns a new *APIRecorder that includes the supplied -// annotations with all recorded events. -func (r *APIRecorder) WithAnnotations(keysAndValues ...string) Recorder { - ar := NewAPIRecorder(r.kube) - maps.Copy(ar.annotations, r.annotations) - - sliceMap(keysAndValues, ar.annotations) - - return ar -} - -func sliceMap(from []string, to map[string]string) { - for i := 0; i+1 < len(from); i += 2 { - k, v := from[i], from[i+1] - to[k] = v - } + r.kube.Eventf(obj, nil, string(e.Type), string(e.Reason), string(e.Reason), "%s", e.Message) } // A NopRecorder does nothing. @@ -133,6 +108,3 @@ func NewNopRecorder() *NopRecorder { // Event does nothing. func (r *NopRecorder) Event(_ runtime.Object, _ Event) {} - -// WithAnnotations does nothing. -func (r *NopRecorder) WithAnnotations(_ ...string) Recorder { return r } diff --git a/pkg/event/event_test.go b/pkg/event/event_test.go index 656b0a9ec..3494af35e 100644 --- a/pkg/event/event_test.go +++ b/pkg/event/event_test.go @@ -17,72 +17,60 @@ limitations under the License. package event import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ) -func TestSliceMap(t *testing.T) { - type args struct { - from []string - to map[string]string - } +// mockKubeRecorder satisfies events.EventRecorder. +type mockKubeRecorder struct { + events []mockEvent +} + +type mockEvent struct { + obj runtime.Object + typeStr string + reason string + action string + msg string +} + +func (m *mockKubeRecorder) Eventf(obj runtime.Object, _ runtime.Object, eventtype, reason, action, note string, args ...any) { + msg := fmt.Sprintf(note, args...) + m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, action: action, msg: msg}) +} + +type mockObj struct{} - cases := map[string]struct { - reason string - args args - want map[string]string - }{ - "OnePair": { - reason: "One key value pair should be added.", - args: args{ - from: []string{"key", "val"}, - to: map[string]string{}, - }, - want: map[string]string{"key": "val"}, - }, - "TwoPairs": { - reason: "Two key value pairs should be added.", - args: args{ - from: []string{ - "key", "val", - "another", "value", - }, - to: map[string]string{}, - }, - want: map[string]string{ - "key": "val", - "another": "value", - }, - }, - "NoValue": { - reason: "Two key value pairs should be added.", - args: args{ - from: []string{"key"}, - to: map[string]string{}, - }, - want: map[string]string{}, - }, - "ExtraneousKey": { - reason: "One key value pair should be added.", - args: args{ - from: []string{ - "key", "val", - "extraneous", - }, - to: map[string]string{}, - }, - want: map[string]string{"key": "val"}, - }, +func (m *mockObj) GetObjectKind() schema.ObjectKind { return nil } +func (m *mockObj) DeepCopyObject() runtime.Object { + return &mockObj{} +} + +func TestAPIRecorderEvent(t *testing.T) { + mr := &mockKubeRecorder{} + rec := NewAPIRecorder(mr) + + rec.Event(&mockObj{}, Normal("testReason", "test message")) + + want := mockEvent{typeStr: "Normal", reason: "testReason", action: "testReason", msg: "test message"} + if diff := cmp.Diff(want, mr.events[0], cmp.AllowUnexported(mockEvent{}), cmpopts.IgnoreFields(mockEvent{}, "obj")); diff != "" { + t.Errorf("unexpected event: -want, +got:\n%s", diff) } +} + +func TestAPIRecorderFilter(t *testing.T) { + mr := &mockKubeRecorder{} + filter := func(_ runtime.Object, _ Event) bool { return true } + rec := NewAPIRecorder(mr, filter) - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - sliceMap(tc.args.from, tc.args.to) + rec.Event(&mockObj{}, Normal("testReason", "test message")) - if diff := cmp.Diff(tc.want, tc.args.to); diff != "" { - t.Errorf("%s\nsliceMap(...): -want, +got:\n%s", tc.reason, diff) - } - }) + if diff := cmp.Diff(0, len(mr.events)); diff != "" { + t.Errorf("expected no events, got %d: %s", len(mr.events), diff) } } diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index d27c9d393..878f25dec 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -1021,7 +1021,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu r.metricRecorder.recordFirstTimeReconciled(managed) status := r.conditions.For(managed) - record := r.record.WithAnnotations("external-name", meta.GetExternalName(managed)) + record := r.record log = log.WithValues( "uid", managed.GetUID(), "version", managed.GetResourceVersion(), @@ -1050,8 +1050,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // Log, publish an event and update the SYNC status condition. if meta.IsPaused(managed) || policy.IsPaused() { log.Debug("Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation", "annotation", meta.AnnotationKeyReconciliationPaused) - record.Event(managed, event.Normal(reasonReconciliationPaused, "Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation", - "annotation", meta.AnnotationKeyReconciliationPaused)) + record.Event(managed, event.Normal(reasonReconciliationPaused, "Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation")) status.MarkConditions(xpv2.ReconcilePaused()) // if the pause annotation is removed or the management policies changed, we will have a chance to reconcile // again and resume and if status update fails, we will reconcile again to retry to update the status @@ -1067,7 +1066,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu if tracker, ok := managed.(reconcileRequestTracker); ok { if tracker.GetLastHandledReconcileAt() != token { log.Debug("Processing reconcile request", "token", token) - record.Event(managed, event.Normal(reasonReconcileRequestHandled, "Handling reconcile request", "token", token)) + record.Event(managed, event.Normal(reasonReconcileRequestHandled, "Handling reconcile request")) reconcileRequestToken = token } } @@ -1542,7 +1541,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // In some cases our external-name may be set by Create above. log = log.WithValues("external-name", meta.GetExternalName(managed)) - record = r.record.WithAnnotations("external-name", meta.GetExternalName(managed)) + record = r.record if err := r.change.Log(ctx, managedPreOp, v1alpha1.OperationType_OPERATION_TYPE_CREATE, nil, creation.AdditionalDetails); err != nil { log.Info(errRecordChangeLog, "error", err) diff --git a/pkg/reconciler/providerconfig/reconciler_test.go b/pkg/reconciler/providerconfig/reconciler_test.go index 2be2a4e17..9c194beed 100644 --- a/pkg/reconciler/providerconfig/reconciler_test.go +++ b/pkg/reconciler/providerconfig/reconciler_test.go @@ -678,8 +678,6 @@ type recorder struct{ events []event.Event } func (r *recorder) Event(_ runtime.Object, e event.Event) { r.events = append(r.events, e) } -func (r *recorder) WithAnnotations(_ ...string) event.Recorder { return r } - // terminatingUsage returns a usage that is being deleted, and that is waiting // for its owner to release it. func terminatingUsage(name, owner string, uid types.UID, deleted *metav1.Time) *fake.ProviderConfigUsage {