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: Add support for EFS #2196

Merged
merged 3 commits into from
Oct 5, 2015
Merged

Conversation

radeksimko
Copy link
Member

EFS

It's actually in preview, where you have to request access to it and Amazon will eventually approve it, but since it's publicly available in the SDK, I was thinking it's not so "hush-hush" that I'd have to wait. Either way, whoever is going to run the acceptance tests will likely need the EFS to be enabled in that test AWS account.

Checklist

  • efs_file_system resource
  • efs_file_system.tags
  • efs_mount_target resource
  • docs

Test plan

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=EFS' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=EFS -timeout 90m
=== RUN   TestAccAWSEFSFileSystem
--- PASS: TestAccAWSEFSFileSystem (18.35s)
=== RUN   TestAccAWSEFSMountTarget
--- PASS: TestAccAWSEFSMountTarget (347.33s)
=== RUN   TestDiffEFSTags
--- PASS: TestDiffEFSTags (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    365.699s

@radeksimko
Copy link
Member Author

Implementation of mount targets is currently blocked by upstream because AWS for some reason doesn't allow search (i.e. describe-mount-targets) by Mount Target ID, only by File System ID, which is not scalable at all as we'd have to get all mount targets attached to a file system and then locally look for the right one.

http://docs.aws.amazon.com/cli/latest/reference/efs/describe-mount-targets.html

This has been raised with AWS and looks like a feature that should be there (according to support representative).

@radeksimko
Copy link
Member Author

After talking to AWS and re-reading relevant docs, it makes more sense to create mount_target as a subresource - e.g.

resource "efs_file_system" "one" {
  mount_target {
    subnet_id = "subnet-12345678" # AZ 1
    ip_address = "10.1.2.3"
    security_groups = ["sg-12345", "sg-98765"]
  }

  mount_target {
    subnet_id = "subnet-9876543" # AZ 2
    ip_address = "10.2.1.1"
    security_groups = ["sg-aaaaaa", "sg-bbbbbb"]
  }
}

The highlight from docs is:

Number of mount targets per file system per Availability Zone: 1

Therefore the original concern about scalability of the algorithm where we list through all mount targets related to a FS is becoming invalid. It should scale fine.

Very often people are going to have 3-4 mount targets, eventually twice or 3 times as much, but maximum is number of regions * number of AZs per a single EFS file system.

@radeksimko
Copy link
Member Author

This is now ready for review.

@josephholsten
Copy link
Contributor

@phinze ready for review

@catsby
Copy link
Contributor

catsby commented Oct 2, 2015

Hey @radeksimko – sorry for the lag here, the code looks good but the tests won't run, I think the SDK has changed out from under this. Could you take a look? Seems the API around MountTargetId has changed 😦

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label Oct 2, 2015
@radeksimko
Copy link
Member Author

Hey @catsby
I think it was just a conflict w/ the CloudWatch log group that has been merged recently.

Fixed now. I also tried running acceptance tests again and everything seems fine.

@radeksimko radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Oct 3, 2015
@radeksimko radeksimko force-pushed the efs branch 7 times, most recently from 594c04e to f1189ad Compare October 3, 2015 19:59
@catsby
Copy link
Contributor

catsby commented Oct 5, 2015

LGTM, merge away...

radeksimko added a commit that referenced this pull request Oct 5, 2015
provider/aws: Add support for EFS
@radeksimko radeksimko merged commit 71d3f18 into hashicorp:master Oct 5, 2015
@radeksimko radeksimko deleted the efs branch October 5, 2015 14:06
@stack72
Copy link
Contributor

stack72 commented Oct 5, 2015

nice work @radeksimko :)

@ghost
Copy link

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

5 participants