adding more refactoring and unit tests

Author: PatrickLaabs

Makefile

@@ -231,7 +231,7 @@ CAPI_KIND_CLUSTER_NAME ?= capi-test
 # It is set by Prow GIT_TAG, a git-based tag of the form vYYYYMMDD-hash, e.g., v20210120-v0.3.10-308-gc61521971
 
 # Next release is: v1.0.0-preview
-TAG ?= v1.0.0-preview.1
+TAG ?= v1.0.0-preview.2
 ARCH ?= $(shell go env GOARCH)
 ALL_ARCH = amd64 arm arm64
 

config/default/manager_image_patch.yaml

@@ -7,5 +7,5 @@ spec:
   template:
     spec:
       containers:
-      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview
+      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview.1
         name: manager

config/default/manager_image_patch.yaml-e

@@ -7,5 +7,5 @@ spec:
   template:
     spec:
       containers:
-      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview
+      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview.1
         name: manager

controllers/cdk8sappproxy/cdk8sappproxy_consts.go

@@ -4,9 +4,3 @@ package cdk8sappproxy
 const (
 	Finalizer = "cdk8sappproxy.addons.cluster.x-k8s.io/finalizer"
 )
-
-// Operation represents the type of operation being performed by the cdk8sappproxy controller.
-const (
-	OperationDeletion = "deletion"
-	OperationNormal   = "normal"
-)

controllers/cdk8sappproxy/cdk8sappproxy_controller.go

@@ -48,19 +48,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-func (r *Reconciler) getCdk8sAppProxyForPolling(ctx context.Context, proxyName types.NamespacedName) (*addonsv1alpha1.Cdk8sAppProxy, error) {
-	cdk8sAppProxy := &addonsv1alpha1.Cdk8sAppProxy{}
-	if err := r.Get(ctx, proxyName, cdk8sAppProxy); err != nil {
-		if apierrors.IsNotFound(err) {
-			return nil, nil
-		}
-
-		return nil, err
-	}
-
-	return cdk8sAppProxy, nil
-}
-
 func (r *Reconciler) checkIfResourceExists(ctx context.Context, dynClient dynamic.Interface, gvr schema.GroupVersionResource, namespace string, name string) (bool, error) {
 	resourceGetter := dynClient.Resource(gvr)
 	if namespace != "" {

controllers/cdk8sappproxy/cdk8sappproxy_git_operator.go

@@ -8,7 +8,7 @@ import (
 	"path/filepath"
 )
 
-func (r *Reconciler) prepareSource(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger, operation string) (appSourcePath string, currentCommitHash string, err error) {
+func (r *Reconciler) prepareSource(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (appSourcePath string, currentCommitHash string, err error) {
 	gitImpl := &gitoperator.GitImplementer{}
 	var buf bytes.Buffer
 	gitSpec := cdk8sAppProxy.Spec.GitRepository
@@ -33,7 +33,7 @@ func (r *Reconciler) prepareSource(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy,
 
 		if gitSpec.Path != "" {
 			appSourcePath = filepath.Join(directory, gitSpec.Path)
-			logger.Info("Adjusted appSourcePath for repository subpath", "subPath", gitSpec.Path, "finalPath", appSourcePath, "operation", operation)
+			logger.Info("Adjusted appSourcePath for repository subpath", "subPath", gitSpec.Path, "finalPath", appSourcePath)
 		}
 
 		return appSourcePath, currentCommitHash, err

controllers/cdk8sappproxy/cdk8sappproxy_reconciler.go

@@ -89,7 +89,7 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 	}
 
 	// Prepare a source path and get current commit hash
-	appSourcePath, _, err := r.prepareSource(ctx, cdk8sAppProxy, proxyNamespacedName, logger, OperationNormal)
+	appSourcePath, _, err := r.prepareSource(cdk8sAppProxy, logger)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -146,7 +146,7 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 				}
 				if polling {
 					logger.Info("Detected changes in git repository, proceeding with reconciliation.")
-					appSourcePath, _, err = r.prepareSource(ctx, cdk8sAppProxy, proxyNamespacedName, logger, OperationNormal)
+					appSourcePath, _, err = r.prepareSource(cdk8sAppProxy, logger)
 					if err != nil {
 						logger.Error(err, "Prepare source for reconciliation")
 					}
@@ -195,34 +195,6 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 	return ctrl.Result{}, nil
 }
 
-func (r *Reconciler) handleDeletionTriggerAnnotation(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (bool, error) {
-	deletionTriggerAnnotationKey := "cdk8s.addons.cluster.x-k8s.io/reconcile-on-delete-trigger"
-	forceSynthAndApplyDueToDeletion := false
-
-	if cdk8sAppProxy.Annotations != nil {
-		if _, ok := cdk8sAppProxy.Annotations[deletionTriggerAnnotationKey]; ok {
-			forceSynthAndApplyDueToDeletion = true
-			logger.Info("Reconciliation was triggered by a resource deletion annotation.", "annotationKey", deletionTriggerAnnotationKey)
-
-			// Clear the annotation
-			logger.Info("Attempting to clear the resource deletion trigger annotation.", "annotationKey", deletionTriggerAnnotationKey)
-			delete(cdk8sAppProxy.Annotations, deletionTriggerAnnotationKey)
-			if len(cdk8sAppProxy.Annotations) == 0 {
-				cdk8sAppProxy.Annotations = nil
-			}
-
-			if err := r.Update(ctx, cdk8sAppProxy); err != nil {
-				logger.Error(err, "Failed to clear the resource deletion trigger annotation. Requeuing.", "annotationKey", deletionTriggerAnnotationKey)
-
-				return false, err
-			}
-			logger.Info("Successfully cleared the resource deletion trigger annotation.", "annotationKey", deletionTriggerAnnotationKey)
-		}
-	}
-
-	return forceSynthAndApplyDueToDeletion, nil
-}
-
 func (r *Reconciler) ensureFinalizer(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (bool, error) {
 	if !controllerutil.ContainsFinalizer(cdk8sAppProxy, Finalizer) {
 		logger.Info("Adding finalizer", "finalizer", Finalizer)
@@ -252,26 +224,6 @@ func (r *Reconciler) handleNoResources(ctx context.Context, cdk8sAppProxy *addon
 	return nil
 }
 
-func (r *Reconciler) determineIfApplyNeeded(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, parsedResources []*unstructured.Unstructured, currentCommitHash string, forceSynthAndApplyDueToDeletion bool, logger logr.Logger) (bool, clusterv1.ClusterList, error) {
-	var clusterList clusterv1.ClusterList
-
-	// Check for git or annotation triggers
-	triggeredByGitOrAnnotation := r.checkGitOrAnnotationTriggers(cdk8sAppProxy, currentCommitHash, forceSynthAndApplyDueToDeletion, logger)
-
-	if !triggeredByGitOrAnnotation {
-		// Check if resources are missing on clusters
-		foundMissingResources, list, err := r.verifyResourcesOnClusters(ctx, cdk8sAppProxy, parsedResources, logger)
-		if err != nil {
-			return false, clusterList, err
-		}
-		clusterList = list
-
-		return foundMissingResources, clusterList, nil
-	}
-
-	return true, clusterList, nil
-}
-
 func (r *Reconciler) applyNeeded(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, parsedResources []*unstructured.Unstructured, logger logr.Logger) (bool, clusterv1.ClusterList, error) {
 	var clusterList clusterv1.ClusterList
 
@@ -402,34 +354,9 @@ func (r *Reconciler) checkGitOrAnnotationTriggers(cdk8sAppProxy *addonsv1alpha1.
 	return false
 }
 
-func (r *Reconciler) handleSkipApply(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, currentCommitHash string, logger logr.Logger) error {
-	logger.Info("Skipping resource application: no Git changes, no deletion annotation, and all resources verified present.")
-
-	// Re-establish watches for existing resources after controller restart
-	if err := r.reestablishWatchesForExistingResources(ctx, cdk8sAppProxy, logger); err != nil {
-		logger.Error(err, "Failed to re-establish watches for existing resources")
-	}
-
-	cdk8sAppProxy.Status.ObservedGeneration = cdk8sAppProxy.Generation
-	conditions.MarkTrue(cdk8sAppProxy, addonsv1alpha1.DeploymentProgressingCondition)
-
-	if cdk8sAppProxy.Spec.GitRepository != nil && cdk8sAppProxy.Spec.GitRepository.URL != "" && currentCommitHash != "" {
-		cdk8sAppProxy.Status.LastProcessedGitHash = currentCommitHash
-		logger.Info("Updated LastProcessedGitHash to current commit hash as no changes or missing resources were found.", "hash", currentCommitHash)
-	}
-
-	if err := r.Status().Update(ctx, cdk8sAppProxy); err != nil {
-		logger.Error(err, "Failed to update status after skipping resource application.")
-
-		return err
-	}
-
-	return nil
-}
-
 func (r *Reconciler) reestablishWatchesForExistingResources(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) error {
 	// Get the source and parse resources to know what should be watched
-	appSourcePath, _, err := r.prepareSource(ctx, cdk8sAppProxy, types.NamespacedName{Name: cdk8sAppProxy.Name, Namespace: cdk8sAppProxy.Namespace}, logger, OperationNormal)
+	appSourcePath, _, err := r.prepareSource(cdk8sAppProxy, logger)
 	if err != nil {
 		return err
 	}
@@ -588,27 +515,3 @@ func (r *Reconciler) applyResourcesToClusters(ctx context.Context, cdk8sAppProxy
 
 	return ctrl.Result{}, nil
 }
-
-func (r *Reconciler) triggerReconciliation(ctx context.Context, proxyName types.NamespacedName, logger logr.Logger) error {
-	proxyToAnnotate := &addonsv1alpha1.Cdk8sAppProxy{}
-	if err := r.Get(ctx, proxyName, proxyToAnnotate); err != nil {
-		logger.Error(err, "Failed to get latest Cdk8sAppProxy for annotation update")
-
-		return err
-	}
-
-	if proxyToAnnotate.Annotations == nil {
-		proxyToAnnotate.Annotations = make(map[string]string)
-	}
-	proxyToAnnotate.Annotations["cdk8s.addons.cluster.x-k8s.io/git-poll-trigger"] = time.Now().Format(time.RFC3339Nano)
-
-	if err := r.Update(ctx, proxyToAnnotate); err != nil {
-		logger.Error(err, "Failed to update Cdk8sAppProxy annotations to trigger reconciliation")
-
-		return err
-	}
-
-	logger.Info("Successfully updated annotations to trigger reconciliation.")
-
-	return nil
-}