chore: Remove unnecessary ctx pointer (kyma-project#839)

* chore: remove unnecessary ctx pointer * after updateKyma, it should immediately in queue make sure getSyncedContext return original ctx even return error * extract error handling out of getSyncedContext --------- Co-authored-by: Xin Ruan <[email protected]>
nesmabadr · Sep 25, 2023 · a029af8 · a029af8
1 parent b5b026e
commit a029af8
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 34 deletions.
diff --git a/controllers/kyma_controller.go b/controllers/kyma_controller.go
@@ -118,26 +118,50 @@ func (r *KymaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{RequeueAfter: r.RequeueIntervals.Success}, nil
 	}
 
-	return r.reconcile(ctx, kyma)
-}
+	ctx, err := r.getSyncedContext(ctx, kyma)
 
-//nolint:gocognit
-func (r *KymaReconciler) reconcile(ctx context.Context, kyma *v1beta2.Kyma) (ctrl.Result, error) {
-	if r.SyncKymaEnabled(kyma) {
-		remoteClient := remote.NewClientWithConfig(r.Client, r.KcpRestConfig)
-		if err := remote.InitializeSyncContext(&ctx, kyma,
-			r.RemoteSyncNamespace, remoteClient, r.RemoteClientCache); err != nil {
-			if !kyma.DeletionTimestamp.IsZero() && errors.Is(err, remote.ErrAccessSecretNotFound) {
-				return ctrl.Result{}, r.removeAllFinalizersAndUpdateKyma(ctx, kyma)
-			}
-			if util.IsConnectionRefused(err) {
-				r.RemoteClientCache.Del(client.ObjectKeyFromObject(kyma))
-			}
+	if !kyma.DeletionTimestamp.IsZero() && errors.Is(err, remote.ErrAccessSecretNotFound) {
+		logger.Info("access secret not found for kyma, assuming already deleted cluster")
+		r.removeAllFinalizers(kyma)
+		return ctrl.Result{Requeue: true}, r.updateKyma(ctx, kyma)
+	}
+
+	if err != nil {
+		if util.IsConnectionRefused(err) {
+			r.deleteRemoteClientCache(ctx, kyma)
 			r.enqueueWarningEvent(kyma, syncContextError, err)
-			return r.requeueWithError(ctx, kyma, err)
 		}
+		return r.requeueWithError(ctx, kyma, err)
+	}
+
+	return r.reconcile(ctx, kyma)
+}
+
+func (r *KymaReconciler) deleteRemoteClientCache(ctx context.Context, kyma *v1beta2.Kyma) {
+	logger := ctrlLog.FromContext(ctx)
+	logger.Info("connection refused, assuming connection is invalid and resetting cache-entry for kyma")
+	r.RemoteClientCache.Del(client.ObjectKeyFromObject(kyma))
+}
+
+// getSyncedContext returns either the original context (in case Syncing is disabled) or initiates a sync-context
+// with a remote client and returns that context instead.
+// In case of failure, original context should be returned.
+func (r *KymaReconciler) getSyncedContext(ctx context.Context, kyma *v1beta2.Kyma) (context.Context, error) {
+	if !r.SyncKymaEnabled(kyma) {
+		return ctx, nil
+	}
+
+	remoteClient := remote.NewClientWithConfig(r.Client, r.KcpRestConfig)
+	ctxWithSync, err := remote.InitializeSyncContext(ctx, kyma,
+		r.RemoteSyncNamespace, remoteClient, r.RemoteClientCache)
+	if err != nil {
+		return ctx, err
 	}
 
+	return ctxWithSync, nil
+}
+
+func (r *KymaReconciler) reconcile(ctx context.Context, kyma *v1beta2.Kyma) (ctrl.Result, error) {
 	if !kyma.DeletionTimestamp.IsZero() && kyma.Status.State != v1beta2.StateDeleting {
 		if err := r.deleteRemoteKyma(ctx, kyma); err != nil {
 			return r.requeueWithError(ctx, kyma, err)
@@ -394,27 +418,18 @@ func (r *KymaReconciler) handleDeletingState(ctx context.Context, kyma *v1beta2.
 		logger.Info("removed remote finalizer")
 	}
 
-	if err := r.removeFinalizerAndUpdateKyma(ctx, kyma); err != nil {
-		return false, err
-	}
-
 	if err := metrics.RemoveKymaStateMetrics(kyma); err != nil {
 		logger.V(log.DebugLevel).Info(fmt.Sprintf("error occurred while removing kyma state metrics: %s", err))
 	}
 
-	return false, nil
+	controllerutil.RemoveFinalizer(kyma, v1beta2.Finalizer)
+	return false, r.updateKyma(ctx, kyma)
 }
 
-func (r *KymaReconciler) removeAllFinalizersAndUpdateKyma(ctx context.Context, kyma *v1beta2.Kyma) error {
+func (r *KymaReconciler) removeAllFinalizers(kyma *v1beta2.Kyma) {
 	for _, finalizer := range kyma.Finalizers {
 		controllerutil.RemoveFinalizer(kyma, finalizer)
 	}
-	return r.updateKyma(ctx, kyma)
-}
-
-func (r *KymaReconciler) removeFinalizerAndUpdateKyma(ctx context.Context, kyma *v1beta2.Kyma) error {
-	controllerutil.RemoveFinalizer(kyma, v1beta2.Finalizer)
-	return r.updateKyma(ctx, kyma)
 }
 
 func (r *KymaReconciler) updateKyma(ctx context.Context, kyma *v1beta2.Kyma) error {
@@ -550,7 +565,6 @@ func (r *KymaReconciler) SyncKymaEnabled(kyma *v1beta2.Kyma) bool {
 	if !r.InKCPMode {
 		return false
 	}
-
 	return kyma.HasSyncLabelEnabled()
 }
 

diff --git a/pkg/remote/context.go b/pkg/remote/context.go
@@ -13,15 +13,14 @@ type syncContextKey = struct{}
 
 var ErrIsNoSyncContext = errors.New("the given value is not a pointer to a kyma synchronization context")
 
-func InitializeSyncContext(ctx *context.Context, kyma *v1beta2.Kyma,
+func InitializeSyncContext(ctx context.Context, kyma *v1beta2.Kyma,
 	syncNamespace string, kcp Client, cache *ClientCache,
-) error {
-	syncContext, err := InitializeKymaSynchronizationContext(*ctx, kcp, cache, kyma, syncNamespace)
+) (context.Context, error) {
+	syncContext, err := InitializeKymaSynchronizationContext(ctx, kcp, cache, kyma, syncNamespace)
 	if err != nil {
-		return fmt.Errorf("initializing sync context failed: %w", err)
+		return nil, fmt.Errorf("initializing sync context failed: %w", err)
 	}
-	*ctx = context.WithValue(*ctx, syncContextKey{}, syncContext)
-	return nil
+	return context.WithValue(ctx, syncContextKey{}, syncContext), nil
 }
 
 func SyncContextFromContext(ctx context.Context) (*KymaSynchronizationContext, error) {