From 7337a346ecfa68615ad921e47c717c01705b11fe Mon Sep 17 00:00:00 2001 From: Clint Date: Thu, 29 Sep 2016 13:01:09 -0500 Subject: [PATCH] provider/aws: Fix issue with updating ELB subnets for subnets in the same AZ (#9131) * provider/aws: Regression test for #9120 * provider/aws: Fix issue with updating ELB subnets for subnets in the same AZ --- builtin/providers/aws/resource_aws_elb.go | 40 +++-- .../providers/aws/resource_aws_elb_test.go | 154 ++++++++++++++++++ 2 files changed, 181 insertions(+), 13 deletions(-) diff --git a/builtin/providers/aws/resource_aws_elb.go b/builtin/providers/aws/resource_aws_elb.go index 84323a698..6f2fb610c 100644 --- a/builtin/providers/aws/resource_aws_elb.go +++ b/builtin/providers/aws/resource_aws_elb.go @@ -695,19 +695,6 @@ func resourceAwsElbUpdate(d *schema.ResourceData, meta interface{}) error { removed := expandStringList(os.Difference(ns).List()) added := expandStringList(ns.Difference(os).List()) - if len(added) > 0 { - attachOpts := &elb.AttachLoadBalancerToSubnetsInput{ - LoadBalancerName: aws.String(d.Id()), - Subnets: added, - } - - log.Printf("[DEBUG] ELB attach subnets opts: %s", attachOpts) - _, err := elbconn.AttachLoadBalancerToSubnets(attachOpts) - if err != nil { - return fmt.Errorf("Failure adding ELB subnets: %s", err) - } - } - if len(removed) > 0 { detachOpts := &elb.DetachLoadBalancerFromSubnetsInput{ LoadBalancerName: aws.String(d.Id()), @@ -721,6 +708,33 @@ func resourceAwsElbUpdate(d *schema.ResourceData, meta interface{}) error { } } + if len(added) > 0 { + attachOpts := &elb.AttachLoadBalancerToSubnetsInput{ + LoadBalancerName: aws.String(d.Id()), + Subnets: added, + } + + log.Printf("[DEBUG] ELB attach subnets opts: %s", attachOpts) + err := resource.Retry(1*time.Minute, func() *resource.RetryError { + _, err := elbconn.AttachLoadBalancerToSubnets(attachOpts) + if err != nil { + if awsErr, ok := err.(awserr.Error); ok { + // eventually consistent issue with removing a subnet in AZ1 and + // immediately adding a new one in the same AZ + if awsErr.Code() == "InvalidConfigurationRequest" && strings.Contains(awsErr.Message(), "cannot be attached to multiple subnets in the same AZ") { + log.Printf("[DEBUG] retrying az association") + return resource.RetryableError(awsErr) + } + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { + return fmt.Errorf("Failure adding ELB subnets: %s", err) + } + } + d.SetPartial("subnets") } diff --git a/builtin/providers/aws/resource_aws_elb_test.go b/builtin/providers/aws/resource_aws_elb_test.go index 6a676fd4d..0115705fd 100644 --- a/builtin/providers/aws/resource_aws_elb_test.go +++ b/builtin/providers/aws/resource_aws_elb_test.go @@ -248,6 +248,36 @@ func TestAccAWSELB_iam_server_cert(t *testing.T) { }) } +func TestAccAWSELB_swap_subnets(t *testing.T) { + var conf elb.LoadBalancerDescription + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_elb.ourapp", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSELBDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSELBConfig_subnets, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSELBExists("aws_elb.ourapp", &conf), + resource.TestCheckResourceAttr( + "aws_elb.ourapp", "subnets.#", "2"), + ), + }, + + resource.TestStep{ + Config: testAccAWSELBConfig_subnet_swap, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSELBExists("aws_elb.ourapp", &conf), + resource.TestCheckResourceAttr( + "aws_elb.ourapp", "subnets.#", "2"), + ), + }, + }, + }) +} + func testAccLoadTags(conf *elb.LoadBalancerDescription, td *elb.TagDescription) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).elbconn @@ -1329,3 +1359,127 @@ resource "aws_elb" "bar" { } `, certName) } + +const testAccAWSELBConfig_subnets = ` +provider "aws" { + region = "us-west-2" +} + +resource "aws_vpc" "azelb" { + cidr_block = "10.1.0.0/16" + enable_dns_hostnames = true + + tags { + Name = "subnet-vpc" + } +} + +resource "aws_subnet" "public_a_one" { + vpc_id = "${aws_vpc.azelb.id}" + + cidr_block = "10.1.1.0/24" + availability_zone = "us-west-2a" +} + +resource "aws_subnet" "public_b_one" { + vpc_id = "${aws_vpc.azelb.id}" + + cidr_block = "10.1.7.0/24" + availability_zone = "us-west-2b" +} + +resource "aws_subnet" "public_a_two" { + vpc_id = "${aws_vpc.azelb.id}" + + cidr_block = "10.1.2.0/24" + availability_zone = "us-west-2a" +} + +resource "aws_elb" "ourapp" { + name = "terraform-asg-deployment-example" + + subnets = [ + "${aws_subnet.public_a_one.id}", + "${aws_subnet.public_b_one.id}", + ] + + listener { + instance_port = 80 + instance_protocol = "http" + lb_port = 80 + lb_protocol = "http" + } + + depends_on = ["aws_internet_gateway.gw"] +} + +resource "aws_internet_gateway" "gw" { + vpc_id = "${aws_vpc.azelb.id}" + + tags { + Name = "main" + } +} +` + +const testAccAWSELBConfig_subnet_swap = ` +provider "aws" { + region = "us-west-2" +} + +resource "aws_vpc" "azelb" { + cidr_block = "10.1.0.0/16" + enable_dns_hostnames = true + + tags { + Name = "subnet-vpc" + } +} + +resource "aws_subnet" "public_a_one" { + vpc_id = "${aws_vpc.azelb.id}" + + cidr_block = "10.1.1.0/24" + availability_zone = "us-west-2a" +} + +resource "aws_subnet" "public_b_one" { + vpc_id = "${aws_vpc.azelb.id}" + + cidr_block = "10.1.7.0/24" + availability_zone = "us-west-2b" +} + +resource "aws_subnet" "public_a_two" { + vpc_id = "${aws_vpc.azelb.id}" + + cidr_block = "10.1.2.0/24" + availability_zone = "us-west-2a" +} + +resource "aws_elb" "ourapp" { + name = "terraform-asg-deployment-example" + + subnets = [ + "${aws_subnet.public_a_two.id}", + "${aws_subnet.public_b_one.id}", + ] + + listener { + instance_port = 80 + instance_protocol = "http" + lb_port = 80 + lb_protocol = "http" + } + + depends_on = ["aws_internet_gateway.gw"] +} + +resource "aws_internet_gateway" "gw" { + vpc_id = "${aws_vpc.azelb.id}" + + tags { + Name = "main" + } +} +`