cleanup

Author: PatrickLaabs

Dockerfile

@@ -80,7 +80,7 @@ RUN apt-get update && \
     apt-get install -y --no-install-recommends ca-certificates=20240203~22.04.1 curl=7.81.0-1ubuntu1.20 && \
     curl -fsSL https://deb.nodesource.com/setup_18.x | bash - && \
     apt-get install -y nodejs=18.19.1-1nodesource1 && \
-    npm install -g [email protected].108 && \
+    npm install -g [email protected].109 && \
     curl -fsSL -o go1.24.4.linux-amd64.tar.gz https://go.dev/dl/go1.24.4.linux-amd64.tar.gz && \
     tar -C /usr/local -xzf go1.24.4.linux-amd64.tar.gz && \
     rm go1.24.4.linux-amd64.tar.gz && \

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: v0.3.2
-TAG ?= v0.3.2-preview.22
+TAG ?= v0.3.2-preview.24
 ARCH ?= $(shell go env GOARCH)
 ALL_ARCH = amd64 arm arm64
 

README.md

@@ -36,24 +36,25 @@ The `Cdk8sAppProxy` CustomResourceDefinition (CRD) is used to manage the deploym
 
 ### Example Manifest
 
-An example of a `Cdk8sAppProxy` manifest can be found in [`examples/cdk8sappproxy_sample.yaml`](./examples/cdk8sappproxy_sample.yaml). Below is a snippet:
+An example of a `Cdk8sAppProxy` manifest can be found in [`examples/cdk8sappproxy_sample-go.yaml`](./examples/cdk8sappproxy_sample.yaml). Below is a snippet:
 
 ```yaml
 apiVersion: addons.cluster.x-k8s.io/v1alpha1
 kind: Cdk8sAppProxy
 metadata:
-  name: cdk8s-sample-app
+  name: cdk8s-sample-app-go
   namespace: default
 spec:
   gitRepository:
-    url: "https://github.com/PatrickLaabs/cluster-api-addon-provider-cdk8s/examples/cdk8s-sample-deployment"
+    url: "https://github.com/PatrickLaabs/cluster-api-addon-provider-cdk8s"
     reference: "main"
-    path: "."
+    referencePollInterval: '5'
+    path: "examples/cdk8s-sample-deployment"
     # authSecretRef:
     #   name: git-credentials
   clusterSelector: {}
-    # matchLabels:
-      # environment: development
+          # matchLabels:
+  # environment: development
 # ---
 # apiVersion: v1
 # kind: Secret

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:v0.3.2-preview.21
+      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v0.3.2-preview.24
         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:v0.3.2-preview.21
+      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v0.3.2-preview.24
         name: manager

controllers/cdk8sappproxy/cdk8sappproxy_controller.go

@@ -139,12 +139,12 @@ func (r *Reconciler) synthesizeAndParseResources(appSourcePath string, logger lo
 func (r *Reconciler) synthesizeCdk8sApp(appSourcePath string, logger logr.Logger, operation string) error {
 	logger.Info("Synthesizing cdk8s application", "effectiveSourcePath", appSourcePath, "operation", OperationSynthesize)
 
-	npmInstall := cmdRunnerFactory("npm", "install")
-	npmInstall.SetDir(appSourcePath)
-	output, err := npmInstall.CombinedOutput()
-	if err != nil {
-		logger.Error(err, "npm installation failed", "output", string(output), "operation:", OperationNpmInstall)
-	}
+	// npmInstall := cmdRunnerFactory("npm", "install")
+	// npmInstall.SetDir(appSourcePath)
+	// output, err := npmInstall.CombinedOutput()
+	// if err != nil {
+	//	logger.Error(err, "npm installation failed", "output", string(output), "operation:", OperationNpmInstall)
+	// }
 
 	synthCmd := cmdRunnerFactory("cdk8s", "synth")
 	synthCmd.SetDir(appSourcePath)
@@ -156,6 +156,7 @@ func (r *Reconciler) synthesizeCdk8sApp(appSourcePath string, logger logr.Logger
 	}
 
 	logger.Info("cdk8s synth successful", "outputSummary", truncateString(string(output), 200), "operation", operation)
+
 	return nil
 }
 

controllers/cdk8sappproxy/cdk8sappproxy_git_operator.go

@@ -96,13 +96,6 @@ func (r *Reconciler) cloneGitRepository(ctx context.Context, cdk8sAppProxy *addo
 	return nil
 }
 
-func getPollInterval(c *addonsv1alpha1.Cdk8sAppProxy) time.Duration {
-	if c.Spec.GitRepository != nil && c.Spec.GitRepository.ReferencePollInterval != nil {
-		return c.Spec.GitRepository.ReferencePollInterval.Duration
-	}
-	return 5 * time.Minute // default
-}
-
 // pollGitRepository periodically checks the remote git repository for changes.
 func (r *Reconciler) pollGitRepository(ctx context.Context, proxyName types.NamespacedName) {
 	logger := log.FromContext(ctx).WithValues("cdk8sappproxy", proxyName.String(), "goroutine", "pollGitRepository")
@@ -114,7 +107,13 @@ func (r *Reconciler) pollGitRepository(ctx context.Context, proxyName types.Name
 
 		return
 	}
-	pollInterval := getPollInterval(cdk8sAppProxy)
+
+	var pollInterval time.Duration
+	if cdk8sAppProxy.Spec.GitRepository.ReferencePollInterval == nil {
+		pollInterval = 5 * time.Minute
+	} else {
+		pollInterval = cdk8sAppProxy.Spec.GitRepository.ReferencePollInterval.Duration
+	}
 
 	ticker := time.NewTicker(pollInterval)
 	defer ticker.Stop()
@@ -156,58 +155,6 @@ func (r *Reconciler) pollGitRepositoryOnce(ctx context.Context, proxyName types.
 	return r.handleGitRepositoryChange(ctx, proxyName, cdk8sAppProxy, remoteCommitHash, logger)
 }
 
-func (r *Reconciler) prepareSourceForDeletion(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (string, func(), error) {
-	if cdk8sAppProxy.Spec.GitRepository != nil && cdk8sAppProxy.Spec.GitRepository.URL != "" {
-		return r.prepareGitSourceForDeletion(ctx, cdk8sAppProxy, logger)
-	}
-
-	err := errors.New("GitRepository not specified, cannot determine resources to delete")
-	logger.Info(err.Error())
-	_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.SourceNotSpecifiedReason, "Cannot determine resources to delete during deletion", err, true)
-
-	return "", func() {}, err
-}
-
-func (r *Reconciler) prepareGitSourceForDeletion(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (string, func(), error) {
-	gitSpec := cdk8sAppProxy.Spec.GitRepository
-	logger.Info("Using GitRepository source for deletion logic", "url", gitSpec.URL, "reference", gitSpec.Reference, "path", gitSpec.Path)
-
-	tempDir, err := os.MkdirTemp("", "cdk8s-git-delete-")
-	if err != nil {
-		logger.Error(err, "failed to create temp dir for git clone during deletion")
-
-		return "", func() {}, err
-	}
-
-	cleanupFunc := func() {
-		if err := os.RemoveAll(tempDir); err != nil {
-			logger.Error(err, "Failed to remove temporary clone directory for deletion", "tempDir", tempDir)
-		}
-	}
-
-	logger.Info("Created temporary directory for clone during deletion", "tempDir", tempDir)
-
-	if err := r.cloneGitRepository(ctx, nil, gitSpec, tempDir, logger, OperationDeletion); err != nil {
-		cleanupFunc()
-
-		return "", func() {}, err
-	}
-
-	if err := r.checkoutGitReference(ctx, nil, gitSpec, tempDir, logger, OperationDeletion); err != nil {
-		cleanupFunc()
-
-		return "", func() {}, err
-	}
-
-	appSourcePath := tempDir
-	if gitSpec.Path != "" {
-		appSourcePath = filepath.Join(tempDir, gitSpec.Path)
-		logger.Info("Adjusted appSourcePath for deletion", "subPath", gitSpec.Path, "finalPath", appSourcePath)
-	}
-
-	return appSourcePath, cleanupFunc, nil
-}
-
 func (r *Reconciler) findRemoteCommitHash(refs []*plumbing.Reference, refName plumbing.ReferenceName, logger logr.Logger) (string, error) {
 	// First, try to find the exact reference
 	for _, ref := range refs {
@@ -330,111 +277,108 @@ func (r *Reconciler) updateRemoteGitHashStatus(ctx context.Context, proxyName ty
 	return nil
 }
 
-func (r *Reconciler) prepareSource(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, proxyNamespacedName types.NamespacedName, logger logr.Logger) (string, string, func(), error) {
+// prepareSource determines the source path for cdk8s application files, handling Git or local paths.
+// It returns the application source path, current commit hash (if applicable), a cleanup function, and an error.
+// The 'operation' parameter informs logging and error handling, especially for deletion scenarios.
+func (r *Reconciler) prepareSource(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, proxyNamespacedName types.NamespacedName, logger logr.Logger, operation string) (string, string, func(), error) {
 	var appSourcePath string
 	var currentCommitHash string
 	var cleanupFunc func()
 
 	switch {
 	case cdk8sAppProxy.Spec.GitRepository != nil && cdk8sAppProxy.Spec.GitRepository.URL != "":
-		path, hash, cleanup, err := r.prepareGitSource(ctx, cdk8sAppProxy, logger)
+		gitSpec := cdk8sAppProxy.Spec.GitRepository
+		logger.Info("Determined source type: GitRepository", "url", gitSpec.URL, "reference", gitSpec.Reference, "path", gitSpec.Path, "operation", operation)
+
+		tempDirPattern := "cdk8s-git-clone-"
+		if operation == OperationDeletion {
+			tempDirPattern = "cdk8s-git-delete-"
+		}
+		tempDir, err := os.MkdirTemp("", tempDirPattern)
 		if err != nil {
+			logger.Error(err, "Failed to create temp directory for git clone", "operation", operation)
+			// For normal operation, update status. For deletion, the Cdk8sAppProxy might be nil or finalizer needs removal.
+			if operation == OperationNormal && cdk8sAppProxy != nil {
+				_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.GitCloneFailedReason, "Failed to create temp dir for git clone", err, false)
+			}
+
 			return "", "", nil, err
 		}
-		appSourcePath = path
-		currentCommitHash = hash
-		cleanupFunc = cleanup
-
-		// Store current commit hash in status
-		if currentCommitHash != "" {
-			cdk8sAppProxy.Status.LastRemoteGitHash = currentCommitHash
-			logger.Info("Updated cdk8sAppProxy.Status.LastRemoteGitHash with the latest commit hash from remote", "lastRemoteGitHash", currentCommitHash)
-		}
 
-		// Stop any existing git poller for a local path
-		if cancel, ok := r.ActiveGitPollers[proxyNamespacedName]; ok {
-			logger.Info("GitRepository spec removed or empty, stopping existing git poller.")
-			cancel()
-			delete(r.ActiveGitPollers, proxyNamespacedName)
+		cleanupFunc = func() {
+			if err := os.RemoveAll(tempDir); err != nil {
+				logger.Error(err, "Failed to remove temporary clone directory", "tempDir", tempDir, "operation", operation)
+			}
 		}
+		logger.Info("Created temporary directory for clone", "tempDir", tempDir, "operation", operation)
 
-	default:
-		err := errors.New("no source specified")
-		logger.Error(err, "No source specified")
-		if cancel, ok := r.ActiveGitPollers[proxyNamespacedName]; ok {
-			logger.Info("Source spec is invalid or removed, stopping existing git poller.")
-			cancel()
-			delete(r.ActiveGitPollers, proxyNamespacedName)
+		// Pass cdk8sAppProxy as nil if it's a deletion operation where status updates are handled differently or not needed.
+		var proxyForGitOps *addonsv1alpha1.Cdk8sAppProxy
+		if operation == OperationNormal {
+			proxyForGitOps = cdk8sAppProxy
 		}
-		// Use the new consolidated error handler - removeFinalizer = false for normal operations
-		_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.SourceNotSpecifiedReason, "No GitRepository specified", err, false)
-
-		return "", "", nil, err
-	}
-
-	return appSourcePath, currentCommitHash, cleanupFunc, nil
-}
 
-func (r *Reconciler) prepareGitSource(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (string, string, func(), error) {
-	gitSpec := cdk8sAppProxy.Spec.GitRepository
-	logger.Info("Determined source type: GitRepository", "url", gitSpec.URL, "reference", gitSpec.Reference, "path", gitSpec.Path)
-
-	tempDir, err := os.MkdirTemp("", "cdk8s-git-clone-")
-	if err != nil {
-		logger.Error(err, "Failed to create temp directory for git clone")
-		_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.GitCloneFailedReason, "Failed to create temp dir for git clone", err, false)
+		if err := r.cloneGitRepository(ctx, proxyForGitOps, gitSpec, tempDir, logger, operation); err != nil {
+			cleanupFunc()
 
-		return "", "", nil, err
-	}
-
-	cleanupFunc := func() {
-		if err := os.RemoveAll(tempDir); err != nil {
-			logger.Error(err, "Failed to remove temporary clone directory", "tempDir", tempDir)
+			return "", "", nil, err
 		}
-	}
 
-	logger.Info("Created temporary directory for clone", "tempDir", tempDir)
+		if err := r.checkoutGitReference(ctx, proxyForGitOps, gitSpec, tempDir, logger, operation); err != nil {
+			cleanupFunc()
 
-	// Clone repository
-	if err := r.cloneGitRepository(ctx, cdk8sAppProxy, gitSpec, tempDir, logger, OperationNormal); err != nil {
-		cleanupFunc()
-
-		return "", "", nil, err
-	}
+			return "", "", nil, err
+		}
 
-	// Checkout-specific reference if specified
-	if err := r.checkoutGitReference(ctx, cdk8sAppProxy, gitSpec, tempDir, logger, OperationNormal); err != nil {
-		cleanupFunc()
+		appSourcePath = tempDir
+		if gitSpec.Path != "" {
+			appSourcePath = filepath.Join(tempDir, gitSpec.Path)
+			logger.Info("Adjusted appSourcePath for repository subpath", "subPath", gitSpec.Path, "finalPath", appSourcePath, "operation", operation)
+		}
 
-		return "", "", nil, err
-	}
+		// Only get commit hash for normal operations, not for deletion prep.
+		if operation == OperationNormal {
+			logger.Info("Attempting to retrieve current commit hash from Git repository", "repoDir", tempDir)
+			repo, err := git.PlainOpen(tempDir)
+			if err != nil {
+				cleanupFunc()
+				_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.GitCloneFailedReason, "Failed to open git repository post-clone: "+err.Error(), err, false)
 
-	// Determine a final app source path
-	appSourcePath := tempDir
-	if gitSpec.Path != "" {
-		appSourcePath = filepath.Join(tempDir, gitSpec.Path)
-		logger.Info("Adjusted appSourcePath for repository subpath", "subPath", gitSpec.Path, "finalPath", appSourcePath)
-	}
+				return "", "", nil, err
+			}
+			headRef, err := repo.Head()
+			if err != nil {
+				cleanupFunc()
+				_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.GitCloneFailedReason, "Failed to get HEAD reference from git repository post-clone: "+err.Error(), err, false)
 
-	// Get current commit hash inline
-	logger.Info("Attempting to retrieve current commit hash from Git repository", "repoDir", tempDir)
-	repo, err := git.PlainOpen(tempDir)
-	if err != nil {
-		cleanupFunc()
+				return "", "", nil, err
+			}
+			currentCommitHash = headRef.Hash().String()
+			logger.Info("Successfully retrieved current commit hash from Git repository", "commitHash", currentCommitHash)
 
-		return "", "", nil, r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.GitCloneFailedReason, "Failed to open git repository: "+err.Error(), err, false)
-	}
+			// Store current commit hash in status
+			if currentCommitHash != "" && cdk8sAppProxy != nil {
+				cdk8sAppProxy.Status.LastRemoteGitHash = currentCommitHash
+				logger.Info("Updated cdk8sAppProxy.Status.LastRemoteGitHash with the latest commit hash from remote", "lastRemoteGitHash", currentCommitHash)
+			}
+		}
 
-	headRef, err := repo.Head()
-	if err != nil {
-		cleanupFunc()
+	default:
+		err := errors.New("no source specified (neither GitRepository nor LocalPath)")
+		logger.Error(err, "No source specified", "operation", operation)
+		if operation == OperationNormal { // Poller management and status updates for normal flow
+			r.stopGitPoller(proxyNamespacedName, logger)
+			if cdk8sAppProxy != nil {
+				_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.SourceNotSpecifiedReason, err.Error(), err, false)
+			}
+		} else if operation == OperationDeletion && cdk8sAppProxy != nil {
+			// For deletion, if source can't be determined, we might still want to remove finalizer.
+			_ = r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.SourceNotSpecifiedReason, "Cannot determine resources to delete: "+err.Error(), err, true)
+		}
 
-		return "", "", nil, r.updateStatusWithError(ctx, cdk8sAppProxy, addonsv1alpha1.GitCloneFailedReason, "Failed to get HEAD reference from git repository: "+err.Error(), err, false)
+		return "", "", nil, err
 	}
 
-	currentCommitHash := headRef.Hash().String()
-	logger.Info("Successfully retrieved current commit hash from Git repository", "commitHash", currentCommitHash)
-
 	return appSourcePath, currentCommitHash, cleanupFunc, nil
 }
 

controllers/cdk8sappproxy/cdk8sappproxy_reconciler.go

@@ -71,7 +71,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cdk8sAppProxy *addonsv
 	}
 
 	// Get a source path for deletion
-	appSourcePath, cleanup, err := r.prepareSourceForDeletion(ctx, cdk8sAppProxy, logger)
+	appSourcePath, _, cleanup, err := r.prepareSource(ctx, cdk8sAppProxy, proxyNamespacedName, logger, OperationDeletion)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -114,7 +114,7 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 	}
 
 	// Prepare a source path and get current commit hash
-	appSourcePath, currentCommitHash, cleanup, err := r.prepareSource(ctx, cdk8sAppProxy, proxyNamespacedName, logger)
+	appSourcePath, currentCommitHash, cleanup, err := r.prepareSource(ctx, cdk8sAppProxy, proxyNamespacedName, logger, OperationNormal)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -377,7 +377,7 @@ func (r *Reconciler) handleSkipApply(ctx context.Context, cdk8sAppProxy *addonsv
 
 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, _, cleanup, err := r.prepareSource(ctx, cdk8sAppProxy, types.NamespacedName{Name: cdk8sAppProxy.Name, Namespace: cdk8sAppProxy.Namespace}, logger)
+	appSourcePath, _, cleanup, err := r.prepareSource(ctx, cdk8sAppProxy, types.NamespacedName{Name: cdk8sAppProxy.Name, Namespace: cdk8sAppProxy.Namespace}, logger, OperationNormal)
 	if err != nil {
 		return err
 	}