Drop ObjectReference parameters and improve logger

This commit addresses the two rough edges mentioned by Michael.

> You have to supply an ObjectReference to both the metrics and the
> events methods.

As the helper methods are always called from within a reconciler, and
both events and metrics must only be recorderd for well known
resources, the methods of the helpers should only be called for objects
that do contain data about the kind and/or version.

This means that calling `reference.GetReference` within the helper
methods will generally not result in any errors, nor be a really
expensive calculation.

In cases where the GVK information for some reason is not available
on the object, `GetReference` will still be capable of falling back to
the manager's `runtime.Scheme` that is now injected to the helper.

Resulting in a - in my personal opinion - fair trade-off, and a much
more friendly API from the consumer's perspective.

> You have to pass logs in to the Events helper.

By relying on the context of the reconcile operation we get a logger
(with object metadata preconfigured) for free.

Signed-off-by: Hidde Beydals <hello@hidde.co>

runtime/controller/events.go

@@ -17,13 +17,14 @@ limitations under the License.
 package controller
 
 import (
-	"fmt"
+	"context"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kuberecorder "k8s.io/client-go/tools/record"
+	"k8s.io/client-go/tools/reference"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	"github.com/fluxcd/pkg/runtime/events"
@@ -39,20 +40,21 @@ import (
 //         controller.Events
 //     }
 //
-//  You initialise a suitable value with MakeEvents(); each reconciler
-//  will probably need its own value, since it's specialised to a
-//  particular controller and log.
+// You initialise a suitable value with MakeEvents; in most cases the
+// value only needs to be initialized 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
-	Log                   logr.Logger
 }
 
-func MakeEvents(mgr ctrl.Manager, controllerName string, ext *events.Recorder, log logr.Logger) Events {
+func MakeEvents(mgr ctrl.Manager, controllerName string, ext *events.Recorder) Events {
 	return Events{
+		Scheme:                mgr.GetScheme(),
 		EventRecorder:         mgr.GetEventRecorderFor(controllerName),
 		ExternalEventRecorder: ext,
-		Log:                   log,
 	}
 }
 
@@ -63,22 +65,24 @@ type runtimeAndMetaObject interface {
 
 // Event emits a Kubernetes event, and forwards the event to the
 // notification controller if configured.
-func (e Events) Event(ref *corev1.ObjectReference, obj runtimeAndMetaObject, severity, reason, msg string) {
-	e.Eventf(ref, obj, severity, reason, "%s", msg)
+func (e Events) Event(ctx context.Context, obj runtimeAndMetaObject, severity, reason, msg string) {
+	e.Eventf(ctx, obj, severity, reason, msg)
 }
 
 // Eventf emits a Kubernetes event, and forwards the event to the
 // notification controller if configured.
-func (e Events) Eventf(ref *corev1.ObjectReference, obj runtimeAndMetaObject, severity, reason, msgFmt string, args ...interface{}) {
+func (e Events) Eventf(ctx context.Context, obj runtimeAndMetaObject, severity, reason, msgFmt string, args ...interface{}) {
 	if e.EventRecorder != nil {
 		e.EventRecorder.Eventf(obj, severityToEventType(severity), reason, msgFmt, args)
 	}
 	if e.ExternalEventRecorder != nil {
+		ref, err := reference.GetReference(e.Scheme, obj)
+		if err != nil {
+			logr.FromContextOrDiscard(ctx).Error(err, "unable to send event")
+			return
+		}
 		if err := e.ExternalEventRecorder.Eventf(*ref, nil, severity, reason, msgFmt, args); err != nil {
-			e.Log.WithValues(
-				"request",
-				fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName()),
-			).Error(err, "unable to send event")
+			logr.FromContextOrDiscard(ctx).Error(err, "unable to send event")
 			return
 		}
 	}

runtime/controller/metrics.go

@@ -17,11 +17,15 @@ limitations under the License.
 package controller
 
 import (
+	"context"
 	"time"
 
-	corev1 "k8s.io/api/core/v1"
+	"github.com/go-logr/logr"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/reference"
+	ctrl "sigs.k8s.io/controller-runtime"
 	crtlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 
 	"github.com/fluxcd/pkg/apis/meta"
@@ -37,49 +41,70 @@ import (
 //       controller.Metrics
 //     }
 //
-// then you can call either or both of `RecordDuration` and
-// `RecordReadinessMetric`. API types used in GOTK will usually
+// then you can call either or both of RecordDuration and
+// RecordReadinessMetric. API types used in GOTK will usually
 // already be suitable for passing (as a pointer) as the second
 // argument to `RecordReadinessMetric`.
 //
-// When initialising controllers in main.go, use `MustMakeMetrics` to
+// When initialising controllers in main.go, use MustMakeMetrics to
 // create a working Metrics value; you can supply the same value to
 // all reconcilers.
 type Metrics struct {
+	Scheme          *runtime.Scheme
 	MetricsRecorder *metrics.Recorder
 }
 
-func MustMakeMetrics() Metrics {
+func MustMakeMetrics(mgr ctrl.Manager) Metrics {
 	metricsRecorder := metrics.NewRecorder()
 	crtlmetrics.Registry.MustRegister(metricsRecorder.Collectors()...)
-	return Metrics{MetricsRecorder: metricsRecorder}
+
+	return Metrics{
+		Scheme:          mgr.GetScheme(),
+		MetricsRecorder: metricsRecorder,
+	}
 }
 
-func (m Metrics) RecordDuration(ref *corev1.ObjectReference, startTime time.Time) {
+func (m Metrics) RecordDuration(ctx context.Context, obj readinessMetricsable, startTime time.Time) {
 	if m.MetricsRecorder != nil {
+		ref, err := reference.GetReference(m.Scheme, obj)
+		if err != nil {
+			logr.FromContextOrDiscard(ctx).Error(err, "unable to record duration")
+			return
+		}
 		m.MetricsRecorder.RecordDuration(*ref, startTime)
 	}
 }
 
-func (m Metrics) RecordSuspend(ref *corev1.ObjectReference, suspend bool) {
+func (m Metrics) RecordSuspend(ctx context.Context, obj readinessMetricsable, suspend bool) {
 	if m.MetricsRecorder != nil {
+		ref, err := reference.GetReference(m.Scheme, obj)
+		if err != nil {
+			logr.FromContextOrDiscard(ctx).Error(err, "unable to record suspend")
+			return
+		}
 		m.MetricsRecorder.RecordSuspend(*ref, suspend)
 	}
 }
 
 type readinessMetricsable interface {
+	runtime.Object
 	metav1.Object
 	meta.ObjectWithStatusConditions
 }
 
-func (m Metrics) RecordReadinessMetric(ref *corev1.ObjectReference, obj readinessMetricsable) {
-	m.RecordConditionMetric(ref, obj, meta.ReadyCondition)
+func (m Metrics) RecordReadinessMetric(ctx context.Context, obj readinessMetricsable) {
+	m.RecordConditionMetric(ctx, obj, meta.ReadyCondition)
 }
 
-func (m Metrics) RecordConditionMetric(ref *corev1.ObjectReference, obj readinessMetricsable, conditionType string) {
+func (m Metrics) RecordConditionMetric(ctx context.Context, obj readinessMetricsable, conditionType string) {
 	if m.MetricsRecorder == nil {
 		return
 	}
+	ref, err := reference.GetReference(m.Scheme, obj)
+	if err != nil {
+		logr.FromContextOrDiscard(ctx).Error(err, "unable to record condition metric")
+		return
+	}
 	rc := apimeta.FindStatusCondition(*obj.GetStatusConditions(), conditionType)
 	if rc == nil {
 		rc = &metav1.Condition{