feat: enhance addon lifecycle with dynamic templates and robust cleanup (#122)

- Replace static AddOnTemplate with per-GitOpsCluster dynamic generation
- Implement pre-delete cleanup job for proper ArgoCD uninstall sequence
- Add automatic placement tolerations for unreachable/unavailable clusters
- Clean up addons when clusters are removed from placement
- Use Kubernetes downward API for controller image configuration
- Add comprehensive cleanup e2e test suite and CI workflow

Signed-off-by: Mike Ng <[email protected]>

Author: mikeshng

.github/workflows/e2e.yml

@@ -77,3 +77,62 @@ jobs:
         env:
           KUBECONFIG: /home/runner/.kube/config
         run: make test-e2e
+
+  e2e-cleanup:
+    name: e2e-cleanup
+    runs-on: ubuntu-latest
+    steps:
+      - name: checkout code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+          path: go/src/open-cluster-management.io/argocd-pull-integration
+
+      - name: install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+
+      - name: Add GOPATH/bin to PATH
+        run: echo "$GOPATH/bin" >> "$GITHUB_PATH"
+
+      - name: install imagebuilder
+        run: go install github.com/openshift/imagebuilder/cmd/[email protected]
+
+      - name: Set up Helm
+        uses: azure/setup-helm@v4
+        with:
+          version: v3.15.2
+
+      - name: Set up kubectl
+        uses: azure/setup-kubectl@v4
+        with:
+          version: v1.29.0
+
+      - name: build-images
+        run: make build-images
+
+      - name: setup kind (cluster1)
+        uses: engineerd/[email protected]
+        with:
+          version: v0.14.0
+          name: cluster1
+
+      - name: setup kind (hub)
+        uses: engineerd/[email protected]
+        with:
+          version: v0.14.0
+          name: hub
+
+      - name: Load image on the nodes of the hub
+        run: |
+          kind load docker-image --name=hub quay.io/open-cluster-management/argocd-pull-integration:latest
+
+      - name: Load image on the nodes of the cluster1
+        run: |
+          kind load docker-image --name=cluster1 quay.io/open-cluster-management/argocd-pull-integration:latest
+
+      - name: Run e2e cleanup test
+        env:
+          KUBECONFIG: /home/runner/.kube/config
+        run: make test-e2e-cleanup

Makefile

@@ -163,6 +163,57 @@ test-e2e-full: ## Complete e2e test with kind cluster setup, build, deployment,
 	@echo "  kubectl get application -n argocd --context kind-$(SPOKE_CLUSTER)"
 	@echo "  sleep 2"
 
+.PHONY: test-e2e-cleanup
+test-e2e-cleanup: manifests generate fmt vet ## Run e2e cleanup tests (checks addon cleanup behavior)
+	@echo "===== Installing MetalLB ====="
+	./test/e2e/scripts/install_metallb.sh
+	@echo ""
+	@echo "===== Setting up OCM environment ====="
+	./test/e2e/scripts/setup_ocm_env.sh
+	@echo ""
+	@echo "===== Installing addon via Helm ====="
+	$(KUBECTL) config use-context kind-$(HUB_CLUSTER)
+	$(KUBECTL) create namespace argocd || true
+	$(KUBECTL) label namespace argocd app.kubernetes.io/managed-by=Helm --overwrite
+	$(KUBECTL) annotate namespace argocd meta.helm.sh/release-name=argocd-agent-addon meta.helm.sh/release-namespace=argocd --overwrite
+	helm install argocd-agent-addon \
+		./charts/argocd-agent-addon \
+		--namespace argocd \
+		--set image=quay.io/open-cluster-management/argocd-pull-integration \
+		--set tag=latest \
+		--set addonImage=quay.io/open-cluster-management/argocd-pull-integration \
+		--set addonTag=latest \
+		--wait \
+		--timeout 10m
+	@echo ""
+	@echo "===== Running cleanup e2e tests ====="
+	go test -tags=e2e ./test/e2e/ -v -ginkgo.v --ginkgo.label-filter="cleanup"
+	@echo ""
+	@echo "===== E2E Cleanup Tests Complete ====="
+
+.PHONY: test-e2e-cleanup-full
+test-e2e-cleanup-full: ## Complete e2e test with cleanup verification (cluster setup + deployment + cleanup)
+	@echo "===== Cleaning up existing clusters ====="
+	$(KIND) delete clusters --all || true
+	@echo ""
+	@echo "===== Creating KinD clusters ====="
+	$(KIND) create cluster --name $(HUB_CLUSTER)
+	$(KIND) create cluster --name $(SPOKE_CLUSTER)
+	@echo ""
+	@echo "===== Building controller image ====="
+	$(MAKE) docker-build IMG=$(E2E_IMG)
+	@echo ""
+	@echo "===== Loading image to clusters ====="
+	$(KIND) load docker-image $(E2E_IMG) --name $(HUB_CLUSTER)
+	$(KIND) load docker-image $(E2E_IMG) --name $(SPOKE_CLUSTER)
+	@echo ""
+	@echo "===== Running cleanup e2e tests ====="
+	$(MAKE) test-e2e-cleanup
+	@echo ""
+	@echo "===== E2E Cleanup Tests Complete ====="
+	@echo "Hub context: kind-$(HUB_CLUSTER)"
+	@echo "Spoke context: kind-$(SPOKE_CLUSTER)"
+
 ##@ Build
 
 .PHONY: build

api/v1alpha1/gitopscluster_types.go

@@ -54,10 +54,6 @@ type ArgoCDAgentAddonSpec struct {
 	// AgentImage is the ArgoCD agent image to use
 	// +optional
 	AgentImage string `json:"agentImage,omitempty"`
-
-	// Uninstall indicates whether to uninstall the addon
-	// +optional
-	Uninstall bool `json:"uninstall,omitempty"`
 }
 
 // GitOpsClusterStatus defines the observed state of GitOpsCluster
@@ -98,6 +94,15 @@ const (
 
 	// ConditionAddonConfigured indicates that addon config and ManagedClusterAddon are configured
 	ConditionAddonConfigured = "AddonConfigured"
+
+	// ConditionPlacementTolerationConfigured indicates that placement tolerations have been configured
+	ConditionPlacementTolerationConfigured = "PlacementTolerationConfigured"
+
+	// ConditionAddOnTemplateReady indicates that AddOnTemplate has been created/updated
+	ConditionAddOnTemplateReady = "AddOnTemplateReady"
+
+	// ConditionRemovedClustersCleanedUp indicates that removed clusters have been cleaned up
+	ConditionRemovedClustersCleanedUp = "RemovedClustersCleanedUp"
 )
 
 // Condition reasons

api/v1alpha1/gitopscluster_types_test.go

@@ -88,7 +88,6 @@ func TestArgoCDAgentAddonSpec(t *testing.T) {
 				Mode:                   "managed",
 				OperatorImage:          "quay.io/operator:v1.0.0",
 				AgentImage:             "quay.io/agent:v1.0.0",
-				Uninstall:              false,
 			},
 		},
 		{
@@ -103,9 +102,7 @@ func TestArgoCDAgentAddonSpec(t *testing.T) {
 		},
 		{
 			name: "uninstall mode",
-			spec: ArgoCDAgentAddonSpec{
-				Uninstall: true,
-			},
+			spec: ArgoCDAgentAddonSpec{},
 		},
 	}
 

charts/argocd-agent-addon/templates/addontemplate.yaml

@@ -6,6 +6,8 @@ metadata:
 spec:
   controller:
     enabled: false
+  applicationSet:
+    enabled: true
   argoCDAgent:
     principal:
       enabled: true

charts/argocd-agent-addon/templates/argocd-operator/argocd.yaml

@@ -11,5 +11,3 @@ spec:
     resource: addondeploymentconfigs
   - group: addon.open-cluster-management.io
     resource: addontemplates
-    defaultConfig:
-      name: argocd-agent-addon

charts/argocd-agent-addon/templates/clustermanagementaddon.yaml

@@ -33,6 +33,9 @@ spec:
         args:
         - --mode=controller
         - --leader-elect
+        env:
+        - name: CONTROLLER_IMAGE
+          value: "{{ .Values.image }}:{{ .Values.tag }}"
         securityContext:
           readOnlyRootFilesystem: true
           allowPrivilegeEscalation: false

charts/argocd-agent-addon/templates/controller/deployment.yaml

@@ -72,7 +72,6 @@ var (
 	ArgoCDAgentServerAddress = ""
 	ArgoCDAgentServerPort    = ""
 	ArgoCDAgentMode          = "managed"
-	ArgoCDAgentUninstall     = false
 )
 
 var (
@@ -104,7 +103,7 @@ func main() {
 	var enableHTTP2 bool
 	var tlsOpts []func(*tls.Config)
 
-	flag.StringVar(&mode, "mode", "controller", "Mode to run: controller or addon")
+	flag.StringVar(&mode, "mode", "controller", "Mode to run: controller, addon, or cleanup")
 	flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+
 		"Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
@@ -143,6 +142,12 @@ func main() {
 		return
 	}
 
+	// Check if running in cleanup mode
+	if mode == "cleanup" {
+		runCleanupMode()
+		return
+	}
+
 	// Continue with normal controller mode
 
 	// if the enable-http2 flag is false (the default), http/2 should be disabled
@@ -295,10 +300,6 @@ func runAddonMode() {
 		ArgoCDAgentMode = newArgoCDAgentMode
 	}
 
-	if newArgoCDAgentUninstall, found := os.LookupEnv("ARGOCD_AGENT_UNINSTALL"); found && newArgoCDAgentUninstall == "true" {
-		ArgoCDAgentUninstall = true
-	}
-
 	setupLog.Info("Addon mode settings",
 		"syncInterval", addonOptions.SyncInterval,
 		"leaseDuration", addonOptions.LeaderElectionLeaseDuration,
@@ -309,7 +310,6 @@ func runAddonMode() {
 		"ArgoCDAgentServerAddress", ArgoCDAgentServerAddress,
 		"ArgoCDAgentServerPort", ArgoCDAgentServerPort,
 		"ArgoCDAgentMode", ArgoCDAgentMode,
-		"ArgoCDAgentUninstall", ArgoCDAgentUninstall,
 	)
 
 	// Create a new manager for addon mode
@@ -331,7 +331,7 @@ func runAddonMode() {
 
 	// Setup the addon with the manager
 	if err = addon.SetupWithManager(mgr, addonOptions.SyncInterval,
-		ArgoCDOperatorImage, ArgoCDAgentImage, ArgoCDAgentServerAddress, ArgoCDAgentServerPort, ArgoCDAgentMode, ArgoCDAgentUninstall); err != nil {
+		ArgoCDOperatorImage, ArgoCDAgentImage, ArgoCDAgentServerAddress, ArgoCDAgentServerPort, ArgoCDAgentMode); err != nil {
 		setupLog.Error(err, "unable to create addon controller")
 		os.Exit(1)
 	}
@@ -342,3 +342,47 @@ func runAddonMode() {
 		os.Exit(1)
 	}
 }
+
+// runCleanupMode runs the application in cleanup mode for pre-delete hook
+func runCleanupMode() {
+	setupLog.Info("Starting in cleanup mode")
+
+	// Read environment variables for cleanup configuration
+	if newArgoCDOperatorImage, found := os.LookupEnv("ARGOCD_OPERATOR_IMAGE"); found && newArgoCDOperatorImage != "" {
+		ArgoCDOperatorImage = newArgoCDOperatorImage
+	}
+
+	if newArgoCDAgentImage, found := os.LookupEnv("ARGOCD_AGENT_IMAGE"); found && newArgoCDAgentImage != "" {
+		ArgoCDAgentImage = newArgoCDAgentImage
+	}
+
+	setupLog.Info("Cleanup mode settings",
+		"ArgoCDOperatorImage", ArgoCDOperatorImage,
+		"ArgoCDAgentImage", ArgoCDAgentImage,
+	)
+
+	// Create a manager for cleanup mode (no leader election for cleanup job)
+	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
+		Scheme: scheme,
+		Metrics: metricsserver.Options{
+			BindAddress: "0", // Disable metrics in cleanup mode
+		},
+		LeaderElection: false, // Cleanup job doesn't need leader election
+	})
+	if err != nil {
+		setupLog.Error(err, "unable to start cleanup manager")
+		os.Exit(1)
+	}
+
+	// Setup the cleanup with the manager
+	if err = addon.SetupCleanupWithManager(mgr, ArgoCDOperatorImage, ArgoCDAgentImage); err != nil {
+		setupLog.Error(err, "unable to create cleanup controller")
+		os.Exit(1)
+	}
+
+	setupLog.Info("starting cleanup manager")
+	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
+		setupLog.Error(err, "problem running cleanup manager")
+		os.Exit(1)
+	}
+}

cmd/main.go

@@ -68,9 +68,6 @@ spec:
                     description: PrincipalServerPort is the port of the ArgoCD principal
                       server
                     type: string
-                  uninstall:
-                    description: Uninstall indicates whether to uninstall the addon
-                    type: boolean
                 type: object
               placementRef:
                 description: PlacementRef references a Placement resource to select