Strip glog from the event recorder

The event recorded was using glog to log, which it should not have been.
Plus, the event information is inherently structured, so it's a win to
provide structured logging of the event.  We probably want to have the
recorder have a unique logger per controller (or consumer in general) --
right now, it's just using the glog manager logger.

Author: DirectXMan12

pkg/internal/recorder/recorder.go

@@ -19,7 +19,7 @@ package recorder
 import (
 	"fmt"
 
-	"github.com/golang/glog"
+	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
@@ -34,19 +34,24 @@ type provider struct {
 	scheme *runtime.Scheme
 	// eventBroadcaster to create new recorder instance
 	eventBroadcaster record.EventBroadcaster
+	// logger is the logger to use when logging diagnostic event info
+	logger logr.Logger
 }
 
 // NewProvider create a new Provider instance.
-func NewProvider(config *rest.Config, scheme *runtime.Scheme) (recorder.Provider, error) {
+func NewProvider(config *rest.Config, scheme *runtime.Scheme, logger logr.Logger) (recorder.Provider, error) {
 	clientSet, err := kubernetes.NewForConfig(config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to init clientSet: %v", err)
 	}
 
-	p := &provider{scheme: scheme}
+	p := &provider{scheme: scheme, logger: logger}
 	p.eventBroadcaster = record.NewBroadcaster()
-	p.eventBroadcaster.StartLogging(glog.Infof)
 	p.eventBroadcaster.StartRecordingToSink(&typedcorev1.EventSinkImpl{Interface: clientSet.CoreV1().Events("")})
+	p.eventBroadcaster.StartEventWatcher(
+		func(e *corev1.Event) {
+			p.logger.V(1).Info(e.Type, "object", e.InvolvedObject, "reason", e.Reason, "message", e.Message)
+		})
 
 	return p, nil
 }

pkg/internal/recorder/recorder_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package recorder_test
 
 import (
+	tlog "github.com/go-logr/logr/testing"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -26,7 +27,7 @@ import (
 var _ = Describe("recorder.Provider", func() {
 	Describe("NewProvider", func() {
 		It("should return a provider instance and a nil error.", func() {
-			provider, err := recorder.NewProvider(cfg, scheme.Scheme)
+			provider, err := recorder.NewProvider(cfg, scheme.Scheme, tlog.NullLogger{})
 			Expect(provider).NotTo(BeNil())
 			Expect(err).NotTo(HaveOccurred())
 		})
@@ -35,13 +36,13 @@ var _ = Describe("recorder.Provider", func() {
 			// Invalid the config
 			cfg1 := *cfg
 			cfg1.ContentType = "invalid-type"
-			_, err := recorder.NewProvider(&cfg1, scheme.Scheme)
+			_, err := recorder.NewProvider(&cfg1, scheme.Scheme, tlog.NullLogger{})
 			Expect(err.Error()).To(ContainSubstring("failed to init clientSet"))
 		})
 	})
 	Describe("GetEventRecorder", func() {
 		It("should return a recorder instance.", func() {
-			provider, err := recorder.NewProvider(cfg, scheme.Scheme)
+			provider, err := recorder.NewProvider(cfg, scheme.Scheme, tlog.NullLogger{})
 			Expect(err).NotTo(HaveOccurred())
 
 			recorder := provider.GetEventRecorderFor("test")

pkg/manager/manager.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -85,7 +86,7 @@ type Options struct {
 	// Dependency injection for testing
 	newCache            func(config *rest.Config, opts cache.Options) (cache.Cache, error)
 	newClient           func(config *rest.Config, options client.Options) (client.Client, error)
-	newRecorderProvider func(config *rest.Config, scheme *runtime.Scheme) (recorder.Provider, error)
+	newRecorderProvider func(config *rest.Config, scheme *runtime.Scheme, logger logr.Logger) (recorder.Provider, error)
 }
 
 // Runnable allows a component to be started.
@@ -132,7 +133,9 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		return nil, err
 	}
 	// Create the recorder provider to inject event recorders for the components.
-	recorderProvider, err := options.newRecorderProvider(config, options.Scheme)
+	// TODO(directxman12): the log for the event provider should have a context (name, tags, etc) specific
+	// to the particular controller that it's being injected into, rather than a generic one like is here.
+	recorderProvider, err := options.newRecorderProvider(config, options.Scheme, log.WithName("events"))
 	if err != nil {
 		return nil, err
 	}

pkg/manager/manager_test.go

@@ -19,6 +19,7 @@ package manager
 import (
 	"fmt"
 
+	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -86,7 +87,7 @@ var _ = Describe("manger.Manager", func() {
 		})
 		It("should return an error it can't create a recorder.Provider", func(done Done) {
 			m, err := New(cfg, Options{
-				newRecorderProvider: func(config *rest.Config, scheme *runtime.Scheme) (recorder.Provider, error) {
+				newRecorderProvider: func(config *rest.Config, scheme *runtime.Scheme, logger logr.Logger) (recorder.Provider, error) {
 					return nil, fmt.Errorf("expected error")
 				}})
 			Expect(m).To(BeNil())