Skip to content

Commit

Permalink
resource/aws_vpc: Apply attribute waiter logic to enable_dns_hostname…
Browse files Browse the repository at this point in the history
…s and enable_dns_support attributes (#17461)

Reference: #16697

This resource attribute has long been the cause of flakey acceptance testing across the codebase, such as:

```
=== CONT  TestAccAWSEMRInstanceGroup_InstanceCount
TestAccAWSEMRInstanceGroup_InstanceCount: resource_aws_emr_instance_group_test.go:181: Step 1/3 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:
~ resource "aws_vpc" "main" {
~ enable_dns_hostnames             = false -> true
id                               = "vpc-0b7f7f7c1601ee31f"
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccAWSEMRInstanceGroup_InstanceCount (809.62s)
```

Adding logic, similar to `SubnetMapCustomerOwnedIpOnLaunchUpdated` in #16676 can be used to ensure the attribute value has flipped correctly after calling the `ModifyVpcAttribute` API.

This type of waiter logic will be documented in an upcoming Retries and Waiters section of the Contribution Guide.

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSVpc_AssignGeneratedIpv6CidrBlock (79.17s)
--- PASS: TestAccAWSVpc_basic (33.46s)
--- PASS: TestAccAWSVpc_bothDnsOptionsSet (42.70s)
--- PASS: TestAccAWSVpc_classiclinkDnsSupportOptionSet (36.56s)
--- PASS: TestAccAWSVpc_classiclinkOptionSet (34.72s)
--- PASS: TestAccAWSVpc_coreMismatchedDiffs (27.89s)
--- PASS: TestAccAWSVpc_DisabledDnsSupport (42.71s)
--- PASS: TestAccAWSVpc_disappears (20.00s)
--- PASS: TestAccAWSVpc_ignoreTags (58.33s)
--- PASS: TestAccAWSVpc_tags (75.02s)
--- PASS: TestAccAWSVpc_Tenancy (77.02s)
--- PASS: TestAccAWSVpc_update (62.26s)

--- PASS: TestAccDataSourceAwsVpc_basic (32.46s)
--- PASS: TestAccDataSourceAwsVpc_ipv6Associated (33.00s)
--- PASS: TestAccDataSourceAwsVpc_multipleCidr (56.10s)

--- PASS: TestAccAWSEMRCluster_additionalInfo (407.52s)

--- PASS: TestAccAWSEMRInstanceGroup_InstanceCount (881.55s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- FAIL: TestAccAWSVpc_classiclinkDnsSupportOptionSet (17.50s) # #17460
--- FAIL: TestAccAWSVpc_classiclinkOptionSet (18.04s) # #17460
--- PASS: TestAccAWSVpc_AssignGeneratedIpv6CidrBlock (95.03s)
--- PASS: TestAccAWSVpc_basic (42.63s)
--- PASS: TestAccAWSVpc_bothDnsOptionsSet (52.27s)
--- PASS: TestAccAWSVpc_coreMismatchedDiffs (38.50s)
--- PASS: TestAccAWSVpc_DisabledDnsSupport (53.42s)
--- PASS: TestAccAWSVpc_disappears (28.33s)
--- PASS: TestAccAWSVpc_ignoreTags (69.32s)
--- PASS: TestAccAWSVpc_tags (90.39s)
--- PASS: TestAccAWSVpc_Tenancy (94.96s)
--- PASS: TestAccAWSVpc_update (72.18s)

--- PASS: TestAccDataSourceAwsVpc_basic (32.39s)
--- PASS: TestAccDataSourceAwsVpc_ipv6Associated (51.24s)
--- PASS: TestAccDataSourceAwsVpc_multipleCidr (67.27s)

--- PASS: TestAccAWSEMRCluster_additionalInfo (355.27s)

--- PASS: TestAccAWSEMRInstanceGroup_InstanceCount (856.03s)
```
  • Loading branch information
bflad authored Feb 19, 2021
1 parent 33309dd commit e257ac0
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 45 deletions.
17 changes: 11 additions & 6 deletions aws/data_source_aws_vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"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/service/ec2/finder"
)

func dataSourceAwsVpc() *schema.Resource {
Expand Down Expand Up @@ -216,17 +217,21 @@ func dataSourceAwsVpcRead(d *schema.ResourceData, meta interface{}) error {
d.Set("ipv6_cidr_block", vpc.Ipv6CidrBlockAssociationSet[0].Ipv6CidrBlock)
}

attResp, err := awsVpcDescribeVpcAttribute("enableDnsSupport", aws.StringValue(vpc.VpcId), conn)
enableDnsHostnames, err := finder.VpcAttribute(conn, aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsHostnames)

if err != nil {
return err
return fmt.Errorf("error reading EC2 VPC (%s) Attribute (%s): %w", aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsHostnames, err)
}
d.Set("enable_dns_support", attResp.EnableDnsSupport.Value)

attResp, err = awsVpcDescribeVpcAttribute("enableDnsHostnames", aws.StringValue(vpc.VpcId), conn)
d.Set("enable_dns_hostnames", enableDnsHostnames)

enableDnsSupport, err := finder.VpcAttribute(conn, aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsSupport)

if err != nil {
return err
return fmt.Errorf("error reading EC2 VPC (%s) Attribute (%s): %w", aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsSupport, err)
}
d.Set("enable_dns_hostnames", attResp.EnableDnsHostnames.Value)

d.Set("enable_dns_support", enableDnsSupport)

routeTableId, err := resourceAwsVpcSetMainRouteTable(conn, aws.StringValue(vpc.VpcId))
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions aws/internal/service/ec2/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ const (
ErrCodeInvalidSubnetIDNotFound = "InvalidSubnetID.NotFound"
)

const (
ErrCodeInvalidVpcIDNotFound = "InvalidVpcID.NotFound"
)

const (
ErrCodeInvalidVpcEndpointIdNotFound = "InvalidVpcEndpointId.NotFound"
ErrCodeInvalidVpcEndpointServiceIdNotFound = "InvalidVpcEndpointServiceId.NotFound"
Expand Down
35 changes: 35 additions & 0 deletions aws/internal/service/ec2/finder/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,41 @@ func TransitGatewayPrefixListReferenceByID(conn *ec2.EC2, resourceID string) (*e
return TransitGatewayPrefixListReference(conn, transitGatewayRouteTableID, prefixListID)
}

// VpcAttribute looks up a VPC attribute.
func VpcAttribute(conn *ec2.EC2, vpcID string, attribute string) (*bool, error) {
input := &ec2.DescribeVpcAttributeInput{
Attribute: aws.String(attribute),
VpcId: aws.String(vpcID),
}

output, err := conn.DescribeVpcAttribute(input)

if err != nil {
return nil, err
}

if output == nil {
return nil, nil
}

switch attribute {
case ec2.VpcAttributeNameEnableDnsHostnames:
if output.EnableDnsHostnames == nil {
return nil, nil
}

return output.EnableDnsHostnames.Value, nil
case ec2.VpcAttributeNameEnableDnsSupport:
if output.EnableDnsSupport == nil {
return nil, nil
}

return output.EnableDnsSupport.Value, nil
}

return nil, fmt.Errorf("unimplemented VPC attribute: %s", attribute)
}

// VpcPeeringConnectionByID returns the VPC peering connection corresponding to the specified identifier.
// Returns nil and potentially an error if no VPC peering connection is found.
func VpcPeeringConnectionByID(conn *ec2.EC2, id string) (*ec2.VpcPeeringConnection, error) {
Expand Down
21 changes: 21 additions & 0 deletions aws/internal/service/ec2/waiter/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,27 @@ func TransitGatewayPrefixListReferenceState(conn *ec2.EC2, transitGatewayRouteTa
}
}

// VpcAttribute fetches the Vpc and its attribute value
func VpcAttribute(conn *ec2.EC2, id string, attribute string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
attributeValue, err := finder.VpcAttribute(conn, id, attribute)

if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcIDNotFound) {
return nil, "", nil
}

if err != nil {
return nil, "", err
}

if attributeValue == nil {
return nil, "", nil
}

return attributeValue, strconv.FormatBool(aws.BoolValue(attributeValue)), nil
}
}

const (
vpcPeeringConnectionStatusNotFound = "NotFound"
vpcPeeringConnectionStatusUnknown = "Unknown"
Expand Down
22 changes: 22 additions & 0 deletions aws/internal/service/ec2/waiter/waiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,28 @@ func TransitGatewayPrefixListReferenceStateUpdated(conn *ec2.EC2, transitGateway
return nil, err
}

const (
VpcAttributePropagationTimeout = 5 * time.Minute
)

func VpcAttributeUpdated(conn *ec2.EC2, vpcID string, attribute string, expectedValue bool) (*ec2.Vpc, error) {
stateConf := &resource.StateChangeConf{
Target: []string{strconv.FormatBool(expectedValue)},
Refresh: VpcAttribute(conn, vpcID, attribute),
Timeout: VpcAttributePropagationTimeout,
Delay: 10 * time.Second,
MinTimeout: 3 * time.Second,
}

outputRaw, err := stateConf.WaitForState()

if output, ok := outputRaw.(*ec2.Vpc); ok {
return output, err
}

return nil, err
}

const (
VpnGatewayVpcAttachmentAttachedTimeout = 15 * time.Minute

Expand Down
79 changes: 40 additions & 39 deletions aws/resource_aws_vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter"
)

func resourceAwsVpc() *schema.Resource {
Expand Down Expand Up @@ -183,7 +185,11 @@ func resourceAwsVpcCreate(d *schema.ResourceData, meta interface{}) error {
}

if _, err := conn.ModifyVpcAttribute(input); err != nil {
return fmt.Errorf("error enabling VPC (%s) DNS hostnames: %s", d.Id(), err)
return fmt.Errorf("error enabling EC2 VPC (%s) DNS Hostnames: %w", d.Id(), err)
}

if _, err := waiter.VpcAttributeUpdated(conn, d.Id(), ec2.VpcAttributeNameEnableDnsHostnames, d.Get("enable_dns_hostnames").(bool)); err != nil {
return fmt.Errorf("error waiting for EC2 VPC (%s) DNS Hostnames to enable: %w", d.Id(), err)
}
}

Expand All @@ -199,7 +205,11 @@ func resourceAwsVpcCreate(d *schema.ResourceData, meta interface{}) error {
}

if _, err := conn.ModifyVpcAttribute(input); err != nil {
return fmt.Errorf("error disabling VPC (%s) DNS support: %s", d.Id(), err)
return fmt.Errorf("error disabling EC2 VPC (%s) DNS Support: %w", d.Id(), err)
}

if _, err := waiter.VpcAttributeUpdated(conn, d.Id(), ec2.VpcAttributeNameEnableDnsSupport, d.Get("enable_dns_support").(bool)); err != nil {
return fmt.Errorf("error waiting for EC2 VPC (%s) DNS Support to disable: %w", d.Id(), err)
}
}

Expand Down Expand Up @@ -276,17 +286,21 @@ func resourceAwsVpcRead(d *schema.ResourceData, meta interface{}) error {
}
}

resp, err := awsVpcDescribeVpcAttribute("enableDnsSupport", vpcid, conn)
enableDnsHostnames, err := finder.VpcAttribute(conn, aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsHostnames)

if err != nil {
return err
return fmt.Errorf("error reading EC2 VPC (%s) Attribute (%s): %w", aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsHostnames, err)
}
d.Set("enable_dns_support", resp.EnableDnsSupport.Value)

resp, err = awsVpcDescribeVpcAttribute("enableDnsHostnames", vpcid, conn)
d.Set("enable_dns_hostnames", enableDnsHostnames)

enableDnsSupport, err := finder.VpcAttribute(conn, aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsSupport)

if err != nil {
return err
return fmt.Errorf("error reading EC2 VPC (%s) Attribute (%s): %w", aws.StringValue(vpc.VpcId), ec2.VpcAttributeNameEnableDnsSupport, err)
}
d.Set("enable_dns_hostnames", resp.EnableDnsHostnames.Value)

d.Set("enable_dns_support", enableDnsSupport)

describeClassiclinkOpts := &ec2.DescribeVpcClassicLinkInput{
VpcIds: []*string{&vpcid},
Expand Down Expand Up @@ -362,38 +376,38 @@ func resourceAwsVpcUpdate(d *schema.ResourceData, meta interface{}) error {

vpcid := d.Id()
if d.HasChange("enable_dns_hostnames") {
val := d.Get("enable_dns_hostnames").(bool)
modifyOpts := &ec2.ModifyVpcAttributeInput{
VpcId: &vpcid,
input := &ec2.ModifyVpcAttributeInput{
VpcId: aws.String(d.Id()),
EnableDnsHostnames: &ec2.AttributeBooleanValue{
Value: &val,
Value: aws.Bool(d.Get("enable_dns_hostnames").(bool)),
},
}

log.Printf(
"[INFO] Modifying enable_dns_hostnames vpc attribute for %s: %s",
d.Id(), modifyOpts)
if _, err := conn.ModifyVpcAttribute(modifyOpts); err != nil {
return err
if _, err := conn.ModifyVpcAttribute(input); err != nil {
return fmt.Errorf("error updating EC2 VPC (%s) DNS Hostnames: %w", d.Id(), err)
}

if _, err := waiter.VpcAttributeUpdated(conn, d.Id(), ec2.VpcAttributeNameEnableDnsHostnames, d.Get("enable_dns_hostnames").(bool)); err != nil {
return fmt.Errorf("error waiting for EC2 VPC (%s) DNS Hostnames update: %w", d.Id(), err)
}
}

_, hasEnableDnsSupportOption := d.GetOk("enable_dns_support")

if !hasEnableDnsSupportOption || d.HasChange("enable_dns_support") {
val := d.Get("enable_dns_support").(bool)
modifyOpts := &ec2.ModifyVpcAttributeInput{
VpcId: &vpcid,
input := &ec2.ModifyVpcAttributeInput{
VpcId: aws.String(d.Id()),
EnableDnsSupport: &ec2.AttributeBooleanValue{
Value: &val,
Value: aws.Bool(d.Get("enable_dns_support").(bool)),
},
}

log.Printf(
"[INFO] Modifying enable_dns_support vpc attribute for %s: %s",
d.Id(), modifyOpts)
if _, err := conn.ModifyVpcAttribute(modifyOpts); err != nil {
return err
if _, err := conn.ModifyVpcAttribute(input); err != nil {
return fmt.Errorf("error updating EC2 VPC (%s) DNS Support: %w", d.Id(), err)
}

if _, err := waiter.VpcAttributeUpdated(conn, d.Id(), ec2.VpcAttributeNameEnableDnsSupport, d.Get("enable_dns_support").(bool)); err != nil {
return fmt.Errorf("error waiting for EC2 VPC (%s) DNS Support update: %w", d.Id(), err)
}
}

Expand Down Expand Up @@ -733,19 +747,6 @@ func resourceAwsVpcInstanceImport(
return []*schema.ResourceData{d}, nil
}

func awsVpcDescribeVpcAttribute(attribute string, vpcId string, conn *ec2.EC2) (*ec2.DescribeVpcAttributeOutput, error) {
describeAttrOpts := &ec2.DescribeVpcAttributeInput{
Attribute: aws.String(attribute),
VpcId: aws.String(vpcId),
}
resp, err := conn.DescribeVpcAttribute(describeAttrOpts)
if err != nil {
return nil, err
}

return resp, nil
}

// vpcDescribe returns EC2 API information about the specified VPC.
// If the VPC doesn't exist, return nil.
func vpcDescribe(conn *ec2.EC2, vpcId string) (*ec2.Vpc, error) {
Expand Down

0 comments on commit e257ac0

Please sign in to comment.