Skip to content

Commit 38f07ef

Browse files
sbueringersbueringer
committed
ClusterClass: use exact versions from ClusterClass, stop api bump in CC
1 parent c7c0c59 commit 38f07ef

File tree

9 files changed

+641
-101
lines changed

9 files changed

+641
-101
lines changed

‎internal/controllers/clusterclass/clusterclass_controller.go‎

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import (
3434
"sigs.k8s.io/cluster-api/controllers/external"
3535
tlog "sigs.k8s.io/cluster-api/internal/log"
3636
"sigs.k8s.io/cluster-api/util/annotations"
37-
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
3837
"sigs.k8s.io/cluster-api/util/patch"
3938
"sigs.k8s.io/cluster-api/util/predicates"
4039
)
@@ -91,21 +90,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
9190
return ctrl.Result{}, nil
9291
}
9392

94-
// We use the patchHelper to patch potential changes to the ObjectReferences in ClusterClass.
95-
patchHelper, err := patch.NewHelper(clusterClass, r.Client)
96-
if err != nil {
97-
return ctrl.Result{}, err
98-
}
99-
100-
defer func() {
101-
if err := patchHelper.Patch(ctx, clusterClass); err != nil {
102-
reterr = kerrors.NewAggregate([]error{
103-
reterr,
104-
errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: clusterClass})},
105-
)
106-
}
107-
}()
108-
10993
return r.reconcile(ctx, clusterClass)
11094
}
11195

@@ -133,38 +117,35 @@ func (r *Reconciler) reconcile(ctx context.Context, clusterClass *clusterv1.Clus
133117
}
134118
}
135119

136-
// Ensure all the referenced objects are owned by the ClusterClass and that references are
137-
// upgraded to the latest contract.
138-
// Nb. Some external objects can be referenced multiple times in the ClusterClass. We
139-
// update the API contracts of all the references but we set the owner reference on the unique
140-
// external object only once.
120+
// Ensure all referenced objects are owned by the ClusterClass.
121+
// Nb. Some external objects can be referenced multiple times in the ClusterClass,
122+
// but we only want to set the owner reference once per unique external object.
123+
// For example the same KubeadmConfigTemplate could be referenced in multiple MachineDeployment
124+
// classes.
141125
errs := []error{}
142-
patchedRefs := sets.NewString()
126+
reconciledRefs := sets.NewString()
143127
for i := range refs {
144128
ref := refs[i]
145129
uniqueKey := uniqueObjectRefKey(ref)
146-
if err := r.reconcileExternal(ctx, clusterClass, ref, !patchedRefs.Has(uniqueKey)); err != nil {
130+
131+
// Continue as we only have to reconcile every referenced object once.
132+
if reconciledRefs.Has(uniqueKey) {
133+
continue
134+
}
135+
136+
if err := r.reconcileExternal(ctx, clusterClass, ref); err != nil {
147137
errs = append(errs, err)
148138
continue
149139
}
150-
patchedRefs.Insert(uniqueKey)
140+
reconciledRefs.Insert(uniqueKey)
151141
}
152142

153143
return ctrl.Result{}, kerrors.NewAggregate(errs)
154144
}
155145

156-
func (r *Reconciler) reconcileExternal(ctx context.Context, clusterClass *clusterv1.ClusterClass, ref *corev1.ObjectReference, setOwnerRef bool) error {
146+
func (r *Reconciler) reconcileExternal(ctx context.Context, clusterClass *clusterv1.ClusterClass, ref *corev1.ObjectReference) error {
157147
log := ctrl.LoggerFrom(ctx)
158148

159-
if err := utilconversion.UpdateReferenceAPIContract(ctx, r.Client, r.APIReader, ref); err != nil {
160-
return errors.Wrapf(err, "failed to update reference API contract of %s", tlog.KRef{Ref: ref})
161-
}
162-
163-
// If we dont need to set the ownerReference then return early.
164-
if !setOwnerRef {
165-
return nil
166-
}
167-
168149
obj, err := external.Get(ctx, r.UnstructuredCachingClient, ref, clusterClass.Namespace)
169150
if err != nil {
170151
if apierrors.IsNotFound(errors.Cause(err)) {
@@ -173,7 +154,7 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, clusterClass *cluste
173154
return errors.Wrapf(err, "failed to get the external object for the cluster class. refGroupVersionKind: %s, refName: %s", ref.GroupVersionKind(), ref.Name)
174155
}
175156

176-
// If external ref is paused, return early.
157+
// If referenced object is paused, return early.
177158
if annotations.HasPaused(obj) {
178159
log.V(3).Info("External object referenced is paused", "refGroupVersionKind", ref.GroupVersionKind(), "refName", ref.Name)
179160
return nil

‎internal/controllers/topology/cluster/current_state.go‎

Lines changed: 85 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ import (
2121
"fmt"
2222

2323
"github.com/pkg/errors"
24+
corev1 "k8s.io/api/core/v1"
2425
apierrors "k8s.io/apimachinery/pkg/api/errors"
2526
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
27+
"k8s.io/apimachinery/pkg/runtime/schema"
2628
"sigs.k8s.io/controller-runtime/pkg/client"
2729

2830
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -41,7 +43,7 @@ func (r *Reconciler) getCurrentState(ctx context.Context, s *scope.Scope) (*scop
4143
// Reference to the InfrastructureCluster can be nil and is expected to be on the first reconcile.
4244
// In this case the method should still be allowed to continue.
4345
if currentState.Cluster.Spec.InfrastructureRef != nil {
44-
infra, err := r.getCurrentInfrastructureClusterState(ctx, currentState.Cluster)
46+
infra, err := r.getCurrentInfrastructureClusterState(ctx, s.Blueprint.InfrastructureClusterTemplate, currentState.Cluster)
4547
if err != nil {
4648
return nil, err
4749
}
@@ -52,7 +54,7 @@ func (r *Reconciler) getCurrentState(ctx context.Context, s *scope.Scope) (*scop
5254
// should still be allowed to continue.
5355
currentState.ControlPlane = &scope.ControlPlaneState{}
5456
if currentState.Cluster.Spec.ControlPlaneRef != nil {
55-
cp, err := r.getCurrentControlPlaneState(ctx, currentState.Cluster, s.Blueprint)
57+
cp, err := r.getCurrentControlPlaneState(ctx, s.Blueprint.ControlPlane, s.Blueprint.HasControlPlaneInfrastructureMachine(), currentState.Cluster)
5658
if err != nil {
5759
return nil, err
5860
}
@@ -61,7 +63,7 @@ func (r *Reconciler) getCurrentState(ctx context.Context, s *scope.Scope) (*scop
6163

6264
// A Cluster may have zero or more MachineDeployments and a Cluster is expected to have zero MachineDeployments on
6365
// first reconcile.
64-
m, err := r.getCurrentMachineDeploymentState(ctx, currentState.Cluster)
66+
m, err := r.getCurrentMachineDeploymentState(ctx, s.Blueprint.MachineDeployments, currentState.Cluster)
6567
if err != nil {
6668
return nil, err
6769
}
@@ -72,8 +74,12 @@ func (r *Reconciler) getCurrentState(ctx context.Context, s *scope.Scope) (*scop
7274

7375
// getCurrentInfrastructureClusterState looks for the state of the InfrastructureCluster. If a reference is set but not
7476
// found, either from an error or the object not being found, an error is thrown.
75-
func (r *Reconciler) getCurrentInfrastructureClusterState(ctx context.Context, cluster *clusterv1.Cluster) (*unstructured.Unstructured, error) {
76-
infra, err := r.getReference(ctx, cluster.Spec.InfrastructureRef)
77+
func (r *Reconciler) getCurrentInfrastructureClusterState(ctx context.Context, blueprintInfrastructureClusterTemplate *unstructured.Unstructured, cluster *clusterv1.Cluster) (*unstructured.Unstructured, error) {
78+
ref, err := alignRefAPIVersion(blueprintInfrastructureClusterTemplate, cluster.Spec.InfrastructureRef)
79+
if err != nil {
80+
return nil, errors.Wrapf(err, "failed to read %s", tlog.KRef{Ref: cluster.Spec.InfrastructureRef})
81+
}
82+
infra, err := r.getReference(ctx, ref)
7783
if err != nil {
7884
return nil, errors.Wrapf(err, "failed to read %s", tlog.KRef{Ref: cluster.Spec.InfrastructureRef})
7985
}
@@ -89,12 +95,16 @@ func (r *Reconciler) getCurrentInfrastructureClusterState(ctx context.Context, c
8995
// getCurrentControlPlaneState returns information on the ControlPlane being used by the Cluster. If a reference is not found,
9096
// an error is thrown. If the ControlPlane requires MachineInfrastructure according to its ClusterClass an error will be
9197
// thrown if the ControlPlane has no MachineTemplates.
92-
func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, cluster *clusterv1.Cluster, blueprint *scope.ClusterBlueprint) (*scope.ControlPlaneState, error) {
98+
func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, blueprintControlPlane *scope.ControlPlaneBlueprint, blueprintHasControlPlaneInfrastructureMachine bool, cluster *clusterv1.Cluster) (*scope.ControlPlaneState, error) {
9399
var err error
94100
res := &scope.ControlPlaneState{}
95101

96102
// Get the control plane object.
97-
res.Object, err = r.getReference(ctx, cluster.Spec.ControlPlaneRef)
103+
ref, err := alignRefAPIVersion(blueprintControlPlane.Template, cluster.Spec.ControlPlaneRef)
104+
if err != nil {
105+
return nil, errors.Wrapf(err, "failed to read %s", tlog.KRef{Ref: cluster.Spec.ControlPlaneRef})
106+
}
107+
res.Object, err = r.getReference(ctx, ref)
98108
if err != nil {
99109
return nil, errors.Wrapf(err, "failed to read %s", tlog.KRef{Ref: cluster.Spec.ControlPlaneRef})
100110
}
@@ -106,7 +116,7 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, cluster *c
106116
}
107117

108118
// If the clusterClass does not mandate the controlPlane has infrastructureMachines, return.
109-
if !blueprint.HasControlPlaneInfrastructureMachine() {
119+
if !blueprintHasControlPlaneInfrastructureMachine {
110120
return res, nil
111121
}
112122

@@ -115,7 +125,11 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, cluster *c
115125
if err != nil {
116126
return res, errors.Wrapf(err, "failed to get InfrastructureMachineTemplate reference for %s", tlog.KObj{Obj: res.Object})
117127
}
118-
res.InfrastructureMachineTemplate, err = r.getReference(ctx, machineInfrastructureRef)
128+
ref, err = alignRefAPIVersion(blueprintControlPlane.InfrastructureMachineTemplate, machineInfrastructureRef)
129+
if err != nil {
130+
return nil, errors.Wrapf(err, "failed to get InfrastructureMachineTemplate for %s", tlog.KObj{Obj: res.Object})
131+
}
132+
res.InfrastructureMachineTemplate, err = r.getReference(ctx, ref)
119133
if err != nil {
120134
return nil, errors.Wrapf(err, "failed to get InfrastructureMachineTemplate for %s", tlog.KObj{Obj: res.Object})
121135
}
@@ -143,7 +157,7 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, cluster *c
143157
// whether they are managed by a ClusterClass using labels. A Cluster may have zero or more MachineDeployments. Zero is
144158
// expected on first reconcile. If MachineDeployments are found for the Cluster their Infrastructure and Bootstrap references
145159
// are inspected. Where these are not found the function will throw an error.
146-
func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, cluster *clusterv1.Cluster) (map[string]*scope.MachineDeploymentState, error) {
160+
func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, blueprintMachineDeployments map[string]*scope.MachineDeploymentBlueprint, cluster *clusterv1.Cluster) (map[string]*scope.MachineDeploymentState, error) {
147161
state := make(scope.MachineDeploymentsStateMap)
148162

149163
// List all the machine deployments in the current cluster and in a managed topology.
@@ -178,12 +192,26 @@ func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, clust
178192
return nil, fmt.Errorf("duplicate %s found for label %s: %s", tlog.KObj{Obj: m}, clusterv1.ClusterTopologyMachineDeploymentLabelName, mdTopologyName)
179193
}
180194

195+
mdClassName := getMDClassName(cluster, mdTopologyName)
196+
if mdClassName == "" {
197+
return nil, fmt.Errorf("failed to find MachineDeployment topology %s in %s", mdTopologyName, tlog.KObj{Obj: cluster})
198+
}
199+
200+
mdBluePrint, ok := blueprintMachineDeployments[mdClassName]
201+
if !ok {
202+
return nil, fmt.Errorf("failed to find MachineDeployment class %s in ClusterClass", mdClassName)
203+
}
204+
181205
// Gets the BootstrapTemplate
182206
bootstrapRef := m.Spec.Template.Spec.Bootstrap.ConfigRef
183207
if bootstrapRef == nil {
184208
return nil, fmt.Errorf("%s does not have a reference to a Bootstrap Config", tlog.KObj{Obj: m})
185209
}
186-
b, err := r.getReference(ctx, bootstrapRef)
210+
ref, err := alignRefAPIVersion(mdBluePrint.BootstrapTemplate, bootstrapRef)
211+
if err != nil {
212+
return nil, errors.Wrap(err, fmt.Sprintf("%s Bootstrap reference could not be retrieved", tlog.KObj{Obj: m}))
213+
}
214+
b, err := r.getReference(ctx, ref)
187215
if err != nil {
188216
return nil, errors.Wrap(err, fmt.Sprintf("%s Bootstrap reference could not be retrieved", tlog.KObj{Obj: m}))
189217
}
@@ -199,7 +227,11 @@ func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, clust
199227
if infraRef.Name == "" {
200228
return nil, fmt.Errorf("%s does not have a reference to a InfrastructureMachineTemplate", tlog.KObj{Obj: m})
201229
}
202-
infra, err := r.getReference(ctx, &infraRef)
230+
ref, err = alignRefAPIVersion(mdBluePrint.InfrastructureMachineTemplate, &infraRef)
231+
if err != nil {
232+
return nil, errors.Wrap(err, fmt.Sprintf("%s Infrastructure reference could not be retrieved", tlog.KObj{Obj: m}))
233+
}
234+
infra, err := r.getReference(ctx, ref)
203235
if err != nil {
204236
return nil, errors.Wrap(err, fmt.Sprintf("%s Infrastructure reference could not be retrieved", tlog.KObj{Obj: m}))
205237
}
@@ -232,3 +264,44 @@ func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, clust
232264
}
233265
return state, nil
234266
}
267+
268+
// alignRefAPIVersion returns an aligned copy of the currentRef so it matches the apiVersion in ClusterClass.
269+
// This is required so the topology controller can diff current and desired state objects of the same
270+
// version during reconcile.
271+
// If group or kind was changed in the ClusterClass, an exact copy of the currentRef is returned because
272+
// it will end up in a diff and a rollout anyway.
273+
// Only bootstrap template refs in a ClusterClass can change their group and kind.
274+
func alignRefAPIVersion(templateFromClusterClass *unstructured.Unstructured, currentRef *corev1.ObjectReference) (*corev1.ObjectReference, error) {
275+
currentGV, err := schema.ParseGroupVersion(currentRef.APIVersion)
276+
if err != nil {
277+
return nil, errors.Wrapf(err, "failed to parse apiVersion: %q", currentRef.APIVersion)
278+
}
279+
280+
apiVersion := currentRef.APIVersion
281+
// Use apiVersion from ClusterClass if group and kind is the same.
282+
if templateFromClusterClass.GroupVersionKind().Group == currentGV.Group &&
283+
templateFromClusterClass.GetKind() == currentRef.Kind {
284+
apiVersion = templateFromClusterClass.GetAPIVersion()
285+
}
286+
287+
return &corev1.ObjectReference{
288+
APIVersion: apiVersion,
289+
Kind: currentRef.Kind,
290+
Namespace: currentRef.Namespace,
291+
Name: currentRef.Name,
292+
}, nil
293+
}
294+
295+
// getMDClassName retrieves the MDClass name by looking up the MDTopology in the Cluster.
296+
func getMDClassName(cluster *clusterv1.Cluster, mdTopologyName string) string {
297+
if cluster.Spec.Topology.Workers == nil {
298+
return ""
299+
}
300+
301+
for _, mdTopology := range cluster.Spec.Topology.Workers.MachineDeployments {
302+
if mdTopology.Name == mdTopologyName {
303+
return mdTopology.Class
304+
}
305+
}
306+
return ""
307+
}

0 commit comments

Comments
 (0)