Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/v1beta1/cluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/index/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion cmd/clusterctl/client/cluster/objectgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/clusterctl/client/cluster/topology.go
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,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
}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/clusterctl/client/clusterclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/topology/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,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
Expand Down
10 changes: 5 additions & 5 deletions internal/controllers/topology/cluster/cluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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))

Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
}
Expand Down
20 changes: 10 additions & 10 deletions internal/webhooks/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()),
)
}
}
Expand All @@ -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
}

Expand Down
Loading