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.

Sign up for GitHub

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

Jump to bottom

service/ec2: Updated handling for Lambda Hyperplane ENIs #10347

Merged
merged 5 commits into from
Oct 3, 2019
Merged

service/ec2: Updated handling for Lambda Hyperplane ENIs #10347

merged 5 commits into from
Oct 3, 2019

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Oct 2, 2019 •
edited
Loading

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #10044
Builds on #10114
Next steps are #10329

Release note for CHANGELOG:

* resource/aws_security_group: Handle longer deletion timeframe for new Lambda Hyperplane ENIs
* resource/aws_subnet: Handle longer deletion timeframe for new Lambda Hyperplane ENIs

The introduction of improved VPC networking for Lambda brought some welcome enhancements to Lambda functionality, but initially has some unintentional consequences when working with Terraform due to the underlying infrastructure changes. The main issue is that these new Hyperplane ENIs associated with Lambda take additional time currently to detach/delete and that the Lambda service itself is the owner of these ENIs, which prevents early detachment.

In working with the AWS Lambda service team, we have received some confirmation on expected detachment/deletion timeframes for Lambda Hyperplane ENIs. Using this information, we set the Lambda ENI timeout to be at a minimum the expected deletion time to match the service expectations without adjusting the overall default aws_security_group or aws_subnet resource deletion timeouts. This is to ensure legitimate DependencyViolation errors return to operators in a fairly timely manner (left as 10 minutes and 20 minutes respectfully).

Output from AWS Commercial (us-east-2 - Hyperplane enabled)

--- PASS: TestAccAWSLambdaFunction_basic (23.37s)
--- PASS: TestAccAWSLambdaFunction_concurrency (30.76s)
--- PASS: TestAccAWSLambdaFunction_concurrencyCycle (43.12s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfig (42.40s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfigUpdated (41.70s)
--- PASS: TestAccAWSLambdaFunction_EmptyVpcConfig (22.99s)
--- PASS: TestAccAWSLambdaFunction_encryptedEnvVariables (51.21s)
--- PASS: TestAccAWSLambdaFunction_envVariables (45.14s)
--- PASS: TestAccAWSLambdaFunction_expectFilenameAndS3Attributes (10.90s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile (31.12s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile_VPC (1422.82s)
--- PASS: TestAccAWSLambdaFunction_importS3 (22.66s)
--- PASS: TestAccAWSLambdaFunction_Layers (34.75s)
--- PASS: TestAccAWSLambdaFunction_LayersUpdate (54.60s)
--- PASS: TestAccAWSLambdaFunction_localUpdate (31.40s)
--- PASS: TestAccAWSLambdaFunction_localUpdate_nameOnly (24.21s)
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (12.71s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_java8 (23.05s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs10x (26.99s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs810 (26.53s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_noRuntime (0.72s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_provided (18.66s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python27 (27.62s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python36 (22.87s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python37 (27.09s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_ruby25 (27.87s)
--- PASS: TestAccAWSLambdaFunction_s3 (22.59s)
--- PASS: TestAccAWSLambdaFunction_s3Update_basic (32.58s)
--- PASS: TestAccAWSLambdaFunction_s3Update_unversioned (31.07s)
--- PASS: TestAccAWSLambdaFunction_tags (42.41s)
--- PASS: TestAccAWSLambdaFunction_tracingConfig (39.12s)
--- PASS: TestAccAWSLambdaFunction_updateRuntime (29.16s)
--- PASS: TestAccAWSLambdaFunction_versioned (28.09s)
--- PASS: TestAccAWSLambdaFunction_versionedUpdate (47.13s)
--- PASS: TestAccAWSLambdaFunction_VPC (1331.55s)
--- PASS: TestAccAWSLambdaFunction_VPC_withInvocation (1376.24s)
--- PASS: TestAccAWSLambdaFunction_VpcConfig_ProperIamDependencies (1327.69s)
--- PASS: TestAccAWSLambdaFunction_VPCRemoval (1490.19s)
--- PASS: TestAccAWSLambdaFunction_VPCUpdate (1685.40s)

Output from AWS Commercial (us-west-2 - Hyperplane not deployed)

--- PASS: TestAccAWSLambdaFunction_basic (40.50s)
--- PASS: TestAccAWSLambdaFunction_concurrency (47.79s)
--- PASS: TestAccAWSLambdaFunction_concurrencyCycle (62.65s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfig (55.95s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfigUpdated (50.23s)
--- PASS: TestAccAWSLambdaFunction_EmptyVpcConfig (37.47s)
--- PASS: TestAccAWSLambdaFunction_encryptedEnvVariables (73.66s)
--- PASS: TestAccAWSLambdaFunction_envVariables (80.88s)
--- PASS: TestAccAWSLambdaFunction_expectFilenameAndS3Attributes (22.59s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile (42.78s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile_VPC (39.40s)
--- PASS: TestAccAWSLambdaFunction_importS3 (36.62s)
--- PASS: TestAccAWSLambdaFunction_Layers (53.78s)
--- PASS: TestAccAWSLambdaFunction_LayersUpdate (89.78s)
--- PASS: TestAccAWSLambdaFunction_localUpdate (54.31s)
--- PASS: TestAccAWSLambdaFunction_localUpdate_nameOnly (56.10s)
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (26.12s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_java8 (46.49s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs10x (52.25s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs810 (43.59s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_noRuntime (2.71s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_provided (43.88s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python27 (47.91s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python36 (45.95s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python37 (41.40s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_ruby25 (50.32s)
--- PASS: TestAccAWSLambdaFunction_s3 (35.28s)
--- PASS: TestAccAWSLambdaFunction_s3Update_basic (57.89s)
--- PASS: TestAccAWSLambdaFunction_s3Update_unversioned (58.81s)
--- PASS: TestAccAWSLambdaFunction_tags (75.77s)
--- PASS: TestAccAWSLambdaFunction_tracingConfig (55.61s)
--- PASS: TestAccAWSLambdaFunction_updateRuntime (57.19s)
--- PASS: TestAccAWSLambdaFunction_versioned (33.52s)
--- PASS: TestAccAWSLambdaFunction_versionedUpdate (58.25s)
--- PASS: TestAccAWSLambdaFunction_VPC (56.81s)
--- PASS: TestAccAWSLambdaFunction_VPC_withInvocation (86.81s)
--- PASS: TestAccAWSLambdaFunction_VpcConfig_ProperIamDependencies (42.99s)
--- PASS: TestAccAWSLambdaFunction_VPCRemoval (80.28s)
--- PASS: TestAccAWSLambdaFunction_VPCUpdate (81.84s)

--- PASS: TestAccAWSSecurityGroup_basic (10.14s)
--- PASS: TestAccAWSSecurityGroup_Change (19.36s)
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (31.78s)
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (6.53s)
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (25.29s)
--- PASS: TestAccAWSSecurityGroup_drift (7.55s)
--- PASS: TestAccAWSSecurityGroup_drift_complex (31.62s)
--- PASS: TestAccAWSSecurityGroup_Egress_ConfigMode (23.76s)
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (24.51s)
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (12.13s)
--- PASS: TestAccAWSSecurityGroup_forceRevokeRules_false (1228.05s)
--- PASS: TestAccAWSSecurityGroup_forceRevokeRules_true (1242.70s)
--- PASS: TestAccAWSSecurityGroup_generatedName (25.26s)
--- PASS: TestAccAWSSecurityGroup_importBasic (12.91s)
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (14.68s)
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (12.19s)
--- PASS: TestAccAWSSecurityGroup_importIpv6 (30.08s)
--- PASS: TestAccAWSSecurityGroup_importPrefixList (25.01s)
--- PASS: TestAccAWSSecurityGroup_importSelf (31.64s)
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (30.19s)
--- PASS: TestAccAWSSecurityGroup_Ingress_ConfigMode (23.47s)
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (31.60s)
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (9.86s)
--- PASS: TestAccAWSSecurityGroup_ingressWithPrefixList (44.12s)
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (1.28s)
--- PASS: TestAccAWSSecurityGroup_ipv4andipv6Egress (11.90s)
--- PASS: TestAccAWSSecurityGroup_ipv6 (12.77s)
--- PASS: TestAccAWSSecurityGroup_MultiIngress (12.33s)
--- PASS: TestAccAWSSecurityGroup_namePrefix (6.47s)
--- PASS: TestAccAWSSecurityGroup_RuleDescription (26.52s)
--- PASS: TestAccAWSSecurityGroup_ruleGathering (24.55s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitCidrBlockExceededAppend (48.89s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitExceededAllNew (53.89s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitExceededAppend (50.48s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitExceededPrepend (54.09s)
--- PASS: TestAccAWSSecurityGroup_rulesDropOnError (22.40s)
--- PASS: TestAccAWSSecurityGroup_self (11.93s)
--- PASS: TestAccAWSSecurityGroup_tags (40.86s)
--- PASS: TestAccAWSSecurityGroup_vpc (10.39s)
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (10.55s)
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (11.84s)

--- PASS: TestAccAWSSubnet_availabilityZoneId (26.56s)
--- PASS: TestAccAWSSubnet_basic (26.69s)
--- PASS: TestAccAWSSubnet_enableIpv6 (42.97s)
--- PASS: TestAccAWSSubnet_ipv6 (69.30s)

@bflad bflad added this to the v2.31.0 milestone Oct 2, 2019
@bflad bflad requested a review from a team October 2, 2019 17:56
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/ec2 Issues and PRs that pertain to the ec2 service. service/lambda Issues and PRs that pertain to the lambda service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Oct 2, 2019
@bflad bflad added the bug Addresses a defect in current functionality. label Oct 2, 2019
@richardgavel richardgavel mentioned this pull request Oct 2, 2019
Closed
@bflad bflad force-pushed the ewbankkit-issue-10044 branch 2 times, most recently from 59ba637 to 922037e Compare October 2, 2019 22:02
@bflad
service/ec2: Updated handling for Lambda Hyperplane ENIs
09596a0 
Reference: #10044
Reference: #10114
Reference: #10329

The introduction of [improved VPC networking for Lambda]() brought some welcome enhancements to Lambda functionality, but initially has some unintentional consequences when working with Terraform due to the underlying infrastructure changes. The main issue is that these new Hyperplane ENIs associated with Lambda take additional time currently to detach/delete and that the Lambda service itself is the owner of these ENIs, which prevents early detachment.

In working with the AWS Lambda service team, we have received some confirmation on expected detachment/deletion timeframes for Lambda Hyperplane ENIs. Using this information, we set the Lambda ENI timeout to be at a minimum the expected deletion time to match the service expectations without adjusting the overall default `aws_security_group` or `aws_subnet` resource deletion timeouts. This is to ensure legitimate `DependencyViolation` errors return to operators in a fairly timely manner (left as 10 minutes and 20 minutes respectfully).

Output from AWS Commerical (us-east-2 - Hyperplane enabled)

```
--- PASS: TestAccAWSLambdaFunction_basic (23.37s)
--- PASS: TestAccAWSLambdaFunction_concurrency (30.76s)
--- PASS: TestAccAWSLambdaFunction_concurrencyCycle (43.12s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfig (42.40s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfigUpdated (41.70s)
--- PASS: TestAccAWSLambdaFunction_EmptyVpcConfig (22.99s)
--- PASS: TestAccAWSLambdaFunction_encryptedEnvVariables (51.21s)
--- PASS: TestAccAWSLambdaFunction_envVariables (45.14s)
--- PASS: TestAccAWSLambdaFunction_expectFilenameAndS3Attributes (10.90s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile (31.12s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile_VPC (1422.82s)
--- PASS: TestAccAWSLambdaFunction_importS3 (22.66s)
--- PASS: TestAccAWSLambdaFunction_Layers (34.75s)
--- PASS: TestAccAWSLambdaFunction_LayersUpdate (54.60s)
--- PASS: TestAccAWSLambdaFunction_localUpdate (31.40s)
--- PASS: TestAccAWSLambdaFunction_localUpdate_nameOnly (24.21s)
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (12.71s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_java8 (23.05s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs10x (26.99s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs810 (26.53s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_noRuntime (0.72s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_provided (18.66s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python27 (27.62s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python36 (22.87s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python37 (27.09s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_ruby25 (27.87s)
--- PASS: TestAccAWSLambdaFunction_s3 (22.59s)
--- PASS: TestAccAWSLambdaFunction_s3Update_basic (32.58s)
--- PASS: TestAccAWSLambdaFunction_s3Update_unversioned (31.07s)
--- PASS: TestAccAWSLambdaFunction_tags (42.41s)
--- PASS: TestAccAWSLambdaFunction_tracingConfig (39.12s)
--- PASS: TestAccAWSLambdaFunction_updateRuntime (29.16s)
--- PASS: TestAccAWSLambdaFunction_versioned (28.09s)
--- PASS: TestAccAWSLambdaFunction_versionedUpdate (47.13s)
--- PASS: TestAccAWSLambdaFunction_VPC (1331.55s)
--- PASS: TestAccAWSLambdaFunction_VPC_withInvocation (1376.24s)
--- PASS: TestAccAWSLambdaFunction_VpcConfig_ProperIamDependencies (1327.69s)
--- PASS: TestAccAWSLambdaFunction_VPCRemoval (1490.19s)
--- PASS: TestAccAWSLambdaFunction_VPCUpdate (1685.40s)
```

Output from AWS Commercial (us-west-2 - Hyperplane not deployed)

```
--- PASS: TestAccAWSLambdaFunction_basic (40.50s)
--- PASS: TestAccAWSLambdaFunction_concurrency (47.79s)
--- PASS: TestAccAWSLambdaFunction_concurrencyCycle (62.65s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfig (55.95s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfigUpdated (50.23s)
--- PASS: TestAccAWSLambdaFunction_EmptyVpcConfig (37.47s)
--- PASS: TestAccAWSLambdaFunction_encryptedEnvVariables (73.66s)
--- PASS: TestAccAWSLambdaFunction_envVariables (80.88s)
--- PASS: TestAccAWSLambdaFunction_expectFilenameAndS3Attributes (22.59s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile (42.78s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile_VPC (39.40s)
--- PASS: TestAccAWSLambdaFunction_importS3 (36.62s)
--- PASS: TestAccAWSLambdaFunction_Layers (53.78s)
--- PASS: TestAccAWSLambdaFunction_LayersUpdate (89.78s)
--- PASS: TestAccAWSLambdaFunction_localUpdate (54.31s)
--- PASS: TestAccAWSLambdaFunction_localUpdate_nameOnly (56.10s)
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (26.12s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_java8 (46.49s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs10x (52.25s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_NodeJs810 (43.59s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_noRuntime (2.71s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_provided (43.88s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python27 (47.91s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python36 (45.95s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python37 (41.40s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_ruby25 (50.32s)
--- PASS: TestAccAWSLambdaFunction_s3 (35.28s)
--- PASS: TestAccAWSLambdaFunction_s3Update_basic (57.89s)
--- PASS: TestAccAWSLambdaFunction_s3Update_unversioned (58.81s)
--- PASS: TestAccAWSLambdaFunction_tags (75.77s)
--- PASS: TestAccAWSLambdaFunction_tracingConfig (55.61s)
--- PASS: TestAccAWSLambdaFunction_updateRuntime (57.19s)
--- PASS: TestAccAWSLambdaFunction_versioned (33.52s)
--- PASS: TestAccAWSLambdaFunction_versionedUpdate (58.25s)
--- PASS: TestAccAWSLambdaFunction_VPC (56.81s)
--- PASS: TestAccAWSLambdaFunction_VPC_withInvocation (86.81s)
--- PASS: TestAccAWSLambdaFunction_VpcConfig_ProperIamDependencies (42.99s)
--- PASS: TestAccAWSLambdaFunction_VPCRemoval (80.28s)
--- PASS: TestAccAWSLambdaFunction_VPCUpdate (81.84s)

--- PASS: TestAccAWSSecurityGroup_basic (10.14s)
--- PASS: TestAccAWSSecurityGroup_Change (19.36s)
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (31.78s)
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (6.53s)
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (25.29s)
--- PASS: TestAccAWSSecurityGroup_drift (7.55s)
--- PASS: TestAccAWSSecurityGroup_drift_complex (31.62s)
--- PASS: TestAccAWSSecurityGroup_Egress_ConfigMode (23.76s)
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (24.51s)
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (12.13s)
--- PASS: TestAccAWSSecurityGroup_forceRevokeRules_false (1228.05s)
--- PASS: TestAccAWSSecurityGroup_forceRevokeRules_true (1242.70s)
--- PASS: TestAccAWSSecurityGroup_generatedName (25.26s)
--- PASS: TestAccAWSSecurityGroup_importBasic (12.91s)
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (14.68s)
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (12.19s)
--- PASS: TestAccAWSSecurityGroup_importIpv6 (30.08s)
--- PASS: TestAccAWSSecurityGroup_importPrefixList (25.01s)
--- PASS: TestAccAWSSecurityGroup_importSelf (31.64s)
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (30.19s)
--- PASS: TestAccAWSSecurityGroup_Ingress_ConfigMode (23.47s)
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (31.60s)
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (9.86s)
--- PASS: TestAccAWSSecurityGroup_ingressWithPrefixList (44.12s)
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (1.28s)
--- PASS: TestAccAWSSecurityGroup_ipv4andipv6Egress (11.90s)
--- PASS: TestAccAWSSecurityGroup_ipv6 (12.77s)
--- PASS: TestAccAWSSecurityGroup_MultiIngress (12.33s)
--- PASS: TestAccAWSSecurityGroup_namePrefix (6.47s)
--- PASS: TestAccAWSSecurityGroup_RuleDescription (26.52s)
--- PASS: TestAccAWSSecurityGroup_ruleGathering (24.55s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitCidrBlockExceededAppend (48.89s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitExceededAllNew (53.89s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitExceededAppend (50.48s)
--- PASS: TestAccAWSSecurityGroup_ruleLimitExceededPrepend (54.09s)
--- PASS: TestAccAWSSecurityGroup_rulesDropOnError (22.40s)
--- PASS: TestAccAWSSecurityGroup_self (11.93s)
--- PASS: TestAccAWSSecurityGroup_tags (40.86s)
--- PASS: TestAccAWSSecurityGroup_vpc (10.39s)
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (10.55s)
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (11.84s)

--- PASS: TestAccAWSSubnet_availabilityZoneId (26.56s)
--- PASS: TestAccAWSSubnet_basic (26.69s)
--- PASS: TestAccAWSSubnet_enableIpv6 (42.97s)
--- PASS: TestAccAWSSubnet_ipv6 (69.30s)
```
@bflad bflad force-pushed the ewbankkit-issue-10044 branch from 922037e to 09596a0 Compare October 2, 2019 23:06
nywilken
nywilken approved these changes Oct 3, 2019
View reviewed changes
Copy link
Contributor

@nywilken nywilken left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good to go. I left a couple of documentation suggestions and tested in us-east-2, us-west-2, and ap-northeast-1

highlighting only a few of the key Hyperplane test cases that wait for ENIs to dettach

--- PASS: TestAccAWSLambdaFunction_Layers (56.17s)
--- PASS: TestAccAWSLambdaFunction_tracingConfig (60.76s)
--- PASS: TestAccAWSLambdaFunction_basic (37.49s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfigUpdated (62.65s)
--- PASS: TestAccAWSLambdaFunction_LayersUpdate (68.84s)
--- PASS: TestAccAWSLambdaFunction_versionedUpdate (61.02s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfig (63.43s)
--- PASS: TestAccAWSLambdaFunction_importS3 (40.34s)
--- PASS: TestAccAWSLambdaFunction_concurrencyCycle (73.99s)
--- PASS: TestAccAWSLambdaFunction_encryptedEnvVariables (84.76s)
--- PASS: TestAccAWSLambdaFunction_VpcConfig_ProperIamDependencies (1285.11s)
--- PASS: TestAccAWSLambdaFunction_VPC (1407.01s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile_VPC (1368.80s)
--- PASS: TestAccAWSLambdaFunction_VPCRemoval (1411.94s)
--- PASS: TestAccAWSLambdaFunction_VPC_withInvocation (1454.78s)
--- PASS: TestAccAWSLambdaFunction_VPCUpdate (1489.02s)

website/docs/r/security_group.html.markdown
- `create` - (Default `10 minutes`) How long to wait for a security group to be created.
- `delete` - (Default `10 minutes`) How long to wait for a security group to be deleted.
- `create` - (Default `10m`) How long to wait for a security group to be created.
- `delete` - (Default `10m`) How long to retry on `DependencyViolation` errors during security group deletion from lingering ENIs left by certain AWS services such as Elastic Load Balancing. NOTE: Lambda ENIs can take up to 45 minutes to delete, which is not affected by changing this customizable timeout (in version 2.31.0 and later of the Terraform AWS Provider) unless it is increased above 45 minutes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `delete` - (Default `10m`) How long to retry on `DependencyViolation` errors during security group deletion from lingering ENIs left by certain AWS services such as Elastic Load Balancing. NOTE: Lambda ENIs can take up to 45 minutes to delete, which is not affected by changing this customizable timeout (in version 2.31.0 and later of the Terraform AWS Provider) unless it is increased above 45 minutes.
- `delete` - (Default `10m`) How long to retry on `DependencyViolation` errors during security group deletion. NOTE: Dependencies such as lingering ENIs left by Elastic Load Balancing or Lambda Functions can take up to 45 minutes to delete, which is not affected by changing this customizable timeout (in version 2.31.0 and later of the Terraform AWS Provider) unless it is increased above 45 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's chat about this out of band and submit a docs fix after 😄 ELB ENIs are a separate concern (not handled by the 45 minute logic) and generally only take 5 minutes to delete if I remember correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good - documentation is hard 👍

website/docs/r/subnet.html.markdown
configuration options:

- `create` - (Default `10m`) How long to wait for a subnet to be created.
- `delete` - (Default `20m`) How long to retry on `DependencyViolation` errors during subnet deletion from lingering ENIs left by certain AWS services such as Elastic Load Balancing. NOTE: Lambda ENIs can take up to 45 minutes to delete, which is not affected by changing this customizable timeout (in version 2.31.0 and later of the Terraform AWS Provider) unless it is increased above 45 minutes.

This comment was marked as off-topic.

Sign in to view

aws/resource_aws_security_group.go

if isAWSErr(detachNetworkInterfaceErr, "InvalidNetworkInterfaceID.NotFound", "") {
return nil
if eni.Attachment != nil && aws.StringValue(eni.Attachment.InstanceOwnerId) == "amazon-aws" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Mentioning for visibility - although it doesn't appear to be an issue with using the InstanceOwnerId amazon-aws - it was mentioned when speaking with AWS that the Attachment ID for these ENIs ela-attach was a good way of checking if the ENI was Amazon owned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Do you think its worth switching this up in the future?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no hard evidence to suggest that the InstanceOwnerId will not be amazon-aws right now, so we can circle back if needed.

@bflad bflad merged commit d9102b1 into master Oct 3, 2019
@bflad bflad deleted the ewbankkit-issue-10044 branch October 3, 2019 02:20
bflad added a commit that referenced this pull request Oct 3, 2019
@bflad
Update CHANGELOG for #10114 and #10347
279917a
@bflad bflad mentioned this pull request Oct 3, 2019
Closed
@ewbankkit ewbankkit mentioned this pull request Oct 3, 2019
Closed
@ghost
Copy link

ghost commented Oct 3, 2019

This has been released in version 2.31.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
Copy link

ghost commented Nov 2, 2019

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!

@ghost ghost locked and limited conversation to collaborators Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nywilken nywilken nywilken approved these changes

Assignees
No one assigned
Labels
bug Addresses a defect in current functionality. documentation Introduces or discusses updates to documentation. service/ec2 Issues and PRs that pertain to the ec2 service. service/lambda Issues and PRs that pertain to the lambda service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Milestone
v2.31.0
Development

Successfully merging this pull request may close these issues.

AWS changes in eu-west-1 region impacting aws_lambda_function proper deletion
3 participants
@bflad @nywilken @ewbankkit