✨ Compute ControlPlane version for a managed topology

controllers/topology/desired_state.go

@@ -19,13 +19,15 @@ package topology
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apiserver/pkg/storage/names"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
 	"sigs.k8s.io/cluster-api/controllers/external"
+	"sigs.k8s.io/cluster-api/util/version"
 )
 
 // computeDesiredState computes the desired state of the cluster topology.
@@ -136,15 +138,86 @@ func computeControlPlane(class *clusterTopologyClass, current *clusterTopologySt
 		}
 	}
 
+	// Computes the desired Kubernetes version for the control plane.
+	controlPlaneVersion, err := computeControlPlaneVersion(current)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to compute spec.version for the ControlPlane object")
+	}
+
 	// Sets the desired Kubernetes version for the control plane.
-	// TODO: improve this logic by adding support for version upgrade component by component
-	if err := unstructured.SetNestedField(controlPlane.UnstructuredContent(), current.cluster.Spec.Topology.Version, "spec", "version"); err != nil {
+	if err := unstructured.SetNestedField(controlPlane.UnstructuredContent(), controlPlaneVersion, "spec", "version"); err != nil {
 		return nil, errors.Wrap(err, "failed to set spec.version in the ControlPlane object")
 	}
 
 	return controlPlane, nil
 }
 
+// computeControlPlaneVersion computes the ControlPlane version based on the current clusterTopologyState.
+// TODO: we also have to handle the following cases:
+// * ControlPlane.spec.version != MachineDeployment[].spec.template.spec.version, i.e. one of the MachineDeployments has
+//   a different version then the ControlPlane.
+func computeControlPlaneVersion(current *clusterTopologyState) (string, error) {
+	topologyVersion := current.cluster.Spec.Topology.Version
+
+	// ControlPlane does not exist yet, use topologyVersion for new ControlPlane.
+	if current.controlPlane.object == nil {
+		return topologyVersion, nil
+	}
+
+	controlPlaneVersion, ok, err := getControlPlaneVersion(current.controlPlane.object, "spec", "version")
+	if err != nil {
+		return "", err
+	}
+	if !ok {
+		return "", errors.Errorf("failed to get .spec.version from ControlPlane: not found")
+	}
+	controlPlaneStatusVersion, statusVersionSet, err := getControlPlaneVersion(current.controlPlane.object, "status", "version")
+	if err != nil {
+		return "", err
+	}
+
+	// Do not trigger another rollout, while a rollout is already in progress. A rollout is still in progress if:
+	// * .status.version is not set
+	// * .status.version is set but not equal to .spec.version
+	if !statusVersionSet || controlPlaneVersion != controlPlaneStatusVersion {
+		return controlPlaneVersion, nil
+	}
+
+	// ControlPlane downgrade is not allowed.
+	if err := isDowngrade(controlPlaneVersion, topologyVersion); err != nil {
+		return "", err
+	}
+
+	// Either no-op or ControlPlane will be upgraded.
+	return topologyVersion, nil
+}
+
+// getControlPlaneVersion gets the version from the specified path of a ControlPlane.
+func getControlPlaneVersion(controlPlane *unstructured.Unstructured, path ...string) (string, bool, error) {
+	version, ok, err := unstructured.NestedString(controlPlane.UnstructuredContent(), path...)
+	if err != nil {
+		return "", ok, errors.Wrapf(err, "failed to get %s from ControlPlane", strings.Join(path, "."))
+	}
+	return version, ok, nil
+}
+
+// isDowngrade compares currentVersion and desiredVersion and returns an error if they either cannot be parsed
+// or if a downgrade is detected.
+func isDowngrade(currentVersion, desiredVersion string) error {
+	currentVersionParsed, err := version.ParseMajorMinorPatch(currentVersion)
+	if err != nil {
+		return errors.Errorf("failed to parse current version %q: %v", currentVersion, err)
+	}
+	desiredVersionParsed, err := version.ParseMajorMinorPatch(desiredVersion)
+	if err != nil {
+		return errors.Errorf("failed to parse desired version %q: %v", desiredVersion, err)
+	}
+	if desiredVersionParsed.LT(currentVersionParsed) {
+		return errors.Errorf("downgrading from %s to %s is not supported", currentVersion, desiredVersion)
+	}
+	return nil
+}
+
 // computeCluster computes the desired state for the Cluster object.
 // NOTE: Some fields of the Cluster’s fields contribute to defining how a Cluster should look like (e.g. Cluster.Spec.Topology),
 // while some other fields should be managed as part of the actual Cluster (e.g. Cluster.Spec.ControlPlaneRef); in this func

controllers/topology/desired_state_test.go

@@ -381,6 +381,118 @@ func TestComputeControlPlane(t *testing.T) {
 	})
 }
 
+func TestComputeControlPlaneVersion(t *testing.T) {
+	tests := []struct {
+		name                string
+		topologyVersion     string
+		currentControlPlane *unstructured.Unstructured
+		want                string
+		wantErr             bool
+	}{
+		{
+			name:                "ControlPlane does not exist",
+			topologyVersion:     "v1.21.0",
+			currentControlPlane: nil,
+			want:                "v1.21.0",
+		},
+		{
+			name:                "ControlPlane does exist, but without version",
+			topologyVersion:     "v1.21.0",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").Obj(),
+			wantErr:             true,
+		},
+		{
+			name:            "ControlPlane does exist, with same version (no-op)",
+			topologyVersion: "v1.21.0",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").
+				WithVersion("v1.21.0").
+				WithStatusVersion("v1.21.0").
+				Obj(),
+			want: "v1.21.0",
+		},
+		{
+			name:            "ControlPlane does exist, with newer version (downgrade)",
+			topologyVersion: "v1.21.0",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").
+				WithVersion("v1.22.0").
+				WithStatusVersion("v1.22.0").
+				Obj(),
+			wantErr: true,
+		},
+		{
+			name:            "ControlPlane does exist, with invalid version",
+			topologyVersion: "v1.21.0",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").
+				WithVersion("invalid-version").
+				WithStatusVersion("invalid-version").
+				Obj(),
+			wantErr: true,
+		},
+		{
+			name:            "ControlPlane does exist, with valid version but invalid topology version",
+			topologyVersion: "invalid-version",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").
+				WithVersion("v1.21.0").
+				WithStatusVersion("v1.21.0").
+				Obj(),
+			wantErr: true,
+		},
+		{
+			name:            "ControlPlane does exist, with older version (upgrade)",
+			topologyVersion: "v1.21.0",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").
+				WithVersion("v1.20.0").
+				WithStatusVersion("v1.20.0").
+				Obj(),
+			want: "v1.21.0",
+		},
+		{
+			name:            "ControlPlane does exist, with older version (upgrade) but another upgrade is already in progress (.status.version has older version)",
+			topologyVersion: "v1.22.0",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").
+				WithVersion("v1.21.0").
+				WithStatusVersion("v1.20.0").
+				Obj(),
+			want: "v1.21.0",
+		},
+		{
+			name:            "ControlPlane does exist, with older version (upgrade) but another upgrade is already in progress (.status.version is not set)",
+			topologyVersion: "v1.22.0",
+			currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").
+				WithVersion("v1.21.0").
+				Obj(),
+			want: "v1.21.0",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			current := &clusterTopologyState{
+				cluster: &clusterv1.Cluster{
+					Spec: clusterv1.ClusterSpec{
+						Topology: &clusterv1.Topology{
+							Version: tt.topologyVersion,
+						},
+					},
+				},
+				controlPlane: controlPlaneTopologyState{
+					object: tt.currentControlPlane,
+				},
+			}
+
+			got, err := computeControlPlaneVersion(current)
+			if tt.wantErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+			}
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
 func TestComputeCluster(t *testing.T) {
 	// generated objects
 	infrastructureCluster := newFakeInfrastructureCluster(metav1.NamespaceDefault, "infrastructureCluster1").Obj()

controllers/topology/object_builders_test.go

@@ -395,6 +395,8 @@ type fakeControlPlane struct {
 	namespace                     string
 	name                          string
 	infrastructureMachineTemplate *unstructured.Unstructured
+	version                       *string
+	statusVersion                 *string
 }
 
 func newFakeControlPlane(namespace, name string) *fakeControlPlane {
@@ -409,6 +411,16 @@ func (f *fakeControlPlane) WithInfrastructureMachineTemplate(t *unstructured.Uns
 	return f
 }
 
+func (f *fakeControlPlane) WithVersion(version string) *fakeControlPlane {
+	f.version = &version
+	return f
+}
+
+func (f *fakeControlPlane) WithStatusVersion(version string) *fakeControlPlane {
+	f.statusVersion = &version
+	return f
+}
+
 func (f *fakeControlPlane) Obj() *unstructured.Unstructured {
 	obj := &unstructured.Unstructured{}
 	obj.SetAPIVersion(fakeControlPlaneProviderGroupVersion.String())
@@ -428,5 +440,16 @@ func (f *fakeControlPlane) Obj() *unstructured.Unstructured {
 			panic(err)
 		}
 	}
+
+	if f.version != nil {
+		if err := unstructured.SetNestedField(obj.UnstructuredContent(), *f.version, "spec", "version"); err != nil {
+			panic(err)
+		}
+	}
+	if f.statusVersion != nil {
+		if err := unstructured.SetNestedField(obj.UnstructuredContent(), *f.statusVersion, "status", "version"); err != nil {
+			panic(err)
+		}
+	}
 	return obj
 }