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

provider/aws: Change aws_elastic_ip_association to have computed parameters #6552

Merged
merged 5 commits into from
May 9, 2016

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented May 9, 2016

This PR takes the awesome work by @jkinred in #5236 and changes the parameters to be computed. This is being done so we can get this work into 0.6.16 :)

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEIPAssociation' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEIPAssociation -timeout 120m
=== RUN   TestAccAWSEIPAssociation_basic
--- PASS: TestAccAWSEIPAssociation_basic (181.41s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    181.432s

request := &ec2.DescribeAddressesInput{
Filters: []*ec2.Filter{
&ec2.Filter{
Name: aws.String("association-id"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be allocation-id ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When an EIP is associated with something (ENI, Instance, Public IP etc), it get's an association-id. This was used as the ID. Therefore, the call to get the association uses the ID rather than the different scenarios it can be. An association_id will always be specific AFAICT

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I misread and thought allocation was the ID

@catsby
Copy link
Contributor

catsby commented May 9, 2016

Looks good so far, I've left some comments that we need addressed or discussed before we can merge. Thanks for taking this up, @stack72 !

@stack72 stack72 force-pushed the jkinred-f-2680-aws-eip-association branch 2 times, most recently from 0ee38f3 to db55024 Compare May 9, 2016 16:20
parameters

The AWS API was send ing more parameters than we had set. Therefore,
Terraform was showing constant changes when plans were being formed
@stack72 stack72 force-pushed the jkinred-f-2680-aws-eip-association branch from db55024 to 8201f1d Compare May 9, 2016 16:28
@stack72
Copy link
Contributor Author

stack72 commented May 9, 2016

Test's still pass as expected:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEIPAssociation' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEIPAssociation -timeout 120m
=== RUN   TestAccAWSEIPAssociation_basic
--- PASS: TestAccAWSEIPAssociation_basic (163.37s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    163.391s

@catsby
Copy link
Contributor

catsby commented May 9, 2016

Good to go here

@stack72 stack72 merged commit af29a61 into master May 9, 2016
@stack72 stack72 deleted the jkinred-f-2680-aws-eip-association branch May 9, 2016 17:42
cristicalin pushed a commit to cristicalin/terraform that referenced this pull request May 24, 2016
…rameters (hashicorp#6552)

* New top level AWS resource aws_eip_association

* Add documentation for aws_eip_association

* Add tests for aws_eip_association

* provider/aws: Change `aws_elastic_ip_association` to have computed
parameters

The AWS API was send ing more parameters than we had set. Therefore,
Terraform was showing constant changes when plans were being formed
@ghost
Copy link

ghost commented Apr 26, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants