🐛 set finalizer later in unified mode; unblock hub delete when init failed

fleetconfig-controller/api/v1alpha1/webhook_suite_test.go

@@ -83,7 +83,7 @@ var _ = BeforeSuite(func() {
 			filepath.Join(root, "charts", "fleetconfig-controller", "crds"),
 			filepath.Join(root, "config", "crds"),
 		},
-		ErrorIfCRDPathMissing: false,
+		ErrorIfCRDPathMissing: true,
 		WebhookInstallOptions: envtest.WebhookInstallOptions{
 			Paths: []string{filepath.Join("..", "..", "config", "webhook")},
 		},

fleetconfig-controller/charts/fleetconfig-controller/README.md

@@ -158,7 +158,7 @@ Resource specifications for all klusterlet-managed containers.
 | `replicas`                                          | fleetconfig-controller replica count                                                                                                                                       | `1`                                                      |
 | `imageRegistry`                                     | Image registry                                                                                                                                                             | `""`                                                     |
 | `image.repository`                                  | Image repository                                                                                                                                                           | `quay.io/open-cluster-management/fleetconfig-controller` |
-| `image.tag`                                         | Image tag                                                                                                                                                                  | `v0.1.3`                                                 |
+| `image.tag`                                         | Image tag                                                                                                                                                                  | `v0.1.4`                                                 |
 | `image.pullPolicy`                                  | Image pull policy                                                                                                                                                          | `IfNotPresent`                                           |
 | `imagePullSecrets`                                  | Image pull secrets                                                                                                                                                         | `[]`                                                     |
 | `serviceAccount.annotations`                        | Annotations to add to the service account                                                                                                                                  | `{}`                                                     |

fleetconfig-controller/charts/fleetconfig-controller/templates/clusterissuer.yaml

@@ -4,6 +4,8 @@ apiVersion: cert-manager.io/v1
 kind: ClusterIssuer
 metadata:
   name: fleetconfig-controller
+  labels:
+  {{ include "chart.labels" . | nindent 4 }}
   annotations:
   {{ include "chart.annotations" . | nindent 4 }}
 spec:

fleetconfig-controller/charts/fleetconfig-controller/values.yaml

@@ -294,7 +294,7 @@ imageRegistry: ""
 ## @param image.pullPolicy Image pull policy
 image:
   repository: quay.io/open-cluster-management/fleetconfig-controller
-  tag: v0.1.3
+  tag: v0.1.4
   pullPolicy: IfNotPresent
 
 ## @param imagePullSecrets Image pull secrets

fleetconfig-controller/devspace.yaml

@@ -49,7 +49,6 @@ pipelines:
     run_dependencies --all
     build_images fleetconfig-controller-dev
     create_deployments cert-manager
-    kubectl apply -f ./hack/dev/cluster-issuer.yaml
     create_deployments fleetconfig-controller-dev
     start_dev fleetconfig-controller-dev-hub
   deploy: |-
@@ -62,7 +61,6 @@ pipelines:
   deploy-local: |-
     run_pipelines load-local
     create_deployments cert-manager
-    kubectl apply -f ./hack/dev/cluster-issuer.yaml
     create_deployments fleetconfig-controller-local
   debug: |-
     run_dependencies --all

fleetconfig-controller/internal/controller/v1beta1/hub_controller.go

@@ -221,6 +221,22 @@ func (r *HubReconciler) cleanHub(ctx context.Context, hub *v1beta1.Hub, hubKubec
 
 	logger.Info("All Spokes have been deleted, proceeding with Hub cleanup")
 
+	operatorC, err := common.OperatorClient(hubKubeconfig)
+	if err != nil {
+		return true, fmt.Errorf("failed to create operator client for cleanup: %w", err)
+	}
+	_, err = operatorC.OperatorV1().ClusterManagers().Get(ctx, "cluster-manager", metav1.GetOptions{})
+	if err != nil {
+		if kerrs.IsNotFound(err) {
+			logger.Info("ClusterManager not found; skip cleanup.", "hub", hub.Name)
+			hub.Finalizers = slices.DeleteFunc(hub.Finalizers, func(s string) bool {
+				return s == v1beta1.HubCleanupFinalizer
+			})
+			return false, nil
+		}
+		return true, fmt.Errorf("failed to get ClusterManager: %w", err)
+	}
+
 	addonC, err := common.AddOnClient(hubKubeconfig)
 	if err != nil {
 		return true, fmt.Errorf("failed to create addon client for cleanup: %w", err)

fleetconfig-controller/internal/controller/v1beta1/spoke_controller.go

@@ -97,38 +97,38 @@ func (r *SpokeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		spoke.Status.Phase = v1beta1.Unhealthy
 	}
 
-	switch r.InstanceType {
-	case v1beta1.InstanceTypeManager:
-		if !slices.Contains(spoke.Finalizers, v1beta1.HubCleanupFinalizer) {
-			setDefaults(ctx, spoke, hubMeta)
-			spoke.Finalizers = append(
-				spoke.Finalizers,
-				v1beta1.HubCleanupPreflightFinalizer, // removed by the hub to signal to the spoke that preflight is completed
-				v1beta1.HubCleanupFinalizer,          // removed by the hub after post-unjoin cleanup is finished
-			)
-			if spoke.IsHubAsSpoke() {
-				spoke.Finalizers = append(spoke.Finalizers, v1beta1.SpokeCleanupFinalizer)
+	if spoke.DeletionTimestamp.IsZero() {
+		switch r.InstanceType {
+		case v1beta1.InstanceTypeManager:
+			if !slices.Contains(spoke.Finalizers, v1beta1.HubCleanupFinalizer) {
+				setDefaults(ctx, spoke, hubMeta)
+				spoke.Finalizers = append(
+					spoke.Finalizers,
+					v1beta1.HubCleanupPreflightFinalizer, // removed by the hub to signal to the spoke that preflight is completed
+					v1beta1.HubCleanupFinalizer,          // removed by the hub after post-unjoin cleanup is finished
+				)
+				return ret(ctx, ctrl.Result{RequeueAfter: spokeRequeuePreJoin}, nil)
 			}
-			return ret(ctx, ctrl.Result{RequeueAfter: spokeRequeuePreJoin}, nil)
-		}
-	case v1beta1.InstanceTypeUnified:
-		if !slices.Contains(spoke.Finalizers, v1beta1.HubCleanupFinalizer) {
-			setDefaults(ctx, spoke, hubMeta)
-			spoke.Finalizers = append(
-				spoke.Finalizers,
-				v1beta1.HubCleanupPreflightFinalizer, // removed by the hub to signal to the spoke that preflight is completed
-				v1beta1.SpokeCleanupFinalizer,        // removed by the hub after successful unjoin
-				v1beta1.HubCleanupFinalizer,          // removed by the hub after post-unjoin cleanup is finished
-			)
-		}
-	case v1beta1.InstanceTypeAgent:
-		if !slices.Contains(spoke.Finalizers, v1beta1.SpokeCleanupFinalizer) && spoke.DeletionTimestamp.IsZero() {
-			spoke.Finalizers = append(spoke.Finalizers, v1beta1.SpokeCleanupFinalizer) // removed by the spoke to signal to the hub that unjoin succeeded
-			return ret(ctx, ctrl.Result{RequeueAfter: spokeRequeuePreJoin}, nil)
+		case v1beta1.InstanceTypeUnified:
+			if !slices.Contains(spoke.Finalizers, v1beta1.HubCleanupFinalizer) {
+				setDefaults(ctx, spoke, hubMeta)
+				spoke.Finalizers = append(
+					spoke.Finalizers,
+					v1beta1.HubCleanupPreflightFinalizer, // removed by the hub to signal to the spoke that preflight is completed
+					v1beta1.HubCleanupFinalizer,          // removed by the hub after post-unjoin cleanup is finished
+				)
+				// SpokeCleanupFinalizer is added later after successful join
+				return ret(ctx, ctrl.Result{RequeueAfter: spokeRequeuePreJoin}, nil)
+			}
+		case v1beta1.InstanceTypeAgent:
+			if !slices.Contains(spoke.Finalizers, v1beta1.SpokeCleanupFinalizer) {
+				spoke.Finalizers = append(spoke.Finalizers, v1beta1.SpokeCleanupFinalizer) // removed by the spoke to signal to the hub that unjoin succeeded
+				return ret(ctx, ctrl.Result{RequeueAfter: spokeRequeuePreJoin}, nil)
+			}
+		default:
+			// this is guarded against when the manager is initialized. should never reach this point
+			panic(fmt.Sprintf("unknown instance type %s. Must be one of %v", r.InstanceType, v1beta1.SupportedInstanceTypes))
 		}
-	default:
-		// this is guarded against when the manager is initialized. should never reach this point
-		panic(fmt.Sprintf("unknown instance type %s. Must be one of %v", r.InstanceType, v1beta1.SupportedInstanceTypes))
 	}
 
 	// Handle deletion logic with finalizer

fleetconfig-controller/internal/controller/v1beta1/spoke_controller_test.go

@@ -116,7 +116,7 @@ var _ = Describe("Spoke Controller", Ordered, func() {
 
 			By("Verifying the Spoke's finalizer")
 			Expect(k8sClient.Get(ctx, spokeNN, spoke)).To(Succeed())
-			Expect(spoke.Finalizers).To(ContainElement(v1beta1.SpokeCleanupFinalizer),
+			Expect(spoke.Finalizers).To(ContainElement(v1beta1.HubCleanupFinalizer),
 				"Spoke %s wasn't given a finalizer", spokeNN.Name)
 		})
 

fleetconfig-controller/internal/controller/v1beta1/spoke_handler.go

@@ -254,6 +254,12 @@ func (r *SpokeReconciler) doHubWork(ctx context.Context, spoke *v1beta1.Spoke, h
 		}
 	}
 
+	// Add SpokeCleanupFinalizer now that join succeeded and there's something to clean up
+	if r.InstanceType != v1beta1.InstanceTypeAgent && !slices.Contains(spoke.Finalizers, v1beta1.SpokeCleanupFinalizer) {
+		spoke.Finalizers = append(spoke.Finalizers, v1beta1.SpokeCleanupFinalizer)
+		logger.V(1).Info("Added SpokeCleanupFinalizer after successful join")
+	}
+
 	// TODO - handle this via `klusterlet upgrade` once https://github.com/open-cluster-management-io/ocm/issues/1210 is resolved
 	if managedCluster != nil {
 		klusterletValuesAnnotations := map[string]string{}
@@ -686,6 +692,7 @@ func (r *SpokeReconciler) waitForAgentAddonDeleted(ctx context.Context, spoke *v
 // doSpokeCleanup handles all the required cleanup of a spoke cluster when deregistering a Spoke
 func (r *SpokeReconciler) doSpokeCleanup(ctx context.Context, spoke *v1beta1.Spoke, pivotComplete bool) (bool, error) {
 	logger := log.FromContext(ctx)
+
 	// requeue until preflight is complete by the hub's controller
 	if slices.Contains(spoke.Finalizers, v1beta1.HubCleanupPreflightFinalizer) {
 		logger.V(1).Info("Cleanup initiated, waiting for hub to complete preflight")

fleetconfig-controller/internal/webhook/v1beta1/webhook_suite_test.go

@@ -90,10 +90,10 @@ var _ = BeforeSuite(func() {
 			filepath.Join(root, "charts", "fleetconfig-controller", "crds"),
 			filepath.Join(root, "config", "crds"),
 		},
-		ErrorIfCRDPathMissing: false,
+		ErrorIfCRDPathMissing: true,
 
 		WebhookInstallOptions: envtest.WebhookInstallOptions{
-			Paths: []string{filepath.Join("..", "..", "..", "config", "webhook")},
+			Paths: []string{filepath.Join(root, "config", "webhook")},
 		},
 	}