string type, use current UpdateClusterConfig

Author: phuhung273

config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml

@@ -2985,12 +2985,13 @@ spec:
               upgradePolicy:
                 description: |-
                   The support policy to use for the cluster.
-                  Extended support allows you to remain on specific Kubernetes versions for longer.
+                  Extended support indicates that the cluster will not be automatically upgraded
+                  when it leaves the standard support period, and will enter extended support.
                   Clusters in extended support have higher costs.
-                  The default value is EXTENDED. Use STANDARD to disable extended support.
+                  The default value is extended. Use standard to disable extended support.
                 enum:
-                - EXTENDED
-                - STANDARD
+                - extended
+                - standard
                 type: string
               version:
                 description: |-

controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go

@@ -211,12 +211,13 @@ type AWSManagedControlPlaneSpec struct { //nolint: maligned
 	KubeProxy KubeProxy `json:"kubeProxy,omitempty"`
 
 	// The support policy to use for the cluster.
-	// Extended support allows you to remain on specific Kubernetes versions for longer.
+	// Extended support indicates that the cluster will not be automatically upgraded
+	// when it leaves the standard support period, and will enter extended support.
 	// Clusters in extended support have higher costs.
-	// The default value is EXTENDED. Use STANDARD to disable extended support.
-	// +kubebuilder:validation:Enum=EXTENDED;STANDARD
+	// The default value is extended. Use standard to disable extended support.
+	// +kubebuilder:validation:Enum=extended;standard
 	// +optional
-	UpgradePolicy *string `json:"upgradePolicy,omitempty"`
+	UpgradePolicy *UpgradePolicy `json:"upgradePolicy,omitempty"`
 }
 
 // KubeProxy specifies how the kube-proxy daemonset is managed.

controlplane/eks/api/v1beta2/types.go

@@ -212,6 +212,24 @@ type AddonIssue struct {
 	ResourceIDs []string `json:"resourceIds,omitempty"`
 }
 
+// UpgradePolicy defines the support policy to use for the cluster.
+type UpgradePolicy string
+
+var (
+	// UpgradePolicyExtended indicates that the cluster will not be automatically upgraded
+	// when it leaves the standard support period, and will enter extended support.
+	// Clusters in extended support have higher costs.
+	UpgradePolicyExtended = UpgradePolicy("extended")
+
+	// UpgradePolicyStandard indicates that the cluster will be automatically upgraded
+	// when it leaves the standard support period.
+	UpgradePolicyStandard = UpgradePolicy("standard")
+)
+
+func (e UpgradePolicy) String() string {
+	return string(e)
+}
+
 const (
 	// SecurityGroupCluster is the security group for communication between EKS
 	// control plane and managed node groups.

controlplane/eks/api/v1beta2/zz_generated.deepcopy.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -139,10 +140,6 @@ func (s *Service) reconcileCluster(ctx context.Context) error {
 		return errors.Wrap(err, "failed reconciling OIDC provider for cluster")
 	}
 
-	if err := s.reconcileClusterUpgradePolicy(cluster); err != nil {
-		return errors.Wrap(err, "failed reconciling cluster upgrade policy")
-	}
-
 	return nil
 }
 
@@ -472,7 +469,7 @@ func (s *Service) createCluster(eksClusterName string) (*eks.Cluster, error) {
 
 	if s.scope.ControlPlane.Spec.UpgradePolicy != nil {
 		upgradePolicy = &eks.UpgradePolicyRequest{
-			SupportType: s.scope.ControlPlane.Spec.UpgradePolicy,
+			SupportType: aws.String(s.scope.ControlPlane.Spec.UpgradePolicy.String()),
 		}
 	}
 
@@ -549,6 +546,15 @@ func (s *Service) reconcileClusterConfig(cluster *eks.Cluster) error {
 		input.ResourcesVpcConfig = updateVpcConfig
 	}
 
+	updateUpgradePolicy, err := s.reconcileUpgradePolicy(cluster.UpgradePolicy)
+	if err != nil {
+		return errors.Wrap(err, "couldn't update upgrade policy for cluster")
+	}
+	if updateUpgradePolicy != nil {
+		needsUpdate = true
+		input.UpgradePolicy = updateUpgradePolicy
+	}
+
 	if needsUpdate {
 		if err := input.Validate(); err != nil {
 			return errors.Wrap(err, "created invalid UpdateClusterConfigInput")
@@ -739,61 +745,29 @@ func (s *Service) reconcileClusterVersion(cluster *eks.Cluster) error {
 	return nil
 }
 
-func (s *Service) reconcileClusterUpgradePolicy(cluster *eks.Cluster) error {
+func (s *Service) reconcileUpgradePolicy(upgradePolicy *eks.UpgradePolicyResponse) (*eks.UpgradePolicyRequest, error) {
 	s.Info("reconciling upgrade policy")
 
-	clusterUpgradePolicy := cluster.UpgradePolicy.SupportType
+	clusterUpgradePolicy := upgradePolicy.SupportType
 
 	if s.scope.ControlPlane.Spec.UpgradePolicy == nil {
 		s.Debug("upgrade policy not given, no action")
-		return nil
+		return nil, nil
 	}
 
 	if clusterUpgradePolicy == nil {
 		s.Debug("cannot get cluster upgrade policy, no action")
-		return nil
+		return nil, nil
 	}
 
-	if *clusterUpgradePolicy == *s.scope.ControlPlane.Spec.UpgradePolicy {
+	if strings.ToLower(*clusterUpgradePolicy) == s.scope.ControlPlane.Spec.UpgradePolicy.String() {
 		s.Debug("upgrade policy unchanged, no action")
-		return nil
-	}
-
-	input := &eks.UpdateClusterConfigInput{
-		Name: aws.String(s.scope.KubernetesClusterName()),
-		UpgradePolicy: &eks.UpgradePolicyRequest{
-			SupportType: s.scope.ControlPlane.Spec.UpgradePolicy,
-		},
-	}
-
-	if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) {
-		if _, err := s.EKSClient.UpdateClusterConfig(input); err != nil {
-			if aerr, ok := err.(awserr.Error); ok {
-				return false, aerr
-			}
-			return false, err
-		}
-
-		// Wait until status transitions to UPDATING because there's a short
-		// window after UpdateClusterConfig returns where the cluster
-		// status is ACTIVE and the update would be tried again
-		if err := s.EKSClient.WaitUntilClusterUpdating(
-			&eks.DescribeClusterInput{Name: aws.String(s.scope.KubernetesClusterName())},
-			request.WithWaiterLogger(&awslog{s.GetLogger()}),
-		); err != nil {
-			return false, err
-		}
-
-		conditions.MarkTrue(s.scope.ControlPlane, ekscontrolplanev1.EKSControlPlaneUpdatingCondition)
-		record.Eventf(s.scope.ControlPlane, "InitiatedUpdateEKSControlPlane", "Initiated update of EKS control plane %s to upgrade policy %s", s.scope.KubernetesClusterName(), s.scope.ControlPlane.Spec.UpgradePolicy)
-
-		return true, nil
-	}); err != nil {
-		record.Warnf(s.scope.ControlPlane, "FailedUpdateEKSControlPlane", "failed to update the EKS control plane: %v", err)
-		return errors.Wrapf(err, "failed to update EKS cluster")
+		return nil, nil
 	}
 
-	return nil
+	return &eks.UpgradePolicyRequest{
+		SupportType: convertUpgradePolicy(*s.scope.ControlPlane.Spec.UpgradePolicy),
+	}, nil
 }
 
 func (s *Service) describeEKSCluster(eksClusterName string) (*eks.Cluster, error) {
@@ -925,3 +899,10 @@ func getKeyArn(encryptionConfig *eks.EncryptionConfig) string {
 	}
 	return ""
 }
+
+func convertUpgradePolicy(input ekscontrolplanev1.UpgradePolicy) *string {
+	if input == ekscontrolplanev1.UpgradePolicyStandard {
+		return aws.String(eks.SupportTypeStandard)
+	}
+	return aws.String(eks.SupportTypeExtended)
+}

pkg/cloud/services/eks/cluster.go

@@ -524,7 +524,7 @@ func TestCreateCluster(t *testing.T) {
 						RoleName:                   tc.role,
 						NetworkSpec:                infrav1.NetworkSpec{Subnets: tc.subnets},
 						BootstrapSelfManagedAddons: false,
-						UpgradePolicy:              aws.String(eks.SupportTypeStandard),
+						UpgradePolicy:              &ekscontrolplanev1.UpgradePolicyStandard,
 					},
 				},
 			})
@@ -679,57 +679,43 @@ func TestReconcileEKSEncryptionConfig(t *testing.T) {
 	}
 }
 
-func TestReconcileEKSUpgradePolicy(t *testing.T) {
+func TestReconcileUpgradePolicy(t *testing.T) {
 	clusterName := "default.cluster"
 	tests := []struct {
 		name             string
 		oldUpgradePolicy *string
-		newUpgradePolicy *string
-		expect           func(m *mock_eksiface.MockEKSAPIMockRecorder)
+		newUpgradePolicy *ekscontrolplanev1.UpgradePolicy
+		expect           *eks.UpgradePolicyRequest
 		expectError      bool
 	}{
 		{
 			name:             "no update necessary - no upgrade policy specified",
 			oldUpgradePolicy: aws.String(eks.SupportTypeStandard),
-			expect:           func(m *mock_eksiface.MockEKSAPIMockRecorder) {},
+			expect:           nil,
 			expectError:      false,
 		},
 		{
 			name:             "no update necessary - cannot get cluster upgrade policy",
-			newUpgradePolicy: aws.String(eks.SupportTypeStandard),
-			expect:           func(m *mock_eksiface.MockEKSAPIMockRecorder) {},
+			newUpgradePolicy: &ekscontrolplanev1.UpgradePolicyStandard,
+			expect:           nil,
 			expectError:      false,
 		},
 		{
 			name:             "no update necessary - upgrade policy unchanged",
 			oldUpgradePolicy: aws.String(eks.SupportTypeStandard),
-			newUpgradePolicy: aws.String(eks.SupportTypeStandard),
-			expect:           func(m *mock_eksiface.MockEKSAPIMockRecorder) {},
+			newUpgradePolicy: &ekscontrolplanev1.UpgradePolicyStandard,
+			expect:           nil,
 			expectError:      false,
 		},
 		{
 			name:             "needs update",
 			oldUpgradePolicy: aws.String(eks.SupportTypeStandard),
-			newUpgradePolicy: aws.String(eks.SupportTypeExtended),
-			expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {
-				m.WaitUntilClusterUpdating(
-					gomock.AssignableToTypeOf(&eks.DescribeClusterInput{}), gomock.Any(),
-				).Return(nil)
-				m.UpdateClusterConfig(gomock.AssignableToTypeOf(&eks.UpdateClusterConfigInput{})).Return(&eks.UpdateClusterConfigOutput{}, nil)
+			newUpgradePolicy: &ekscontrolplanev1.UpgradePolicyExtended,
+			expect: &eks.UpgradePolicyRequest{
+				SupportType: aws.String(eks.SupportTypeExtended),
 			},
 			expectError: false,
 		},
-		{
-			name:             "api error",
-			oldUpgradePolicy: aws.String(eks.SupportTypeExtended),
-			newUpgradePolicy: aws.String(eks.SupportTypeStandard),
-			expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {
-				m.
-					UpdateClusterConfig(gomock.AssignableToTypeOf(&eks.UpdateClusterConfigInput{})).
-					Return(&eks.UpdateClusterConfigOutput{}, errors.New(""))
-			},
-			expectError: true,
-		},
 	}
 
 	for _, tc := range tests {
@@ -739,8 +725,6 @@ func TestReconcileEKSUpgradePolicy(t *testing.T) {
 			mockControl := gomock.NewController(t)
 			defer mockControl.Finish()
 
-			eksMock := mock_eksiface.NewMockEKSAPI(mockControl)
-
 			scheme := runtime.NewScheme()
 			_ = infrav1.AddToScheme(scheme)
 			_ = ekscontrolplanev1.AddToScheme(scheme)
@@ -762,20 +746,16 @@ func TestReconcileEKSUpgradePolicy(t *testing.T) {
 			})
 			g.Expect(err).To(BeNil())
 
-			tc.expect(eksMock.EXPECT())
 			s := NewService(scope)
-			s.EKSClient = eksMock
 
-			err = s.reconcileClusterUpgradePolicy(&eks.Cluster{
-				UpgradePolicy: &eks.UpgradePolicyResponse{
-					SupportType: tc.oldUpgradePolicy,
-				},
+			upgradePolicyRequest, err := s.reconcileUpgradePolicy(&eks.UpgradePolicyResponse{
+				SupportType: tc.oldUpgradePolicy,
 			})
 			if tc.expectError {
 				g.Expect(err).To(HaveOccurred())
 				return
 			}
-			g.Expect(err).To(BeNil())
+			g.Expect(upgradePolicyRequest).To(Equal(tc.expect))
 		})
 	}
 }