From d1209065e9a34aa210aed56309438cf882840def Mon Sep 17 00:00:00 2001 From: Vacant2333 Date: Fri, 1 Nov 2024 16:59:33 +0800 Subject: [PATCH 1/4] fix: avoid selecting subnets with insufficient available IP addresses Signed-off-by: Vacant2333 --- pkg/providers/subnet/subnet.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/providers/subnet/subnet.go b/pkg/providers/subnet/subnet.go index 959b8bc4ddc8..45c5e7848c2c 100644 --- a/pkg/providers/subnet/subnet.go +++ b/pkg/providers/subnet/subnet.go @@ -168,6 +168,13 @@ func (p *DefaultProvider) ZonalSubnetsForLaunch(ctx context.Context, nodeClass * if trackedIPs, ok := p.inflightIPs[subnet.ID]; ok { prevIPs = trackedIPs } + + // Check if the remaining IP count is insufficient to meet the predicted IP usage; + // if so, remove this subnet zone record from inflightIPs and continue to the next item in the loop。 + if prevIPs-predictedIPsUsed < 0 { + delete(zonalSubnets, subnet.Zone) + continue + } p.inflightIPs[subnet.ID] = prevIPs - predictedIPsUsed } return zonalSubnets, nil From 282a26118a9ed6bc0770e8d088e6e8156d554622 Mon Sep 17 00:00:00 2001 From: tal balash ios <12tooltool@gmail.com> Date: Sat, 21 Dec 2024 18:33:33 +0200 Subject: [PATCH 2/4] added a test where there is no ips --- pkg/providers/instance/suite_test.go | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pkg/providers/instance/suite_test.go b/pkg/providers/instance/suite_test.go index 58b3ebdecf63..508bec9f088d 100644 --- a/pkg/providers/instance/suite_test.go +++ b/pkg/providers/instance/suite_test.go @@ -205,4 +205,40 @@ var _ = Describe("InstanceProvider", func() { retrievedIDs := sets.New[string](lo.Map(instances, func(i *instance.Instance, _ int) string { return i.ID })...) Expect(ids.Equal(retrievedIDs)).To(BeTrue()) }) + It("should not consider subnet with no available IPs for instance creation", func() { + // Prepare the context, nodeClass, and nodeClaim as in the other tests + ExpectApplied(ctx, env.Client, nodeClaim, nodePool, nodeClass) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + + awsEnv.EC2API.Subnets.Store(subnetID, ec2types.Subnet{ + SubnetId: aws.String(fake.subnetID), + AvailableIpAddressCount: aws.Int32(0), // 0 available IPs + AvailabilityZone: aws.String(fake.AvailabilityZone), + }) + + // Update the EC2 API mock to include this subnet + awsEnv.EC2API.DescribeSubnetsReturns = func(input *ec2.DescribeSubnetsInput) (*ec2.DescribeSubnetsOutput, error) { + return &ec2.DescribeSubnetsOutput{ + Subnets: []ec2types.Subnet{ + { + SubnetId: aws.String(fake.subnetID), + AvailableIpAddressCount: aws.Int32(0), // 0 available IPs + AvailabilityZone: aws.String(fake.AvailabilityZone), + }, + }, + }, nil + } + + // Now we attempt to create instances + instanceTypes, err := cloudProvider.GetInstanceTypes(ctx, nodePool) + Expect(err).ToNot(HaveOccurred()) + + instanceTypes = lo.Filter(instanceTypes, func(i *corecloudprovider.InstanceType, _ int) bool { return i.Name == "m5.xlarge" }) + instance, err := awsEnv.InstanceProvider.Create(ctx, nodeClass, nodeClaim, nil, instanceTypes) + + // Assert that the instance was not created due to IP exhaustion + Expect(corecloudprovider.IsInsufficientCapacityError(err)).To(BeTrue()) + Expect(instance).To(BeNil()) + }) + }) From 15b9f6176da5fbb241e9cbca5c8ea9332de4974c Mon Sep 17 00:00:00 2001 From: Tal Balash <12tooltool@gmail.com> Date: Wed, 22 Jan 2025 23:36:53 +0200 Subject: [PATCH 3/4] fix test and ci errors --- pkg/providers/instance/suite_test.go | 60 ++++++++++++++-------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/pkg/providers/instance/suite_test.go b/pkg/providers/instance/suite_test.go index 508bec9f088d..0ba947c2c6e7 100644 --- a/pkg/providers/instance/suite_test.go +++ b/pkg/providers/instance/suite_test.go @@ -205,40 +205,38 @@ var _ = Describe("InstanceProvider", func() { retrievedIDs := sets.New[string](lo.Map(instances, func(i *instance.Instance, _ int) string { return i.ID })...) Expect(ids.Equal(retrievedIDs)).To(BeTrue()) }) - It("should not consider subnet with no available IPs for instance creation", func() { + It("should not consider subnet with no available IPs for instance creation", func() { // Prepare the context, nodeClass, and nodeClaim as in the other tests ExpectApplied(ctx, env.Client, nodeClaim, nodePool, nodeClass) nodeClass = ExpectExists(ctx, env.Client, nodeClass) - - awsEnv.EC2API.Subnets.Store(subnetID, ec2types.Subnet{ - SubnetId: aws.String(fake.subnetID), - AvailableIpAddressCount: aws.Int32(0), // 0 available IPs - AvailabilityZone: aws.String(fake.AvailabilityZone), - }) - + // Update the EC2 API mock to include this subnet - awsEnv.EC2API.DescribeSubnetsReturns = func(input *ec2.DescribeSubnetsInput) (*ec2.DescribeSubnetsOutput, error) { - return &ec2.DescribeSubnetsOutput{ - Subnets: []ec2types.Subnet{ - { - SubnetId: aws.String(fake.subnetID), - AvailableIpAddressCount: aws.Int32(0), // 0 available IPs - AvailabilityZone: aws.String(fake.AvailabilityZone), - }, - }, - }, nil - } - - // Now we attempt to create instances - instanceTypes, err := cloudProvider.GetInstanceTypes(ctx, nodePool) - Expect(err).ToNot(HaveOccurred()) - - instanceTypes = lo.Filter(instanceTypes, func(i *corecloudprovider.InstanceType, _ int) bool { return i.Name == "m5.xlarge" }) - instance, err := awsEnv.InstanceProvider.Create(ctx, nodeClass, nodeClaim, nil, instanceTypes) - - // Assert that the instance was not created due to IP exhaustion - Expect(corecloudprovider.IsInsufficientCapacityError(err)).To(BeTrue()) - Expect(instance).To(BeNil()) + awsEnv.EC2API.DescribeSubnetsOutput.Set(&ec2.DescribeSubnetsOutput{ + Subnets: []ec2types.Subnet{ + { + SubnetId: aws.String("test-subnet-1"), + AvailabilityZone: aws.String("test-zone-1a"), + AvailableIpAddressCount: aws.Int32(0), // Exhausted + Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("test-subnet-1")}}, + }, + { + SubnetId: aws.String("test-subnet-2"), + AvailabilityZone: aws.String("test-zone-1b"), + AvailableIpAddressCount: aws.Int32(5), // Has IPs + Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("test-subnet-2")}}, + }, + }, + }) + + instanceTypes, err := cloudProvider.GetInstanceTypes(ctx, nodePool) + Expect(err).ToNot(HaveOccurred()) + + instanceTypes = lo.Filter(instanceTypes, func(i *corecloudprovider.InstanceType, _ int) bool { return i.Name == "m5.xlarge" }) + instance, err := awsEnv.InstanceProvider.Create(ctx, nodeClass, nodeClaim, nil, instanceTypes) + + // Assert that the instance is created using the subnet with available IPs + Expect(err).ToNot(HaveOccurred()) + Expect(instance).ToNot(BeNil()) + Expect(instance.SubnetID).To(Equal("test-subnet-2")) }) - }) From bbe5e6ba6fa31020d9f19d870dd35990dcde460f Mon Sep 17 00:00:00 2001 From: Tal Balash <12tooltool@gmail.com> Date: Thu, 23 Jan 2025 08:05:06 +0200 Subject: [PATCH 4/4] fixes imports --- pkg/providers/instance/suite_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/providers/instance/suite_test.go b/pkg/providers/instance/suite_test.go index 0ba947c2c6e7..e999eb6fef08 100644 --- a/pkg/providers/instance/suite_test.go +++ b/pkg/providers/instance/suite_test.go @@ -23,6 +23,7 @@ import ( "sigs.k8s.io/karpenter/pkg/test/v1alpha1" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/ec2" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/awslabs/operatorpkg/object" "github.com/samber/lo"