From 1c85ddd32771ca61cc89e781eb3aa9392ea835d1 Mon Sep 17 00:00:00 2001 From: Oded Viner Date: Wed, 21 May 2025 15:43:05 +0300 Subject: [PATCH] core: improve reporting for reconcile requeue cases when a reconcile returns a requeue result without an error (e.g. due to a missing cephcluster), the previous logic reported it as a successful reconcile. this was misleading and made it hard to understand the actual controller behavior this change adds a new condition to log and emit a "requeuing" event when reconcileResponse.Requeue is true and err is nil. this ensures more accurate event reporting and improves observability across all controllers using reportreconcileresult. Signed-off-by: Oded Viner (cherry picked from commit 83ce9a6a63ab6bc72b5e6f2208a43dc7e1b51b68) --- Documentation/CRDs/specification.md | 3 +++ pkg/apis/ceph.rook.io/v1/types.go | 3 ++- pkg/operator/ceph/reporting/reporting.go | 4 ++++ pkg/operator/ceph/reporting/reporting_test.go | 6 ++++-- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md index 57c79c9ab..0597ca9b1 100644 --- a/Documentation/CRDs/specification.md +++ b/Documentation/CRDs/specification.md @@ -5306,6 +5306,9 @@ deletion.

"ReconcileFailed"

ReconcileFailed represents when a resource reconciliation failed.

+

"ReconcileRequeuing"

+

ReconcileRequeuing represents when a resource reconciliation requeue.

+

"ReconcileStarted"

ReconcileStarted represents when a resource reconciliation started.

diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index a251f9254..383934744 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -559,7 +559,8 @@ const ( ReconcileFailed ConditionReason = "ReconcileFailed" // ReconcileStarted represents when a resource reconciliation started. ReconcileStarted ConditionReason = "ReconcileStarted" - + // ReconcileRequeuing represents when a resource reconciliation requeue. + ReconcileRequeuing ConditionReason = "ReconcileRequeuing" // DeletingReason represents when Rook has detected a resource object should be deleted. DeletingReason ConditionReason = "Deleting" // ObjectHasDependentsReason represents when a resource object has dependents that are blocking diff --git a/pkg/operator/ceph/reporting/reporting.go b/pkg/operator/ceph/reporting/reporting.go index 0c4c9a0e6..dde20385a 100644 --- a/pkg/operator/ceph/reporting/reporting.go +++ b/pkg/operator/ceph/reporting/reporting.go @@ -156,6 +156,10 @@ func ReportReconcileResult( // intent. return reconcileResponse, nil } + } else if reconcileResponse.Requeue { + msg := fmt.Sprintf("requeuing %s %q", kind, nsName) + logger.Debug(msg) + recorder.Event(objCopy, corev1.EventTypeNormal, string(cephv1.ReconcileRequeuing), msg) } else { successMsg := fmt.Sprintf("successfully configured %s %q", kind, nsName) diff --git a/pkg/operator/ceph/reporting/reporting_test.go b/pkg/operator/ceph/reporting/reporting_test.go index e86a3aef8..5a9fa7029 100644 --- a/pkg/operator/ceph/reporting/reporting_test.go +++ b/pkg/operator/ceph/reporting/reporting_test.go @@ -94,6 +94,8 @@ func TestReportReconcileResult(t *testing.T) { successMsg := `successfully configured CephCluster "rook-ceph/my-cluster"` successEvent := `Normal ReconcileSucceeded ` + successMsg + requeueMsg := `requeuing CephCluster "rook-ceph/my-cluster"` + requeueEvent := `Normal ReconcileRequeuing ` + requeueMsg fakeErr := errors.New("fake-err") errorMsg := `failed to reconcile CephCluster "rook-ceph/my-cluster". fake-err` @@ -131,9 +133,9 @@ func TestReportReconcileResult(t *testing.T) { cephCluster, inResult, nil) assert.NoError(t, err) assert.Equal(t, inResult, result) - assert.Equal(t, successMsg, strings.TrimSpace(logBuf.String())) + assert.Equal(t, requeueMsg, strings.TrimSpace(logBuf.String())) assert.Len(t, recorder.Events, 1) - assert.Equal(t, successEvent, <-recorder.Events) + assert.Equal(t, requeueEvent, <-recorder.Events) }) t.Run("failed reconcile with requeue", func(t *testing.T) {