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.

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

r/aws_network_interface - read after create + add support for ipv6 #12281

Merged
merged 29 commits into from
Oct 16, 2020

Conversation

DrFaust92
Copy link
Collaborator

@DrFaust92 DrFaust92 commented Mar 5, 2020

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

Closes #12260
Closes #1707
Closes #8408
Relates #1060

Release note for CHANGELOG:

resource_aws_network_interface: add support for adding IPV6 addresses

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSENI_'
--- PASS: TestAccAWSENI_ipv6 (239.85s)
--- PASS: TestAccAWSENI_tags (241.01s)
--- PASS: TestAccAWSENI_ipv6_count (301.36s)
--- PASS: TestAccAWSENI_sourceDestCheck (235.46s)
--- PASS: TestAccAWSENI_computedIPs (117.03s)
--- PASS: TestAccAWSENI_PrivateIpsCount (253.85s)
--- PASS: TestAccAWSENI_basic (122.11s)
--- PASS: TestAccAWSENI_disappears (100.06s)
--- PASS: TestAccAWSENI_updatedDescription (194.35s)
--- PASS: TestAccAWSENI_attached (214.29s)
--- PASS: TestAccAWSENI_ignoreExternalAttachment (173.77s)

@DrFaust92 DrFaust92 requested a review from a team March 5, 2020 22:33
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. service/ec2 Issues and PRs that pertain to the ec2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. labels Mar 5, 2020
@DrFaust92 DrFaust92 changed the title [WIP]r/aws_network_interface - add support for ipv6 r/aws_network_interface - add support for ipv6 Mar 6, 2020
@DrFaust92 DrFaust92 removed the request for review from a team March 6, 2020 15:30
@DrFaust92
Copy link
Collaborator Author

@bflad, i have seem to have removed terraform-providers/aws-provider from the reviewers. can you assist in fixing this?

@ewbankkit
Copy link
Contributor

ewbankkit commented Mar 6, 2020

Verified acceptance tests:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSENI_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 4 -run=TestAccAWSENI_ -timeout 120m
go: downloading github.com/aws/aws-sdk-go v1.29.16
go: extracting github.com/aws/aws-sdk-go v1.29.16
go: finding github.com/aws/aws-sdk-go v1.29.16
=== RUN   TestAccAWSENI_basic
=== PAUSE TestAccAWSENI_basic
=== RUN   TestAccAWSENI_ipv6
=== PAUSE TestAccAWSENI_ipv6
=== RUN   TestAccAWSENI_ipv6_count
=== PAUSE TestAccAWSENI_ipv6_count
=== RUN   TestAccAWSENI_disappears
=== PAUSE TestAccAWSENI_disappears
=== RUN   TestAccAWSENI_updatedDescription
=== PAUSE TestAccAWSENI_updatedDescription
=== RUN   TestAccAWSENI_attached
=== PAUSE TestAccAWSENI_attached
=== RUN   TestAccAWSENI_ignoreExternalAttachment
=== PAUSE TestAccAWSENI_ignoreExternalAttachment
=== RUN   TestAccAWSENI_sourceDestCheck
=== PAUSE TestAccAWSENI_sourceDestCheck
=== RUN   TestAccAWSENI_computedIPs
=== PAUSE TestAccAWSENI_computedIPs
=== RUN   TestAccAWSENI_PrivateIpsCount
=== PAUSE TestAccAWSENI_PrivateIpsCount
=== RUN   TestAccAWSENI_tags
=== PAUSE TestAccAWSENI_tags
=== CONT  TestAccAWSENI_basic
=== CONT  TestAccAWSENI_ignoreExternalAttachment
=== CONT  TestAccAWSENI_tags
=== CONT  TestAccAWSENI_PrivateIpsCount
--- PASS: TestAccAWSENI_basic (76.47s)
=== CONT  TestAccAWSENI_computedIPs
--- PASS: TestAccAWSENI_tags (126.85s)
=== CONT  TestAccAWSENI_sourceDestCheck
--- PASS: TestAccAWSENI_ignoreExternalAttachment (127.13s)
=== CONT  TestAccAWSENI_disappears
--- PASS: TestAccAWSENI_PrivateIpsCount (149.81s)
=== CONT  TestAccAWSENI_attached
--- PASS: TestAccAWSENI_computedIPs (74.37s)
=== CONT  TestAccAWSENI_updatedDescription
--- PASS: TestAccAWSENI_disappears (74.29s)
=== CONT  TestAccAWSENI_ipv6_count
--- PASS: TestAccAWSENI_sourceDestCheck (122.47s)
=== CONT  TestAccAWSENI_ipv6
--- PASS: TestAccAWSENI_updatedDescription (100.56s)
--- PASS: TestAccAWSENI_ipv6 (75.66s)
--- PASS: TestAccAWSENI_ipv6_count (146.90s)
--- PASS: TestAccAWSENI_attached (425.23s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	575.097s

@euank
Copy link

euank commented Mar 26, 2020

I'm not a valid reviewer in this repo, but for what it's worth, this change LGTM, both code-wise and from using it. I've patched this into the plugin locally, and it's worked well for managing ipv6 addresses on my ENIs.

@DrFaust92 DrFaust92 force-pushed the eni-ipv6-support branch 2 times, most recently from 601c490 to 593bd8c Compare April 11, 2020 14:58
@DrFaust92
Copy link
Collaborator Author

Rebased

resource "aws_vpc" "foo" {
cidr_block = "172.16.0.0/16"
enable_dns_hostnames = true
data "aws_availability_zones" "available" {
Copy link
Contributor

@ewbankkit ewbankkit May 25, 2020

Choose a reason for hiding this comment

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

Change (plus testAccAWSENIConfigExternalAttachment) to

data "aws_availability_zones" "available" {
  state = "available"
 
  filter {
    name   = "opt-in-status"
    values = ["opt-in-not-required"]
  }
}

(see #12412, #12517), else I get the error

--- FAIL: TestAccAWSENI_ignoreExternalAttachment (62.85s)
    testing.go:669: Step 0 error: errors during apply:
        
        Error: Error launching source instance: Unsupported: The requested configuration is currently not supported. Please check the documentation for supported configurations.
        	status code: 400, request id: b71e5cb8-fcca-44ae-85ee-8a976eb52998
        
          on /tmp/tf-test739947919/main.tf line 41:
          (source code not available)

Name = "terraform-testacc-network-interface"
}
const testAccAWSENIConfigBase = `
data "aws_availability_zones" "available" {
Copy link
Contributor

@ewbankkit ewbankkit May 25, 2020

Choose a reason for hiding this comment

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

Change (plus testAccAWSENIIPV6ConfigBase) to

data "aws_availability_zones" "available" {
  state = "available"
 
  filter {
    name   = "opt-in-status"
    values = ["opt-in-not-required"]
  }
}

(see #12412, #12517), else I get the error

--- FAIL: TestAccAWSENI_basic (62.89s)
    testing.go:669: Step 0 error: Check failed: Check 2/8 error: expected availability_zone to be us-west-2a, but was us-west-2-lax-1a

or

--- FAIL: TestAccAWSENI_ipv6_count (32.10s)
    testing.go:669: Step 0 error: errors during apply:
        
        Error: Error creating subnet: InvalidSubnet.Range: The IPv6 CIDR '2600:1f13:32d:8910::/64' is invalid.
        	status code: 400, request id: cf69d107-1eb1-4fc6-bb3c-e0802b8c0571
        
          on /tmp/tf-test080041457/main.tf line 16:
          (source code not available)

for the IPv6 case.

@DrFaust92
Copy link
Collaborator Author

@ewbankkit, fixed!

@ewbankkit
Copy link
Contributor

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSENI_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSENI_ -timeout 120m
=== RUN   TestAccAWSENI_basic
=== PAUSE TestAccAWSENI_basic
=== RUN   TestAccAWSENI_ipv6
=== PAUSE TestAccAWSENI_ipv6
=== RUN   TestAccAWSENI_ipv6_count
=== PAUSE TestAccAWSENI_ipv6_count
=== RUN   TestAccAWSENI_disappears
=== PAUSE TestAccAWSENI_disappears
=== RUN   TestAccAWSENI_updatedDescription
=== PAUSE TestAccAWSENI_updatedDescription
=== RUN   TestAccAWSENI_attached
=== PAUSE TestAccAWSENI_attached
=== RUN   TestAccAWSENI_ignoreExternalAttachment
=== PAUSE TestAccAWSENI_ignoreExternalAttachment
=== RUN   TestAccAWSENI_sourceDestCheck
=== PAUSE TestAccAWSENI_sourceDestCheck
=== RUN   TestAccAWSENI_computedIPs
=== PAUSE TestAccAWSENI_computedIPs
=== RUN   TestAccAWSENI_PrivateIpsCount
=== PAUSE TestAccAWSENI_PrivateIpsCount
=== RUN   TestAccAWSENI_tags
=== PAUSE TestAccAWSENI_tags
=== CONT  TestAccAWSENI_basic
=== CONT  TestAccAWSENI_tags
--- PASS: TestAccAWSENI_basic (76.21s)
=== CONT  TestAccAWSENI_PrivateIpsCount
--- PASS: TestAccAWSENI_tags (127.02s)
=== CONT  TestAccAWSENI_computedIPs
--- PASS: TestAccAWSENI_computedIPs (74.49s)
=== CONT  TestAccAWSENI_sourceDestCheck
--- PASS: TestAccAWSENI_PrivateIpsCount (152.00s)
=== CONT  TestAccAWSENI_ignoreExternalAttachment
--- PASS: TestAccAWSENI_sourceDestCheck (127.38s)
=== CONT  TestAccAWSENI_attached
--- PASS: TestAccAWSENI_ignoreExternalAttachment (115.07s)
=== CONT  TestAccAWSENI_updatedDescription
--- FAIL: TestAccAWSENI_attached (28.63s)
    testing.go:684: Step 0 error: errors during apply:
        
        Error: Error launching source instance: Unsupported: The requested configuration is currently not supported. Please check the documentation for supported configurations.
        	status code: 400, request id: 05f5ab80-b6b3-4354-8874-b0d3620e7f18
        
          on /tmp/tf-test893588910/main.tf line 39:
          (source code not available)
        
        
=== CONT  TestAccAWSENI_disappears
--- PASS: TestAccAWSENI_disappears (74.96s)
=== CONT  TestAccAWSENI_ipv6_count
--- PASS: TestAccAWSENI_updatedDescription (102.77s)
=== CONT  TestAccAWSENI_ipv6
--- PASS: TestAccAWSENI_ipv6 (76.82s)
--- PASS: TestAccAWSENI_ipv6_count (151.75s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	584.297s
FAIL
GNUmakefile:26: recipe for target 'testacc' failed
make: *** [testacc] Error 1

It looks like we need to add

data "aws_availability_zones" "available" {
  state = "available"
 
  filter {
    name   = "opt-in-status"
    values = ["opt-in-not-required"]
  }
}

to TestAccAWSENI_attached.

@DrFaust92
Copy link
Collaborator Author

fixed again

@ewbankkit
Copy link
Contributor

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSENI_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSENI_ -timeout 120m
=== RUN   TestAccAWSENI_basic
=== PAUSE TestAccAWSENI_basic
=== RUN   TestAccAWSENI_ipv6
=== PAUSE TestAccAWSENI_ipv6
=== RUN   TestAccAWSENI_ipv6_count
=== PAUSE TestAccAWSENI_ipv6_count
=== RUN   TestAccAWSENI_disappears
=== PAUSE TestAccAWSENI_disappears
=== RUN   TestAccAWSENI_updatedDescription
=== PAUSE TestAccAWSENI_updatedDescription
=== RUN   TestAccAWSENI_attached
=== PAUSE TestAccAWSENI_attached
=== RUN   TestAccAWSENI_ignoreExternalAttachment
=== PAUSE TestAccAWSENI_ignoreExternalAttachment
=== RUN   TestAccAWSENI_sourceDestCheck
=== PAUSE TestAccAWSENI_sourceDestCheck
=== RUN   TestAccAWSENI_computedIPs
=== PAUSE TestAccAWSENI_computedIPs
=== RUN   TestAccAWSENI_PrivateIpsCount
=== PAUSE TestAccAWSENI_PrivateIpsCount
=== RUN   TestAccAWSENI_tags
=== PAUSE TestAccAWSENI_tags
=== CONT  TestAccAWSENI_basic
=== CONT  TestAccAWSENI_ignoreExternalAttachment
--- PASS: TestAccAWSENI_basic (77.45s)
=== CONT  TestAccAWSENI_tags
--- PASS: TestAccAWSENI_ignoreExternalAttachment (114.93s)
=== CONT  TestAccAWSENI_PrivateIpsCount
--- PASS: TestAccAWSENI_tags (126.79s)
=== CONT  TestAccAWSENI_computedIPs
--- PASS: TestAccAWSENI_PrivateIpsCount (151.92s)
=== CONT  TestAccAWSENI_sourceDestCheck
--- PASS: TestAccAWSENI_computedIPs (75.15s)
=== CONT  TestAccAWSENI_disappears
--- PASS: TestAccAWSENI_disappears (74.97s)
=== CONT  TestAccAWSENI_attached
--- PASS: TestAccAWSENI_sourceDestCheck (126.19s)
=== CONT  TestAccAWSENI_updatedDescription
--- PASS: TestAccAWSENI_updatedDescription (103.28s)
=== CONT  TestAccAWSENI_ipv6_count
--- PASS: TestAccAWSENI_attached (271.79s)
=== CONT  TestAccAWSENI_ipv6
--- PASS: TestAccAWSENI_ipv6_count (153.18s)
--- PASS: TestAccAWSENI_ipv6 (77.86s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	704.046s

@DrFaust92
Copy link
Collaborator Author

@ewbankkit, as always thanks a lot!

@bryceml
Copy link

bryceml commented Jul 17, 2020

Just curious if there is something holding this up from being merged, I'd like to be able to use this. Last comment was about 6 weeks ago.

@DrFaust92 DrFaust92 added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Jul 18, 2020
@DrFaust92 DrFaust92 marked this pull request as draft August 28, 2020 20:06
@DrFaust92 DrFaust92 changed the title r/aws_network_interface - add support for ipv6 r/aws_network_interface - read after create + add support for ipv6 Aug 28, 2020
@DrFaust92 DrFaust92 marked this pull request as draft October 9, 2020 10:05
@DrFaust92 DrFaust92 marked this pull request as ready for review October 9, 2020 10:07
@DrFaust92
Copy link
Collaborator Author

Rebased + more test refactor:

--- PASS: TestAccAWSENI_computedIPs (114.66s)
--- PASS: TestAccAWSENI_basic (121.19s)
--- PASS: TestAccAWSENI_attached (194.78s)
--- PASS: TestAccAWSENI_PrivateIpsCount (259.21s)
--- PASS: TestAccAWSENI_ignoreExternalAttachment (182.95s)
--- PASS: TestAccAWSENI_sourceDestCheck (238.15s)
--- PASS: TestAccAWSENI_disappears (99.56s)
--- PASS: TestAccAWSENI_updatedDescription (191.10s)
--- PASS: TestAccAWSENI_ipv6_count (298.87s)
--- PASS: TestAccAWSENI_tags (238.05s)
--- PASS: TestAccAWSENI_ipv6 (248.03s)

@bflad bflad self-assigned this Oct 16, 2020
@bflad bflad added this to the v3.12.0 milestone Oct 16, 2020
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @DrFaust92 🚀

Output from acceptance testing in AWS Commercial:

--- PASS: TestAccAWSENI_attached (183.12s)
--- PASS: TestAccAWSENI_basic (102.76s)
--- PASS: TestAccAWSENI_computedIPs (101.72s)
--- PASS: TestAccAWSENI_disappears (87.26s)
--- PASS: TestAccAWSENI_ignoreExternalAttachment (185.44s)
--- PASS: TestAccAWSENI_ipv6 (141.80s)
--- PASS: TestAccAWSENI_ipv6_count (159.75s)
--- PASS: TestAccAWSENI_PrivateIpsCount (151.93s)
--- PASS: TestAccAWSENI_sourceDestCheck (143.44s)
--- PASS: TestAccAWSENI_tags (145.50s)
--- PASS: TestAccAWSENI_updatedDescription (123.99s)

--- PASS: TestAccAWSNetworkInterfaceSGAttachment_basic (70.59s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_DataSource (119.79s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_disappears (108.71s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Instance (139.32s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Multiple (90.30s)

Output from acceptance testing in AWS GovCloud (US):

--- PASS: TestAccAWSENI_attached (178.91s)
--- PASS: TestAccAWSENI_basic (95.41s)
--- PASS: TestAccAWSENI_computedIPs (93.75s)
--- PASS: TestAccAWSENI_disappears (85.16s)
--- PASS: TestAccAWSENI_ignoreExternalAttachment (146.43s)
--- PASS: TestAccAWSENI_ipv6 (131.01s)
--- PASS: TestAccAWSENI_ipv6_count (141.75s)
--- PASS: TestAccAWSENI_PrivateIpsCount (85.91s)
--- PASS: TestAccAWSENI_sourceDestCheck (131.61s)
--- PASS: TestAccAWSENI_tags (130.60s)
--- PASS: TestAccAWSENI_updatedDescription (112.40s)

--- PASS: TestAccAWSNetworkInterfaceSGAttachment_basic (68.81s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_DataSource (86.84s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_disappears (102.20s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Instance (121.17s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Multiple (83.78s)

@bflad bflad merged commit 3a02b4f into hashicorp:master Oct 16, 2020
bflad added a commit that referenced this pull request Oct 16, 2020
@ghost
Copy link

ghost commented Oct 22, 2020

This has been released in version 3.12.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 16, 2020

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 as resolved and limited conversation to collaborators Nov 16, 2020
@DrFaust92 DrFaust92 deleted the eni-ipv6-support branch April 15, 2021 10:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 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
5 participants