From 5a8000dc408ada0e89432c05e29c1eab8d40bf4e Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Tue, 27 Jul 2021 20:05:03 +0200 Subject: [PATCH] Compute desired ControlPlane version for a managed topology MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- .../clustertopology_controller_compute.go | 71 +++++++++++++- ...clustertopology_controller_compute_test.go | 93 +++++++++++++++++++ 2 files changed, 162 insertions(+), 2 deletions(-) diff --git a/controllers/clustertopology_controller_compute.go b/controllers/clustertopology_controller_compute.go index 92bd74325a6b..14e2fbbdec61 100644 --- a/controllers/clustertopology_controller_compute.go +++ b/controllers/clustertopology_controller_compute.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/cluster-api/controllers/external" utilconversion "sigs.k8s.io/cluster-api/util/conversion" "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/version" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -274,15 +275,81 @@ 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 != ControlPlane.status.version, i.e. ControlPLane rollout is already in progress. +// * 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) { + currentTopologyVersion := current.cluster.Spec.Topology.Version + + // ControlPlane does not exist yet, use currentTopologyVersion for new ControlPlane. + if current.controlPlane.object == nil { + return currentTopologyVersion, nil + } + + currentControlPlaneVersion, err := getControlPlaneVersion(current.controlPlane.object) + if err != nil { + return "", err + } + + // ControlPlane already has the currentTopologyVersion. + if currentControlPlaneVersion == currentTopologyVersion { + return currentTopologyVersion, nil + } + + // ControlPlane downgrade is not allowed. + if err := detectDowngrade(currentControlPlaneVersion, currentTopologyVersion); err != nil { + return "", err + } + + // ControlPlane will be upgraded. + return currentTopologyVersion, nil +} + +// getControlPlaneVersion gets the .spec.version of a ControlPlane. +func getControlPlaneVersion(controlPlane *unstructured.Unstructured) (string, error) { + controlPlaneVersion, ok, err := unstructured.NestedString(controlPlane.UnstructuredContent(), "spec", "version") + if err != nil { + return "", errors.Wrap(err, "failed to get spec.version from ControlPlane") + } + if !ok { + return "", errors.New("failed to get spec.version from ControlPlane: not found") + } + return controlPlaneVersion, nil +} + +// detectDowngrade compares currentVersion and desiredVersion and returns an error if they either cannot be parsed +// or if a downgrade is detected. +func detectDowngrade(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("downgrade 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 diff --git a/controllers/clustertopology_controller_compute_test.go b/controllers/clustertopology_controller_compute_test.go index 3d0bd915363a..c33375a5571b 100644 --- a/controllers/clustertopology_controller_compute_test.go +++ b/controllers/clustertopology_controller_compute_test.go @@ -689,6 +689,87 @@ 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").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").Obj(), + wantErr: true, + }, + { + name: "ControlPlane does exist, with invalid version", + topologyVersion: "v1.21.0", + currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").WithVersion("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").Obj(), + wantErr: true, + }, + { + name: "ControlPlane does exist, with older version (upgrade)", + topologyVersion: "v1.21.0", + currentControlPlane: newFakeControlPlane(metav1.NamespaceDefault, "cp").WithVersion("v1.20.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() @@ -1385,6 +1466,7 @@ type fakeControlPlane struct { namespace string name string infrastructureMachineTemplate *unstructured.Unstructured + version *string } func newFakeControlPlane(namespace, name string) *fakeControlPlane { @@ -1399,6 +1481,11 @@ func (f *fakeControlPlane) WithInfrastructureMachineTemplate(t *unstructured.Uns return f } +func (f *fakeControlPlane) WithVersion(version string) *fakeControlPlane { + f.version = &version + return f +} + func (f *fakeControlPlane) Obj() *unstructured.Unstructured { obj := &unstructured.Unstructured{} obj.SetAPIVersion(fakeControlPlaneProviderGroupVersion.String()) @@ -1418,5 +1505,11 @@ 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) + } + } return obj }