diff --git a/pkg/cloud/services/autoscaling/autoscalinggroup.go b/pkg/cloud/services/autoscaling/autoscalinggroup.go index e3355b3eb7..e7204b3165 100644 --- a/pkg/cloud/services/autoscaling/autoscalinggroup.go +++ b/pkg/cloud/services/autoscaling/autoscalinggroup.go @@ -277,16 +277,28 @@ func (s *Service) UpdateASG(machinePoolScope *scope.MachinePoolScope) error { return fmt.Errorf("getting subnets for ASG: %w", err) } + minSize := machinePoolScope.AWSMachinePool.Spec.MinSize + maxSize := machinePoolScope.AWSMachinePool.Spec.MaxSize + input := &autoscaling.UpdateAutoScalingGroupInput{ AutoScalingGroupName: aws.String(machinePoolScope.Name()), // TODO: define dynamically - borrow logic from ec2 - MaxSize: aws.Int32(machinePoolScope.AWSMachinePool.Spec.MaxSize), - MinSize: aws.Int32(machinePoolScope.AWSMachinePool.Spec.MinSize), + MaxSize: aws.Int32(maxSize), + MinSize: aws.Int32(minSize), VPCZoneIdentifier: aws.String(strings.Join(subnetIDs, ",")), CapacityRebalance: aws.Bool(machinePoolScope.AWSMachinePool.Spec.CapacityRebalance), } if machinePoolScope.MachinePool.Spec.Replicas != nil && !annotations.ReplicasManagedByExternalAutoscaler(machinePoolScope.MachinePool) { - input.DesiredCapacity = aws.Int32(*machinePoolScope.MachinePool.Spec.Replicas) + replicas := *machinePoolScope.MachinePool.Spec.Replicas + + if replicas < minSize { + replicas = minSize + } + if replicas > maxSize { + replicas = maxSize + } + + input.DesiredCapacity = aws.Int32(replicas) } if machinePoolScope.AWSMachinePool.Spec.MixedInstancesPolicy != nil { diff --git a/pkg/cloud/services/autoscaling/autoscalinggroup_test.go b/pkg/cloud/services/autoscaling/autoscalinggroup_test.go index 392fbfc93f..37a841f241 100644 --- a/pkg/cloud/services/autoscaling/autoscalinggroup_test.go +++ b/pkg/cloud/services/autoscaling/autoscalinggroup_test.go @@ -764,6 +764,42 @@ func TestServiceUpdateASG(t *testing.T) { }) }, }, + { + name: "replicas below minSize", + machinePoolName: "update-asg-replicas-below-min", + wantErr: false, + setupMachinePoolScope: func(mps *scope.MachinePoolScope) { + mps.MachinePool.Spec.Replicas = ptr.To[int32](0) + mps.AWSMachinePool.Spec.MinSize = 2 + mps.AWSMachinePool.Spec.MaxSize = 5 + }, + expect: func(e *mocks.MockEC2APIMockRecorder, m *mock_autoscalingiface.MockAutoScalingAPIMockRecorder, g *WithT) { + m.UpdateAutoScalingGroup(context.TODO(), gomock.AssignableToTypeOf(&autoscaling.UpdateAutoScalingGroupInput{})).DoAndReturn(func(ctx context.Context, input *autoscaling.UpdateAutoScalingGroupInput, options ...autoscaling.Options) (*autoscaling.UpdateAutoScalingGroupOutput, error) { + g.Expect(input.MinSize).To(BeComparableTo(ptr.To[int32](2))) + g.Expect(input.MaxSize).To(BeComparableTo(ptr.To[int32](5))) + g.Expect(input.DesiredCapacity).To(BeComparableTo(ptr.To[int32](2))) + return &autoscaling.UpdateAutoScalingGroupOutput{}, nil + }) + }, + }, + { + name: "replicas above maxSize", + machinePoolName: "update-asg-replicas-above-max", + wantErr: false, + setupMachinePoolScope: func(mps *scope.MachinePoolScope) { + mps.MachinePool.Spec.Replicas = ptr.To[int32](10) + mps.AWSMachinePool.Spec.MinSize = 1 + mps.AWSMachinePool.Spec.MaxSize = 5 + }, + expect: func(e *mocks.MockEC2APIMockRecorder, m *mock_autoscalingiface.MockAutoScalingAPIMockRecorder, g *WithT) { + m.UpdateAutoScalingGroup(context.TODO(), gomock.AssignableToTypeOf(&autoscaling.UpdateAutoScalingGroupInput{})).DoAndReturn(func(ctx context.Context, input *autoscaling.UpdateAutoScalingGroupInput, options ...autoscaling.Options) (*autoscaling.UpdateAutoScalingGroupOutput, error) { + g.Expect(input.MinSize).To(BeComparableTo(ptr.To[int32](1))) + g.Expect(input.MaxSize).To(BeComparableTo(ptr.To[int32](5))) + g.Expect(input.DesiredCapacity).To(BeComparableTo(ptr.To[int32](5))) + return &autoscaling.UpdateAutoScalingGroupOutput{}, nil + }) + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cloud/services/eks/nodegroup.go b/pkg/cloud/services/eks/nodegroup.go index e89c97b16a..62cf45052c 100644 --- a/pkg/cloud/services/eks/nodegroup.go +++ b/pkg/cloud/services/eks/nodegroup.go @@ -94,23 +94,35 @@ func (s *NodegroupService) describeASGs(ctx context.Context, ng *ekstypes.Nodegr } func (s *NodegroupService) scalingConfig() *ekstypes.NodegroupScalingConfig { - var replicas int32 = 1 - if s.scope.MachinePool.Spec.Replicas != nil { - replicas = *s.scope.MachinePool.Spec.Replicas - } - cfg := ekstypes.NodegroupScalingConfig{ - DesiredSize: aws.Int32(replicas), - } + cfg := ekstypes.NodegroupScalingConfig{} + scaling := s.scope.ManagedMachinePool.Spec.Scaling - if scaling == nil { + if scaling != nil { + if scaling.MaxSize != nil { + cfg.MaxSize = aws.Int32(*scaling.MaxSize) + } + if scaling.MinSize != nil { + cfg.MinSize = aws.Int32(*scaling.MinSize) + } + } + + if annotations.ReplicasManagedByExternalAutoscaler(s.scope.MachinePool) { return &cfg } - if scaling.MaxSize != nil { - cfg.MaxSize = aws.Int32(*scaling.MaxSize) + + replicas := int32(1) + if s.scope.MachinePool.Spec.Replicas != nil { + replicas = *s.scope.MachinePool.Spec.Replicas + } + + if cfg.MinSize != nil && replicas < *cfg.MinSize { + replicas = *cfg.MinSize } - if scaling.MaxSize != nil { - cfg.MinSize = aws.Int32(*scaling.MinSize) + if cfg.MaxSize != nil && replicas > *cfg.MaxSize { + replicas = *cfg.MaxSize } + + cfg.DesiredSize = aws.Int32(replicas) return &cfg } diff --git a/pkg/cloud/services/eks/nodegroup_test.go b/pkg/cloud/services/eks/nodegroup_test.go new file mode 100644 index 0000000000..5087485bd1 --- /dev/null +++ b/pkg/cloud/services/eks/nodegroup_test.go @@ -0,0 +1,175 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package eks + +import ( + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" + "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" +) + +func TestScalingConfig(t *testing.T) { + tests := []struct { + name string + replicas *int32 + minSize *int32 + maxSize *int32 + externalAutoscalerManaged bool + expectDesiredSize *int32 + expectMinSize *int32 + expectMaxSize *int32 + }{ + { + name: "external autoscaler", + replicas: aws.Int32(3), + minSize: aws.Int32(1), + maxSize: aws.Int32(5), + externalAutoscalerManaged: true, + expectDesiredSize: nil, + expectMinSize: aws.Int32(1), + expectMaxSize: aws.Int32(5), + }, + { + name: "replicas within bounds", + replicas: aws.Int32(3), + minSize: aws.Int32(1), + maxSize: aws.Int32(5), + expectDesiredSize: aws.Int32(3), + expectMinSize: aws.Int32(1), + expectMaxSize: aws.Int32(5), + }, + { + name: "replicas below minSize", + replicas: aws.Int32(0), + minSize: aws.Int32(2), + maxSize: aws.Int32(5), + expectDesiredSize: aws.Int32(2), + expectMinSize: aws.Int32(2), + expectMaxSize: aws.Int32(5), + }, + { + name: "replicas above maxSize", + replicas: aws.Int32(10), + minSize: aws.Int32(1), + maxSize: aws.Int32(5), + expectDesiredSize: aws.Int32(5), + expectMinSize: aws.Int32(1), + expectMaxSize: aws.Int32(5), + }, + { + name: "nil replicas defaults to 1", + replicas: nil, + minSize: aws.Int32(0), + maxSize: aws.Int32(5), + expectDesiredSize: aws.Int32(1), + expectMinSize: aws.Int32(0), + expectMaxSize: aws.Int32(5), + }, + { + name: "nil replicas clamped to minSize", + replicas: nil, + minSize: aws.Int32(3), + maxSize: aws.Int32(5), + expectDesiredSize: aws.Int32(3), + expectMinSize: aws.Int32(3), + expectMaxSize: aws.Int32(5), + }, + { + name: "no scaling config", + replicas: aws.Int32(3), + minSize: nil, + maxSize: nil, + expectDesiredSize: aws.Int32(3), + expectMinSize: nil, + expectMaxSize: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + machinePool := &clusterv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pool", + Namespace: "default", + }, + Spec: clusterv1.MachinePoolSpec{ + Replicas: tc.replicas, + }, + } + + if tc.externalAutoscalerManaged { + machinePool.ObjectMeta.Annotations = map[string]string{ + clusterv1.ReplicasManagedByAnnotation: "cluster-autoscaler", + } + } + + var scaling *expinfrav1.ManagedMachinePoolScaling + if tc.minSize != nil || tc.maxSize != nil { + scaling = &expinfrav1.ManagedMachinePoolScaling{ + MinSize: tc.minSize, + MaxSize: tc.maxSize, + } + } + + managedMachinePool := &expinfrav1.AWSManagedMachinePool{ + Spec: expinfrav1.AWSManagedMachinePoolSpec{ + Scaling: scaling, + }, + } + + mockScope := &scope.ManagedMachinePoolScope{ + MachinePool: machinePool, + ManagedMachinePool: managedMachinePool, + } + + service := &NodegroupService{ + scope: mockScope, + } + + cfg := service.scalingConfig() + + if tc.expectDesiredSize == nil { + g.Expect(cfg.DesiredSize).To(BeNil()) + } else { + g.Expect(cfg.DesiredSize).ToNot(BeNil()) + g.Expect(*cfg.DesiredSize).To(Equal(*tc.expectDesiredSize)) + } + + if tc.expectMinSize == nil { + g.Expect(cfg.MinSize).To(BeNil()) + } else { + g.Expect(cfg.MinSize).ToNot(BeNil()) + g.Expect(*cfg.MinSize).To(Equal(*tc.expectMinSize)) + } + + if tc.expectMaxSize == nil { + g.Expect(cfg.MaxSize).To(BeNil()) + } else { + g.Expect(cfg.MaxSize).ToNot(BeNil()) + g.Expect(*cfg.MaxSize).To(Equal(*tc.expectMaxSize)) + } + }) + } +}