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: Updating the aws_efs_mount_target dns_name #11023

Merged
merged 1 commit into from
Jan 4, 2017

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Jan 4, 2017

Fixes:#10902

AWS introduced a change to the Mount Target DNS Name to remove the
availability_zone from it -
https://aws.amazon.com/about-aws/whats-new/2016/12/simplified-mounting-of-amazon-efs-file-systems/

This was because there used to be a limit of 1 mount target per AZ -
this has been raised.

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEFSMountTarget_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/04 10:45:35 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEFSMountTarget_ -timeout 120m
=== RUN   TestAccAWSEFSMountTarget_importBasic
--- PASS: TestAccAWSEFSMountTarget_importBasic (236.19s)
=== RUN   TestAccAWSEFSMountTarget_basic
--- PASS: TestAccAWSEFSMountTarget_basic (445.52s)
=== RUN   TestAccAWSEFSMountTarget_disappears
--- PASS: TestAccAWSEFSMountTarget_disappears (228.31s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	910.044s

Fixes:#10902

AWS introduced a change to the Mount Target DNS Name to remove the
availability_zone from it -
https://aws.amazon.com/about-aws/whats-new/2016/12/simplified-mounting-of-amazon-efs-file-systems/

This was because there used to be a limit of 1 mount target per AZ -
this has been raised.

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEFSMountTarget_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/04 10:45:35 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEFSMountTarget_ -timeout 120m
=== RUN   TestAccAWSEFSMountTarget_importBasic
--- PASS: TestAccAWSEFSMountTarget_importBasic (236.19s)
=== RUN   TestAccAWSEFSMountTarget_basic
--- PASS: TestAccAWSEFSMountTarget_basic (445.52s)
=== RUN   TestAccAWSEFSMountTarget_disappears
--- PASS: TestAccAWSEFSMountTarget_disappears (228.31s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	910.044s
```
@catsby
Copy link
Contributor

catsby commented Jan 4, 2017

I'm thinking ( 🤔 ), should this region-only DNS Name be a separate computed attribute? Is the AZ specific one still valid, or is it deprecated? I'm thinking that anyone referencing efs_mount_target.mount.dns_name would see a downstream change because of this. Maybe that's unavoidable if the AZ one is deprecated 🐼

@catsby
Copy link
Contributor

catsby commented Jan 4, 2017

so, I re-read #10902 and it seems the AZ is just dropped on AWS side? If so, then this LGTM 👍

@stack72 stack72 merged commit 8542715 into master Jan 4, 2017
@stack72
Copy link
Contributor Author

stack72 commented Jan 4, 2017

Thanks @catsby :) yes, they dropped it on their side

@stack72 stack72 deleted the b-aws-efs-mount-target_dns_name branch January 4, 2017 23:12
@@ -200,13 +200,13 @@ func resourceAwsEfsMountTargetRead(d *schema.ResourceData, meta interface{}) err
}

// DNS name per http://docs.aws.amazon.com/efs/latest/ug/mounting-fs-mount-cmd-dns-name.html
az, err := getAzFromSubnetId(*mt.SubnetId, meta.(*AWSClient).ec2conn)
_, err = getAzFromSubnetId(*mt.SubnetId, meta.(*AWSClient).ec2conn)

Choose a reason for hiding this comment

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

Is there any point of calling getAzFromSubnetId if the return value isn't used?

@tjwallace
Copy link

Could the dns_name attribute also be added to the aws_efs_file_system resource now, since the value of efs_mount_target.dns_name will be the same for the same file_system_id?

@ghost
Copy link

ghost commented Apr 18, 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 18, 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.

3 participants