Skip to content

🐛 Fix continuous reconciles because of apiVersion differences in Cluster topology controller #12341

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 11, 2025
Merged

🐛 Fix continuous reconciles because of apiVersion differences in Cluster topology controller #12341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 22 additions & 11 deletions internal/controllers/topology/cluster/current_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package cluster
import (
"context"
"fmt"
"strings"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -83,7 +84,7 @@ func (r *Reconciler) getCurrentState(ctx context.Context, s *scope.Scope) (*scop
// getCurrentInfrastructureClusterState looks for the state of the InfrastructureCluster. If a reference is set but not
// found, either from an error or the object not being found, an error is thrown.
func (r *Reconciler) getCurrentInfrastructureClusterState(ctx context.Context, blueprintInfrastructureClusterTemplate *unstructured.Unstructured, cluster *clusterv1.Cluster) (*unstructured.Unstructured, error) {
ref, err := alignRefAPIVersion(blueprintInfrastructureClusterTemplate, cluster.Spec.InfrastructureRef)
ref, err := alignRefAPIVersion(blueprintInfrastructureClusterTemplate, cluster.Spec.InfrastructureRef, false)
if err != nil {
return nil, errors.Wrapf(err, "failed to read %s %s", cluster.Spec.InfrastructureRef.Kind, klog.KRef(cluster.Spec.InfrastructureRef.Namespace, cluster.Spec.InfrastructureRef.Name))
}
Expand All @@ -108,7 +109,7 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, blueprintC
res := &scope.ControlPlaneState{}

// Get the control plane object.
ref, err := alignRefAPIVersion(blueprintControlPlane.Template, cluster.Spec.ControlPlaneRef)
ref, err := alignRefAPIVersion(blueprintControlPlane.Template, cluster.Spec.ControlPlaneRef, false)
if err != nil {
return nil, errors.Wrapf(err, "failed to read %s %s", cluster.Spec.ControlPlaneRef.Kind, klog.KRef(cluster.Spec.ControlPlaneRef.Namespace, cluster.Spec.ControlPlaneRef.Name))
}
Expand All @@ -133,7 +134,7 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, blueprintC
if err != nil {
return res, errors.Wrapf(err, "failed to get InfrastructureMachineTemplate reference for %s %s", res.Object.GetKind(), klog.KObj(res.Object))
}
ref, err = alignRefAPIVersion(blueprintControlPlane.InfrastructureMachineTemplate, machineInfrastructureRef)
ref, err = alignRefAPIVersion(blueprintControlPlane.InfrastructureMachineTemplate, machineInfrastructureRef, true)
if err != nil {
return nil, errors.Wrapf(err, "failed to get InfrastructureMachineTemplate for %s %s", res.Object.GetKind(), klog.KObj(res.Object))
}
Expand Down Expand Up @@ -228,11 +229,11 @@ func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, bluep
if !ok {
return nil, fmt.Errorf("failed to find MachineDeployment class %s in ClusterClass", mdClassName)
}
bootstrapRef, err = alignRefAPIVersion(mdBluePrint.BootstrapTemplate, bootstrapRef)
bootstrapRef, err = alignRefAPIVersion(mdBluePrint.BootstrapTemplate, bootstrapRef, true)
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("MachineDeployment %s Bootstrap reference could not be retrieved", klog.KObj(m)))
}
infraRef, err = alignRefAPIVersion(mdBluePrint.InfrastructureMachineTemplate, infraRef)
infraRef, err = alignRefAPIVersion(mdBluePrint.InfrastructureMachineTemplate, infraRef, true)
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("MachineDeployment %s Infrastructure reference could not be retrieved", klog.KObj(m)))
}
Expand Down Expand Up @@ -348,11 +349,11 @@ func (r *Reconciler) getCurrentMachinePoolState(ctx context.Context, blueprintMa
if !ok {
return nil, fmt.Errorf("failed to find MachinePool class %s in ClusterClass", mpClassName)
}
bootstrapRef, err = alignRefAPIVersion(mpBluePrint.BootstrapTemplate, bootstrapRef)
bootstrapRef, err = alignRefAPIVersion(mpBluePrint.BootstrapTemplate, bootstrapRef, false)
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("MachinePool %s Bootstrap reference could not be retrieved", klog.KObj(m)))
}
infraRef, err = alignRefAPIVersion(mpBluePrint.InfrastructureMachinePoolTemplate, infraRef)
infraRef, err = alignRefAPIVersion(mpBluePrint.InfrastructureMachinePoolTemplate, infraRef, false)
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("MachinePool %s Infrastructure reference could not be retrieved", klog.KObj(m)))
}
Expand Down Expand Up @@ -397,17 +398,27 @@ func (r *Reconciler) getCurrentMachinePoolState(ctx context.Context, blueprintMa
// If group or kind was changed in the ClusterClass, an exact copy of the currentRef is returned because
// it will end up in a diff and a rollout anyway.
// Only bootstrap template refs in a ClusterClass can change their group and kind.
func alignRefAPIVersion(templateFromClusterClass *unstructured.Unstructured, currentRef *corev1.ObjectReference) (*corev1.ObjectReference, error) {
func alignRefAPIVersion(templateFromClusterClass *unstructured.Unstructured, currentRef *corev1.ObjectReference, isCurrentTemplate bool) (*corev1.ObjectReference, error) {
currentGV, err := schema.ParseGroupVersion(currentRef.APIVersion)
if err != nil {
return nil, errors.Wrapf(err, "failed to parse apiVersion: %q", currentRef.APIVersion)
}

apiVersion := currentRef.APIVersion
// Use apiVersion from ClusterClass if group and kind is the same.
if templateFromClusterClass.GroupVersionKind().Group == currentGV.Group &&
templateFromClusterClass.GetKind() == currentRef.Kind {
apiVersion = templateFromClusterClass.GetAPIVersion()
if templateFromClusterClass.GroupVersionKind().Group == currentGV.Group {
if isCurrentTemplate {
// If the current object is a template, kind has to be identical.
if templateFromClusterClass.GetKind() == currentRef.Kind {
apiVersion = templateFromClusterClass.GetAPIVersion()
}
} else {
// If the current object is not a template, currentRef.Kind should be the kind from CC without the Template suffix,
// e.g. KubeadmControlPlaneTemplate == KubeadmControlPlane
if strings.TrimSuffix(templateFromClusterClass.GetKind(), "Template") == currentRef.Kind {
apiVersion = templateFromClusterClass.GetAPIVersion()
}
}
}

return &corev1.ObjectReference{
Expand Down
40 changes: 33 additions & 7 deletions internal/controllers/topology/cluster/current_state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1187,43 +1187,67 @@ func TestAlignRefAPIVersion(t *testing.T) {
name string
templateFromClusterClass *unstructured.Unstructured
currentRef *corev1.ObjectReference
isCurrentTemplate bool
want *corev1.ObjectReference
wantErr bool
}{
{
name: "Error for invalid apiVersion",
templateFromClusterClass: &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": clusterv1.GroupVersionInfrastructure.String(),
"kind": "DockerCluster",
"kind": "DockerClusterTemplate",
}},
currentRef: &corev1.ObjectReference{
APIVersion: "invalid/api/version",
Kind: "DockerCluster",
Name: "my-cluster-abc",
Namespace: metav1.NamespaceDefault,
},
wantErr: true,
isCurrentTemplate: false,
wantErr: true,
},
{
name: "Use apiVersion from ClusterClass: group and kind is the same",
name: "Use apiVersion from ClusterClass: group and kind is the same (+/- Template suffix)",
templateFromClusterClass: &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": clusterv1.GroupVersionInfrastructure.String(),
"kind": "DockerCluster",
"kind": "DockerClusterTemplate",
}},
currentRef: &corev1.ObjectReference{
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta2",
APIVersion: "infrastructure.cluster.x-k8s.io/different", // should be overwritten with version from CC
Kind: "DockerCluster",
Name: "my-cluster-abc",
Namespace: metav1.NamespaceDefault,
},
isCurrentTemplate: false,
want: &corev1.ObjectReference{
// Group & kind is the same => apiVersion is taken from ClusterClass.
// Group & kind is the same (+/- Template suffix) => apiVersion is taken from ClusterClass.
APIVersion: clusterv1.GroupVersionInfrastructure.String(),
Kind: "DockerCluster",
Name: "my-cluster-abc",
Namespace: metav1.NamespaceDefault,
},
},
{
name: "Use apiVersion from ClusterClass: group and kind is the same",
templateFromClusterClass: &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": clusterv1.GroupVersionBootstrap.String(),
"kind": "KubeadmConfigTemplate",
}},
currentRef: &corev1.ObjectReference{
APIVersion: "bootstrap.cluster.x-k8s.io/different", // should be overwritten with version from CC
Kind: "KubeadmConfigTemplate",
Name: "my-cluster-abc",
Namespace: metav1.NamespaceDefault,
},
isCurrentTemplate: true,
want: &corev1.ObjectReference{
// Group & kind is the same => apiVersion is taken from ClusterClass.
APIVersion: clusterv1.GroupVersionBootstrap.String(),
Kind: "KubeadmConfigTemplate",
Name: "my-cluster-abc",
Namespace: metav1.NamespaceDefault,
},
},
{
name: "Use apiVersion from currentRef: kind is different",
templateFromClusterClass: &unstructured.Unstructured{Object: map[string]interface{}{
Expand All @@ -1236,6 +1260,7 @@ func TestAlignRefAPIVersion(t *testing.T) {
Name: "my-cluster-abc",
Namespace: metav1.NamespaceDefault,
},
isCurrentTemplate: true,
want: &corev1.ObjectReference{
// kind is different => apiVersion is taken from currentRef.
APIVersion: clusterv1.GroupVersionBootstrap.String(),
Expand All @@ -1259,6 +1284,7 @@ func TestAlignRefAPIVersion(t *testing.T) {
Name: "my-cluster-abc",
Namespace: metav1.NamespaceDefault,
},
isCurrentTemplate: true,
want: &corev1.ObjectReference{
// group is different => apiVersion is taken from currentRef.
APIVersion: clusterv1.GroupVersionBootstrap.String(),
Expand All @@ -1272,7 +1298,7 @@ func TestAlignRefAPIVersion(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

got, err := alignRefAPIVersion(tt.templateFromClusterClass, tt.currentRef)
got, err := alignRefAPIVersion(tt.templateFromClusterClass, tt.currentRef, tt.isCurrentTemplate)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
return
Expand Down
Loading