diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index 46754e2c385c..1467d765a77d 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/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(), diff --git a/pkg/operator/ceph/cluster/controller_test.go b/pkg/operator/ceph/cluster/controller_test.go index 477f36046ad8..0c5ea2219d3f 100644 --- a/pkg/operator/ceph/cluster/controller_test.go +++ b/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 diff --git a/pkg/operator/ceph/object/controller.go b/pkg/operator/ceph/object/controller.go index 570d1df8c4dd..e155d376f118 100644 --- a/pkg/operator/ceph/object/controller.go +++ b/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, } diff --git a/pkg/operator/ceph/object/controller_test.go b/pkg/operator/ceph/object/controller_test.go index 0a19ada55b0d..06f0ca2475dc 100644 --- a/pkg/operator/ceph/object/controller_test.go +++ b/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, } diff --git a/pkg/operator/ceph/reporting/reporting.go b/pkg/operator/ceph/reporting/reporting.go index 98ec711ab971..19d816a40ded 100644 --- a/pkg/operator/ceph/reporting/reporting.go +++ b/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) diff --git a/pkg/operator/k8sutil/events.go b/pkg/operator/k8sutil/events.go deleted file mode 100644 index 5d2486ee3d2f..000000000000 --- a/pkg/operator/k8sutil/events.go +++ /dev/null @@ -1,79 +0,0 @@ -/* -Copyright 2021 The Rook Authors. All rights reserved. - -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 k8sutil for Kubernetes helpers. -package k8sutil - -import ( - "fmt" - "time" - - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/record" -) - -// EventReporter is custom events reporter type which allows user to limit the events -type EventReporter struct { - recorder record.EventRecorder - - // lastReportedEvent will have a last captured event - lastReportedEvent map[string]string - - // lastReportedEventTime will be the time of lastReportedEvent - lastReportedEventTime map[string]time.Time -} - -// NewEventReporter returns EventReporter object -func NewEventReporter(recorder record.EventRecorder) *EventReporter { - return &EventReporter{ - recorder: recorder, - lastReportedEvent: make(map[string]string), - lastReportedEventTime: make(map[string]time.Time), - } -} - -// ReportIfNotPresent will report event if lastReportedEvent is not the same in last 60 minutes -func (rep *EventReporter) ReportIfNotPresent(instance runtime.Object, eventType, eventReason, msg string) { - - nameSpacedName, err := getNameSpacedName(instance) - if err != nil { - return - } - - eventKey := getEventKey(eventType, eventReason, msg) - - if rep.lastReportedEvent[nameSpacedName] != eventKey || rep.lastReportedEventTime[nameSpacedName].Add(time.Minute*60).Before(time.Now()) { - logger.Info("Reporting Event ", nameSpacedName, " ", eventKey) - rep.lastReportedEvent[nameSpacedName] = eventKey - rep.lastReportedEventTime[nameSpacedName] = time.Now() - rep.recorder.Event(instance, eventType, eventReason, msg) - } else { - logger.Debug("Not Reporting Event because event is same as the old one:", nameSpacedName, " ", eventKey) - } -} - -func getNameSpacedName(instance runtime.Object) (string, error) { - objMeta, err := meta.Accessor(instance) - if err != nil { - return "", err - } - return objMeta.GetNamespace() + ":" + objMeta.GetName(), nil -} - -func getEventKey(eventType, eventReason, msg string) string { - return fmt.Sprintf("%s:%s:%s", eventType, eventReason, msg) -} diff --git a/pkg/operator/k8sutil/events_test.go b/pkg/operator/k8sutil/events_test.go deleted file mode 100644 index fb25f5c1478e..000000000000 --- a/pkg/operator/k8sutil/events_test.go +++ /dev/null @@ -1,147 +0,0 @@ -/* -Copyright 2021 The Rook Authors. All rights reserved. - -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 k8sutil for Kubernetes helpers. -package k8sutil - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/record" -) - -func getEventsOccurences(channel chan string) map[string]int { - - foundEvents := make(map[string]int) - - for len(channel) > 0 { - e := <-channel - foundEvents[e]++ - } - - return foundEvents -} - -func TestReportIfNotPresent(t *testing.T) { - pod1 := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod1", - Namespace: "rook-ceph", - }, - } - - pod2 := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod2", - Namespace: "rook-ceph", - }, - } - - testCases := []struct { - eventReported int - changeTime bool - ReportAnotherEvent bool - ReportEventForDifferentObject bool - }{ - { - // verify ReportIfNotPresent is called once and event is reported once - eventReported: 1, - }, - { - // verify ReportIfNotPresent is called twice and event is reported once - eventReported: 2, - }, - { - // verify ReportIfNotPresent report same event again when event is not present on cluster - eventReported: 1, - changeTime: true, - }, - { - // verify ReportIfNotPresent report same event again when event is not present on cluster - eventReported: 2, - changeTime: true, - }, - { - // verify it report event "a" both the times if events came like "a", "b", "a" - eventReported: 1, - ReportAnotherEvent: true, - }, - { - // verify it report event "a" both the times if events came like "a", "b", "a" - eventReported: 2, - ReportAnotherEvent: true, - }, - { - // verify it does not report the same event for same objects if multiple objects come into the picture - eventReported: 1, - ReportEventForDifferentObject: true, - }, - { - // verify it does not report the same event for same objects if multiple objects come into the picture - eventReported: 2, - ReportEventForDifferentObject: true, - }, - } - - for _, tc := range testCases { - eventType, eventReason, eventMsg := corev1.EventTypeNormal, "Created", "Pod has been created" - - frecorder := record.NewFakeRecorder(1024) - reporter := NewEventReporter(frecorder) - - for i := 0; i < tc.eventReported; i++ { - reporter.ReportIfNotPresent(pod1, eventType, eventReason, eventMsg) - } - - foundEvents := getEventsOccurences(frecorder.Events) - assert.Equal(t, 1, foundEvents[eventType+" "+eventReason+" "+eventMsg]) - - if tc.changeTime { - nameSpacedName, err := getNameSpacedName(pod1) - assert.NoError(t, err) - ftime := reporter.lastReportedEventTime[nameSpacedName].Add(time.Minute * -60) - reporter.lastReportedEventTime[nameSpacedName] = ftime - - reporter.ReportIfNotPresent(pod1, eventType, eventReason, eventMsg) - foundEvents := getEventsOccurences(frecorder.Events) - assert.Equal(t, 1, foundEvents[eventType+" "+eventReason+" "+eventMsg]) - } - - if tc.ReportAnotherEvent { - reporter.ReportIfNotPresent(pod1, corev1.EventTypeWarning, eventReason, eventMsg) - foundEvents := getEventsOccurences(frecorder.Events) - assert.Equal(t, 1, foundEvents[corev1.EventTypeWarning+" "+eventReason+" "+eventMsg]) - - reporter.ReportIfNotPresent(pod1, eventType, eventReason, eventMsg) - foundEvents = getEventsOccurences(frecorder.Events) - assert.Equal(t, 1, foundEvents[eventType+" "+eventReason+" "+eventMsg]) - } - - if tc.ReportEventForDifferentObject { - reporter.ReportIfNotPresent(pod2, eventType, eventReason, eventMsg) - foundEvents := getEventsOccurences(frecorder.Events) - assert.Equal(t, 1, foundEvents[eventType+" "+eventReason+" "+eventMsg]) - - reporter.ReportIfNotPresent(pod1, eventType, eventReason, eventMsg) - foundEvents = getEventsOccurences(frecorder.Events) - assert.Equal(t, 0, foundEvents[eventType+" "+eventReason+" "+eventMsg]) - } - } -}