test: fix cluster version comparison (#1386)

The previous logic was not fully correct, for example it would consider 1.31.0 to be less than 1.30.2. This makes the logic a proper semver comparison, which should prevent breakage when newer cluster versions are tested in the future.
GoogleContainerTools · Aug 12, 2024 · 1e9ddcc · 1e9ddcc
1 parent 1a5a851
commit 1e9ddcc
Show file tree

Hide file tree

Showing 4 changed files with 174 additions and 13 deletions.
diff --git a/e2e/nomostest/clean.go b/e2e/nomostest/clean.go
@@ -213,7 +213,7 @@ func uninstallUnmanagedPackagesOfType(nt *NT, gvk schema.GroupVersionKind) error
 		testpredicates.Or(nt.Logger, conditions...),
 	}
 	// Delete the unmanaged packages in parallel
-	nt.T.Log("[CLEANUP] Deleting unmanaged %s objects...", gvk.Kind)
+	nt.T.Logf("[CLEANUP] Deleting unmanaged %s objects...", gvk.Kind)
 	tg := taskgroup.New()
 	for _, obj := range rsObjs {
 		nn := client.ObjectKeyFromObject(obj)

diff --git a/e2e/nomostest/clusterversion/cluster_version.go b/e2e/nomostest/clusterversion/cluster_version.go
@@ -53,6 +53,27 @@ func (cv ClusterVersion) GKESuffix() (int, error) {
 	return intVal, nil
 }
 
+// IsAtLeast returns whether the ClusterVersion is at least the provided version.
+func (cv ClusterVersion) IsAtLeast(other ClusterVersion) bool {
+	if cv.Major != other.Major {
+		return cv.Major > other.Major
+	}
+	if cv.Minor != other.Minor {
+		return cv.Minor > other.Minor
+	}
+	if cv.Patch != other.Patch {
+		return cv.Patch > other.Patch
+	}
+	// Compare -gke suffix, if they exist
+	myGKESuffix, _ := cv.GKESuffix()
+	otherGKESuffix, _ := other.GKESuffix()
+	if myGKESuffix != otherGKESuffix {
+		return myGKESuffix > otherGKESuffix
+	}
+	// Base case - Equal
+	return true
+}
+
 // ParseClusterVersion parses the string "gitVersion" of a Kubernetes cluster.
 func ParseClusterVersion(version string) (ClusterVersion, error) {
 	re := regexp.MustCompile(clusterVersionPattern)

diff --git a/e2e/nomostest/clusterversion/cluster_version_test.go b/e2e/nomostest/clusterversion/cluster_version_test.go
@@ -87,3 +87,150 @@ func TestParseClusterVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestIsAtLeast(t *testing.T) {
+	testcases := map[string]struct {
+		myClusterVersion    ClusterVersion
+		otherClusterVersion ClusterVersion
+		expectAtLeast       bool
+	}{
+		"less than minor version on earlier minor version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  25,
+				Patch:  11,
+				Suffix: "-gke.1062001",
+			},
+			otherClusterVersion: ClusterVersion{Major: 1, Minor: 26},
+			expectAtLeast:       false,
+		},
+		"greater than minor version on same minor version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  26,
+				Patch:  11,
+				Suffix: "-gke.1062001",
+			},
+			otherClusterVersion: ClusterVersion{Major: 1, Minor: 26},
+			expectAtLeast:       true,
+		},
+		"greater than minor version on later minor version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  27,
+				Patch:  11,
+				Suffix: "-gke.1062001",
+			},
+			otherClusterVersion: ClusterVersion{Major: 1, Minor: 26},
+			expectAtLeast:       true,
+		},
+		"greater than gke patch version on earlier minor version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  29,
+				Patch:  4,
+				Suffix: "-gke.1994000",
+			},
+			otherClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1394000",
+			},
+			expectAtLeast: false,
+		},
+		"less than gke patch version on same patch version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1004000",
+			},
+			otherClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1394000",
+			},
+			expectAtLeast: false,
+		},
+		"equal to gke patch version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1394000",
+			},
+			otherClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1394000",
+			},
+			expectAtLeast: true,
+		},
+		"greater than gke patch version on same patch version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1994000",
+			},
+			otherClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1394000",
+			},
+			expectAtLeast: true,
+		},
+		"greater than gke patch version on later minor version": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  31,
+				Patch:  0,
+				Suffix: "-gke.1004000",
+			},
+			otherClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  30,
+				Patch:  2,
+				Suffix: "-gke.1394000",
+			},
+			expectAtLeast: true,
+		},
+		"less than gke patch version on my unspecified GKE patch": {
+			myClusterVersion: ClusterVersion{
+				Major: 1,
+				Minor: 31,
+				Patch: 0,
+			},
+			otherClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  31,
+				Patch:  0,
+				Suffix: "-gke.1004000",
+			},
+		},
+		"greater than gke patch version on other unspecified GKE patch": {
+			myClusterVersion: ClusterVersion{
+				Major:  1,
+				Minor:  31,
+				Patch:  0,
+				Suffix: "-gke.1004000",
+			},
+			otherClusterVersion: ClusterVersion{
+				Major: 1,
+				Minor: 31,
+				Patch: 0,
+			},
+			expectAtLeast: true,
+		},
+	}
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			got := tc.myClusterVersion.IsAtLeast(tc.otherClusterVersion)
+			testutil.AssertEqual(t, tc.expectAtLeast, got)
+		})
+	}
+}
diff --git a/e2e/nomostest/nt.go b/e2e/nomostest/nt.go
@@ -907,18 +907,11 @@ func (nt *NT) autopilotClusterSupportsBursting() (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	if initialClusterVersion.Major < 1 || initialClusterVersion.Minor < 26 {
-		return false, nil
-	}
-	gkeSuffix, err := nt.ClusterVersion.GKESuffix()
-	if err != nil {
-		return false, err
-	}
-	if nt.ClusterVersion.Major < 1 || nt.ClusterVersion.Minor < 30 ||
-		nt.ClusterVersion.Patch < 2 || gkeSuffix < 1394000 {
-		return false, nil
-	}
-	return true, nil
+	minInitialVersion := clusterversion.ClusterVersion{Major: 1, Minor: 26}
+	minCurrentVersion := clusterversion.ClusterVersion{
+		Major: 1, Minor: 30, Patch: 2, Suffix: "-gke.1394000"}
+	return initialClusterVersion.IsAtLeast(minInitialVersion) &&
+		nt.ClusterVersion.IsAtLeast(minCurrentVersion), nil
 }
 
 func (nt *NT) detectClusterSupportsBursting() {