-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Batch up requests with more than 10 target group changes #10435
Conversation
c2fb983
to
8cb02b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @beezly 👋 Thanks so much for submitting this fix! This appears to be on the right track, but it requires a covering acceptance test (example described below) which uncovered that we need to wait until each batch is fully applied, otherwise it will still return the same error.
Once the acceptance testing and batch waiting is added, we should be able to get this in. Thanks again.
Failing test prior to code fix:
--- FAIL: TestAccAWSAutoScalingGroup_TargetGroupArns (81.59s)
testing.go:569: Step 2 error: errors during apply:
Error: Error updating Load Balancers Target Groups for AutoScaling Group (tf-asg-2019101000215510330000000d), error: ValidationError: Trying to update too many Load Balancers/Target Groups at once. The limit is 10
Associated testing code:
// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/256
func TestAccAWSAutoScalingGroup_TargetGroupArns(t *testing.T) {
var group autoscaling.Group
resourceName := "aws_autoscaling_group.test"
rName := acctest.RandomWithPrefix("tf-acc-test")
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSAutoScalingGroupConfig_TargetGroupArns(rName, 11),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAutoScalingGroupExists(resourceName, &group),
resource.TestCheckResourceAttr(resourceName, "target_group_arns.#", "11"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"force_delete",
"initial_lifecycle_hook",
"name_prefix",
"tag",
"tags",
"wait_for_capacity_timeout",
"wait_for_elb_capacity",
},
},
{
Config: testAccAWSAutoScalingGroupConfig_TargetGroupArns(rName, 0),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAutoScalingGroupExists(resourceName, &group),
resource.TestCheckResourceAttr(resourceName, "target_group_arns.#", "0"),
),
},
{
Config: testAccAWSAutoScalingGroupConfig_TargetGroupArns(rName, 11),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAutoScalingGroupExists(resourceName, &group),
resource.TestCheckResourceAttr(resourceName, "target_group_arns.#", "11"),
),
},
},
})
}
func testAccAWSAutoScalingGroupConfig_TargetGroupArns(rName string, tgCount int) string {
return fmt.Sprintf(`
data "aws_ami" "test" {
most_recent = true
owners = ["amazon"]
filter {
name = "name"
values = ["amzn-ami-hvm-*-x86_64-gp2"]
}
}
data "aws_availability_zones" "available" {
state = "available"
}
resource "aws_launch_template" "test" {
image_id = data.aws_ami.test.id
instance_type = "t3.micro"
name = %[1]q
}
resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"
tags = {
Name = %[1]q
}
}
resource "aws_subnet" "test" {
availability_zone = data.aws_availability_zones.available.names[0]
cidr_block = "10.0.0.0/24"
vpc_id = aws_vpc.test.id
tags = {
Name = %[1]q
}
}
resource "aws_lb_target_group" "test" {
count = %[2]d
port = 80
protocol = "HTTP"
vpc_id = "${aws_vpc.test.id}"
}
resource "aws_autoscaling_group" "test" {
force_delete = true
max_size = 0
min_size = 0
target_group_arns = length(aws_lb_target_group.test) > 0 ? aws_lb_target_group.test[*].arn : []
vpc_zone_identifier = [aws_subnet.test.id]
launch_template {
id = aws_launch_template.test.id
}
}
`, rName, tgCount)
}
New test passes after adding the batch waiting:
--- PASS: TestAccAWSAutoScalingGroup_TargetGroupArns (187.93s)
… by 10 to prevent API and rate limiting errors Reference: #256 Reference: #10435 Reference: https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_AttachLoadBalancers.html Reference: https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_DetachLoadBalancers.html This change is complementary to the target group batching. The AutoScaling API for `AttachLoadBalancers` and `DetachLoadBalancers` only allow 10 elements at a time. In addition to the batching to split the API requests, we must wait for the batch to fully complete before moving onto the next batch, otherwise the API returns a rate limiting error: ``` --- FAIL: TestAccAWSAutoScalingGroup_LoadBalancers (360.22s) testing.go:569: Step 2 error: errors during apply: Error: Error updating Load Balancers for AutoScaling Group (tf-asg-2019101000090127270000000d), error: ValidationError: Trying to update too many Load Balancers/Target Groups at once. The limit is 10 ``` Output from acceptance testing: ``` --- PASS: TestAccAWSAutoScalingGroup_LoadBalancers (443.84s) ```
Co-Authored-By: Brian Flad <bflad417@gmail.com>
cf62651
to
d4fe206
Compare
@bflad thanks for the review! I've added in the new test you suggested and put the waitFor functions in. The new acceptance test now passes...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @beezly, thanks for the update 🚀
--- PASS: TestAccAWSAutoScalingGroup_serviceLinkedRoleARN (47.18s)
--- PASS: TestAccAWSAutoScalingGroup_namePrefix (55.63s)
--- PASS: TestAccAWSAutoScalingGroup_autoGeneratedName (81.21s)
--- PASS: TestAccAWSAutoScalingGroup_withMetrics (81.10s)
--- PASS: TestAccAWSAutoScalingGroup_VpcUpdates (99.23s)
--- PASS: TestAccAWSAutoScalingGroup_terminationPolicies (105.44s)
--- PASS: TestAccAWSAutoScalingGroup_classicVpcZoneIdentifier (108.41s)
--- PASS: TestAccAWSAutoScalingGroup_emptyAvailabilityZones (110.63s)
--- PASS: TestAccAWSAutoScalingGroup_LaunchTemplate_IAMInstanceProfile (71.64s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandAllocationStrategy (49.02s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy (49.92s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotInstancePools (41.75s)
--- PASS: TestAccAWSAutoScalingGroup_withPlacementGroup (181.76s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_LaunchTemplateName (52.19s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandPercentageAboveBaseCapacity (85.01s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_Override_InstanceType (38.75s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandBaseCapacity (100.72s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotMaxPrice (77.26s)
--- PASS: TestAccAWSAutoScalingGroup_launchTemplate_update (165.60s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotAllocationStrategy (111.32s)
--- PASS: TestAccAWSAutoScalingGroup_suspendingProcesses (220.69s)
--- PASS: TestAccAWSAutoScalingGroup_initialLifecycleHook (225.24s)
--- PASS: TestAccAWSAutoScalingGroup_TargetGroupArns (233.59s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_Version (104.85s)
--- PASS: TestAccAWSAutoScalingGroup_launchTemplate (256.90s)
--- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups (257.96s)
--- PASS: TestAccAWSAutoScalingGroup_basic (267.00s)
--- PASS: TestAccAWSAutoScalingGroup_enablingMetrics (291.22s)
--- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups_ELBCapacity (317.17s)
--- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer_ToTargetGroup (330.08s)
--- PASS: TestAccAWSAutoScalingGroup_tags (331.60s)
--- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer (443.13s)
This has been released in version 2.32.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
:parrot.gif: |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
The AWS API only supports adding/removing 10 requests at a time.
Community Note
Closes #256
Release note for CHANGELOG:
Output from acceptance testing: