Add EFS storage driver #231
Conversation
| ### Runtime Behavior | ||
| AWS EFS storage driver creates one EFS FileSystem per volume and provides root | ||
| of the filesystem as NFS mount point. Volumes aren't attached to instances | ||
| directly but rather exposed to each subnet by creating `MountPoint` in each VPC |
There was a problem hiding this comment.
MountTarget? - also should this link out to https://docs.aws.amazon.com/efs/latest/ug/how-it-works.html ?
| }, | ||
| }) | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
should we attempt to purge the volume here if tag creation fails to avoid leaking volumes?
mhrabovcin
force-pushed
the
efs
branch
2 times, most recently
from
448bb9d to
23084a1
Compare
| "github.com/akutz/gofig" | ||
| "github.com/akutz/goof" | ||
|
|
||
| "github.com/aws/aws-sdk-go/aws/ec2metadata" |
There was a problem hiding this comment.
The AWS imports in the executor package need to be refactored per our call.
| return string(subnetID), nil | ||
| } | ||
|
|
||
| func (r *AwsVpcSubnetResolver) getUrl(path string) string { |
mhrabovcin
force-pushed
the
efs
branch
2 times, most recently
from
e59d83d to
f45a29f
Compare
Current coverage is 26.17% (diff: 3.44%)@@ master #231 diff @@
==========================================
Files 36 39 +3
Lines 2001 2407 +406
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 617 630 +13
- Misses 1316 1709 +393
Partials 68 68
|
|
@mhrabovcin : I checked out your test scripts. They look good to me. The naming convention is also good. There are some basics that you may want to add later to do some error handling for any failed commands in your script, or different Cloud Formation status messages (eg CREATE FAILED) or check to make sure the environment shuts down correctly in your test-env-down.sh script - not critical by any means. There is also a small typo I noticed in your test README: |
|
Martin Hrabovcin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
|
@cduchesne thanks for the comments. I've fixed the typo and also improved / simplified error handling in test environment scripts. |
|
Hi @mhrabovcin, Thank you for this sir. It has been merged. I apologize it took longer than I said, but I was thrown into some other priorities for a bit. The good news is now I have help, and so I was able to merge this. Thank you again! |
I've worked on this patch with @kasisnu .