From 6a94f920e1f44b962c3a9e998f70db17e5caf5ed Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczynski Date: Tue, 30 Aug 2016 10:42:54 +0100 Subject: [PATCH] provider/aws: Handle missing EFS mount target in aws_efs_mount_target. (#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 * Add EFS mount target DNS helper function. Signed-off-by: Krzysztof Wilczynski * 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 * 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 --- .../aws/resource_aws_efs_mount_target.go | 61 +++++++--- .../aws/resource_aws_efs_mount_target_test.go | 109 +++++++++++++++++- 2 files changed, 150 insertions(+), 20 deletions(-) diff --git a/builtin/providers/aws/resource_aws_efs_mount_target.go b/builtin/providers/aws/resource_aws_efs_mount_target.go index bc3ffbb936..8529fba690 100644 --- a/builtin/providers/aws/resource_aws_efs_mount_target.go +++ b/builtin/providers/aws/resource_aws_efs_mount_target.go @@ -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) @@ -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"}, @@ -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] @@ -161,11 +162,19 @@ 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] @@ -173,10 +182,10 @@ func resourceAwsEfsMountTargetRead(d *schema.ResourceData, meta interface{}) err 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()), @@ -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 } @@ -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 @@ -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 } @@ -261,7 +277,7 @@ 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()) } @@ -269,3 +285,14 @@ func resourceAwsEfsMountTargetDelete(d *schema.ResourceData, meta interface{}) e 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 +} diff --git a/builtin/providers/aws/resource_aws_efs_mount_target_test.go b/builtin/providers/aws/resource_aws_efs_mount_target_test.go index e93357b332..78f5faa7eb 100644 --- a/builtin/providers/aws/resource_aws_efs_mount_target_test.go +++ b/builtin/providers/aws/resource_aws_efs_mount_target_test.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" "testing" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -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, @@ -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", @@ -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", @@ -45,6 +50,7 @@ func TestAccAWSEFSMountTarget_basic(t *testing.T) { ), testAccCheckEfsMountTarget( "aws_efs_mount_target.beta", + &mount, ), resource.TestMatchResourceAttr( "aws_efs_mount_target.beta", @@ -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 { @@ -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 { @@ -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" { @@ -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" {