Merge pull request #10703 from vincepri/add-class-key-ref

🌱 Add Cluster.GetClassKey() to retrieve a NamespacedName for classes

api/v1beta1/cluster_types.go

@@ -25,6 +25,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 
 	capierrors "sigs.k8s.io/cluster-api/errors"
@@ -509,6 +510,14 @@ type Cluster struct {
 	Status ClusterStatus `json:"status,omitempty"`
 }
 
+// GetClassKey returns the namespaced name for the class associated with this object.
+func (c *Cluster) GetClassKey() types.NamespacedName {
+	if c.Spec.Topology == nil {
+		return types.NamespacedName{}
+	}
+	return types.NamespacedName{Namespace: c.GetNamespace(), Name: c.Spec.Topology.Class}
+}
+
 // GetConditions returns the set of conditions for this object.
 func (c *Cluster) GetConditions() Conditions {
 	return c.Status.Conditions

api/v1beta1/index/cluster.go

@@ -51,7 +51,7 @@ func ClusterByClusterClassClassName(o client.Object) []string {
 		panic(fmt.Sprintf("Expected Cluster but got a %T", o))
 	}
 	if cluster.Spec.Topology != nil {
-		return []string{cluster.Spec.Topology.Class}
+		return []string{cluster.GetClassKey().Name}
 	}
 	return nil
 }

cmd/clusterctl/client/cluster/objectgraph.go

@@ -148,7 +148,7 @@ func (n *node) captureAdditionalInformation(obj *unstructured.Unstructured) erro
 			if n.additionalInfo == nil {
 				n.additionalInfo = map[string]interface{}{}
 			}
-			n.additionalInfo[clusterTopologyNameKey] = cluster.Spec.Topology.Class
+			n.additionalInfo[clusterTopologyNameKey] = cluster.GetClassKey().Name
 		}
 	}
 

cmd/clusterctl/client/cluster/topology.go

@@ -695,7 +695,8 @@ func (t *topologyClient) affectedClusters(ctx context.Context, in *TopologyPlanI
 	// Each of the Cluster that uses the ClusterClass in the input is an affected cluster.
 	for _, cc := range affectedClusterClasses {
 		for i := range clusterList.Items {
-			if clusterList.Items[i].Spec.Topology != nil && clusterList.Items[i].Spec.Topology.Class == cc.Name {
+			cluster := clusterList.Items[i]
+			if cluster.Spec.Topology != nil && cluster.GetClassKey().Name == cc.Name {
 				affectedClusters[client.ObjectKeyFromObject(&clusterList.Items[i])] = true
 			}
 		}

cmd/clusterctl/client/clusterclass.go

@@ -80,7 +80,7 @@ func clusterClassNamesFromTemplate(template Template) ([]string, error) {
 		if cluster.Spec.Topology == nil {
 			continue
 		}
-		classes = append(classes, cluster.Spec.Topology.Class)
+		classes = append(classes, cluster.GetClassKey().Name)
 	}
 	return classes, nil
 }

internal/controllers/topology/cluster/cluster_controller.go

@@ -217,9 +217,9 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope.Scope) (ctrl.Result
 
 	// Get ClusterClass.
 	clusterClass := &clusterv1.ClusterClass{}
-	key := client.ObjectKey{Name: s.Current.Cluster.Spec.Topology.Class, Namespace: s.Current.Cluster.Namespace}
+	key := s.Current.Cluster.GetClassKey()
 	if err := r.Client.Get(ctx, key, clusterClass); err != nil {
-		return ctrl.Result{}, errors.Wrapf(err, "failed to retrieve ClusterClass %s", s.Current.Cluster.Spec.Topology.Class)
+		return ctrl.Result{}, errors.Wrapf(err, "failed to retrieve ClusterClass %s", key)
 	}
 
 	s.Blueprint.ClusterClass = clusterClass

internal/controllers/topology/cluster/cluster_controller_test.go

@@ -303,7 +303,7 @@ func TestClusterReconciler_reconcileUpdatesOnClusterClass(t *testing.T) {
 
 	// Get the clusterClass to update and check if clusterClass updates are being correctly reconciled.
 	clusterClass := &clusterv1.ClusterClass{}
-	g.Expect(env.Get(ctx, client.ObjectKey{Namespace: ns.Name, Name: actualCluster.Spec.Topology.Class}, clusterClass)).To(Succeed())
+	g.Expect(env.Get(ctx, actualCluster.GetClassKey(), clusterClass)).To(Succeed())
 
 	patchHelper, err := patch.NewHelper(clusterClass, env.Client)
 	g.Expect(err).ToNot(HaveOccurred())
@@ -317,7 +317,7 @@ func TestClusterReconciler_reconcileUpdatesOnClusterClass(t *testing.T) {
 		// Check that the clusterClass has been correctly updated to use the new infrastructure template.
 		// This is necessary as sometimes the cache can take a little time to update.
 		class := &clusterv1.ClusterClass{}
-		g.Expect(env.Get(ctx, client.ObjectKey{Namespace: ns.Name, Name: actualCluster.Spec.Topology.Class}, class)).To(Succeed())
+		g.Expect(env.Get(ctx, actualCluster.GetClassKey(), class)).To(Succeed())
 		g.Expect(class.Spec.Workers.MachineDeployments[0].Template.Infrastructure.Ref.Name).To(Equal(infrastructureMachineTemplateName2))
 		g.Expect(class.Spec.Workers.MachinePools[0].Template.Infrastructure.Ref.Name).To(Equal(infrastructureMachinePoolTemplateName2))
 
@@ -412,7 +412,7 @@ func TestClusterReconciler_reconcileClusterClassRebase(t *testing.T) {
 			return err
 		}
 		// Check to ensure the spec.topology.class has been successfully updated in the API server and cache.
-		g.Expect(updatedCluster.Spec.Topology.Class).To(Equal(clusterClassName2))
+		g.Expect(updatedCluster.GetClassKey().Name).To(Equal(clusterClassName2))
 		// Check if Cluster has relevant Infrastructure and ControlPlane and labels and annotations.
 		g.Expect(assertClusterReconcile(updatedCluster)).Should(Succeed())
 
@@ -633,7 +633,7 @@ func TestClusterReconciler_deleteClusterClass(t *testing.T) {
 
 	// Ensure the clusterClass is available in the API server .
 	clusterClass := &clusterv1.ClusterClass{}
-	g.Expect(env.Get(ctx, client.ObjectKey{Namespace: ns.Name, Name: actualCluster.Spec.Topology.Class}, clusterClass)).To(Succeed())
+	g.Expect(env.Get(ctx, actualCluster.GetClassKey(), clusterClass)).To(Succeed())
 
 	// Attempt to delete the ClusterClass. Expect an error here as the ClusterClass deletion is blocked by the webhook.
 	g.Expect(env.Delete(ctx, clusterClass)).NotTo(Succeed())
@@ -968,7 +968,7 @@ func assertControlPlaneReconcile(cluster *clusterv1.Cluster) error {
 		}
 	}
 	clusterClass := &clusterv1.ClusterClass{}
-	if err := env.Get(ctx, client.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Spec.Topology.Class}, clusterClass); err != nil {
+	if err := env.Get(ctx, cluster.GetClassKey(), clusterClass); err != nil {
 		return err
 	}
 	// Check for the ControlPlaneInfrastructure if it's referenced in the clusterClass.

internal/controllers/topology/cluster/patches/variables/variables.go

@@ -63,7 +63,7 @@ func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster, def
 			Namespace: cluster.Namespace,
 			Topology: &runtimehooksv1.ClusterTopologyBuiltins{
 				Version: cluster.Spec.Topology.Version,
-				Class:   cluster.Spec.Topology.Class,
+				Class:   cluster.GetClassKey().Name,
 			},
 		},
 	}

internal/webhooks/cluster.go

@@ -102,7 +102,7 @@ func (webhook *Cluster) Default(ctx context.Context, obj runtime.Object) error {
 			cluster.Spec.Topology.Version = "v" + cluster.Spec.Topology.Version
 		}
 
-		if cluster.Spec.Topology.Class == "" {
+		if cluster.GetClassKey().Name == "" {
 			allErrs = append(
 				allErrs,
 				field.Required(
@@ -119,7 +119,7 @@ func (webhook *Cluster) Default(ctx context.Context, obj runtime.Object) error {
 			if apierrors.IsNotFound(err) || errors.Is(err, errClusterClassNotReconciled) {
 				return nil
 			}
-			return apierrors.NewInternalError(errors.Wrapf(err, "Cluster %s can't be defaulted. 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.GetClassKey().Name))
 		}
 
 		// Doing both defaulting and validating here prevents a race condition where the ClusterClass could be
@@ -254,7 +254,7 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 	var allErrs field.ErrorList
 
 	// class should be defined.
-	if newCluster.Spec.Topology.Class == "" {
+	if newCluster.GetClassKey().Name == "" {
 		allErrs = append(
 			allErrs,
 			field.Required(
@@ -334,7 +334,7 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 		}
 
 		// Topology or Class can not be added on update unless ClusterTopologyUnsafeUpdateClassNameAnnotation is set.
-		if oldCluster.Spec.Topology == nil || oldCluster.Spec.Topology.Class == "" {
+		if oldCluster.Spec.Topology == nil || oldCluster.GetClassKey().Name == "" {
 			if _, ok := newCluster.Annotations[clusterv1.ClusterTopologyUnsafeUpdateClassNameAnnotation]; ok {
 				return allWarnings, allErrs
 			}
@@ -386,15 +386,15 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 		}
 
 		// If the ClusterClass referenced in the Topology has changed compatibility checks are needed.
-		if oldCluster.Spec.Topology.Class != newCluster.Spec.Topology.Class {
+		if oldCluster.GetClassKey() != newCluster.GetClassKey() {
 			// Check to see if the ClusterClass referenced in the old version of the Cluster exists.
 			oldClusterClass, err := webhook.pollClusterClassForCluster(ctx, oldCluster)
 			if err != nil {
 				allErrs = append(
 					allErrs, field.Forbidden(
 						fldPath.Child("class"),
 						fmt.Sprintf("valid ClusterClass with name %q could not be retrieved, change from class %[1]q to class %q cannot be validated. Error: %s",
-							oldCluster.Spec.Topology.Class, newCluster.Spec.Topology.Class, err.Error())))
+							oldCluster.GetClassKey(), newCluster.GetClassKey(), err.Error())))
 
 				// Return early with errors if the ClusterClass can't be retrieved.
 				return allWarnings, allErrs
@@ -854,20 +854,20 @@ func (webhook *Cluster) validateClusterClassExistsAndIsReconciled(ctx context.Co
 				fmt.Sprintf(
 					"Cluster refers to ClusterClass %s in the topology but it does not exist. "+
 						"Cluster topology has not been fully validated. "+
-						"The ClusterClass must be created to reconcile the Cluster", newCluster.Spec.Topology.Class),
+						"The ClusterClass must be created to reconcile the Cluster", newCluster.GetClassKey()),
 			)
 		case errors.Is(clusterClassPollErr, errClusterClassNotReconciled):
 			allWarnings = append(allWarnings,
 				fmt.Sprintf(
 					"Cluster refers to ClusterClass %s but this object which hasn't yet been reconciled. "+
-						"Cluster topology has not been fully validated. ", newCluster.Spec.Topology.Class),
+						"Cluster topology has not been fully validated. ", newCluster.GetClassKey()),
 			)
 		// If there's any other error return a generic warning with the error message.
 		default:
 			allWarnings = append(allWarnings,
 				fmt.Sprintf(
 					"Cluster refers to ClusterClass %s in the topology but it could not be retrieved. "+
-						"Cluster topology has not been fully validated: %s", newCluster.Spec.Topology.Class, clusterClassPollErr.Error()),
+						"Cluster topology has not been fully validated: %s", newCluster.GetClassKey(), clusterClassPollErr.Error()),
 			)
 		}
 	}
@@ -879,7 +879,7 @@ func (webhook *Cluster) pollClusterClassForCluster(ctx context.Context, cluster
 	clusterClass := &clusterv1.ClusterClass{}
 	var clusterClassPollErr error
 	_ = wait.PollUntilContextTimeout(ctx, 200*time.Millisecond, 2*time.Second, true, func(ctx context.Context) (bool, error) {
-		if clusterClassPollErr = webhook.Client.Get(ctx, client.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Spec.Topology.Class}, clusterClass); clusterClassPollErr != nil {
+		if clusterClassPollErr = webhook.Client.Get(ctx, cluster.GetClassKey(), clusterClass); clusterClassPollErr != nil {
 			return false, nil //nolint:nilerr
 		}