Skip to content

Commit

Permalink
service/ec2: Handle additional EIP association issues and remove EIP …
Browse files Browse the repository at this point in the history 
…hardcoded us-east-1 environment variable handling in tests (#16032)

Reference: #8316
Reference: #15737
Reference: #15791
Reference: #16018 (to run testing locally)

Changes:

```
* resource/aws_eip: In EC2-Classic, wait until Instance returns as associated during create or update
* resource/aws_eip_association: Retry on additional EC2 Address eventual consistency errors on creation
* resource/aws_eip_association: In EC2-Classic, wait until Instance returns as associated during creation
```

Previously in AWS Commercial:

```
=== RUN   TestAccAWSEIP_Ec2Classic
TestAccAWSEIP_Ec2Classic: resource_aws_eip_test.go:96: Step 1/2 error: After applying this test step, the plan was not empty.
stdout:
An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# aws_eip.test will be updated in-place
~ resource "aws_eip" "test" {
domain           = "standard"
id               = "52.0.143.158"
+ instance         = "i-0c0a6ad483e281c59"
public_dns       = "ec2-52-0-143-158.compute-1.amazonaws.com"
public_ip        = "52.0.143.158"
public_ipv4_pool = "amazon"
vpc              = false
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccAWSEIP_Ec2Classic (153.41s)

=== CONT  TestAccAWSEIPAssociation_ec2Classic
    resource_aws_eip_association_test.go:97: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:

        An execution plan has been generated and is shown below.
        Resource actions are indicated with the following symbols:
        -/+ destroy and then create replacement

        Terraform will perform the following actions:

          # aws_eip_association.test must be replaced
        -/+ resource "aws_eip_association" "test" {
              + allocation_id        = (known after apply)
              ~ id                   = "34.224.14.254" -> (known after apply)
              + instance_id          = "i-0ab9e7598ae44485f"
              + network_interface_id = (known after apply)
              + private_ip_address   = (known after apply)
                public_ip            = "34.224.14.254"
            }

        Plan: 1 to add, 0 to change, 1 to destroy.
--- FAIL: TestAccAWSEIPAssociation_ec2Classic (74.15s)

=== CONT  TestAccAWSEIPAssociation_ec2Classic
    resource_aws_eip_association_test.go:97: Step 1/2 error: Error running apply: 2020/11/03 09:35:32 [DEBUG] Using modified User-Agent: Terraform/0.12.29 HashiCorp-terraform-exec/0.10.0

        Error: Error associating EIP: AuthFailure: The address '34.239.37.205' does not belong to you.
          status code: 400, request id: d4163627-4987-4466-a297-aa2a48331dc9

=== CONT  TestAccAWSEIPAssociation_disappears
    resource_aws_eip_association_test.go:154: Step 1/1 error: Error running apply: 2020/11/03 09:35:33 [DEBUG] Using modified User-Agent: Terraform/0.12.29 HashiCorp-terraform-exec/0.10.0

        Error: Error associating EIP: InvalidAllocationID.NotFound: The allocation ID 'eipalloc-0780b47e4b04f970a' does not exist
          status code: 400, request id: 2c1a4d76-0ec2-45d7-9427-89d6e5de03ec

=== CONT  TestAccAWSEIPAssociation_networkInterface
    resource_aws_eip_association_test.go:43: Step 1/2 error: Error running apply: 2020/11/03 09:35:30 [DEBUG] Using modified User-Agent: Terraform/0.12.29 HashiCorp-terraform-exec/0.10.0

        Error: Error associating EIP: InvalidAllocationID.NotFound: The allocation ID 'eipalloc-071e65b698ca98f08' does not exist
          status code: 400, request id: 862d2320-e52a-45ef-854c-9cc90004bf77
```

Previously in AWS GovCloud (US):

```
=== RUN   TestAccAWSEIPAssociation_ec2Classic
TestAccAWSEIPAssociation_ec2Classic: provider_test.go:196: [{0 error configuring Terraform AWS Provider: error validating provider credentials: error calling sts:GetCallerIdentity: InvalidClientTokenId: The security token included in the request is invalid.
  status code: 403, request id: f2a9b7c4-2448-47a0-b5ea-87de84dd9b7a  []}]
--- FAIL: TestAccAWSEIPAssociation_ec2Classic (0.36s)
```

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSEIP_associated_user_private_ip (231.25s)
--- PASS: TestAccAWSEIP_basic (18.88s)
--- PASS: TestAccAWSEIP_disappears (12.62s)
--- PASS: TestAccAWSEIP_Ec2Classic (195.34s)
--- PASS: TestAccAWSEIP_instance (99.75s)
--- PASS: TestAccAWSEIP_Instance_Reassociate (126.92s)
--- PASS: TestAccAWSEIP_networkInterface (81.71s)
--- PASS: TestAccAWSEIP_notAssociated (144.46s)
--- PASS: TestAccAWSEIP_PublicIpv4Pool_default (18.84s)
--- PASS: TestAccAWSEIP_tags_Ec2Classic (7.61s)
--- PASS: TestAccAWSEIP_tags_Vpc (26.35s)
--- PASS: TestAccAWSEIP_twoEIPsOneNetworkInterface (82.93s)
--- SKIP: TestAccAWSEIP_CustomerOwnedIpv4Pool (2.47s)
--- SKIP: TestAccAWSEIP_PublicIpv4Pool_custom (0.00s)

--- PASS: TestAccAWSEIPAssociation_basic (159.35s)
--- PASS: TestAccAWSEIPAssociation_disappears (101.24s)
--- PASS: TestAccAWSEIPAssociation_ec2Classic (93.73s)
--- PASS: TestAccAWSEIPAssociation_instance (93.00s)
--- PASS: TestAccAWSEIPAssociation_networkInterface (71.25s)
--- PASS: TestAccAWSEIPAssociation_spotInstance (71.11s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- PASS: TestAccAWSEIP_associated_user_private_ip (245.88s)
--- PASS: TestAccAWSEIP_basic (23.76s)
--- PASS: TestAccAWSEIP_disappears (16.42s)
--- PASS: TestAccAWSEIP_instance (107.18s)
--- PASS: TestAccAWSEIP_Instance_Reassociate (165.24s)
--- PASS: TestAccAWSEIP_networkInterface (90.37s)
--- PASS: TestAccAWSEIP_notAssociated (146.54s)
--- PASS: TestAccAWSEIP_PublicIpv4Pool_default (24.04s)
--- PASS: TestAccAWSEIP_tags_Vpc (37.31s)
--- PASS: TestAccAWSEIP_twoEIPsOneNetworkInterface (90.85s)
--- SKIP: TestAccAWSEIP_CustomerOwnedIpv4Pool (2.86s)
--- SKIP: TestAccAWSEIP_Ec2Classic (2.89s)
--- SKIP: TestAccAWSEIP_PublicIpv4Pool_custom (0.00s)
--- SKIP: TestAccAWSEIP_tags_Ec2Classic (2.89s)

--- PASS: TestAccAWSEIPAssociation_basic (156.32s)
--- PASS: TestAccAWSEIPAssociation_disappears (130.31s)
--- PASS: TestAccAWSEIPAssociation_instance (89.29s)
--- PASS: TestAccAWSEIPAssociation_networkInterface (79.42s)
--- PASS: TestAccAWSEIPAssociation_spotInstance (68.02s)
--- SKIP: TestAccAWSEIPAssociation_ec2Classic (2.92s)
```
  • Loading branch information
@bflad
bflad authored Nov 12, 2020
1 parent 8410f8c commit 380460d
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 95 deletions.
63 changes: 63 additions & 0 deletions aws/resource_aws_eip.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,16 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

const (
// Maximum amount of time to wait for EIP association with EC2-Classic instances
ec2AddressAssociationClassicTimeout = 2 * time.Minute
)

func resourceAwsEip() *schema.Resource {
Expand Down Expand Up @@ -388,6 +395,12 @@ func resourceAwsEipUpdate(d *schema.ResourceData, meta interface{}) error {
d.Set("network_interface", "")
return fmt.Errorf("Failure associating EIP: %s", err)
}

if assocOpts.AllocationId == nil {
if err := waitForEc2AddressAssociationClassic(ec2conn, aws.StringValue(assocOpts.PublicIp), aws.StringValue(assocOpts.InstanceId)); err != nil {
return fmt.Errorf("error waiting for EC2 Address (%s) to associate with EC2-Classic Instance (%s): %w", aws.StringValue(assocOpts.PublicIp), aws.StringValue(assocOpts.InstanceId), err)
}
}
}

if d.HasChange("tags") && !d.IsNewResource() {
Expand Down Expand Up @@ -501,3 +514,53 @@ func disassociateEip(d *schema.ResourceData, meta interface{}) error {
}
return err
}

// waitForEc2AddressAssociationClassic ensures the correct Instance is associated with an Address
//
// This can take a few seconds to appear correctly for EC2-Classic addresses.
func waitForEc2AddressAssociationClassic(conn *ec2.EC2, publicIP string, instanceID string) error {
input := &ec2.DescribeAddressesInput{
Filters: []*ec2.Filter{
{
Name: aws.String("public-ip"),
Values: []*string{aws.String(publicIP)},
},
{
Name: aws.String("domain"),
Values: []*string{aws.String(ec2.DomainTypeStandard)},
},
},
}

err := resource.Retry(ec2AddressAssociationClassicTimeout, func() *resource.RetryError {
output, err := conn.DescribeAddresses(input)

if tfawserr.ErrCodeEquals(err, "InvalidAddress.NotFound") {
return resource.RetryableError(err)
}

if err != nil {
return resource.NonRetryableError(err)
}

if len(output.Addresses) == 0 || output.Addresses[0] == nil {
return resource.RetryableError(fmt.Errorf("not found"))
}

if aws.StringValue(output.Addresses[0].InstanceId) != instanceID {
return resource.RetryableError(fmt.Errorf("not associated"))
}

return nil
})

if tfresource.TimedOut(err) {
_, err = conn.DescribeAddresses(input)
}

if err != nil {
return fmt.Errorf("error describing EC2 Address (%s) association: %w", publicIP, err)
}

return nil
}
32 changes: 25 additions & 7 deletions aws/resource_aws_eip_association.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -96,12 +97,25 @@ func resourceAwsEipAssociationCreate(d *schema.ResourceData, meta interface{}) e
err := resource.Retry(2*time.Minute, func() *resource.RetryError {
var err error
resp, err = conn.AssociateAddress(request)

// EC2-VPC error for new addresses
if tfawserr.ErrCodeEquals(err, "InvalidAllocationID.NotFound") {
return resource.RetryableError(err)
}

// EC2-Classic error for new addresses
if tfawserr.ErrMessageContains(err, "AuthFailure", "does not belong to you") {
return resource.RetryableError(err)
}

if tfawserr.ErrMessageContains(err, "InvalidInstanceID", "pending instance") {
return resource.RetryableError(err)
}

if err != nil {
if isAWSErr(err, "InvalidInstanceID", "pending instance") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}

return nil
})
if isResourceTimeoutError(err) {
Expand All @@ -121,11 +135,15 @@ func resourceAwsEipAssociationCreate(d *schema.ResourceData, meta interface{}) e
supportedPlatforms, resp)
}

if resp.AssociationId == nil {
// This is required field for EC2 Classic per docs
d.SetId(*request.PublicIp)
if resp.AssociationId != nil {
d.SetId(aws.StringValue(resp.AssociationId))
} else {
d.SetId(*resp.AssociationId)
// EC2-Classic
d.SetId(aws.StringValue(request.PublicIp))

if err := waitForEc2AddressAssociationClassic(conn, aws.StringValue(request.PublicIp), aws.StringValue(request.InstanceId)); err != nil {
return fmt.Errorf("error waiting for EC2 Address (%s) to associate with EC2-Classic Instance (%s): %w", aws.StringValue(request.PublicIp), aws.StringValue(request.InstanceId), err)
}
}

return resourceAwsEipAssociationRead(d, meta)
Expand Down
132 changes: 68 additions & 64 deletions aws/resource_aws_eip_association_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package aws

import (
"fmt"
"os"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -17,9 +16,9 @@ func TestAccAWSEIPAssociation_instance(t *testing.T) {
var a ec2.Address

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: testAccProviderFactories,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEIPAssociationConfig_instance(),
Expand All @@ -42,9 +41,9 @@ func TestAccAWSEIPAssociation_networkInterface(t *testing.T) {
var a ec2.Address

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: testAccProviderFactories,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEIPAssociationConfig_networkInterface,
Expand All @@ -67,9 +66,9 @@ func TestAccAWSEIPAssociation_basic(t *testing.T) {
resourceName := "aws_eip_association.by_allocation_id"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccEC2VPCOnlyPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
PreCheck: func() { testAccPreCheck(t); testAccEC2VPCOnlyPreCheck(t) },
ProviderFactories: testAccProviderFactories,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEIPAssociationConfig(),
Expand All @@ -95,21 +94,16 @@ func TestAccAWSEIPAssociation_ec2Classic(t *testing.T) {
var a ec2.Address
resourceName := "aws_eip_association.test"

oldvar := os.Getenv("AWS_DEFAULT_REGION")
os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
defer os.Setenv("AWS_DEFAULT_REGION", oldvar)

// This test cannot run in parallel with the other EIP Association tests
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) },
ProviderFactories: testAccProviderFactories,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEIPAssociationConfig_ec2Classic,
Config: testAccAWSEIPAssociationConfig_ec2Classic(),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEIPExists("aws_eip.test", &a),
testAccCheckAWSEIPAssociationExists(resourceName, &a),
testAccCheckAWSEIPEc2ClassicExists("aws_eip.test", &a),
testAccCheckAWSEIPAssociationEc2ClassicExists(resourceName, &a),
resource.TestCheckResourceAttrSet(resourceName, "public_ip"),
resource.TestCheckResourceAttr(resourceName, "allocation_id", ""),
testAccCheckAWSEIPAssociationHasIpBasedId(resourceName),
Expand All @@ -130,9 +124,9 @@ func TestAccAWSEIPAssociation_spotInstance(t *testing.T) {
resourceName := "aws_eip_association.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: testAccProviderFactories,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEIPAssociationConfig_spotInstance(rInt),
Expand All @@ -158,9 +152,9 @@ func TestAccAWSEIPAssociation_disappears(t *testing.T) {
resourceName := "aws_eip_association.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: testAccProviderFactories,
CheckDestroy: testAccCheckAWSEIPAssociationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEIPAssociationConfigDisappears(),
Expand Down Expand Up @@ -208,6 +202,42 @@ func testAccCheckAWSEIPAssociationExists(name string, res *ec2.Address) resource
}
}

func testAccCheckAWSEIPAssociationEc2ClassicExists(name string, res *ec2.Address) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("Not found: %s", name)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No EIP Association ID is set")
}

conn := testAccProviderEc2Classic.Meta().(*AWSClient).ec2conn
platforms := testAccProviderEc2Classic.Meta().(*AWSClient).supportedplatforms

request, err := describeAddressesById(rs.Primary.ID, platforms)

if err != nil {
return err
}

describe, err := conn.DescribeAddresses(request)

if err != nil {
return fmt.Errorf("error describing EC2 Address (%s): %w", rs.Primary.ID, err)
}

if len(describe.Addresses) != 1 || aws.StringValue(describe.Addresses[0].PublicIp) != rs.Primary.ID {
return fmt.Errorf("EC2 Address (%s) not found", rs.Primary.ID)
}

*res = *describe.Addresses[0]

return nil
}
}

func testAccCheckAWSEIPAssociationHasIpBasedId(name string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
Expand Down Expand Up @@ -385,51 +415,25 @@ resource "aws_eip_association" "test" {
`)
}

const testAccAWSEIPAssociationConfig_ec2Classic = `
provider "aws" {
region = "us-east-1"
}
func testAccAWSEIPAssociationConfig_ec2Classic() string {
return composeConfig(
testAccEc2ClassicRegionProviderConfig(),
testAccLatestAmazonLinuxPvEbsAmiConfig(),
testAccAvailableEc2InstanceTypeForRegion("t1.micro", "m3.medium", "m3.large", "c3.large", "r3.large"),
`
resource "aws_eip" "test" {}
data "aws_availability_zones" "available" {
state = "available"
filter {
name = "opt-in-status"
values = ["opt-in-not-required"]
}
}
data "aws_ami" "ubuntu" {
most_recent = true
filter {
name = "name"
values = ["ubuntu/images/ebs/ubuntu-trusty-14.04-i386-server-*"]
}
filter {
name = "virtualization-type"
values = ["paravirtual"]
}
owners = ["099720109477"] # Canonical
}
resource "aws_instance" "test" {
ami = data.aws_ami.ubuntu.id
availability_zone = data.aws_availability_zones.available.names[0]
# tflint-ignore: aws_instance_previous_type
instance_type = "t1.micro"
ami = data.aws_ami.amzn-ami-minimal-pv-ebs.id
instance_type = data.aws_ec2_instance_type_offering.available.instance_type
}
resource "aws_eip_association" "test" {
public_ip = aws_eip.test.public_ip
instance_id = aws_instance.test.id
}
`
`)
}

func testAccAWSEIPAssociationConfig_spotInstance(rInt int) string {
return composeConfig(
Expand Down
Loading

0 comments on commit 380460d

Please sign in to comment.