Skip to content

Commit

Permalink
provider/aws: Handle missing EFS mount target in aws_efs_mount_target. (
Browse files Browse the repository at this point in the history 
#8529)

* Handle missing EFS mount target in aws_efs_mount_target.

This commit resolves issue where the EFS mount target would be already
deleted (e.g. it was deleted outside of Terraform, etc.). Also, correct
how values are begin set in the ReadFunc to avoid nil pointer dereference.

Signed-off-by: Krzysztof Wilczynski <[email protected]>

* Add EFS mount target DNS helper function.

Signed-off-by: Krzysztof Wilczynski <[email protected]>

* Add EFS mount target response helper.

This commit adds a helper which can be used to check whether the response
contains a valid and non-empty list of EFS file system mount targets.

Signed-off-by: Krzysztof Wilczynski <[email protected]>

* Add acceptance test to check for non-empty plan.

This commit adds a test to verify the condition where the underlying EFS mount
target would be deleted and/or disappear resulting in a new resource to be
created to replace it.

Signed-off-by: Krzysztof Wilczynski <[email protected]>
  • Loading branch information
@kwilczynski @stack72
kwilczynski authored and stack72 committed Aug 30, 2016
1 parent 76ea6c5 commit 6a94f92
Show file tree
Hide file tree
Showing 2 changed files with 150 additions and 20 deletions.
61 changes: 44 additions & 17 deletions builtin/providers/aws/resource_aws_efs_mount_target.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func resourceAwsEfsMountTargetCreate(d *schema.ResourceData, meta interface{}) e
// So we make it fail by calling 1 request per AZ at a time.
az, err := getAzFromSubnetId(subnetId, meta.(*AWSClient).ec2conn)
if err != nil {
return fmt.Errorf("Failed getting AZ from subnet ID (%s): %s", subnetId, err)
return fmt.Errorf("Failed getting Availability Zone from subnet ID (%s): %s", subnetId, err)
}
mtKey := "efs-mt-" + fsId + "-" + az
awsMutexKV.Lock(mtKey)
Expand All @@ -102,6 +102,7 @@ func resourceAwsEfsMountTargetCreate(d *schema.ResourceData, meta interface{}) e
}

d.SetId(*mt.MountTargetId)
log.Printf("[INFO] EFS mount target ID: %s", d.Id())

stateConf := &resource.StateChangeConf{
Pending: []string{"creating"},
Expand All @@ -114,8 +115,8 @@ func resourceAwsEfsMountTargetCreate(d *schema.ResourceData, meta interface{}) e
return nil, "error", err
}

if len(resp.MountTargets) < 1 {
return nil, "error", fmt.Errorf("EFS mount target %q not found", d.Id())
if hasEmptyMountTargets(resp) {
return nil, "error", fmt.Errorf("EFS mount target %q could not be found.", d.Id())
}

mt := resp.MountTargets[0]
Expand Down Expand Up @@ -161,22 +162,30 @@ func resourceAwsEfsMountTargetRead(d *schema.ResourceData, meta interface{}) err
MountTargetId: aws.String(d.Id()),
})
if err != nil {
return err
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "MountTargetNotFound" {
// The EFS mount target could not be found,
// which would indicate that it might be
// already deleted.
log.Printf("[WARN] EFS mount target %q could not be found.", d.Id())
d.SetId("")
return nil
}
return fmt.Errorf("Error reading EFS mount target %s: %s", d.Id(), err)
}

if len(resp.MountTargets) < 1 {
return fmt.Errorf("EFS mount target %q not found", d.Id())
if hasEmptyMountTargets(resp) {
return fmt.Errorf("EFS mount target %q could not be found.", d.Id())
}

mt := resp.MountTargets[0]

log.Printf("[DEBUG] Found EFS mount target: %#v", mt)

d.SetId(*mt.MountTargetId)
d.Set("file_system_id", *mt.FileSystemId)
d.Set("ip_address", *mt.IpAddress)
d.Set("subnet_id", *mt.SubnetId)
d.Set("network_interface_id", *mt.NetworkInterfaceId)
d.Set("file_system_id", mt.FileSystemId)
d.Set("ip_address", mt.IpAddress)
d.Set("subnet_id", mt.SubnetId)
d.Set("network_interface_id", mt.NetworkInterfaceId)

sgResp, err := conn.DescribeMountTargetSecurityGroups(&efs.DescribeMountTargetSecurityGroupsInput{
MountTargetId: aws.String(d.Id()),
Expand All @@ -185,15 +194,22 @@ func resourceAwsEfsMountTargetRead(d *schema.ResourceData, meta interface{}) err
return err
}

d.Set("security_groups", schema.NewSet(schema.HashString, flattenStringList(sgResp.SecurityGroups)))
err = d.Set("security_groups", schema.NewSet(schema.HashString, flattenStringList(sgResp.SecurityGroups)))
if err != nil {
return 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)
if err != nil {
return fmt.Errorf("Failed getting AZ from subnet ID (%s): %s", *mt.SubnetId, err)
return fmt.Errorf("Failed getting Availability Zone from subnet ID (%s): %s", *mt.SubnetId, err)
}

region := meta.(*AWSClient).region
d.Set("dns_name", fmt.Sprintf("%s.%s.efs.%s.amazonaws.com", az, *mt.FileSystemId, region))
err = d.Set("dns_name", resourceAwsEfsMountTargetDnsName(az, *mt.FileSystemId, region))
if err != nil {
return err
}

return nil
}
Expand All @@ -207,8 +223,8 @@ func getAzFromSubnetId(subnetId string, conn *ec2.EC2) (string, error) {
return "", err
}

if len(out.Subnets) != 1 {
return "", fmt.Errorf("Expected exactly 1 subnet returned for %q", subnetId)
if l := len(out.Subnets); l != 1 {
return "", fmt.Errorf("Expected exactly 1 subnet returned for %q, got: %d", subnetId, l)
}

return *out.Subnets[0].AvailabilityZone, nil
Expand Down Expand Up @@ -245,7 +261,7 @@ func resourceAwsEfsMountTargetDelete(d *schema.ResourceData, meta interface{}) e
return nil, "error", awsErr
}

if len(resp.MountTargets) < 1 {
if hasEmptyMountTargets(resp) {
return nil, "", nil
}

Expand All @@ -261,11 +277,22 @@ func resourceAwsEfsMountTargetDelete(d *schema.ResourceData, meta interface{}) e

_, err = stateConf.WaitForState()
if err != nil {
return fmt.Errorf("Error waiting for EFS mount target (%q) to delete: %q",
return fmt.Errorf("Error waiting for EFS mount target (%q) to delete: %s",
d.Id(), err.Error())
}

log.Printf("[DEBUG] EFS mount target %q deleted.", d.Id())

return nil
}

func resourceAwsEfsMountTargetDnsName(az, fileSystemId, region string) string {
return fmt.Sprintf("%s.%s.efs.%s.amazonaws.com", az, fileSystemId, region)
}

func hasEmptyMountTargets(mto *efs.DescribeMountTargetsOutput) bool {
if mto != nil && len(mto.MountTargets) > 0 {
return false
}
return true
}
109 changes: 106 additions & 3 deletions builtin/providers/aws/resource_aws_efs_mount_target_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"regexp"
"testing"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
Expand All @@ -14,6 +15,8 @@ import (
)

func TestAccAWSEFSMountTarget_basic(t *testing.T) {
var mount efs.MountTargetDescription

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Expand All @@ -24,6 +27,7 @@ func TestAccAWSEFSMountTarget_basic(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckEfsMountTarget(
"aws_efs_mount_target.alpha",
&mount,
),
resource.TestMatchResourceAttr(
"aws_efs_mount_target.alpha",
Expand All @@ -37,6 +41,7 @@ func TestAccAWSEFSMountTarget_basic(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckEfsMountTarget(
"aws_efs_mount_target.alpha",
&mount,
),
resource.TestMatchResourceAttr(
"aws_efs_mount_target.alpha",
Expand All @@ -45,6 +50,7 @@ func TestAccAWSEFSMountTarget_basic(t *testing.T) {
),
testAccCheckEfsMountTarget(
"aws_efs_mount_target.beta",
&mount,
),
resource.TestMatchResourceAttr(
"aws_efs_mount_target.beta",
Expand All @@ -57,6 +63,62 @@ func TestAccAWSEFSMountTarget_basic(t *testing.T) {
})
}

func TestAccAWSEFSMountTarget_disappears(t *testing.T) {
var mount efs.MountTargetDescription

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckVpnGatewayDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSEFSMountTargetConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckEfsMountTarget(
"aws_efs_mount_target.alpha",
&mount,
),
testAccAWSEFSMountTargetDisappears(&mount),
),
ExpectNonEmptyPlan: true,
},
},
})
}

func TestResourceAWSEFSMountTarget_mountTargetDnsName(t *testing.T) {
actual := resourceAwsEfsMountTargetDnsName("non-existant-1c",
"fs-123456ab", "non-existant-1")

expected := "non-existant-1c.fs-123456ab.efs.non-existant-1.amazonaws.com"
if actual != expected {
t.Fatalf("Expected EFS mount target DNS name to be %s, got %s",
expected, actual)
}
}

func TestResourceAWSEFSMountTarget_hasEmptyMountTargets(t *testing.T) {
mto := &efs.DescribeMountTargetsOutput{
MountTargets: []*efs.MountTargetDescription{},
}

var actual bool

actual = hasEmptyMountTargets(mto)
if !actual {
t.Fatalf("Expected return value to be true, got %t", actual)
}

// Add an empty mount target.
mto.MountTargets = append(mto.MountTargets, &efs.MountTargetDescription{})

actual = hasEmptyMountTargets(mto)
if actual {
t.Fatalf("Expected return value to be false, got %t", actual)
}

}

func testAccCheckEfsMountTargetDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).efsconn
for _, rs := range s.RootModule().Resources {
Expand All @@ -82,7 +144,7 @@ func testAccCheckEfsMountTargetDestroy(s *terraform.State) error {
return nil
}

func testAccCheckEfsMountTarget(resourceID string) resource.TestCheckFunc {
func testAccCheckEfsMountTarget(resourceID string, mount *efs.MountTargetDescription) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[resourceID]
if !ok {
Expand Down Expand Up @@ -111,13 +173,54 @@ func testAccCheckEfsMountTarget(resourceID string) resource.TestCheckFunc {
*mt.MountTargets[0].MountTargetId, fs.Primary.ID)
}

*mount = *mt.MountTargets[0]

return nil
}
}

func testAccAWSEFSMountTargetDisappears(mount *efs.MountTargetDescription) resource.TestCheckFunc {
return func(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).efsconn

_, err := conn.DeleteMountTarget(&efs.DeleteMountTargetInput{
MountTargetId: mount.MountTargetId,
})

if err != nil {
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "MountTargetNotFound" {
return nil
}
return err
}

return resource.Retry(3*time.Minute, func() *resource.RetryError {
resp, err := conn.DescribeMountTargets(&efs.DescribeMountTargetsInput{
MountTargetId: mount.MountTargetId,
})
if err != nil {
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "MountTargetNotFound" {
return nil
}
return resource.NonRetryableError(
fmt.Errorf("Error reading EFS mount target: %s", err))
}
if resp.MountTargets == nil || len(resp.MountTargets) < 1 {
return nil
}
if *resp.MountTargets[0].LifeCycleState == "deleted" {
return nil
}
return resource.RetryableError(fmt.Errorf(
"Waiting for EFS mount target: %s", mount.MountTargetId))
})
}

}

const testAccAWSEFSMountTargetConfig = `
resource "aws_efs_file_system" "foo" {
reference_name = "radeksimko"
creation_token = "radeksimko"
}
resource "aws_efs_mount_target" "alpha" {
Expand All @@ -138,7 +241,7 @@ resource "aws_subnet" "alpha" {

const testAccAWSEFSMountTargetConfigModified = `
resource "aws_efs_file_system" "foo" {
reference_name = "radeksimko"
creation_token = "radeksimko"
}
resource "aws_efs_mount_target" "alpha" {
Expand Down

0 comments on commit 6a94f92

Please sign in to comment.