Merge pull request #3337 from richardcase/nodegroupupdate

k8s-ci-robot · web-flow · commit b1ac1ea1ea54 · 2022-03-24T22:05:50.000-07:00
feat: add nodegroup update config support
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml
@@ -659,6 +659,25 @@ spec:
                   - value
                   type: object
                 type: array
+              updateConfig:
+                description: UpdateConfig holds the optional config to control the
+                  behaviour of the update to the nodegroup.
+                properties:
+                  maxUnavailable:
+                    description: MaxUnavailable is the maximum number of nodes unavailable
+                      at once during a version update. Nodes will be updated in parallel.
+                      The maximum number is 100.
+                    maximum: 100
+                    minimum: 1
+                    type: integer
+                  maxUnavailablePrecentage:
+                    description: MaxUnavailablePercentage is the maximum percentage
+                      of nodes unavailable during a version update. This percentage
+                      of nodes will be updated in parallel, up to 100 nodes at once.
+                    maximum: 100
+                    minimum: 1
+                    type: integer
+                type: object
             type: object
           status:
             description: AWSManagedMachinePoolStatus defines the observed state of
diff --git a/exp/api/v1alpha3/conversion.go b/exp/api/v1alpha3/conversion.go
@@ -91,6 +91,7 @@ func (r *AWSManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	dst.Spec.Taints = restored.Spec.Taints
 	dst.Spec.CapacityType = restored.Spec.CapacityType
 	dst.Spec.RoleAdditionalPolicies = restored.Spec.RoleAdditionalPolicies
+	dst.Spec.UpdateConfig = restored.Spec.UpdateConfig
 
 	return nil
 }
diff --git a/exp/api/v1alpha3/zz_generated.conversion.go b/exp/api/v1alpha3/zz_generated.conversion.go
diff --git a/exp/api/v1alpha4/conversion.go b/exp/api/v1alpha4/conversion.go
@@ -64,6 +64,7 @@ func (src *AWSManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Spec.RoleAdditionalPolicies = restored.Spec.RoleAdditionalPolicies
+	dst.Spec.UpdateConfig = restored.Spec.UpdateConfig
 
 	return nil
 }
diff --git a/exp/api/v1alpha4/zz_generated.conversion.go b/exp/api/v1alpha4/zz_generated.conversion.go
diff --git a/exp/api/v1beta1/awsmanagedmachinepool_types.go b/exp/api/v1beta1/awsmanagedmachinepool_types.go
@@ -144,6 +144,11 @@ type AWSManagedMachinePoolSpec struct {
 	// +kubebuilder:default:=onDemand
 	// +optional
 	CapacityType *ManagedMachinePoolCapacityType `json:"capacityType,omitempty"`
+
+	// UpdateConfig holds the optional config to control the behaviour of the update
+	// to the nodegroup.
+	// +optional
+	UpdateConfig *UpdateConfig `json:"updateConfig,omitempty"`
 }
 
 // ManagedMachinePoolScaling specifies scaling options.
diff --git a/exp/api/v1beta1/awsmanagedmachinepool_webhook.go b/exp/api/v1beta1/awsmanagedmachinepool_webhook.go
@@ -76,6 +76,27 @@ func (r *AWSManagedMachinePool) validateScaling() field.ErrorList {
 	return allErrs
 }
 
+func (r *AWSManagedMachinePool) validateNodegroupUpdateConfig() field.ErrorList {
+	var allErrs field.ErrorList
+
+	if r.Spec.UpdateConfig != nil {
+		nodegroupUpdateConfigField := field.NewPath("spec", "updateConfig")
+
+		if r.Spec.UpdateConfig.MaxUnavailable == nil && r.Spec.UpdateConfig.MaxUnavailablePercentage == nil {
+			allErrs = append(allErrs, field.Invalid(nodegroupUpdateConfigField, r.Spec.UpdateConfig, "must specify one of maxUnavailable or maxUnavailablePercentage when using nodegroup updateconfig"))
+		}
+
+		if r.Spec.UpdateConfig.MaxUnavailable != nil && r.Spec.UpdateConfig.MaxUnavailablePercentage != nil {
+			allErrs = append(allErrs, field.Invalid(nodegroupUpdateConfigField, r.Spec.UpdateConfig, "cannot specify both maxUnavailable and maxUnavailablePercentage"))
+		}
+	}
+
+	if len(allErrs) == 0 {
+		return nil
+	}
+	return allErrs
+}
+
 func (r *AWSManagedMachinePool) validateRemoteAccess() field.ErrorList {
 	var allErrs field.ErrorList
 	if r.Spec.RemoteAccess == nil {
@@ -109,6 +130,9 @@ func (r *AWSManagedMachinePool) ValidateCreate() error {
 	if errs := r.validateRemoteAccess(); len(errs) > 0 {
 		allErrs = append(allErrs, errs...)
 	}
+	if errs := r.validateNodegroupUpdateConfig(); len(errs) > 0 {
+		allErrs = append(allErrs, errs...)
+	}
 
 	allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
 
@@ -140,6 +164,9 @@ func (r *AWSManagedMachinePool) ValidateUpdate(old runtime.Object) error {
 	if errs := r.validateScaling(); errs != nil || len(errs) == 0 {
 		allErrs = append(allErrs, errs...)
 	}
+	if errs := r.validateNodegroupUpdateConfig(); len(errs) > 0 {
+		allErrs = append(allErrs, errs...)
+	}
 
 	if len(allErrs) == 0 {
 		return nil
diff --git a/exp/api/v1beta1/awsmanagedmachinepool_webhook_test.go b/exp/api/v1beta1/awsmanagedmachinepool_webhook_test.go
@@ -20,6 +20,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/aws/aws-sdk-go/aws"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -90,6 +91,41 @@ func TestAWSManagedMachinePool_ValidateCreate(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "valid update config",
+			pool: &AWSManagedMachinePool{
+				Spec: AWSManagedMachinePoolSpec{
+					EKSNodegroupName: "eks-node-group-3",
+					UpdateConfig: &UpdateConfig{
+						MaxUnavailable: aws.Int(1),
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "update config with no values",
+			pool: &AWSManagedMachinePool{
+				Spec: AWSManagedMachinePoolSpec{
+					EKSNodegroupName: "eks-node-group-3",
+					UpdateConfig:     &UpdateConfig{},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "update config with both values",
+			pool: &AWSManagedMachinePool{
+				Spec: AWSManagedMachinePoolSpec{
+					EKSNodegroupName: "eks-node-group-3",
+					UpdateConfig: &UpdateConfig{
+						MaxUnavailable:           aws.Int(1),
+						MaxUnavailablePercentage: aws.Int(10),
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -170,6 +206,40 @@ func TestAWSManagedMachinePool_ValidateUpdate(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "adding update config is accepted",
+			old: &AWSManagedMachinePool{
+				Spec: AWSManagedMachinePoolSpec{
+					EKSNodegroupName: "eks-node-group-1",
+				},
+			},
+			new: &AWSManagedMachinePool{
+				Spec: AWSManagedMachinePoolSpec{
+					EKSNodegroupName: "eks-node-group-1",
+					UpdateConfig: &UpdateConfig{
+						MaxUnavailablePercentage: aws.Int(10),
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "removing update config is accepted",
+			old: &AWSManagedMachinePool{
+				Spec: AWSManagedMachinePoolSpec{
+					EKSNodegroupName: "eks-node-group-1",
+					UpdateConfig: &UpdateConfig{
+						MaxUnavailablePercentage: aws.Int(10),
+					},
+				},
+			},
+			new: &AWSManagedMachinePool{
+				Spec: AWSManagedMachinePoolSpec{
+					EKSNodegroupName: "eks-node-group-1",
+				},
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/exp/api/v1beta1/types.go b/exp/api/v1beta1/types.go
@@ -251,3 +251,21 @@ func (t *Taints) Contains(taint *Taint) bool {
 
 	return false
 }
+
+// UpdateConfig is the configuration options for updating a nodegroup. Only one of MaxUnavailable
+// and MaxUnavailablePercentage should be specified.
+type UpdateConfig struct {
+	// MaxUnavailable is the maximum number of nodes unavailable at once during a version update.
+	// Nodes will be updated in parallel. The maximum number is 100.
+	// +optional
+	// +kubebuilder:validation:Maximum=100
+	// +kubebuilder:validation:Minimum=1
+	MaxUnavailable *int `json:"maxUnavailable,omitempty"`
+
+	// MaxUnavailablePercentage is the maximum percentage of nodes unavailable during a version update. This
+	// percentage of nodes will be updated in parallel, up to 100 nodes at once.
+	// +optional
+	// +kubebuilder:validation:Maximum=100
+	// +kubebuilder:validation:Minimum=1
+	MaxUnavailablePercentage *int `json:"maxUnavailablePrecentage,omitempty"`
+}
diff --git a/exp/api/v1beta1/zz_generated.deepcopy.go b/exp/api/v1beta1/zz_generated.deepcopy.go
diff --git a/go.mod b/go.mod
@@ -15,6 +15,7 @@ require (
 	github.com/go-logr/logr v1.2.3
 	github.com/gofrs/flock v0.8.1
 	github.com/golang/mock v1.6.0
+	github.com/google/go-cmp v0.5.6
 	github.com/google/goexpect v0.0.0-20210430020637-ab937bf7fd6f
 	github.com/google/gofuzz v1.2.0
 	github.com/onsi/ginkgo v1.16.5
@@ -75,7 +76,6 @@ require (
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/cel-go v0.9.0 // indirect
-	github.com/google/go-cmp v0.5.6 // indirect
 	github.com/google/go-github/v33 v33.0.0 // indirect
 	github.com/google/go-querystring v1.0.0 // indirect
 	github.com/google/goterm v0.0.0-20190703233501-fc88cf888a3f // indirect
diff --git a/pkg/cloud/converters/eks.go b/pkg/cloud/converters/eks.go
@@ -175,3 +175,37 @@ func CapacityTypeToSDK(capacityType expinfrav1.ManagedMachinePoolCapacityType) (
 		return "", ErrUnknownCapacityType
 	}
 }
+
+// NodegroupUpdateconfigToSDK is used to convert a CAPA UpdateConfig to AWS SDK NodegroupUpdateConfig.
+func NodegroupUpdateconfigToSDK(updateConfig *expinfrav1.UpdateConfig) *eks.NodegroupUpdateConfig {
+	if updateConfig == nil {
+		return nil
+	}
+
+	converted := &eks.NodegroupUpdateConfig{}
+	if updateConfig.MaxUnavailable != nil {
+		converted.MaxUnavailable = aws.Int64(int64(*updateConfig.MaxUnavailable))
+	}
+	if updateConfig.MaxUnavailablePercentage != nil {
+		converted.MaxUnavailablePercentage = aws.Int64(int64(*updateConfig.MaxUnavailablePercentage))
+	}
+
+	return converted
+}
+
+// NodegroupUpdateconfigFromSDK is used to convert a AWS SDK NodegroupUpdateConfig to a CAPA UpdateConfig.
+func NodegroupUpdateconfigFromSDK(ngUpdateConfig *eks.NodegroupUpdateConfig) *expinfrav1.UpdateConfig {
+	if ngUpdateConfig == nil {
+		return nil
+	}
+
+	converted := &expinfrav1.UpdateConfig{}
+	if ngUpdateConfig.MaxUnavailable != nil {
+		converted.MaxUnavailable = aws.Int(int(*ngUpdateConfig.MaxUnavailable))
+	}
+	if ngUpdateConfig.MaxUnavailablePercentage != nil {
+		converted.MaxUnavailablePercentage = aws.Int(int(*ngUpdateConfig.MaxUnavailablePercentage))
+	}
+
+	return converted
+}
diff --git a/pkg/cloud/services/eks/nodegroup.go b/pkg/cloud/services/eks/nodegroup.go
@@ -24,6 +24,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/eks"
 	"github.com/aws/aws-sdk-go/service/iam"
+	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/util/version"
 
@@ -113,6 +114,12 @@ func (s *NodegroupService) scalingConfig() *eks.NodegroupScalingConfig {
 	return &cfg
 }
 
+func (s *NodegroupService) updateConfig() *eks.NodegroupUpdateConfig {
+	updateConfig := s.scope.ManagedMachinePool.Spec.UpdateConfig
+
+	return converters.NodegroupUpdateconfigToSDK(updateConfig)
+}
+
 func (s *NodegroupService) roleArn() (*string, error) {
 	var role *iam.Role
 	if s.scope.RoleName() != "" {
@@ -208,6 +215,7 @@ func (s *NodegroupService) createNodegroup() (*eks.Nodegroup, error) {
 		Labels:        aws.StringMap(managedPool.Labels),
 		Tags:          aws.StringMap(tags),
 		RemoteAccess:  remoteAccess,
+		UpdateConfig:  s.updateConfig(),
 	}
 	if managedPool.AMIType != nil {
 		input.AmiType = aws.String(string(*managedPool.AMIType))
@@ -452,6 +460,12 @@ func (s *NodegroupService) reconcileNodegroupConfig(ng *eks.Nodegroup) error {
 		input.ScalingConfig = s.scalingConfig()
 		needsUpdate = true
 	}
+	currentUpdateConfig := converters.NodegroupUpdateconfigFromSDK(ng.UpdateConfig)
+	if !cmp.Equal(managedPool.UpdateConfig, currentUpdateConfig) {
+		s.V(2).Info("Nodegroup update configuration differs from spec, updating the nodegroup update config", "nodegroup", ng.NodegroupName)
+		input.UpdateConfig = s.updateConfig()
+		needsUpdate = true
+	}
 	if !needsUpdate {
 		s.V(2).Info("node group config update not needed", "cluster", eksClusterName, "name", *ng.NodegroupName)
 		return nil
diff --git a/test/e2e/data/eks/cluster-template-eks-managed-machinepool-only.yaml b/test/e2e/data/eks/cluster-template-eks-managed-machinepool-only.yaml
@@ -23,3 +23,5 @@ spec:
   scaling:
     minSize: 1
     maxSize: 2
+  updateConfig:
+    maxUnavailable: 2

Original file line number	Diff line number	Diff line change
`@@ -91,6 +91,7 @@ func (r *AWSManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error {`
`91`	`91`	`dst.Spec.Taints = restored.Spec.Taints`
`92`	`92`	`dst.Spec.CapacityType = restored.Spec.CapacityType`
`93`	`93`	`dst.Spec.RoleAdditionalPolicies = restored.Spec.RoleAdditionalPolicies`
	`94`	`+ dst.Spec.UpdateConfig = restored.Spec.UpdateConfig`
`94`	`95`
`95`	`96`	`return nil`
`96`	`97`	`}`
Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@ func (src *AWSManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error {`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`dst.Spec.RoleAdditionalPolicies = restored.Spec.RoleAdditionalPolicies`
	`67`	`+ dst.Spec.UpdateConfig = restored.Spec.UpdateConfig`
`67`	`68`
`68`	`69`	`return nil`
`69`	`70`	`}`