diff --git a/cmd/clusterctl/client/cluster/mover.go b/cmd/clusterctl/client/cluster/mover.go index 6c8b01c4c867..6862955b969b 100644 --- a/cmd/clusterctl/client/cluster/mover.go +++ b/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 } diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index 2b4ba8f67cae..8b5bb361556c 100644 --- a/internal/webhooks/cluster.go +++ b/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