Skip to content
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

resource/aws_autoscaling_group: Batch ELB attachments and detachments by 10 to prevent API and rate limiting errors #10445

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 118 additions & 12 deletions aws/resource_aws_autoscaling_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -884,22 +884,56 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{})
add := expandStringList(ns.Difference(os).List())

if len(remove) > 0 {
_, err := conn.DetachLoadBalancers(&autoscaling.DetachLoadBalancersInput{
AutoScalingGroupName: aws.String(d.Id()),
LoadBalancerNames: remove,
})
if err != nil {
return fmt.Errorf("Error updating Load Balancers for AutoScaling Group (%s), error: %s", d.Id(), err)
// API only supports removing 10 at a time
var batches [][]*string

batchSize := 10

for batchSize < len(remove) {
remove, batches = remove[batchSize:], append(batches, remove[0:batchSize:batchSize])
}
batches = append(batches, remove)

for _, batch := range batches {
_, err := conn.DetachLoadBalancers(&autoscaling.DetachLoadBalancersInput{
AutoScalingGroupName: aws.String(d.Id()),
LoadBalancerNames: batch,
})

if err != nil {
return fmt.Errorf("error detaching AutoScaling Group (%s) Load Balancers: %s", d.Id(), err)
}

if err := waitUntilAutoscalingGroupLoadBalancersRemoved(conn, d.Id()); err != nil {
return fmt.Errorf("error describing AutoScaling Group (%s) Load Balancers being removed: %s", d.Id(), err)
}
}
}

if len(add) > 0 {
_, err := conn.AttachLoadBalancers(&autoscaling.AttachLoadBalancersInput{
AutoScalingGroupName: aws.String(d.Id()),
LoadBalancerNames: add,
})
if err != nil {
return fmt.Errorf("Error updating Load Balancers for AutoScaling Group (%s), error: %s", d.Id(), err)
// API only supports adding 10 at a time
batchSize := 10

var batches [][]*string

for batchSize < len(add) {
add, batches = add[batchSize:], append(batches, add[0:batchSize:batchSize])
}
batches = append(batches, add)

for _, batch := range batches {
_, err := conn.AttachLoadBalancers(&autoscaling.AttachLoadBalancersInput{
AutoScalingGroupName: aws.String(d.Id()),
LoadBalancerNames: batch,
})

if err != nil {
return fmt.Errorf("error attaching AutoScaling Group (%s) Load Balancers: %s", d.Id(), err)
}

if err := waitUntilAutoscalingGroupLoadBalancersAdded(conn, d.Id()); err != nil {
return fmt.Errorf("error describing AutoScaling Group (%s) Load Balancers being added: %s", d.Id(), err)
}
}
}
}
Expand Down Expand Up @@ -1493,3 +1527,75 @@ func flattenAutoScalingMixedInstancesPolicy(mixedInstancesPolicy *autoscaling.Mi

return []interface{}{m}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever a case when we'd want to set a timeout on these functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question and yes, there might be! Quickly scanning the recent issues, I don't see any complaints about the similar change to the target group handling (that also does not have timeouts), but for now this is dependent on the AWS side entering some sort of failure state rather than staying in the adding/deleting state forever in those cases. 🤞

func waitUntilAutoscalingGroupLoadBalancersAdded(conn *autoscaling.AutoScaling, asgName string) error {
input := &autoscaling.DescribeLoadBalancersInput{
AutoScalingGroupName: aws.String(asgName),
}
var lbAdding bool

for {
output, err := conn.DescribeLoadBalancers(input)

if err != nil {
return err
}

for _, tg := range output.LoadBalancers {
if aws.StringValue(tg.State) == "Adding" {
lbAdding = true
break
}
}

if lbAdding {
lbAdding = false
input.NextToken = nil
continue
}

if aws.StringValue(output.NextToken) == "" {
break
}

input.NextToken = output.NextToken
}

return nil
}

func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling, asgName string) error {
input := &autoscaling.DescribeLoadBalancersInput{
AutoScalingGroupName: aws.String(asgName),
}
var lbRemoving bool

for {
output, err := conn.DescribeLoadBalancers(input)

if err != nil {
return err
}

for _, tg := range output.LoadBalancers {
if aws.StringValue(tg.State) == "Removing" {
lbRemoving = true
break
}
}

if lbRemoving {
lbRemoving = false
input.NextToken = nil
continue
}

if aws.StringValue(output.NextToken) == "" {
break
}

input.NextToken = output.NextToken
}

return nil
}
122 changes: 122 additions & 0 deletions aws/resource_aws_autoscaling_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,56 @@ func TestAccAWSAutoScalingGroup_LaunchTemplate_IAMInstanceProfile(t *testing.T)
})
}

// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/256
func TestAccAWSAutoScalingGroup_LoadBalancers(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_LoadBalancers(rName, 11),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAutoScalingGroupExists(resourceName, &group),
resource.TestCheckResourceAttr(resourceName, "load_balancers.#", "11"),
),
},
{
ResourceName: resourceName,
ImportState: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎊

ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"force_delete",
"initial_lifecycle_hook",
"name_prefix",
"tag",
"tags",
"wait_for_capacity_timeout",
"wait_for_elb_capacity",
},
},
{
Config: testAccAWSAutoScalingGroupConfig_LoadBalancers(rName, 0),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAutoScalingGroupExists(resourceName, &group),
resource.TestCheckResourceAttr(resourceName, "load_balancers.#", "0"),
),
},
{
Config: testAccAWSAutoScalingGroupConfig_LoadBalancers(rName, 11),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAutoScalingGroupExists(resourceName, &group),
resource.TestCheckResourceAttr(resourceName, "load_balancers.#", "11"),
),
},
},
})
}

func TestAccAWSAutoScalingGroup_MixedInstancesPolicy(t *testing.T) {
var group autoscaling.Group
resourceName := "aws_autoscaling_group.test"
Expand Down Expand Up @@ -3285,6 +3335,78 @@ resource "aws_autoscaling_group" "test" {
`, rName, rName, rName, rName)
}

func testAccAWSAutoScalingGroupConfig_LoadBalancers(rName string, elbCount 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_internet_gateway" "test" {
vpc_id = aws_vpc.test.id
}

resource "aws_elb" "test" {
count = %[2]d
depends_on = ["aws_internet_gateway.test"]

subnets = [aws_subnet.test.id]

listener {
instance_port = 80
instance_protocol = "http"
lb_port = 80
lb_protocol = "http"
}
}

resource "aws_autoscaling_group" "test" {
force_delete = true
max_size = 0
min_size = 0
load_balancers = length(aws_elb.test) > 0 ? aws_elb.test[*].name : []
vpc_zone_identifier = [aws_subnet.test.id]

launch_template {
id = aws_launch_template.test.id
}
}
`, rName, elbCount)
}

func testAccAWSAutoScalingGroupConfig_MixedInstancesPolicy_Base(rName string) string {
return fmt.Sprintf(`
data "aws_ami" "test" {
Expand Down