Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

p0upgradeK8sVersionChecks fix #233

Merged
merged 1 commit into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions hosted/eks/helper/helper_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,14 @@ func UpgradeClusterKubernetesVersion(cluster *management.Cluster, upgradeToVersi
}

// Check if the desired config has been applied in Rancher
Eventually(func() string {
ginkgo.GinkgoLogr.Info("Waiting for k8s upgrade to appear in EKSStatus.UpstreamSpec ...")
// Check if EKSConfig has correct KubernetesVersion after upgrade (Ref: eks-operator/issues/668)
Eventually(func() bool {
ginkgo.GinkgoLogr.Info("Waiting for k8s upgrade to appear in EKSStatus.UpstreamSpec & EKSConfig ...")
cluster, err = client.Management.Cluster.ByID(cluster.ID)
Expect(err).To(BeNil())
return *cluster.EKSStatus.UpstreamSpec.KubernetesVersion
}, tools.SetTimeout(15*time.Minute), 30*time.Second).Should(Equal(upgradeToVersion))
return *cluster.EKSStatus.UpstreamSpec.KubernetesVersion == upgradeToVersion && *cluster.EKSConfig.KubernetesVersion == upgradeToVersion
Copy link
Collaborator

@valaparthvi valaparthvi Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to check EKSConfig again, since we are already checking in on L80.

Copy link
Collaborator Author

@cpinjani cpinjani Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, however eks-operator/issues/668 (fixed already) occurs sometimes due to race condition.
This check ensures condition is met to start Nodegroup upgrade.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind adding a comment about it if you haven't already? It's possible that I or someone else might forget later and remove it.

}, tools.SetTimeout(15*time.Minute), 30*time.Second).Should(BeTrue())

// ensure nodegroup version is same in Rancher
for _, ng := range cluster.EKSStatus.UpstreamSpec.NodeGroups {
Expect(*ng.Version).To(Equal(currentVersion))
Expand All @@ -101,29 +103,34 @@ func UpgradeClusterKubernetesVersion(cluster *management.Cluster, upgradeToVersi
// UpgradeNodeKubernetesVersion upgrades the k8s version of nodegroup to the value defined by upgradeToVersion.
// if wait is set to true, it will wait until the cluster finishes upgrading;
// if checkClusterConfig is set to true, it will validate that nodegroup has been upgraded successfully
func UpgradeNodeKubernetesVersion(cluster *management.Cluster, upgradeToVersion string, client *rancher.Client, wait, checkClusterConfig bool) (*management.Cluster, error) {
upgradedCluster := cluster
for i := range upgradedCluster.EKSConfig.NodeGroups {
upgradedCluster.EKSConfig.NodeGroups[i].Version = &upgradeToVersion
}

// if useEksctl is set to true, nodegroup will be upgraded using eksctl utility instead of updating it from Rancher
func UpgradeNodeKubernetesVersion(cluster *management.Cluster, upgradeToVersion string, client *rancher.Client, wait, checkClusterConfig, useEksctl bool) (*management.Cluster, error) {
var err error
cluster, err = client.Management.Cluster.Update(cluster, &upgradedCluster)
Expect(err).To(BeNil())

// Check if the desired config is set correctly
for _, ng := range cluster.EKSConfig.NodeGroups {
Expect(*ng.Version).To(Equal(upgradeToVersion))
}
if !useEksctl {
upgradedCluster := cluster
for i := range upgradedCluster.EKSConfig.NodeGroups {
upgradedCluster.EKSConfig.NodeGroups[i].Version = &upgradeToVersion
}

if wait {
err = clusters.WaitClusterToBeUpgraded(client, cluster.ID)
cluster, err = client.Management.Cluster.Update(cluster, &upgradedCluster)
Expect(err).To(BeNil())

if wait {
err = clusters.WaitClusterToBeUpgraded(client, cluster.ID)
Expect(err).To(BeNil())
}
} else {
// Upgrade Nodegroup using eksctl due to custom Launch template
for _, ng := range cluster.EKSConfig.NodeGroups {
err = UpgradeEKSNodegroupOnAWS(helpers.GetEKSRegion(), cluster.EKSConfig.DisplayName, *ng.NodegroupName, upgradeToVersion)
Expect(err).To(BeNil())
}
}

if checkClusterConfig {
Eventually(func() bool {
// Check if the desired config has been applied in
// Check if the desired config has been applied
cluster, err = client.Management.Cluster.ByID(cluster.ID)
Expect(err).To(BeNil())
ginkgo.GinkgoLogr.Info("waiting for the nodegroup upgrade to appear in EKSStatus.UpstreamSpec ...")
Expand All @@ -135,6 +142,12 @@ func UpgradeNodeKubernetesVersion(cluster *management.Cluster, upgradeToVersion
return true
}, tools.SetTimeout(15*time.Minute), 30*time.Second).Should(BeTrue())
}

// Ensure nodegroup version is correct in Rancher after upgrade
for _, ng := range cluster.EKSConfig.NodeGroups {
Expect(*ng.Version).To(Equal(upgradeToVersion))
}

return cluster, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,15 @@ func commonchecks(ctx *helpers.Context, cluster *management.Cluster, clusterName

cluster, err = helper.UpgradeClusterKubernetesVersion(cluster, *latestVersion, ctx.RancherAdminClient, true)
Expect(err).To(BeNil())
// Does not upgrade noodegroup version since using custom LT, skip for imported cluster
if !helpers.IsImport {
By("upgrading the NodeGroups", func() {
cluster, err = helper.UpgradeNodeKubernetesVersion(cluster, *latestVersion, ctx.RancherAdminClient, true, true)
Expect(err).To(BeNil())
})

var useEksctl bool
if helpers.IsImport {
useEksctl = true
}
By("upgrading the NodeGroups", func() {
cluster, err = helper.UpgradeNodeKubernetesVersion(cluster, *latestVersion, ctx.RancherAdminClient, true, true, useEksctl)
Expect(err).To(BeNil())
})
})

var downgradeVersion string
Expand Down
13 changes: 7 additions & 6 deletions hosted/eks/p0/p0_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,14 @@ func p0upgradeK8sVersionChecks(cluster *management.Cluster, client *rancher.Clie
Expect(err).To(BeNil())
})

// Does not upgrades version since using custom LT, skip for imported cluster
if !helpers.IsImport {
By("upgrading the NodeGroups", func() {
cluster, err = helper.UpgradeNodeKubernetesVersion(cluster, upgradeToVersion, client, true, true)
Expect(err).To(BeNil())
})
var useEksctl bool
if helpers.IsImport {
useEksctl = true
}
By("upgrading the NodeGroups", func() {
cluster, err = helper.UpgradeNodeKubernetesVersion(cluster, upgradeToVersion, client, true, true, useEksctl)
Expect(err).To(BeNil())
})
}

func p0NodesChecks(cluster *management.Cluster, client *rancher.Client, clusterName string) {
Expand Down
2 changes: 1 addition & 1 deletion hosted/eks/p1/p1_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ func syncRancherToAWSCheck(cluster *management.Cluster, client *rancher.Client,
func upgradeNodeKubernetesVersionGTCPCheck(cluster *management.Cluster, client *rancher.Client, upgradeToVersion string) {
GinkgoLogr.Info("Upgrading only Nodegroup's EKS version to: " + upgradeToVersion)
var err error
cluster, err = helper.UpgradeNodeKubernetesVersion(cluster, upgradeToVersion, client, false, false)
cluster, err = helper.UpgradeNodeKubernetesVersion(cluster, upgradeToVersion, client, false, false, false)
Expect(err).To(BeNil())

// wait until the error is visible on the cluster
Expand Down
Loading