Make Cluster webhook less strict for out of date ClusterClasses

cmd/clusterctl/client/cluster/mover.go

@@ -637,20 +637,23 @@ func pauseClusterClass(proxy Proxy, n *node, pause bool) error {
 	}
 
 	// Update the annotation to the desired state
-	ccAnnotations := clusterClass.GetAnnotations()
-	if ccAnnotations == nil {
-		ccAnnotations = make(map[string]string)
+	currentAnnotations := clusterClass.GetAnnotations()
+
+	// Create a copy of the current annotations to avoid modifying the original map.
+	wantAnnotations := map[string]string{}
+	for k, v := range currentAnnotations {
+		wantAnnotations[k] = v
 	}
 	if pause {
 		// Set the pause annotation.
-		ccAnnotations[clusterv1.PausedAnnotation] = ""
+		wantAnnotations[clusterv1.PausedAnnotation] = ""
 	} else {
 		// Delete the pause annotation.
-		delete(ccAnnotations, clusterv1.PausedAnnotation)
+		delete(wantAnnotations, clusterv1.PausedAnnotation)
 	}
 
 	// If the ClusterClass is already at desired state return early.
-	if !annotations.AddAnnotations(clusterClass, ccAnnotations) {
+	if !annotations.AddAnnotations(clusterClass, wantAnnotations) {
 		return nil
 	}
 

internal/webhooks/cluster.go

@@ -62,6 +62,8 @@ type Cluster struct {
 var _ webhook.CustomDefaulter = &Cluster{}
 var _ webhook.CustomValidator = &Cluster{}
 
+var errClusterClassNotReconciled = errors.New("ClusterClass is not up to date")
+
 // Default satisfies the defaulting webhook interface.
 func (webhook *Cluster) Default(ctx context.Context, obj runtime.Object) error {
 	// We gather all defaulting errors and return them together.
@@ -88,11 +90,11 @@ func (webhook *Cluster) Default(ctx context.Context, obj runtime.Object) error {
 		}
 		clusterClass, err := webhook.pollClusterClassForCluster(ctx, cluster)
 		if err != nil {
-			// If the ClusterClass can't be found ignore the error.
-			if apierrors.IsNotFound(err) {
+			// If the ClusterClass can't be found or is not up to date ignore the error.
+			if apierrors.IsNotFound(err) || errors.Is(err, errClusterClassNotReconciled) {
 				return nil
 			}
-			return apierrors.NewInternalError(errors.Wrapf(err, "Cluster %s can't be defaulted. Valid ClusterClass %s can not be retrieved", cluster.Name, cluster.Spec.Topology.Class))
+			return apierrors.NewInternalError(errors.Wrapf(err, "Cluster %s can't be defaulted. ClusterClass %s can not be retrieved", cluster.Name, cluster.Spec.Topology.Class))
 		}
 
 		allErrs = append(allErrs, DefaultVariables(cluster, clusterClass)...)
@@ -243,8 +245,9 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 
 	// Get the ClusterClass referenced in the Cluster.
 	clusterClass, clusterClassGetErr := webhook.pollClusterClassForCluster(ctx, newCluster)
-	if clusterClassGetErr != nil && !apierrors.IsNotFound(clusterClassGetErr) {
-		// If the error is anything other than "Not Found" return all errors at this point.
+	if clusterClassGetErr != nil &&
+		// If the error is anything other than "NotFound" or "NotReconciled" return all errors at this point.
+		!(apierrors.IsNotFound(clusterClassGetErr) || errors.Is(clusterClassGetErr, errClusterClassNotReconciled)) {
 		allErrs = append(
 			allErrs, field.InternalError(
 				fldPath.Child("class"),
@@ -530,20 +533,21 @@ func ValidateClusterForClusterClass(cluster *clusterv1.Cluster, clusterClass *cl
 func (webhook *Cluster) pollClusterClassForCluster(ctx context.Context, cluster *clusterv1.Cluster) (*clusterv1.ClusterClass, error) {
 	clusterClass := &clusterv1.ClusterClass{}
 	var clusterClassGetErr error
+	// TODO: Add a webhook warning if the ClusterClass is not up to date or not found.
 	_ = util.PollImmediate(200*time.Millisecond, 2*time.Second, func() (bool, error) {
 		if clusterClassGetErr = webhook.Client.Get(ctx, client.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Spec.Topology.Class}, clusterClass); clusterClassGetErr != nil {
 			return false, nil //nolint:nilerr
 		}
 
-		// Return an error if the ClusterClass has not successfully reconciled because variables aren't correctly
-		// reconciled.
-		// TODO: Decide whether to check generation here. This requires creating templates before creating the Cluster and
-		// may interfere with the way clusterctl move works.
-		if !conditions.Has(clusterClass, clusterv1.ClusterClassVariablesReconciledCondition) ||
+		// Return a errClusterClassNotReconciled error if the ClusterClass has not successfully reconciled or if the
+		// ClusterClass variables have not been successfully reconciled.
+		if clusterClass.Generation != clusterClass.Status.ObservedGeneration ||
+			!conditions.Has(clusterClass, clusterv1.ClusterClassVariablesReconciledCondition) ||
 			conditions.IsFalse(clusterClass, clusterv1.ClusterClassVariablesReconciledCondition) {
-			clusterClassGetErr = errors.New("ClusterClass is not up to date. If this persists check ClusterClass status")
+			clusterClassGetErr = errClusterClassNotReconciled
 			return false, nil
 		}
+		clusterClassGetErr = nil
 		return true, nil
 	})
 	return clusterClass, clusterClassGetErr