operator: always report events

The original PR which added event reporting unnecessarily "optimized" to
prevent spamming the API controller[1].

It is sometimes important to get events as they happen and not hide new
events behind preexisting older events. For example, in integration
tests, we may often want to wait for a controller to finish processing
an update, and the best way to do that is to wait for the
"ReconcileSucceeded" event. In order for this to be useful, the events
must be reported each time.

If we begin having problems with events being reported too often, then
we should fix the underlying issue of reconciles happening too often
instead of relying on a time-based "optimization" that hides recent
event reports that may be useful.

[1]: #7222

Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>

pkg/operator/ceph/cluster/controller.go

@@ -41,6 +41,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apituntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -88,7 +89,7 @@ type ClusterController struct {
 	osdChecker     *osd.OSDHealthMonitor
 	client         client.Client
 	namespacedName types.NamespacedName
-	recorder       *k8sutil.EventReporter
+	recorder       record.EventRecorder
 	OpManagerCtx   context.Context
 }
 
@@ -112,7 +113,7 @@ func newReconciler(mgr manager.Manager, ctx *clusterd.Context, clusterController
 	// add "rook-" prefix to the controller name to make sure it is clear to all reading the events
 	// that they are coming from Rook. The controller name already has context that it is for Ceph
 	// and from the cluster controller.
-	clusterController.recorder = k8sutil.NewEventReporter(mgr.GetEventRecorderFor("rook-" + controllerName))
+	clusterController.recorder = mgr.GetEventRecorderFor("rook-" + controllerName)
 
 	return &ReconcileCephCluster{
 		client:            mgr.GetClient(),

pkg/operator/ceph/cluster/controller_test.go

@@ -24,7 +24,6 @@ import (
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	"github.com/rook/rook/pkg/client/clientset/versioned/scheme"
 	"github.com/rook/rook/pkg/clusterd"
-	"github.com/rook/rook/pkg/operator/k8sutil"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -82,7 +81,7 @@ func TestReconcileDeleteCephCluster(t *testing.T) {
 		// create the cluster controller and tell it that the cluster has been deleted
 		controller := NewClusterController(clusterdCtx, "")
 		fakeRecorder := record.NewFakeRecorder(5)
-		controller.recorder = k8sutil.NewEventReporter(fakeRecorder)
+		controller.recorder = fakeRecorder
 
 		// Create a fake client to mock API calls
 		// Make sure it has the fake CephCluster that is to be deleted in it

pkg/operator/ceph/object/controller.go

@@ -42,6 +42,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -87,7 +88,7 @@ type ReconcileCephObjectStore struct {
 	clusterSpec         *cephv1.ClusterSpec
 	clusterInfo         *cephclient.ClusterInfo
 	objectStoreContexts map[string]*objectStoreHealth
-	recorder            *k8sutil.EventReporter
+	recorder            record.EventRecorder
 	opManagerContext    context.Context
 	opConfig            opcontroller.OperatorConfig
 }
@@ -113,7 +114,7 @@ func newReconciler(mgr manager.Manager, context *clusterd.Context, opManagerCont
 		context:             context,
 		bktclient:           bktclient.NewForConfigOrDie(context.KubeConfig),
 		objectStoreContexts: make(map[string]*objectStoreHealth),
-		recorder:            k8sutil.NewEventReporter(mgr.GetEventRecorderFor("rook-" + controllerName)),
+		recorder:            mgr.GetEventRecorderFor("rook-" + controllerName),
 		opManagerContext:    opManagerContext,
 		opConfig:            opConfig,
 	}

pkg/operator/ceph/object/controller_test.go

@@ -349,7 +349,7 @@ func TestCephObjectStoreController(t *testing.T) {
 			scheme:              s,
 			context:             c,
 			objectStoreContexts: make(map[string]*objectStoreHealth),
-			recorder:            k8sutil.NewEventReporter(record.NewFakeRecorder(5)),
+			recorder:            record.NewFakeRecorder(5),
 			opManagerContext:    context.TODO(),
 		}
 
@@ -684,7 +684,7 @@ func TestCephObjectStoreControllerMultisite(t *testing.T) {
 		scheme:              s,
 		context:             c,
 		objectStoreContexts: make(map[string]*objectStoreHealth),
-		recorder:            k8sutil.NewEventReporter(record.NewFakeRecorder(5)),
+		recorder:            record.NewFakeRecorder(5),
 		opManagerContext:    ctx,
 	}
 

pkg/operator/ceph/reporting/reporting.go

@@ -24,10 +24,10 @@ import (
 	"github.com/coreos/pkg/capnslog"
 	"github.com/pkg/errors"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
-	"github.com/rook/rook/pkg/operator/k8sutil"
 	"github.com/rook/rook/pkg/util/dependents"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -40,7 +40,7 @@ import (
 // error returned by the reconcile.
 // The function is designed to return the appropriate values needed for the controller-runtime
 // framework's Reconcile() method.
-func ReportReconcileResult(logger *capnslog.PackageLogger, recorder *k8sutil.EventReporter,
+func ReportReconcileResult(logger *capnslog.PackageLogger, recorder record.EventRecorder,
 	obj client.Object, reconcileResponse reconcile.Result, err error,
 ) (reconcile.Result, error) {
 	kind := obj.GetObjectKind().GroupVersionKind().Kind
@@ -51,7 +51,7 @@ func ReportReconcileResult(logger *capnslog.PackageLogger, recorder *k8sutil.Eve
 		logger.Errorf("failed to reconcile %s %q. %v", kind, nsName, err)
 
 		// 2. event
-		recorder.ReportIfNotPresent(obj, corev1.EventTypeWarning, string(cephv1.ReconcileFailed), err.Error())
+		recorder.Event(obj, corev1.EventTypeWarning, string(cephv1.ReconcileFailed), err.Error())
 
 		if !reconcileResponse.IsZero() {
 			// The framework will requeue immediately if there is an error. If we get an error with
@@ -67,7 +67,7 @@ func ReportReconcileResult(logger *capnslog.PackageLogger, recorder *k8sutil.Eve
 		logger.Debug(successMsg)
 
 		// 2. event
-		recorder.ReportIfNotPresent(obj, corev1.EventTypeNormal, string(cephv1.ReconcileSucceeded), successMsg)
+		recorder.Event(obj, corev1.EventTypeNormal, string(cephv1.ReconcileSucceeded), successMsg)
 	}
 
 	return reconcileResponse, err
@@ -116,7 +116,7 @@ func ReportDeletionBlockedDueToDependents(
 // 2. as an event on the object (via the given event recorder)
 // 3. as a condition on the object (added to the object's conditions list given)
 func ReportDeletionNotBlockedDueToDependents(
-	logger *capnslog.PackageLogger, client client.Client, recorder *k8sutil.EventReporter, obj cephv1.StatusConditionGetter,
+	logger *capnslog.PackageLogger, client client.Client, recorder record.EventRecorder, obj cephv1.StatusConditionGetter,
 ) {
 	kind := obj.GetObjectKind().GroupVersionKind().Kind
 	nsName := types.NamespacedName{
@@ -130,7 +130,7 @@ func ReportDeletionNotBlockedDueToDependents(
 	logger.Infof("%s. %s", safeMsg, deletingMsg)
 
 	// 2. event
-	recorder.ReportIfNotPresent(obj, corev1.EventTypeNormal, string(cephv1.DeletingReason), deletingMsg)
+	recorder.Event(obj, corev1.EventTypeNormal, string(cephv1.DeletingReason), deletingMsg)
 
 	// 3. condition
 	unblockedCond := dependents.DeletionBlockedDueToDependentsCondition(false, safeMsg)