resource/aws_vpc: Apply attribute waiter logic to enable_dns_hostnames and enable_dns_support attributes #17461

bflad · 2021-02-04T16:19:24Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

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:
# aws_vpc.main will be updated in-place
~ resource "aws_vpc" "main" {
~ enable_dns_hostnames             = false -> true
id                               = "vpc-0b7f7f7c1601ee31f"
# (13 unchanged attributes hidden)
}
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) # https://github.com/hashicorp/terraform-provider-aws/issues/17460
--- FAIL: TestAccAWSVpc_classiclinkOptionSet (18.04s) # https://github.com/hashicorp/terraform-provider-aws/issues/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)

…s and enable_dns_support attributes 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) ```

gdavison

Looks good to me 🚀

Waiting on acceptance test results

gdavison · 2021-02-19T00:31:34Z

aws/data_source_aws_vpc.go

@@ -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)


👍 I like the use of finder functions here

bflad · 2021-02-19T15:04:05Z

Test results from yesterday:

--- FAIL: TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount (0.87s)

provider_test.go:702: at least one environment variable of [AWS_ALTERNATE_PROFILE AWS_ALTERNATE_ACCESS_KEY_ID] must be set. Usage: credentials for running acceptance testing in alternate AWS account

--- FAIL: TestAccDataSourceAwsVpc_multipleCidr (46.18s) # https://github.com/hashicorp/terraform-provider-aws/issues/17707

data_source_aws_vpc_test.go:118: Step 1/1 error: Error running apply: exit status 1
Error: multiple VPCs matched; use additional constraints to reduce matches to a single VPC

--- PASS: TestAccAWSVpc_AssignGeneratedIpv6CidrBlock (44.60s)
--- PASS: TestAccAWSVpc_basic (14.33s)
--- PASS: TestAccAWSVpc_bothDnsOptionsSet (41.76s)
--- PASS: TestAccAWSVpc_classiclinkDnsSupportOptionSet (32.42s)
--- PASS: TestAccAWSVpc_classiclinkOptionSet (32.03s)
--- PASS: TestAccAWSVpc_coreMismatchedDiffs (44.93s)
--- PASS: TestAccAWSVpc_DisabledDnsSupport (42.70s)
--- PASS: TestAccAWSVpc_disappears (11.86s)
--- PASS: TestAccAWSVpc_ignoreTags (31.86s)
--- PASS: TestAccAWSVpc_tags (63.12s)
--- PASS: TestAccAWSVpc_Tenancy (52.73s)
--- PASS: TestAccAWSVpc_update (54.38s)

--- PASS: TestAccAWSVpcEndpoint_disappears (23.61s)
--- PASS: TestAccAWSVpcEndpoint_gatewayBasic (32.56s)
--- PASS: TestAccAWSVpcEndpoint_gatewayPolicy (45.05s)
--- PASS: TestAccAWSVpcEndpoint_gatewayWithRouteTableAndPolicy (52.68s)
--- PASS: TestAccAWSVpcEndpoint_interfaceBasic (43.76s)
--- PASS: TestAccAWSVpcEndpoint_interfaceNonAWSServiceAcceptOnCreate (278.55s)
--- PASS: TestAccAWSVpcEndpoint_interfaceNonAWSServiceAcceptOnUpdate (275.53s)
--- PASS: TestAccAWSVpcEndpoint_interfaceWithSubnetAndSecurityGroup (354.73s)
--- PASS: TestAccAWSVpcEndpoint_tags (52.87s)
--- PASS: TestAccAWSVpcEndpoint_VpcEndpointType_GatewayLoadBalancer (439.12s)

--- PASS: TestAccAWSVpcEndpointConnectionNotification_basic (273.59s)

--- PASS: TestAccAWSVpcEndpointRouteTableAssociation_basic (45.03s)

--- PASS: TestAccAWSVpcEndpointService_AllowedPrincipals (259.56s)
--- PASS: TestAccAWSVpcEndpointService_basic (234.42s)
--- PASS: TestAccAWSVpcEndpointService_disappears (253.87s)
--- PASS: TestAccAWSVpcEndpointService_GatewayLoadBalancerArns (188.41s)
--- PASS: TestAccAWSVpcEndpointService_private_dns_name (252.18s)
--- PASS: TestAccAWSVpcEndpointService_tags (284.76s)

--- PASS: TestAccAWSVpcEndpointServiceAllowedPrincipal_basic (221.52s)

--- PASS: TestAccAWSVpcEndpointSubnetAssociation_basic (319.65s)
--- PASS: TestAccAWSVpcEndpointSubnetAssociation_multiple (597.30s)

--- PASS: TestAccAWSVpcPeeringConnectionOptions_basic (37.31s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount (85.62s)

--- PASS: TestAccDataSourceAwsVpc_basic (44.63s)
--- PASS: TestAccDataSourceAwsVpc_ipv6Associated (42.85s)

--- PASS: TestAccDataSourceAwsVpcDhcpOptions_basic (55.39s)
--- PASS: TestAccDataSourceAwsVpcDhcpOptions_Filter (71.97s)

--- PASS: TestAccDataSourceAwsVpcEndpoint_byFilter (71.01s)
--- PASS: TestAccDataSourceAwsVpcEndpoint_byId (65.60s)
--- PASS: TestAccDataSourceAwsVpcEndpoint_byTags (63.64s)
--- PASS: TestAccDataSourceAwsVpcEndpoint_gatewayBasic (67.58s)
--- PASS: TestAccDataSourceAwsVpcEndpoint_gatewayWithRouteTableAndTags (71.23s)
--- PASS: TestAccDataSourceAwsVpcEndpoint_interface (143.95s)

--- PASS: TestAccDataSourceAwsVpcEndpointService_custom (252.82s)
--- PASS: TestAccDataSourceAwsVpcEndpointService_custom_filter (216.28s)
--- PASS: TestAccDataSourceAwsVpcEndpointService_custom_filter_tags (251.95s)
--- PASS: TestAccDataSourceAwsVpcEndpointService_gateway (50.55s)
--- PASS: TestAccDataSourceAwsVpcEndpointService_interface (44.69s)
--- PASS: TestAccDataSourceAwsVpcEndpointService_ServiceType_Gateway (46.38s)
--- PASS: TestAccDataSourceAwsVpcEndpointService_ServiceType_Interface (38.04s)

--- PASS: TestAccDataSourceAwsVpcPeeringConnection_CidrBlock (56.46s)
--- PASS: TestAccDataSourceAwsVpcPeeringConnection_Id (61.75s)
--- PASS: TestAccDataSourceAwsVpcPeeringConnection_PeerCidrBlock (58.44s)
--- PASS: TestAccDataSourceAwsVpcPeeringConnection_PeerVpcId (64.64s)
--- PASS: TestAccDataSourceAwsVpcPeeringConnection_VpcId (47.18s)
--- PASS: TestAccDataSourceAwsVpcPeeringConnections_basic (44.14s)

--- PASS: TestAccDataSourceAwsVpcs_basic (36.58s)
--- PASS: TestAccDataSourceAwsVpcs_filters (35.56s)
--- PASS: TestAccDataSourceAwsVpcs_tags (36.95s)

ghost · 2021-02-19T16:52:25Z

This has been released in version 3.29.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!

ghost · 2021-03-21T17:13:24Z

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!

bflad added the technical-debt Addresses areas of the codebase that need refactoring or redesign. label Feb 4, 2021

bflad requested a review from a team as a code owner February 4, 2021 16:19

ghost added size/L Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. labels Feb 4, 2021

bflad force-pushed the td-aws_vpc-attibute-waiters branch from e1a90fc to b8396c3 Compare February 4, 2021 16:36

bflad force-pushed the td-aws_vpc-attibute-waiters branch from b8396c3 to b022adb Compare February 4, 2021 17:09

bflad mentioned this pull request Feb 5, 2021

tests/service/ec2: Various Tests Returning InvalidVpcID.NotFound Without Context #17458

Closed

gdavison self-assigned this Feb 19, 2021

gdavison approved these changes Feb 19, 2021

View reviewed changes

bflad merged commit e257ac0 into main Feb 19, 2021

bflad deleted the td-aws_vpc-attibute-waiters branch February 19, 2021 15:04

github-actions bot added this to the v3.29.0 milestone Feb 19, 2021

ghost locked as resolved and limited conversation to collaborators Mar 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/aws_vpc: Apply attribute waiter logic to enable_dns_hostnames and enable_dns_support attributes #17461

resource/aws_vpc: Apply attribute waiter logic to enable_dns_hostnames and enable_dns_support attributes #17461

bflad commented Feb 4, 2021

gdavison left a comment

gdavison Feb 19, 2021

bflad commented Feb 19, 2021

ghost commented Feb 19, 2021

ghost commented Mar 21, 2021

resource/aws_vpc: Apply attribute waiter logic to enable_dns_hostnames and enable_dns_support attributes #17461

resource/aws_vpc: Apply attribute waiter logic to enable_dns_hostnames and enable_dns_support attributes #17461

Conversation

bflad commented Feb 4, 2021

Community Note

gdavison left a comment

Choose a reason for hiding this comment

gdavison Feb 19, 2021

Choose a reason for hiding this comment

bflad commented Feb 19, 2021

ghost commented Feb 19, 2021

ghost commented Mar 21, 2021