From 4418bd17b06ed19783341005e3344e85fbbcf89d Mon Sep 17 00:00:00 2001 From: michaelhtm <98621731+michaelhtm@users.noreply.github.com> Date: Tue, 30 Jun 2026 15:24:48 -0700 Subject: [PATCH 1/2] Add cluster version rollback support with rollbackConfig Allow rolling back (downgrading) an EKS cluster's Kubernetes version by one minor version via `eksctl upgrade cluster --version `. - ResolveUpgradeVersion now resolves both the next and previous supported versions, permits a single-version downgrade (N->N-1), and rejects multi-version downgrades and downgrading the oldest supported version, replacing the old "cannot upgrade to a lower version" error. - Add metadata.rollbackConfig.timeoutMinutes (api.RollbackConfig) to configure how long EKS waits before automatically cancelling an in-progress version update. Validated to the API's 120-10080 range and plumbed into UpdateClusterVersion. - Bump aws-sdk-go-v2/service/eks to v1.88.0, which ships RollbackConfig on UpdateClusterVersion (and the CancelUpdate operation); regenerate the awsapi EKS interface, EKS mock, deepcopy helpers, and schema. --- go.mod | 2 +- go.sum | 4 +- pkg/actions/cluster/upgrade.go | 15 +++- .../eksctl.io/v1alpha5/assets/schema.json | 21 ++++++ pkg/apis/eksctl.io/v1alpha5/types.go | 17 +++++ pkg/apis/eksctl.io/v1alpha5/validation.go | 23 ++++++ .../eksctl.io/v1alpha5/validation_test.go | 36 +++++++++ .../v1alpha5/zz_generated.deepcopy.go | 26 +++++++ pkg/awsapi/eks.go | 9 +++ pkg/eks/mocksv2/EKS.go | 74 +++++++++++++++++++ pkg/eks/update.go | 6 ++ pkg/eks/update_test.go | 58 +++++++++++++++ pkg/eks/versions_manager.go | 51 +++++++++---- pkg/eks/versions_manager_test.go | 20 +++-- 14 files changed, 334 insertions(+), 28 deletions(-) diff --git a/go.mod b/go.mod index a2a487da33..e1a29ccede 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs v1.73.0 github.com/aws/aws-sdk-go-v2/service/cognitoidentityprovider v1.60.2 github.com/aws/aws-sdk-go-v2/service/ec2 v1.302.0 - github.com/aws/aws-sdk-go-v2/service/eks v1.87.0 + github.com/aws/aws-sdk-go-v2/service/eks v1.88.0 github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing v1.33.25 github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.54.12 github.com/aws/aws-sdk-go-v2/service/iam v1.53.10 diff --git a/go.sum b/go.sum index 6ebfc2efab..cc3b1370f7 100644 --- a/go.sum +++ b/go.sum @@ -136,8 +136,8 @@ github.com/aws/aws-sdk-go-v2/service/cognitoidentityprovider v1.60.2 h1:pZxE29WA github.com/aws/aws-sdk-go-v2/service/cognitoidentityprovider v1.60.2/go.mod h1:nbe4Nf/HOY+e54Dl+yjv04scYTGTC+4ZthbfOuPTXQs= github.com/aws/aws-sdk-go-v2/service/ec2 v1.302.0 h1:7c0jQaj+QKYUo3pgtEm9fQIePJH6QJA3bVKIgCCLdvM= github.com/aws/aws-sdk-go-v2/service/ec2 v1.302.0/go.mod h1:Y95W0Hm6FYLPa6o0hbnJ+sWgmdc4ifcLFjGkdobWVhY= -github.com/aws/aws-sdk-go-v2/service/eks v1.87.0 h1:bftLltXNWmNr9ed3CaQnVlzNPTNTFdHguNhIsZF6DxM= -github.com/aws/aws-sdk-go-v2/service/eks v1.87.0/go.mod h1:rbIASs+SfCDUXx2EdfMkNpDGptlW8hvMZ9AawRiUBqE= +github.com/aws/aws-sdk-go-v2/service/eks v1.88.0 h1:dP/bd/5AG73UJGXyZh6gQSafVS9T+mBQgIULtJvzEnE= +github.com/aws/aws-sdk-go-v2/service/eks v1.88.0/go.mod h1:rbIASs+SfCDUXx2EdfMkNpDGptlW8hvMZ9AawRiUBqE= github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing v1.33.25 h1:VzmoYPRbNSUqk3pA04ZyGZUg52yfX259XXRqwr1lns4= github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing v1.33.25/go.mod h1:r7chQGimOmFs4oqawhO+i+o3ez2l69rzAco5KTb7bjY= github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.54.12 h1:TJXv7kZjdXA2maPDaJFFEQPBrPmvPtMybN3qYDOpJ4Y= diff --git a/pkg/actions/cluster/upgrade.go b/pkg/actions/cluster/upgrade.go index 01103efa48..e7c92d2cd7 100644 --- a/pkg/actions/cluster/upgrade.go +++ b/pkg/actions/cluster/upgrade.go @@ -10,6 +10,7 @@ import ( api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" "github.com/weaveworks/eksctl/pkg/eks" + "github.com/weaveworks/eksctl/pkg/utils" ) func upgrade(ctx context.Context, cfg *api.ClusterConfig, ctl *eks.ClusterProvider, dryRun bool) (bool, error) { @@ -31,14 +32,24 @@ func upgrade(ctx context.Context, cfg *api.ClusterConfig, ctl *eks.ClusterProvid } if upgradeVersion != "" { + currentVersion := ctl.ControlPlaneVersion() + // A resolved target lower than the current version is a rollback (downgrade); + // use direction-aware wording so the logs don't say "upgrade" for a downgrade. + action, pastAction := "upgrade", "upgraded" + if c, err := utils.CompareVersions(upgradeVersion, currentVersion); err != nil { + return false, err + } else if c < 0 { + action, pastAction = "roll back", "rolled back" + } + msgNodeGroupsAndAddons := "you will need to follow the upgrade procedure for all of nodegroups and add-ons" - cmdutils.LogIntendedAction(dryRun, "upgrade cluster %q control plane from current version %q to %q", cfg.Metadata.Name, ctl.ControlPlaneVersion(), upgradeVersion) + cmdutils.LogIntendedAction(dryRun, "%s cluster %q control plane from current version %q to %q", action, cfg.Metadata.Name, currentVersion, upgradeVersion) if !dryRun { cfg.Metadata.Version = upgradeVersion if err := ctl.UpdateClusterVersionBlocking(ctx, cfg); err != nil { return false, err } - logger.Success("cluster %q control plane has been upgraded to version %q", cfg.Metadata.Name, cfg.Metadata.Version) + logger.Success("cluster %q control plane has been %s to version %q", cfg.Metadata.Name, pastAction, cfg.Metadata.Version) logger.Info(msgNodeGroupsAndAddons) } } else { diff --git a/pkg/apis/eksctl.io/v1alpha5/assets/schema.json b/pkg/apis/eksctl.io/v1alpha5/assets/schema.json index df1142e1fa..b568e9d2a9 100755 --- a/pkg/apis/eksctl.io/v1alpha5/assets/schema.json +++ b/pkg/apis/eksctl.io/v1alpha5/assets/schema.json @@ -1072,6 +1072,11 @@ "description": "the AWS region hosting this cluster", "x-intellij-html-description": "the AWS region hosting this cluster" }, + "rollbackConfig": { + "$ref": "#/definitions/RollbackConfig", + "description": "configures the automatic rollback behaviour when updating the cluster version (including downgrades).", + "x-intellij-html-description": "configures the automatic rollback behaviour when updating the cluster version (including downgrades)." + }, "tags": { "additionalProperties": { "type": "string" @@ -1092,6 +1097,7 @@ "region", "version", "forceUpdateVersion", + "rollbackConfig", "tags", "annotations" ], @@ -3053,6 +3059,21 @@ ], "additionalProperties": false }, + "RollbackConfig": { + "properties": { + "timeoutMinutes": { + "type": "integer", + "description": "length of time in minutes to wait before cancelling the update. The timeout is a minimum-bound: cancellation occurs no sooner than the time specified, but can occur shortly thereafter. Valid values are between 120 (2 hours) and 10080 (7 days). When unset, EKS defaults to 720 (12 hours).", + "x-intellij-html-description": "length of time in minutes to wait before cancelling the update. The timeout is a minimum-bound: cancellation occurs no sooner than the time specified, but can occur shortly thereafter. Valid values are between 120 (2 hours) and 10080 (7 days). When unset, EKS defaults to 720 (12 hours)." + } + }, + "preferredOrder": [ + "timeoutMinutes" + ], + "additionalProperties": false, + "description": "holds the rollback configuration used when updating the cluster version. EKS waits for the configured timeout before automatically cancelling an in-progress cluster version update.", + "x-intellij-html-description": "holds the rollback configuration used when updating the cluster version. EKS waits for the configured timeout before automatically cancelling an in-progress cluster version update." + }, "SSOIdentity": { "required": [ "id", diff --git a/pkg/apis/eksctl.io/v1alpha5/types.go b/pkg/apis/eksctl.io/v1alpha5/types.go index 8cce87aa4e..f43ee0d948 100644 --- a/pkg/apis/eksctl.io/v1alpha5/types.go +++ b/pkg/apis/eksctl.io/v1alpha5/types.go @@ -708,6 +708,10 @@ type ClusterMeta struct { // When updating cluster version, provide the force flag to override upgrade-blocking insights // +optional ForceUpdateVersion *bool `json:"forceUpdateVersion,omitempty"` + // RollbackConfig configures the automatic rollback behaviour when updating + // the cluster version (including downgrades). + // +optional + RollbackConfig *RollbackConfig `json:"rollbackConfig,omitempty"` // Tags are used to tag AWS resources created by eksctl // +optional Tags map[string]string `json:"tags,omitempty"` @@ -727,6 +731,19 @@ type UpgradePolicy struct { SupportType string `json:"supportType,omitempty"` } +// RollbackConfig holds the rollback configuration used when updating the +// cluster version. EKS waits for the configured timeout before automatically +// cancelling an in-progress cluster version update. +type RollbackConfig struct { + // TimeoutMinutes is the length of time in minutes to wait before cancelling + // the update. The timeout is a minimum-bound: cancellation occurs no sooner + // than the time specified, but can occur shortly thereafter. Valid values + // are between 120 (2 hours) and 10080 (7 days). When unset, EKS defaults to + // 720 (12 hours). + // +optional + TimeoutMinutes *int `json:"timeoutMinutes,omitempty"` +} + // KubernetesNetworkConfig contains cluster networking options type KubernetesNetworkConfig struct { // Valid variants are `IPFamily` constants diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index f1e374f92e..0d989aae72 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation.go @@ -182,6 +182,23 @@ func validateSupportType(supportType string) error { return nil } +// RollbackTimeoutMinutesMin and RollbackTimeoutMinutesMax are the inclusive +// bounds EKS accepts for metadata.rollbackConfig.timeoutMinutes. +const ( + RollbackTimeoutMinutesMin = 120 + RollbackTimeoutMinutesMax = 10080 +) + +func validateRollbackConfig(rollbackConfig *RollbackConfig) error { + if rollbackConfig == nil || rollbackConfig.TimeoutMinutes == nil { + return nil + } + if timeout := *rollbackConfig.TimeoutMinutes; timeout < RollbackTimeoutMinutesMin || timeout > RollbackTimeoutMinutesMax { + return fmt.Errorf("metadata.rollbackConfig.timeoutMinutes must be between %d and %d, got %d", RollbackTimeoutMinutesMin, RollbackTimeoutMinutesMax, timeout) + } + return nil +} + // ValidateClusterConfig checks compatible fields of a given ClusterConfig func ValidateClusterConfig(cfg *ClusterConfig) error { if cfg.UpgradePolicy != nil { @@ -192,6 +209,12 @@ func ValidateClusterConfig(cfg *ClusterConfig) error { } } + if cfg.Metadata != nil { + if err := validateRollbackConfig(cfg.Metadata.RollbackConfig); err != nil { + return err + } + } + if IsDisabled(cfg.IAM.WithOIDC) && len(cfg.IAM.ServiceAccounts) > 0 { return fmt.Errorf("iam.withOIDC must be enabled explicitly for iam.serviceAccounts to be created") } diff --git a/pkg/apis/eksctl.io/v1alpha5/validation_test.go b/pkg/apis/eksctl.io/v1alpha5/validation_test.go index 40be54ade6..ce8a9501c7 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation_test.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation_test.go @@ -55,6 +55,42 @@ var _ = Describe("ClusterConfig validation", func() { }) }) + Describe("metadata.rollbackConfig validation", func() { + var cfg *api.ClusterConfig + + BeforeEach(func() { + cfg = api.NewClusterConfig() + }) + + It("accepts a missing rollbackConfig", func() { + Expect(api.ValidateClusterConfig(cfg)).To(Succeed()) + }) + + It("accepts a rollbackConfig without a timeout", func() { + cfg.Metadata.RollbackConfig = &api.RollbackConfig{} + Expect(api.ValidateClusterConfig(cfg)).To(Succeed()) + }) + + DescribeTable("timeoutMinutes bounds", + func(timeoutMinutes int, expectValid bool) { + cfg.Metadata.RollbackConfig = &api.RollbackConfig{ + TimeoutMinutes: aws.Int(timeoutMinutes), + } + err := api.ValidateClusterConfig(cfg) + if expectValid { + Expect(err).NotTo(HaveOccurred()) + } else { + Expect(err).To(MatchError(ContainSubstring("metadata.rollbackConfig.timeoutMinutes must be between 120 and 10080"))) + } + }, + Entry("rejects below the minimum", 119, false), + Entry("accepts the minimum", 120, true), + Entry("accepts a value within range", 720, true), + Entry("accepts the maximum", 10080, true), + Entry("rejects above the maximum", 10081, false), + ) + }) + Describe("nodeGroups[*].name validation", func() { var ( cfg *api.ClusterConfig diff --git a/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go b/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go index 81e57b8638..d375bff55a 100644 --- a/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go +++ b/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go @@ -985,6 +985,11 @@ func (in *ClusterMeta) DeepCopyInto(out *ClusterMeta) { *out = new(bool) **out = **in } + if in.RollbackConfig != nil { + in, out := &in.RollbackConfig, &out.RollbackConfig + *out = new(RollbackConfig) + (*in).DeepCopyInto(*out) + } if in.Tags != nil { in, out := &in.Tags, &out.Tags *out = make(map[string]string, len(*in)) @@ -2583,6 +2588,27 @@ func (in *RemoteNodesIAM) DeepCopy() *RemoteNodesIAM { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RollbackConfig) DeepCopyInto(out *RollbackConfig) { + *out = *in + if in.TimeoutMinutes != nil { + in, out := &in.TimeoutMinutes, &out.TimeoutMinutes + *out = new(int) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RollbackConfig. +func (in *RollbackConfig) DeepCopy() *RollbackConfig { + if in == nil { + return nil + } + out := new(RollbackConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SSOIdentity) DeepCopyInto(out *SSOIdentity) { *out = *in diff --git a/pkg/awsapi/eks.go b/pkg/awsapi/eks.go index 2274e354e7..7c2e9e6f28 100644 --- a/pkg/awsapi/eks.go +++ b/pkg/awsapi/eks.go @@ -39,6 +39,15 @@ type EKS interface { // // [Using RBAC Authorization]: https://kubernetes.io/docs/reference/access-authn-authz/rbac/ AssociateIdentityProviderConfig(ctx context.Context, params *eks.AssociateIdentityProviderConfigInput, optFns ...func(*Options)) (*eks.AssociateIdentityProviderConfigOutput, error) + // Cancels an in-progress update to an Amazon EKS cluster on a best-effort basis. + // Cancellation is only performed if the update can be cancelled. Currently, this + // is supported for VersionRollback update types on EKS Auto Mode clusters when + // nodes are rolling back. + // + // A successful cancellation stops the node rollback. After cancellation, nodes + // converge to the current cluster version honoring configured disruption controls. + // If the control plane rollback has already begun, the cancellation request fails. + CancelUpdate(ctx context.Context, params *eks.CancelUpdateInput, optFns ...func(*Options)) (*eks.CancelUpdateOutput, error) // Creates an access entry. // // An access entry allows an IAM principal to access your cluster. Access entries diff --git a/pkg/eks/mocksv2/EKS.go b/pkg/eks/mocksv2/EKS.go index 2d846d963d..53ac4e8ae5 100644 --- a/pkg/eks/mocksv2/EKS.go +++ b/pkg/eks/mocksv2/EKS.go @@ -244,6 +244,80 @@ func (_c *EKS_AssociateIdentityProviderConfig_Call) RunAndReturn(run func(contex return _c } +// CancelUpdate provides a mock function with given fields: ctx, params, optFns +func (_m *EKS) CancelUpdate(ctx context.Context, params *eks.CancelUpdateInput, optFns ...func(*eks.Options)) (*eks.CancelUpdateOutput, error) { + _va := make([]interface{}, len(optFns)) + for _i := range optFns { + _va[_i] = optFns[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, params) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + if len(ret) == 0 { + panic("no return value specified for CancelUpdate") + } + + var r0 *eks.CancelUpdateOutput + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *eks.CancelUpdateInput, ...func(*eks.Options)) (*eks.CancelUpdateOutput, error)); ok { + return rf(ctx, params, optFns...) + } + if rf, ok := ret.Get(0).(func(context.Context, *eks.CancelUpdateInput, ...func(*eks.Options)) *eks.CancelUpdateOutput); ok { + r0 = rf(ctx, params, optFns...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*eks.CancelUpdateOutput) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *eks.CancelUpdateInput, ...func(*eks.Options)) error); ok { + r1 = rf(ctx, params, optFns...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// EKS_CancelUpdate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CancelUpdate' +type EKS_CancelUpdate_Call struct { + *mock.Call +} + +// CancelUpdate is a helper method to define mock.On call +// - ctx context.Context +// - params *eks.CancelUpdateInput +// - optFns ...func(*eks.Options) +func (_e *EKS_Expecter) CancelUpdate(ctx interface{}, params interface{}, optFns ...interface{}) *EKS_CancelUpdate_Call { + return &EKS_CancelUpdate_Call{Call: _e.mock.On("CancelUpdate", + append([]interface{}{ctx, params}, optFns...)...)} +} + +func (_c *EKS_CancelUpdate_Call) Run(run func(ctx context.Context, params *eks.CancelUpdateInput, optFns ...func(*eks.Options))) *EKS_CancelUpdate_Call { + _c.Call.Run(func(args mock.Arguments) { + variadicArgs := make([]func(*eks.Options), len(args)-2) + for i, a := range args[2:] { + if a != nil { + variadicArgs[i] = a.(func(*eks.Options)) + } + } + run(args[0].(context.Context), args[1].(*eks.CancelUpdateInput), variadicArgs...) + }) + return _c +} + +func (_c *EKS_CancelUpdate_Call) Return(_a0 *eks.CancelUpdateOutput, _a1 error) *EKS_CancelUpdate_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *EKS_CancelUpdate_Call) RunAndReturn(run func(context.Context, *eks.CancelUpdateInput, ...func(*eks.Options)) (*eks.CancelUpdateOutput, error)) *EKS_CancelUpdate_Call { + _c.Call.Return(run) + return _c +} + // CreateAccessEntry provides a mock function with given fields: ctx, params, optFns func (_m *EKS) CreateAccessEntry(ctx context.Context, params *eks.CreateAccessEntryInput, optFns ...func(*eks.Options)) (*eks.CreateAccessEntryOutput, error) { _va := make([]interface{}, len(optFns)) diff --git a/pkg/eks/update.go b/pkg/eks/update.go index 9d9edec1c9..1d8eac78fd 100644 --- a/pkg/eks/update.go +++ b/pkg/eks/update.go @@ -236,6 +236,12 @@ func (c *ClusterProvider) UpdateClusterVersion(ctx context.Context, cfg *api.Clu if cfg.Metadata.ForceUpdateVersion != nil { input.Force = *cfg.Metadata.ForceUpdateVersion } + if rollbackConfig := cfg.Metadata.RollbackConfig; rollbackConfig != nil { + input.RollbackConfig = &ekstypes.RollbackConfig{} + if rollbackConfig.TimeoutMinutes != nil { + input.RollbackConfig.TimeoutMinutes = aws.Int32(int32(*rollbackConfig.TimeoutMinutes)) + } + } output, err := c.AWSProvider.EKS().UpdateClusterVersion(ctx, input) if err != nil { return nil, err diff --git a/pkg/eks/update_test.go b/pkg/eks/update_test.go index 9e0d176c90..1919e93a1a 100644 --- a/pkg/eks/update_test.go +++ b/pkg/eks/update_test.go @@ -180,4 +180,62 @@ var _ = Describe("EKS API wrapper", func() { Expect(ctl.UpdateClusterConfigForLogging(context.Background(), cfg)).To(Succeed()) }) }) + + Describe("UpdateClusterVersion", func() { + var ( + ctl *ClusterProvider + cfg *api.ClusterConfig + p *mockprovider.MockProvider + + sentInput *awseks.UpdateClusterVersionInput + ) + + BeforeEach(func() { + p = mockprovider.NewMockProvider() + ctl = &ClusterProvider{ + AWSProvider: p, + Status: &ProviderStatus{}, + } + + cfg = api.NewClusterConfig() + cfg.Metadata.Name = "testcluster" + cfg.Metadata.Version = "1.30" + + p.MockEKS().On("UpdateClusterVersion", mock.Anything, mock.MatchedBy(func(input *awseks.UpdateClusterVersionInput) bool { + sentInput = input + return true + })).Return(&awseks.UpdateClusterVersionOutput{ + Update: &ekstypes.Update{ + Id: aws.String("u123"), + Type: ekstypes.UpdateTypeVersionUpdate, + }, + }, nil) + }) + + It("does not set RollbackConfig when none is configured", func() { + _, err := ctl.UpdateClusterVersion(context.Background(), cfg) + Expect(err).NotTo(HaveOccurred()) + Expect(sentInput.RollbackConfig).To(BeNil()) + }) + + It("passes the configured rollback timeout through to the API", func() { + cfg.Metadata.RollbackConfig = &api.RollbackConfig{ + TimeoutMinutes: aws.Int(360), + } + + _, err := ctl.UpdateClusterVersion(context.Background(), cfg) + Expect(err).NotTo(HaveOccurred()) + Expect(sentInput.RollbackConfig).NotTo(BeNil()) + Expect(sentInput.RollbackConfig.TimeoutMinutes).To(Equal(aws.Int32(360))) + }) + + It("sets an empty RollbackConfig when configured without a timeout", func() { + cfg.Metadata.RollbackConfig = &api.RollbackConfig{} + + _, err := ctl.UpdateClusterVersion(context.Background(), cfg) + Expect(err).NotTo(HaveOccurred()) + Expect(sentInput.RollbackConfig).NotTo(BeNil()) + Expect(sentInput.RollbackConfig.TimeoutMinutes).To(BeNil()) + }) + }) }) diff --git a/pkg/eks/versions_manager.go b/pkg/eks/versions_manager.go index 5690836933..39e3d69eeb 100644 --- a/pkg/eks/versions_manager.go +++ b/pkg/eks/versions_manager.go @@ -133,8 +133,9 @@ func (cvm *ClusterVersionsManager) ResolveClusterVersion(version string) (string } func (cvm *ClusterVersionsManager) ResolveUpgradeVersion(desiredVersion string, currentVersion string) (string, error) { - // Resolve next version + // Resolve next and previous versions var nextVersion string + var previousVersion string switch { case currentVersion == "": return "", fmt.Errorf("couldn't resolve control plane version") @@ -144,29 +145,27 @@ func (cvm *ClusterVersionsManager) ResolveUpgradeVersion(desiredVersion string, return "", fmt.Errorf("control plane version %q is not supported", currentVersion) default: i := slices.Index(cvm.supportedVersions, currentVersion) - if i == len(cvm.supportedVersions)-1 { - logger.Info("control plane is already on latest version %q", currentVersion) - return "", nil + // Determine next version + if i < len(cvm.supportedVersions)-1 { + nextVersion = cvm.supportedVersions[i+1] + } + // Determine previous version + if i > 0 { + previousVersion = cvm.supportedVersions[i-1] } - nextVersion = cvm.supportedVersions[i+1] } // If the version was not specified, default to the next Kubernetes version, and assume the user intended to upgrade if possible. // Also support "auto" as version (see #2461) if desiredVersion == "" || desiredVersion == "auto" { - if cvm.IsSupportedVersion(nextVersion) { + if nextVersion != "" { return nextVersion, nil } + logger.Info("control plane is already on latest version %q", currentVersion) // There is no new version, stay in the current one return "", nil } - if c, err := utils.CompareVersions(desiredVersion, currentVersion); err != nil { - return "", fmt.Errorf("couldn't compare versions for upgrade: %w", err) - } else if c < 0 { - return "", fmt.Errorf("cannot upgrade to a lower version. Found given target version %q, current cluster version %q", desiredVersion, currentVersion) - } - if cvm.IsDeprecatedVersion(desiredVersion) { return "", fmt.Errorf("control plane version %q has been deprecated", desiredVersion) } @@ -179,15 +178,37 @@ func (cvm *ClusterVersionsManager) ResolveUpgradeVersion(desiredVersion string, return "", nil } - if desiredVersion == nextVersion { + c, err := utils.CompareVersions(desiredVersion, currentVersion) + if err != nil { + return "", fmt.Errorf("couldn't compare versions: %w", err) + } + + // Handle upgrade + if c > 0 { + if desiredVersion == nextVersion { + return desiredVersion, nil + } + return "", fmt.Errorf( + "upgrading more than one version at a time is not supported. Found upgrade from %q to %q. Please upgrade to %q first", + currentVersion, + desiredVersion, + nextVersion) + } + + // Handle downgrade (rollback) + if previousVersion == "" { + return "", fmt.Errorf("cannot downgrade: %q is the oldest supported version", currentVersion) + } + + if desiredVersion == previousVersion { return desiredVersion, nil } return "", fmt.Errorf( - "upgrading more than one version at a time is not supported. Found upgrade from %q to %q. Please upgrade to %q first", + "downgrading more than one version at a time is not supported. Found downgrade from %q to %q. Please downgrade to %q first", currentVersion, desiredVersion, - nextVersion) + previousVersion) } func resolveDeprecatedVersions(currentVersion string) ([]string, error) { diff --git a/pkg/eks/versions_manager_test.go b/pkg/eks/versions_manager_test.go index 9db430ffab..8f843865ba 100644 --- a/pkg/eks/versions_manager_test.go +++ b/pkg/eks/versions_manager_test.go @@ -53,12 +53,10 @@ var _ = Describe("upgrade cluster", func() { upgradeVersion, err := cvm.ResolveUpgradeVersion(c.givenVersion, c.eksVersion) if c.expectedErrorText != "" { - if c.expectedErrorText != "cannot upgrade to a lower version" { - } else { - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(c.expectedErrorText)) - } + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(c.expectedErrorText)) } else { + Expect(err).NotTo(HaveOccurred()) Expect(upgradeVersion).To(Equal(c.expectedUpgradeVersion)) } }, @@ -99,10 +97,16 @@ var _ = Describe("upgrade cluster", func() { expectedErrorText: "upgrading more than one version at a time is not supported", }), - Entry("fails when the given version is lower than the current one", upgradeCase{ - givenVersion: api.Version1_29, + Entry("downgrades to the previous version when specified", upgradeCase{ + givenVersion: api.Version1_29, + eksVersion: api.Version1_30, + expectedUpgradeVersion: api.Version1_29, + }), + + Entry("fails when the downgrade jumps more than one kubernetes version", upgradeCase{ + givenVersion: api.Version1_28, eksVersion: api.Version1_30, - expectedErrorText: "cannot upgrade to a lower version", + expectedErrorText: "downgrading more than one version at a time is not supported", }), Entry("fails when the version is deprecated", upgradeCase{ From c4d88bc96a741ce3a41b142115c3308b13112646 Mon Sep 17 00:00:00 2001 From: michaelhtm <98621731+michaelhtm@users.noreply.github.com> Date: Wed, 1 Jul 2026 01:18:56 -0700 Subject: [PATCH 2/2] Only set RollbackConfig when a timeoutMinutes value is present Address review feedback: when metadata.rollbackConfig omits timeoutMinutes we were sending an empty "rollbackConfig": {} to UpdateClusterVersion. The API tolerates it (it applies its default timeout either way), but it is redundant, so only set input.RollbackConfig when there is a value to send. --- pkg/eks/update.go | 10 ++++++---- pkg/eks/update_test.go | 5 ++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/eks/update.go b/pkg/eks/update.go index 1d8eac78fd..ed97410d80 100644 --- a/pkg/eks/update.go +++ b/pkg/eks/update.go @@ -236,10 +236,12 @@ func (c *ClusterProvider) UpdateClusterVersion(ctx context.Context, cfg *api.Clu if cfg.Metadata.ForceUpdateVersion != nil { input.Force = *cfg.Metadata.ForceUpdateVersion } - if rollbackConfig := cfg.Metadata.RollbackConfig; rollbackConfig != nil { - input.RollbackConfig = &ekstypes.RollbackConfig{} - if rollbackConfig.TimeoutMinutes != nil { - input.RollbackConfig.TimeoutMinutes = aws.Int32(int32(*rollbackConfig.TimeoutMinutes)) + // Only send a RollbackConfig when there is a value to set; an empty struct + // would serialize as "rollbackConfig": {}, which is redundant with omitting it + // (EKS applies its default timeout either way). + if rollbackConfig := cfg.Metadata.RollbackConfig; rollbackConfig != nil && rollbackConfig.TimeoutMinutes != nil { + input.RollbackConfig = &ekstypes.RollbackConfig{ + TimeoutMinutes: aws.Int32(int32(*rollbackConfig.TimeoutMinutes)), } } output, err := c.AWSProvider.EKS().UpdateClusterVersion(ctx, input) diff --git a/pkg/eks/update_test.go b/pkg/eks/update_test.go index 1919e93a1a..3cd3435b91 100644 --- a/pkg/eks/update_test.go +++ b/pkg/eks/update_test.go @@ -229,13 +229,12 @@ var _ = Describe("EKS API wrapper", func() { Expect(sentInput.RollbackConfig.TimeoutMinutes).To(Equal(aws.Int32(360))) }) - It("sets an empty RollbackConfig when configured without a timeout", func() { + It("does not set RollbackConfig when configured without a timeout", func() { cfg.Metadata.RollbackConfig = &api.RollbackConfig{} _, err := ctl.UpdateClusterVersion(context.Background(), cfg) Expect(err).NotTo(HaveOccurred()) - Expect(sentInput.RollbackConfig).NotTo(BeNil()) - Expect(sentInput.RollbackConfig.TimeoutMinutes).To(BeNil()) + Expect(sentInput.RollbackConfig).To(BeNil()) }) }) })