Fix TKG OVA template upgrade mechanism in CSE

.changes/v2.24.0/663-bug-fixes.md

@@ -0,0 +1,5 @@
+* Fixed an issue that prevented CSE Kubernetes clusters from being upgraded to an OVA with higher Kubernetes version but same TKG version,
+  and to an OVA with a higher patch version of Kubernetes [GH-663] 
+* Fixed an issue that prevented CSE Kubernetes clusters from being upgraded to TKG v2.5.0 with Kubernetes v1.26.11 as it
+  performed an invalid upgrade of CoreDNS [GH-663] 
+* Fixed an issue that prevented reading the SSH Public Key from provisioned CSE Kubernetes clusters [GH-663] 

govcd/cse.go

@@ -233,8 +233,8 @@ func (cluster *CseKubernetesCluster) GetSupportedUpgrades(refreshOvas bool) ([]*
 		if err != nil {
 			continue // This means it's not a TKGm OVA, or it is not supported, so we skip it
 		}
-		// The OVA can be used if the TKG version is higher than the actual and the Kubernetes version is at most 1 minor higher.
-		if targetVersions.compareTkgVersion(cluster.TkgVersion.String()) == 1 && targetVersions.kubernetesVersionIsOneMinorHigher(cluster.KubernetesVersion.String()) {
+		// The OVA can be used if the TKG version is equal to the actual or higher, and the Kubernetes version is at most 1 minor higher.
+		if targetVersions.compareTkgVersion(cluster.TkgVersion.String()) >= 0 && targetVersions.kubernetesVersionIsUpgradeableFrom(cluster.KubernetesVersion.String()) {
 			cluster.supportedUpgrades = append(cluster.supportedUpgrades, vAppTemplate.VAppTemplate)
 		}
 	}

govcd/cse/tkg_versions.json

@@ -27,7 +27,7 @@
     "tkg": "v2.4.0",
     "tkr": "v1.26.8---vmware.1-tkg.1",
     "etcd": "v3.5.6_vmware.20",
-    "coreDns": "v1.10.1_vmware.7"
+    "coreDns": "v1.9.3_vmware.16"
   },
   "v1.26.8+vmware.1-tkg.1-0edd4dafbefbdb503f64d5472e500cf8": {
     "tkg": "v2.3.1",
@@ -39,7 +39,7 @@
     "tkg": "v2.4.0",
     "tkr": "v1.25.13---vmware.1-tkg.1",
     "etcd": "v3.5.6_vmware.20",
-    "coreDns": "v1.10.1_vmware.7"
+    "coreDns": "v1.9.3_vmware.16"
   },
   "v1.25.13+vmware.1-tkg.1-6f7650434fd3787d751e8fb3c9e2153d": {
     "tkg": "v2.3.1",

govcd/cse_test.go

@@ -79,6 +79,7 @@ func (vcd *TestVCD) Test_Cse(check *C) {
 	check.Assert(err, IsNil)
 	check.Assert(cseVersion, NotNil)
 
+	sshPublicKey := "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrCI+QkLjgQVqR7c7dJfawJqCslVomo5I25JdolqlteX7RCUq0yncWyS+8MTYWCS03sm1jOroLOeuji8CDKCDCcKwQerJiOFoJS+VOK5xCjJ2u8RBGlIpXNcmIh2VriRJrV7TCKrFMSKLNF4/n83q4gWI/YPf6/dRhpPB72HYrdI4omvRlU4GG09jMmgiz+5Yb8wJEXYMsJni+MwPzFKe6TbMcqjBusDyeFGAhgyN7QJGpdNhAn1sqvqZrW2QjaE8P+4t8RzBo8B2ucyQazd6+lbYmOHq9366LjG160snzXrFzlARc4hhpjMzu9Bcm6i3ZZI70qhIbmi5IonbbVh8t"
 	// Create the cluster
 	clusterSettings := CseClusterSettings{
 		Name:                    "test-cse",
@@ -112,6 +113,7 @@ func (vcd *TestVCD) Test_Cse(check *C) {
 		NodeHealthCheck:    true,
 		PodCidr:            "100.96.0.0/11",
 		ServiceCidr:        "100.64.0.0/13",
+		SshPublicKey:       sshPublicKey,
 		AutoRepairOnErrors: true,
 	}
 	cluster, err := org.CseCreateKubernetesCluster(clusterSettings, 150*time.Minute)
@@ -409,6 +411,7 @@ func assertCseClusterCreation(check *C, createdCluster *CseKubernetesCluster, se
 	check.Assert(createdCluster.ServiceCidr, Equals, settings.ServiceCidr)
 	check.Assert(createdCluster.SshPublicKey, Equals, settings.SshPublicKey)
 	check.Assert(createdCluster.VirtualIpSubnet, Equals, settings.VirtualIpSubnet)
+	check.Assert(createdCluster.SshPublicKey, Equals, settings.SshPublicKey)
 
 	v411, err := semver.NewVersion("4.1.1")
 	check.Assert(err, IsNil)

govcd/cse_util.go

@@ -357,9 +357,9 @@ func cseConvertToCseKubernetesClusterType(rde *DefinedEntity) (*CseKubernetesClu
 			if len(users) == 0 {
 				return nil, fmt.Errorf("expected 'spec.kubeadmConfigSpec.users' slice to not to be empty")
 			}
-			keys := traverseMapAndGet[[]string](users[0], "sshAuthorizedKeys")
+			keys := traverseMapAndGet[[]interface{}](users[0], "sshAuthorizedKeys")
 			if len(keys) > 0 {
-				result.SshPublicKey = keys[0] // Optional field
+				result.SshPublicKey = keys[0].(string) // Optional field
 			}
 
 			version, err := semver.NewVersion(traverseMapAndGet[string](yamlDocument, "spec.version"))
@@ -832,35 +832,47 @@ func (tkgVersions tkgVersionBundle) compareTkgVersion(tkgVersion string) int {
 	return receiverVersion.Compare(inputVersion)
 }
 
-// kubernetesVersionIsOneMinorHigher returns true only if the receiver Kubernetes version is exactly one minor version higher
-// than the given input version, being the minor digit the 'Y' in 'X.Y.Z'.
+// kubernetesVersionIsUpgradeableFrom returns true either if the receiver Kubernetes version is exactly one minor version higher
+// than the given input version (being the minor digit the 'Y' in 'X.Y.Z') or if the minor is the same, but the patch is higher
+// (being the minor digit the 'Z' in 'X.Y.Z').
 // Any malformed version returns false.
 // Examples:
-// * "1.19.2".kubernetesVersionIsOneMinorHigher("1.18.7") = true
-// * "1.19.10".kubernetesVersionIsOneMinorHigher("1.18.0") = true
-// * "1.20.2".kubernetesVersionIsOneMinorHigher("1.18.7") = false
-// * "1.21.2".kubernetesVersionIsOneMinorHigher("1.18.7") = false
-// * "1.18.0".kubernetesVersionIsOneMinorHigher("1.18.7") = false
-func (tkgVersions tkgVersionBundle) kubernetesVersionIsOneMinorHigher(kubernetesVersion string) bool {
-	receiverVersion, err := semver.NewVersion(tkgVersions.KubernetesVersion)
+// * "1.19.2".kubernetesVersionIsUpgradeableFrom("1.18.7") = true
+// * "1.19.2".kubernetesVersionIsUpgradeableFrom("1.19.2") = false
+// * "1.19.2".kubernetesVersionIsUpgradeableFrom("1.19.0") = true
+// * "1.19.10".kubernetesVersionIsUpgradeableFrom("1.18.0") = true
+// * "1.20.2".kubernetesVersionIsUpgradeableFrom("1.18.7") = false
+// * "1.21.2".kubernetesVersionIsUpgradeableFrom("1.18.7") = false
+// * "1.18.0".kubernetesVersionIsUpgradeableFrom("1.18.7") = false
+func (tkgVersions tkgVersionBundle) kubernetesVersionIsUpgradeableFrom(kubernetesVersion string) bool {
+	upgradeToVersion, err := semver.NewVersion(tkgVersions.KubernetesVersion)
 	if err != nil {
 		return false
 	}
-	inputVersion, err := semver.NewVersion(kubernetesVersion)
+	fromVersion, err := semver.NewVersion(kubernetesVersion)
 	if err != nil {
 		return false
 	}
 
-	receiverVersionSegments := receiverVersion.Segments()
-	if len(receiverVersionSegments) < 2 {
+	if upgradeToVersion.Equal(fromVersion) {
 		return false
 	}
-	inputSegments := inputVersion.Segments()
-	if len(inputSegments) < 2 {
+
+	upgradeToVersionSegments := upgradeToVersion.Segments()
+	if len(upgradeToVersionSegments) < 2 {
+		return false
+	}
+	fromVersionSegments := fromVersion.Segments()
+	if len(fromVersionSegments) < 2 {
 		return false
 	}
 
-	return receiverVersionSegments[0] == inputSegments[0] && receiverVersionSegments[1]-1 == inputSegments[1]
+	majorIsEqual := upgradeToVersionSegments[0] == fromVersionSegments[0]
+	minorIsJustOneHigher := upgradeToVersionSegments[1]-1 == fromVersionSegments[1]
+	minorIsEqual := upgradeToVersionSegments[1] == fromVersionSegments[1]
+	patchIsHigher := upgradeToVersionSegments[2] > fromVersionSegments[2]
+
+	return majorIsEqual && (minorIsJustOneHigher || (minorIsEqual && patchIsHigher))
 }
 
 // getVcdKeConfig gets the required information from the CSE Server configuration RDE (VCDKEConfig), such as the

govcd/cse_util_unit_test.go

@@ -235,63 +235,75 @@ func Test_tkgVersionBundle_compareTkgVersion(t *testing.T) {
 				TkgVersion: tt.receiverTkgVersion,
 			}
 			if got := tkgVersions.compareTkgVersion(tt.comparedTkgVersion); got != tt.want {
-				t.Errorf("kubernetesVersionIsOneMinorHigher() = %v, want %v", got, tt.want)
+				t.Errorf("compareTkgVersion() = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
 
-func Test_tkgVersionBundle_kubernetesVersionIsOneMinorHigher(t *testing.T) {
+func Test_tkgVersionBundle_kubernetesVersionIsUpgradeableFrom(t *testing.T) {
 	tests := []struct {
-		name                      string
-		receiverKubernetesVersion string
-		comparedKubernetesVersion string
-		want                      bool
+		name             string
+		upgradeToVersion string
+		fromVersion      string
+		want             bool
 	}{
 		{
-			name:                      "same Kubernetes versions",
-			receiverKubernetesVersion: "1.20.2+vmware.1",
-			comparedKubernetesVersion: "1.20.2+vmware.1",
-			want:                      false,
+			name:             "same Kubernetes versions",
+			upgradeToVersion: "1.20.2+vmware.1",
+			fromVersion:      "1.20.2+vmware.1",
+			want:             false,
 		},
 		{
-			name:                      "one Kubernetes minor higher",
-			receiverKubernetesVersion: "1.21.9+vmware.1",
-			comparedKubernetesVersion: "1.20.2+vmware.1",
-			want:                      true,
+			name:             "the Kubernetes patch is higher",
+			upgradeToVersion: "1.21.9+vmware.1",
+			fromVersion:      "1.21.7+vmware.1",
+			want:             true,
 		},
 		{
-			name:                      "one Kubernetes minor lower",
-			receiverKubernetesVersion: "1.19.9+vmware.1",
-			comparedKubernetesVersion: "1.20.2+vmware.1",
-			want:                      false,
+			name:             "one Kubernetes minor higher",
+			upgradeToVersion: "1.21.9+vmware.1",
+			fromVersion:      "1.20.2+vmware.1",
+			want:             true,
 		},
 		{
-			name:                      "several Kubernetes minors higher",
-			receiverKubernetesVersion: "1.22.9+vmware.1",
-			comparedKubernetesVersion: "1.20.2+vmware.1",
-			want:                      false,
+			name:             "the Kubernetes patch is lower",
+			upgradeToVersion: "1.20.0+vmware.1",
+			fromVersion:      "1.20.7+vmware.1",
+			want:             false,
 		},
 		{
-			name:                      "wrong receiver Kubernetes version",
-			receiverKubernetesVersion: "foo",
-			comparedKubernetesVersion: "1.20.2+vmware.1",
-			want:                      false,
+			name:             "one Kubernetes minor lower",
+			upgradeToVersion: "1.19.9+vmware.1",
+			fromVersion:      "1.20.2+vmware.1",
+			want:             false,
 		},
 		{
-			name:                      "wrong compared Kubernetes version",
-			receiverKubernetesVersion: "1.20.2+vmware.1",
-			comparedKubernetesVersion: "foo",
-			want:                      false,
+			name:             "several Kubernetes minors higher",
+			upgradeToVersion: "1.22.9+vmware.1",
+			fromVersion:      "1.20.2+vmware.1",
+			want:             false,
+		},
+		{
+			name:             "wrong receiver Kubernetes version",
+			upgradeToVersion: "foo",
+			fromVersion:      "1.20.2+vmware.1",
+			want:             false,
+		},
+		{
+			name:             "wrong compared Kubernetes version",
+			upgradeToVersion: "1.20.2+vmware.1",
+			fromVersion:      "foo",
+			want:             false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			tkgVersions := tkgVersionBundle{
-				KubernetesVersion: tt.receiverKubernetesVersion,
+				KubernetesVersion: tt.upgradeToVersion,
 			}
-			if got := tkgVersions.kubernetesVersionIsOneMinorHigher(tt.comparedKubernetesVersion); got != tt.want {
-				t.Errorf("kubernetesVersionIsOneMinorHigher() = %v, want %v", got, tt.want)
+			if got := tkgVersions.kubernetesVersionIsUpgradeableFrom(tt.fromVersion); got != tt.want {
+				t.Errorf("kubernetesVersionIsUpgradeableFrom() = %v, want %v", got, tt.want)
 			}
 		})
 	}

govcd/cse_yaml.go

@@ -74,7 +74,7 @@ func (cluster *CseKubernetesCluster) updateCapiYaml(input CseClusterUpdateInput)
 		if err != nil {
 			return cluster.capvcdType.Spec.CapiYaml, fmt.Errorf("could not retrieve the TKG versions of OVA '%s': %s", *input.KubernetesTemplateOvaId, err)
 		}
-		if versions.compareTkgVersion(cluster.capvcdType.Status.Capvcd.Upgrade.Current.TkgVersion) != 1 || !versions.kubernetesVersionIsOneMinorHigher(cluster.capvcdType.Status.Capvcd.Upgrade.Current.KubernetesVersion) {
+		if versions.compareTkgVersion(cluster.capvcdType.Status.Capvcd.Upgrade.Current.TkgVersion) < 0 || !versions.kubernetesVersionIsUpgradeableFrom(cluster.capvcdType.Status.Capvcd.Upgrade.Current.KubernetesVersion) {
 			return cluster.capvcdType.Spec.CapiYaml, fmt.Errorf("cannot perform an OVA change as the new one '%s' has an older TKG/Kubernetes version (%s/%s)", vAppTemplate.VAppTemplate.Name, versions.TkgVersion, versions.KubernetesVersion)
 		}
 		err = cseUpdateKubernetesTemplateInYaml(yamlDocs, vAppTemplate.VAppTemplate)