🐛 set taint on managedcluster before starting spoke cleanup (#76)

* fix: set taint on managedcluster before starting spoke cleanup

Signed-off-by: Artur Shad Nik <[email protected]>

* fix: order of operations

Signed-off-by: Artur Shad Nik <[email protected]>

* fix: use different taints for regular workloads vs addons

Signed-off-by: Artur Shad Nik <[email protected]>

* chore: split out preflight cleanup to reduce complexity

Signed-off-by: Artur Shad Nik <[email protected]>

* refactor: tighten up requeues

Signed-off-by: Artur Shad Nik <[email protected]>

* chore: shorten requeues during deletion

Signed-off-by: Artur Shad Nik <[email protected]>

* chore: make cluster drain optional, default to false

Signed-off-by: Artur Shad Nik <[email protected]>

* fix: update helm chart

Signed-off-by: Artur Shad Nik <[email protected]>

* test: update e2e test

Signed-off-by: Artur Shad Nik <[email protected]>

* chore: rabbit

Signed-off-by: Artur Shad Nik <[email protected]>

* chore: bump fcc to 0.1.2

Signed-off-by: Artur Shad Nik <[email protected]>

---------

Signed-off-by: Artur Shad Nik <[email protected]>

Author: arturshadnik

fleetconfig-controller/api/v1beta1/constants.go

@@ -129,6 +129,14 @@ const (
 
 	// AgentCleanupWatcherName is the name of the watcher for cleaning up the spoke agent.
 	AgentCleanupWatcherName = "agent-cleanup-watcher"
+
+	// ManagedClusterWorkloadCleanupTaint is applied to a ManagedCluster to remove non-addon workloads.
+	// Addons can tolerate this taint to continue running during initial cleanup phase.
+	ManagedClusterWorkloadCleanupTaint = "fleetconfig.open-cluster-management.io/workload-cleanup"
+
+	// ManagedClusterTerminatingTaint is applied to remove all workloads including addons.
+	// Nothing should tolerate this taint - it signals final cluster termination.
+	ManagedClusterTerminatingTaint = "fleetconfig.open-cluster-management.io/terminating"
 )
 
 // SupportedInstanceTypes are the valid cluster types that the controller can be installed in.

fleetconfig-controller/api/v1beta1/spoke_types.go

@@ -89,7 +89,7 @@ type SpokeSpec struct {
 
 // CleanupConfig is the configuration for cleaning up resources during Spoke cleanup.
 type CleanupConfig struct {
-	// If true, the agent will attempt to garbage collect its own namespace after the spoke cluster is unjoined.
+	// If set, the agent will attempt to garbage collect its own namespace after the spoke cluster is unjoined.
 	// +kubebuilder:default:=false
 	// +optional
 	PurgeAgentNamespace bool `json:"purgeAgentNamespace,omitempty"`
@@ -104,6 +104,14 @@ type CleanupConfig struct {
 	// +kubebuilder:default:=false
 	// +optional
 	PurgeKubeconfigSecret bool `json:"purgeKubeconfigSecret,omitempty"`
+
+	// If set, all ManifestWorks which were created using a Placement will be automatically descheduled from the Spoke cluster during deletion.
+	// This includes AddOns installed using installStrategy.type=Placements. If an AddOn must stay running to reconcile deletion of other ManifestWorks,
+	// it should tolerate the `fleetconfig.open-cluster-management.io/workload-cleanup` taint.
+	// Manually created ManifestWorks will not be affected and must be manually cleaned up for Spoke deletion to proceed.
+	// +kubebuilder:default:=false
+	// +optional
+	ForceClusterDrain bool `json:"forceClusterDrain,omitempty"`
 }
 
 // HubRef is the information required to get a Hub resource.

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

@@ -73,9 +73,17 @@ spec:
                 description: CleanupConfig is used to configure which resources should
                   be automatically garbage collected during cleanup.
                 properties:
+                  forceClusterDrain:
+                    default: false
+                    description: |-
+                      If set, all ManifestWorks which were created using a Placement will be automatically descheduled from the Spoke cluster during deletion.
+                      This includes AddOns installed using installStrategy.type=Placements. If an AddOn must stay running to reconcile deletion of other ManifestWorks,
+                      it should tolerate the `fleetconfig.open-cluster-management.io/workload-cleanup` taint.
+                      Manually created ManifestWorks will not be affected and must be manually cleaned up for Spoke deletion to proceed.
+                    type: boolean
                   purgeAgentNamespace:
                     default: false
-                    description: If true, the agent will attempt to garbage collect
+                    description: If set, the agent will attempt to garbage collect
                       its own namespace after the spoke cluster is unjoined.
                     type: boolean
                   purgeKlusterletOperator:

fleetconfig-controller/charts/fleetconfig-controller/crds/fleetconfig.open-cluster-management.io_spokes.yaml

@@ -75,9 +75,14 @@ metadata:
 spec:
   {{- with .cleanupConfig }}
   cleanupConfig:
-    purgeKlusterletOperator: {{ .purgeKlusterletOperator | default true }}
+    {{- if hasKey . "purgeKlusterletOperator" }}
+    purgeKlusterletOperator: {{ .purgeKlusterletOperator }}
+    {{- else }}
+    purgeKlusterletOperator: true
+    {{- end }}
     purgeKubeconfigSecret: {{ .purgeKubeconfigSecret | default false }}
     purgeAgentNamespace: {{ .purgeAgentNamespace | default false }}
+    forceClusterDrain: {{ .forceClusterDrain | default false }}
   {{- end }}
   hubRef:
     name: {{ .hubRef.name }}

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

@@ -184,6 +184,7 @@ fleetConfig:
   ## @param fleetConfig.spokes[0].cleanupConfig.purgeKlusterletOperator If set, the klusterlet operator will be purged and all open-cluster-management namespaces deleted when the klusterlet is unjoined from its Hub cluster.
   ## @param fleetConfig.spokes[0].cleanupConfig.purgeKubeconfigSecret If set, the kubeconfig secret will be automatically deleted after the agent has taken over managing the Spoke.
   ## @param fleetConfig.spokes[0].cleanupConfig.purgeAgentNamespace If true, the agent will attempt to garbage collect its own namespace after the spoke cluster is unjoined.
+  ## @param fleetConfig.spokes[0].cleanupConfig.forceClusterDrain If set, all ManifestWorks which were created using a Placement will be automatically descheduled from the Spoke cluster during deletion. This includes AddOns installed using installStrategy.type=Placements. If an AddOn must stay running to reconcile deletion of other ManifestWorks, it should tolerate the `fleetconfig.open-cluster-management.io/workload-cleanup` taint. Manually created ManifestWorks will not be affected and must be manually cleaned up for Spoke deletion to proceed.
   ## @param fleetConfig.spokes[0].kubeconfig.context The context to use in the kubeconfig file. Leave empty to use the current context.
   ## @param fleetConfig.spokes[0].kubeconfig.inCluster If set, the kubeconfig will be read from the cluster. Only applicable for same-cluster operations.
   ## @param fleetConfig.spokes[0].kubeconfig.secretReference.name The name of the secret.
@@ -216,6 +217,7 @@ fleetConfig:
       purgeKlusterletOperator: true
       purgeKubeconfigSecret: true
       purgeAgentNamespace: true
+      forceClusterDrain: false
     ## Kubeconfig details for the Spoke cluster.
     kubeconfig:
       context: ""
@@ -292,7 +294,7 @@ imageRegistry: ""
 ## @param image.pullPolicy Image pull policy
 image:
   repository: quay.io/open-cluster-management/fleetconfig-controller
-  tag: v0.1.1
+  tag: v0.1.2
   pullPolicy: IfNotPresent
 
 ## @param imagePullSecrets Image pull secrets

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

@@ -117,6 +117,8 @@ deployments:
           enabled: ${FLEETCONFIG_ENABLED}
         addonMode: ${ADDON_MODE}
         enableLegacyControllers: ${ENABLE_LEGACY_CONTROLLERS}
+        cert-manager:
+          enabled: false
       valuesFiles:
       - ${CONTEXT}/charts/fleetconfig-controller/values.yaml
     updateImageTags: false
@@ -131,6 +133,8 @@ deployments:
           enabled: ${FLEETCONFIG_ENABLED}
         addonMode: ${ADDON_MODE}
         enableLegacyControllers: ${ENABLE_LEGACY_CONTROLLERS}
+        cert-manager:
+          enabled: false
       valuesFiles:
       - ${CONTEXT}/charts/fleetconfig-controller/values.yaml
 
@@ -146,6 +150,8 @@ deployments:
           tag: local
         addonMode: ${ADDON_MODE}
         enableLegacyControllers: ${ENABLE_LEGACY_CONTROLLERS}
+        cert-manager:
+          enabled: false
       valuesFiles:
       - ${CONTEXT}/charts/fleetconfig-controller/values.yaml
       - ${CONTEXT}/test/data/fleetconfig-values.yaml

fleetconfig-controller/devspace.yaml

@@ -7,9 +7,14 @@ import (
 
 // generic
 const (
-	clusteradm     = "clusteradm"
-	requeue        = 30 * time.Second
-	amwExistsError = "you should manually clean them, uninstall kluster will cause those works out of control."
+	clusteradm = "clusteradm"
+
+	hubRequeuePreInit    = 30 * time.Second
+	hubRequeuePostInit   = 2 * time.Minute
+	requeueDeleting      = 5 * time.Second
+	spokeRequeuePreJoin  = 15 * time.Second
+	spokeRequeuePostJoin = 1 * time.Minute
+	spokeWatchInterval   = 30 * time.Second
 )
 
 var csrSuffixPattern = regexp.MustCompile(`-[a-zA-Z0-9]{5}$`)

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

@@ -94,7 +94,7 @@ func (r *HubReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 	// Add a finalizer and requeue if not already present
 	if !slices.Contains(hub.Finalizers, v1beta1.HubCleanupFinalizer) {
 		hub.Finalizers = append(hub.Finalizers, v1beta1.HubCleanupFinalizer)
-		return ret(ctx, ctrl.Result{RequeueAfter: requeue}, nil)
+		return ret(ctx, ctrl.Result{RequeueAfter: hubRequeuePreInit}, nil)
 	}
 
 	hubKubeconfig, err := kube.KubeconfigFromSecretOrCluster(ctx, r.Client, hub.Spec.Kubeconfig, hub.Namespace)
@@ -106,20 +106,22 @@ func (r *HubReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 	if !hub.DeletionTimestamp.IsZero() {
 		if hub.Status.Phase != v1beta1.Deleting {
 			hub.Status.Phase = v1beta1.Deleting
-			return ret(ctx, ctrl.Result{RequeueAfter: requeue}, nil)
+			return ret(ctx, ctrl.Result{RequeueAfter: requeueDeleting}, nil)
 		}
 
 		if slices.Contains(hub.Finalizers, v1beta1.HubCleanupFinalizer) {
-			if err := r.cleanHub(ctx, hub, hubKubeconfig); err != nil {
+			requeue, err := r.cleanHub(ctx, hub, hubKubeconfig)
+			if err != nil {
 				hub.SetConditions(true, v1beta1.NewCondition(
 					err.Error(), v1beta1.CleanupFailed, metav1.ConditionTrue, metav1.ConditionFalse,
 				))
 				return ret(ctx, ctrl.Result{}, err)
 			}
+			if requeue {
+				return ret(ctx, ctrl.Result{RequeueAfter: requeueDeleting}, nil)
+			}
 		}
-		hub.Finalizers = slices.DeleteFunc(hub.Finalizers, func(s string) bool {
-			return s == v1beta1.HubCleanupFinalizer
-		})
+
 		// end reconciliation
 		return ret(ctx, ctrl.Result{}, nil)
 	}
@@ -145,7 +147,7 @@ func (r *HubReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 
 	if previousPhase == "" {
 		// set initial phase/conditions and requeue
-		return ret(ctx, ctrl.Result{RequeueAfter: requeue}, nil)
+		return ret(ctx, ctrl.Result{RequeueAfter: hubRequeuePreInit}, nil)
 	}
 
 	// Handle Hub cluster: initialization and/or upgrade
@@ -155,22 +157,22 @@ func (r *HubReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 	}
 	hubInitializedCond := hub.GetCondition(v1beta1.HubInitialized)
 	if hubInitializedCond == nil || hubInitializedCond.Status == metav1.ConditionFalse {
-		return ret(ctx, ctrl.Result{RequeueAfter: requeue}, nil)
+		return ret(ctx, ctrl.Result{RequeueAfter: hubRequeuePreInit}, nil)
 	}
 
 	// Finalize phase
 	for _, c := range hub.Status.Conditions {
 		if c.Status != c.WantStatus {
 			logger.Info("WARNING: condition does not have the desired status", "type", c.Type, "reason", c.Reason, "message", c.Message, "status", c.Status, "wantStatus", c.WantStatus)
 			hub.Status.Phase = v1beta1.Unhealthy
-			return ret(ctx, ctrl.Result{RequeueAfter: requeue}, nil)
+			return ret(ctx, ctrl.Result{RequeueAfter: hubRequeuePreInit}, nil)
 		}
 	}
 	if hub.Status.Phase == v1beta1.HubStarting {
 		hub.Status.Phase = v1beta1.HubRunning
 	}
 
-	return ret(ctx, ctrl.Result{RequeueAfter: requeue}, nil)
+	return ret(ctx, ctrl.Result{RequeueAfter: hubRequeuePostInit}, nil)
 }
 
 type contextKey int
@@ -185,56 +187,55 @@ func withOriginalHub(ctx context.Context, hub *v1beta1.Hub) context.Context {
 }
 
 // cleanup cleans up a Hub and its associated resources.
-func (r *HubReconciler) cleanHub(ctx context.Context, hub *v1beta1.Hub, hubKubeconfig []byte) error {
+func (r *HubReconciler) cleanHub(ctx context.Context, hub *v1beta1.Hub, hubKubeconfig []byte) (bool, error) {
 	logger := log.FromContext(ctx)
 	logger.V(0).Info("cleanHub", "hub", hub.Name)
 
 	// Check if there are any Spokes that need to be deleted
 	spokeList := &v1beta1.SpokeList{}
 	err := r.List(ctx, spokeList)
 	if err != nil {
-		return err
+		return true, err
 	}
 
-	spokes := spokeList.Items
-	if len(spokes) > 0 {
-		// Mark all Spokes for deletion if they haven't been deleted yet
-		for i := range spokes {
-			spoke := &spokes[i]
-			if spoke.DeletionTimestamp.IsZero() {
-				if !spoke.IsManagedBy(hub.ObjectMeta) {
-					continue
-				}
-				logger.Info("Marking Spoke for deletion", "spoke", spoke.Name)
-				if err := r.Delete(ctx, spoke); err != nil && !kerrs.IsNotFound(err) {
-					return fmt.Errorf("failed to delete spoke %s: %w", spoke.Name, err)
-				}
+	managedRemaining := 0
+	for i := range spokeList.Items {
+		s := &spokeList.Items[i]
+		if !s.IsManagedBy(hub.ObjectMeta) {
+			continue
+		}
+		if s.DeletionTimestamp.IsZero() {
+			logger.Info("Marking Spoke for deletion", "spoke", s.Name)
+			if err := r.Delete(ctx, s); err != nil && !kerrs.IsNotFound(err) {
+				return true, fmt.Errorf("failed to delete spoke %s: %w", s.Name, err)
 			}
 		}
-
-		logger.V(1).Info("Waiting for all Spokes to be deleted before proceeding with Hub cleanup",
-			"remainingSpokes", len(spokes))
-		// Return a retriable error to requeue and check again later
-		return fmt.Errorf("waiting for background spoke deletion. Remaining: %d spokes", len(spokes))
+		// Count managed spokes until they're fully deleted
+		managedRemaining++
+	}
+	if managedRemaining > 0 {
+		logger.V(1).Info("Waiting for managed Spokes to be deleted before proceeding with Hub cleanup",
+			"remainingSpokes", managedRemaining)
+		return true, nil
 	}
 
 	logger.Info("All Spokes have been deleted, proceeding with Hub cleanup")
 
 	addonC, err := common.AddOnClient(hubKubeconfig)
 	if err != nil {
-		return fmt.Errorf("failed to create addon client for cleanup: %w", err)
+		return true, fmt.Errorf("failed to create addon client for cleanup: %w", err)
 	}
 
 	hubCopy := hub.DeepCopy()
 	hubCopy.Spec.AddOnConfigs = nil
 	hubCopy.Spec.HubAddOns = nil
 	_, err = handleAddonConfig(ctx, r.Client, addonC, hubCopy)
 	if err != nil {
-		return err
+		return true, err
 	}
 	_, err = handleHubAddons(ctx, addonC, hubCopy)
 	if err != nil {
-		return err
+		return true, err
 	}
 
 	purgeOperator := false
@@ -253,12 +254,15 @@ func (r *HubReconciler) cleanHub(ctx context.Context, hub *v1beta1.Hub, hubKubec
 	stdout, stderr, err := exec_utils.CmdWithLogs(ctx, cmd, "waiting for 'clusteradm clean' to complete...")
 	if err != nil {
 		out := append(stdout, stderr...)
-		return fmt.Errorf("failed to clean hub cluster: %v, output: %s", err, string(out))
+		return true, fmt.Errorf("failed to clean hub cluster: %v, output: %s", err, string(out))
 	}
 	logger.V(1).Info("hub cleaned", "output", string(stdout))
 
-	return nil
+	hub.Finalizers = slices.DeleteFunc(hub.Finalizers, func(s string) bool {
+		return s == v1beta1.HubCleanupFinalizer
+	})
 
+	return false, nil
 }
 
 // handleHub manages Hub cluster init and upgrade operations