From 47d71a0a034d2d60154b69e224b236babc26f97e Mon Sep 17 00:00:00 2001
From: Milos Tomic <59831542+tmilos77@users.noreply.github.com>
Date: Mon, 27 Jan 2025 10:56:36 +0100
Subject: [PATCH] fix: Scope finalizer do not add finalizer to Kyma being
 deleted (#986)

---
 pkg/composed/bridge.go            | 7 +++++++
 pkg/kcp/scope/addKymaFinalizer.go | 7 ++++++-
 pkg/kcp/scope/reconciler.go       | 8 +++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/pkg/composed/bridge.go b/pkg/composed/bridge.go
index fddd89f52..d699438a1 100644
--- a/pkg/composed/bridge.go
+++ b/pkg/composed/bridge.go
@@ -3,9 +3,16 @@ package composed
 import (
 	"context"
 	"errors"
+	"github.com/go-logr/logr"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
+func HandleWithoutLogging(err error, ctx context.Context) (ctrl.Result, error) {
+	logger := logr.Discard()
+	ctx = LoggerIntoCtx(ctx, logger)
+	return Handle(err, ctx)
+}
+
 func Handle(err error, ctx context.Context) (ctrl.Result, error) {
 	logger := LoggerFromCtx(ctx)
 	if errors.Is(err, context.DeadlineExceeded) {
diff --git a/pkg/kcp/scope/addKymaFinalizer.go b/pkg/kcp/scope/addKymaFinalizer.go
index 054a23179..e411a73a5 100644
--- a/pkg/kcp/scope/addKymaFinalizer.go
+++ b/pkg/kcp/scope/addKymaFinalizer.go
@@ -12,7 +12,12 @@ func addKymaFinalizer(ctx context.Context, st composed.State) (error, context.Co
 
 	if !state.ObjAsScope().DeletionTimestamp.IsZero() {
 		// Scope is being deleted
-		return nil, nil
+		return nil, ctx
+	}
+
+	if !state.kyma.GetDeletionTimestamp().IsZero() {
+		// kyma is being deleted - it has deletionTimestamp and finalizer can not be added in that state
+		return nil, ctx
 	}
 
 	added, err := composed.PatchObjAddFinalizer(ctx, cloudcontrolv1beta1.FinalizerName, state.kyma, state.Cluster().K8sClient())
diff --git a/pkg/kcp/scope/reconciler.go b/pkg/kcp/scope/reconciler.go
index 8e72f8c91..808cf7174 100644
--- a/pkg/kcp/scope/reconciler.go
+++ b/pkg/kcp/scope/reconciler.go
@@ -48,7 +48,13 @@ func (r *scopeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 	state := r.newState(req)
 	action := r.newAction()
 
-	return composed.Handle(action(ctx, state))
+	// Scope reconciler is triggered very often due to KLM constant changes on watched Kyma
+	// HandleWithoutLogging should be used, so no reconciliation outcome is logged since it most cases
+	// the reconciler will do nothing since no change regarding CloudManager was done on Kyma
+	// so it will just produce an unnecessary log entry "Reconciliation finished without control error - doing stop and forget"
+	// To accommodate this non-functional requirement to keep logs tidy and prevent excessive and not so usable log entries
+	// in cases when Scope actually did something we have to accept the discomfort of not having this log entry
+	return composed.HandleWithoutLogging(action(ctx, state))
 }
 
 func (r *scopeReconciler) newState(req ctrl.Request) *State {