Don't allow concurrent patch uppgrades

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

internal/webhooks/cluster.go

@@ -446,6 +446,11 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 }
 
 func (webhook *Cluster) validateTopologyVersion(ctx context.Context, fldPath *field.Path, fldValue string, inVersion, oldVersion semver.Version, oldCluster *clusterv1.Cluster) *field.Error {
+	// Nothing to do if the version doesn't change.
+	if version.Compare(inVersion, oldVersion, version.WithBuildTags()) == 0 {
+		return nil
+	}
+
 	// Version could only be increased.
 	if inVersion.NE(semver.Version{}) && oldVersion.NE(semver.Version{}) && version.Compare(inVersion, oldVersion, version.WithBuildTags()) == -1 {
 		return field.Invalid(
@@ -469,18 +474,6 @@ func (webhook *Cluster) validateTopologyVersion(ctx context.Context, fldPath *fi
 		)
 	}
 
-	// Only check the following cases if the minor version increases by 1 (we already return above for >= 2).
-	ceilVersion = semver.Version{
-		Major: oldVersion.Major,
-		Minor: oldVersion.Minor + 1,
-		Patch: 0,
-	}
-
-	// Return early if its not a minor version upgrade.
-	if !inVersion.GTE(ceilVersion) {
-		return nil
-	}
-
 	allErrs := []error{}
 	// minor version cannot be increased if control plane is upgrading or not yet on the current version
 	if err := validateTopologyControlPlaneVersion(ctx, webhook.Client, oldCluster, oldVersion); err != nil {
@@ -501,7 +494,7 @@ func (webhook *Cluster) validateTopologyVersion(ctx context.Context, fldPath *fi
 		return field.Invalid(
 			fldPath,
 			fldValue,
-			fmt.Sprintf("minor version update cannot happen at this time: %v", kerrors.NewAggregate(allErrs)),
+			fmt.Sprintf("version update cannot happen at this time: %v", kerrors.NewAggregate(allErrs)),
 		)
 	}
 

internal/webhooks/cluster_test.go

@@ -1833,6 +1833,7 @@ func TestClusterTopologyValidation(t *testing.T) {
 			name:      "should update",
 			expectErr: false,
 			old: builder.Cluster("fooboo", "cluster1").
+				WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()).
 				WithTopology(builder.ClusterTopology().
 					WithClass("foo").
 					WithVersion("v1.19.1").
@@ -1855,6 +1856,7 @@ func TestClusterTopologyValidation(t *testing.T) {
 					Build()).
 				Build(),
 			in: builder.Cluster("fooboo", "cluster1").
+				WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()).
 				WithTopology(builder.ClusterTopology().
 					WithClass("foo").
 					WithVersion("v1.19.2").
@@ -1876,6 +1878,21 @@ func TestClusterTopologyValidation(t *testing.T) {
 							Build()).
 					Build()).
 				Build(),
+			additionalObjects: []client.Object{
+				builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.19.1").
+					WithStatusFields(map[string]interface{}{"status.version": "v1.19.1"}).
+					Build(),
+				builder.MachineDeployment("fooboo", "cluster1-workers1").WithLabels(map[string]string{
+					clusterv1.ClusterNameLabel:                          "cluster1",
+					clusterv1.ClusterTopologyOwnedLabel:                 "",
+					clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1",
+				}).WithVersion("v1.19.1").Build(),
+				builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{
+					clusterv1.ClusterNameLabel:                    "cluster1",
+					clusterv1.ClusterTopologyOwnedLabel:           "",
+					clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1",
+				}).WithVersion("v1.19.1").Build(),
+			},
 		},
 		{
 			name:      "should return error when upgrade concurrency annotation value is < 1",