From a26ceb22f06588001efe2d00d90f8dc992a3d097 Mon Sep 17 00:00:00 2001 From: Sunny Date: Mon, 11 Oct 2021 02:46:52 +0530 Subject: [PATCH] Remove controller.Events Since events.Recorder implements the k8s EventRecorder interface, there is no need of the controller.Events any more. The reconcilers can embed the k8s EventRecorder and use a events.Recorder recorder. Update the events.Recorder to embed a k8s event recorder to pass events to k8s along with an external recorder. The trace events are passed to k8s recorder as a normal event since it only accepts normal and warning event types. Update tests to use testenv with suite_test. Signed-off-by: Sunny --- runtime/controller/events.go | 97 --------------------------------- runtime/events/recorder.go | 41 +++++++++++--- runtime/events/recorder_test.go | 16 ++---- runtime/events/suite_test.go | 76 ++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 116 deletions(-) delete mode 100644 runtime/controller/events.go create mode 100644 runtime/events/suite_test.go diff --git a/runtime/controller/events.go b/runtime/controller/events.go deleted file mode 100644 index becdd0b6..00000000 --- a/runtime/controller/events.go +++ /dev/null @@ -1,97 +0,0 @@ -/* -Copyright 2020 The Flux 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 controller - -import ( - "context" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - kuberecorder "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/fluxcd/pkg/runtime/events" -) - -// Events is a helper struct that adds the capability of sending events to the Kubernetes API and an external event -// recorder, like the GitOps Toolkit notification-controller. -// -// Use it by embedding it in your reconciler struct: -// -// type MyTypeReconciler { -// client.Client -// // ... etc. -// controller.Events -// } -// -// Use MakeEvents to create a working Events value; in most cases the value needs to be initialised just once per -// controller, as the specialised logger and object reference data are gathered from the arguments provided to the -// Eventf method. -type Events struct { - Scheme *runtime.Scheme - EventRecorder kuberecorder.EventRecorder - ExternalEventRecorder *events.Recorder -} - -// MakeEvents creates a new Events, with the Events.Scheme set to that of the given mgr and a newly initialised -// Events.EventRecorder for the given controllerName. -func MakeEvents(mgr ctrl.Manager, controllerName string, ext *events.Recorder) Events { - return Events{ - Scheme: mgr.GetScheme(), - EventRecorder: mgr.GetEventRecorderFor(controllerName), - ExternalEventRecorder: ext, - } -} - -// Event emits a Kubernetes event, and forwards the event to the ExternalEventRecorder if configured. -// Use EventWithMeta or EventWithMetaf if you want to attach metadata to the external event. -func (e Events) Event(ctx context.Context, obj client.Object, severity, reason, msg string) { - e.EventWithMetaf(ctx, obj, nil, severity, reason, msg) -} - -// Eventf emits a Kubernetes event, and forwards the event to the ExternalEventRecorder if configured. -// Use EventWithMeta or EventWithMetaf if you want to attach metadata to the external event. -func (e Events) Eventf(ctx context.Context, obj client.Object, severity, reason, msgFmt string, args ...interface{}) { - e.EventWithMetaf(ctx, obj, nil, severity, reason, msgFmt, args...) -} - -// EventWithMeta emits a Kubernetes event, and forwards the event and metadata to the ExternalEventRecorder if configured. -func (e Events) EventWithMeta(ctx context.Context, obj client.Object, metadata map[string]string, severity, reason, msg string) { - e.EventWithMetaf(ctx, obj, metadata, severity, reason, msg) -} - -// EventWithMetaf emits a Kubernetes event, and forwards the event and metadata to the ExternalEventRecorder if configured. -func (e Events) EventWithMetaf(ctx context.Context, obj client.Object, metadata map[string]string, severity, reason, msgFmt string, args ...interface{}) { - if e.EventRecorder != nil { - e.EventRecorder.AnnotatedEventf(obj, metadata, severityToEventType(severity), reason, msgFmt, args...) - } - if e.ExternalEventRecorder != nil { - e.ExternalEventRecorder.AnnotatedEventf(obj, metadata, severityToEventType(severity), reason, msgFmt, args...) - } -} - -// severityToEventType maps the given severity string to a corev1 EventType. -// In case of an unrecognised severity, EventTypeNormal is returned. -func severityToEventType(severity string) string { - switch severity { - case events.EventSeverityError: - return corev1.EventTypeWarning - default: - return corev1.EventTypeNormal - } -} diff --git a/runtime/events/recorder.go b/runtime/events/recorder.go index 9c2a828e..b72fa426 100644 --- a/runtime/events/recorder.go +++ b/runtime/events/recorder.go @@ -32,9 +32,27 @@ import ( "k8s.io/apimachinery/pkg/runtime" kuberecorder "k8s.io/client-go/tools/record" "k8s.io/client-go/tools/reference" + ctrl "sigs.k8s.io/controller-runtime" ) -// Recorder posts events to the webhook address. +// Recorder posts events to the Kubernetes API and any other event recorder webhook address, like the GitOps Toolkit +// notification-controller. +// +// Use it by embedding EventRecorder in reconciler struct: +// +// import ( +// ... +// kuberecorder "k8s.io/client-go/tools/record" +// ... +// ) +// +// type MyTypeReconciler { +// client.Client +// // ... etc. +// kuberecorder.EventRecorder +// } +// +// Use NewRecorder to create a working Recorder. type Recorder struct { // URL address of the events endpoint. Webhook string @@ -45,7 +63,10 @@ type Recorder struct { // Retryable HTTP client. Client *retryablehttp.Client - // Scheme of the recorded objects. + // EventRecorder is the Kubernetes event recorder. + EventRecorder kuberecorder.EventRecorder + + // Scheme to look up the recorded objects. Scheme *runtime.Scheme // Log is the recorder logger. @@ -54,9 +75,10 @@ type Recorder struct { var _ kuberecorder.EventRecorder = &Recorder{} -// NewRecorder creates an event Recorder with default settings. -// The recorder performs automatic retries for connection errors and 500-range response codes. -func NewRecorder(scheme *runtime.Scheme, log logr.Logger, webhook, reportingController string) (*Recorder, error) { +// NewRecorder creates an event Recorder with a Kubernetes event recorder and an external event recorder based on the +// given webhook. The recorder performs automatic retries for connection errors and 500-range response codes from the +// external recorder. +func NewRecorder(mgr ctrl.Manager, log logr.Logger, webhook, reportingController string) (*Recorder, error) { if _, err := url.Parse(webhook); err != nil { return nil, err } @@ -67,10 +89,11 @@ func NewRecorder(scheme *runtime.Scheme, log logr.Logger, webhook, reportingCont httpClient.Logger = nil return &Recorder{ - Scheme: scheme, + Scheme: mgr.GetScheme(), Webhook: webhook, ReportingController: reportingController, Client: httpClient, + EventRecorder: mgr.GetEventRecorderFor(reportingController), Log: log, }, nil } @@ -101,11 +124,15 @@ func (r *Recorder) AnnotatedEventf( severity := eventTypeToSeverity(eventtype) // Do not send trace events to notification controller, - // traces are persisted as Kubernetes events only. + // traces are persisted as Kubernetes events only as normal events. if severity == EventSeverityTrace { + r.EventRecorder.AnnotatedEventf(object, annotations, corev1.EventTypeNormal, reason, messageFmt, args...) return } + // Forward the event to the Kubernetes recorder. + r.EventRecorder.AnnotatedEventf(object, annotations, eventtype, reason, messageFmt, args...) + if r.Client == nil { err := fmt.Errorf("retryable HTTP client has not been initialized") r.Log.Error(err, "unable to record event") diff --git a/runtime/events/recorder_test.go b/runtime/events/recorder_test.go index 36918f4e..2c539fd0 100644 --- a/runtime/events/recorder_test.go +++ b/runtime/events/recorder_test.go @@ -25,8 +25,6 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" ) @@ -50,9 +48,7 @@ func TestEventRecorder_AnnotatedEventf(t *testing.T) { })) defer ts.Close() - scheme := runtime.NewScheme() - require.NoError(t, clientgoscheme.AddToScheme(scheme)) - eventRecorder, err := NewRecorder(scheme, ctrl.Log, ts.URL, "test-controller") + eventRecorder, err := NewRecorder(env, ctrl.Log, ts.URL, "test-controller") require.NoError(t, err) obj := &corev1.ConfigMap{} @@ -86,9 +82,7 @@ func TestEventRecorder_AnnotatedEventf_Retry(t *testing.T) { })) defer ts.Close() - scheme := runtime.NewScheme() - require.NoError(t, clientgoscheme.AddToScheme(scheme)) - eventRecorder, err := NewRecorder(scheme, ctrl.Log, ts.URL, "test-controller") + eventRecorder, err := NewRecorder(env, ctrl.Log, ts.URL, "test-controller") require.NoError(t, err) eventRecorder.Client.RetryMax = 2 @@ -115,9 +109,7 @@ func TestEventRecorder_AnnotatedEventf_RateLimited(t *testing.T) { })) defer ts.Close() - scheme := runtime.NewScheme() - require.NoError(t, clientgoscheme.AddToScheme(scheme)) - eventRecorder, err := NewRecorder(scheme, ctrl.Log, ts.URL, "test-controller") + eventRecorder, err := NewRecorder(env, ctrl.Log, ts.URL, "test-controller") require.NoError(t, err) eventRecorder.Client.RetryMax = 2 @@ -125,6 +117,6 @@ func TestEventRecorder_AnnotatedEventf_RateLimited(t *testing.T) { obj.Namespace = "gitops-system" obj.Name = "webapp" - eventRecorder.AnnotatedEventf(obj, nil, "sync", "sync %s", obj.Name) + eventRecorder.AnnotatedEventf(obj, nil, corev1.EventTypeNormal, "sync", "sync %s", obj.Name) require.Equal(t, 1, requestCount) } diff --git a/runtime/events/suite_test.go b/runtime/events/suite_test.go new file mode 100644 index 00000000..d53a94fa --- /dev/null +++ b/runtime/events/suite_test.go @@ -0,0 +1,76 @@ +/* +Copyright 2021 The Flux 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 ( + "fmt" + "os" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/fluxcd/pkg/runtime/testenv" +) + +var ( + env *testenv.Environment + ctx = ctrl.SetupSignalHandler() +) + +func TestMain(m *testing.M) { + scheme := runtime.NewScheme() + utilruntime.Must(corev1.AddToScheme(scheme)) + + env = testenv.New( + testenv.WithScheme(scheme), + ) + + go func() { + fmt.Println("Starting the test environment") + if err := env.Start(ctx); err != nil { + panic(fmt.Sprintf("Failed to start the test environment manager: %v", err)) + } + }() + <-env.Manager.Elected() + + // Create test namespace. + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gitops-system", + }, + } + if err := env.Client.Create(ctx, ns); err != nil { + panic(fmt.Sprintf("Failed to create gitops-system namespace: %v", err)) + } + + code := m.Run() + + if err := env.Client.Delete(ctx, ns); err != nil { + panic(fmt.Sprintf("Failed to delete gitops-system namespace: %v", err)) + } + + fmt.Println("Stopping the test environment") + if err := env.Stop(); err != nil { + panic(fmt.Sprintf("Failed to stop the test environment: %v", err)) + } + + os.Exit(code) +}