From fceb3ac381cfa6669cb6905b0dae5d00d18e7ce5 Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczynski Date: Sat, 3 Sep 2016 11:40:24 +0100 Subject: [PATCH] Maintenance and clean-up for aws_efs_file_system. This commit is purely a maintenance and clean-up of the resource. Signed-off-by: Krzysztof Wilczynski --- .../aws/resource_aws_efs_file_system.go | 45 ++++++++------ .../aws/resource_aws_efs_file_system_test.go | 61 +++++++++++-------- 2 files changed, 62 insertions(+), 44 deletions(-) diff --git a/builtin/providers/aws/resource_aws_efs_file_system.go b/builtin/providers/aws/resource_aws_efs_file_system.go index d0524aef3..e3369c8d0 100644 --- a/builtin/providers/aws/resource_aws_efs_file_system.go +++ b/builtin/providers/aws/resource_aws_efs_file_system.go @@ -24,14 +24,14 @@ func resourceAwsEfsFileSystem() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "creation_token": &schema.Schema{ + "creation_token": { Type: schema.TypeString, Optional: true, ForceNew: true, ValidateFunc: validateMaxLength(64), }, - "reference_name": &schema.Schema{ + "reference_name": { Type: schema.TypeString, Optional: true, ForceNew: true, @@ -40,7 +40,7 @@ func resourceAwsEfsFileSystem() *schema.Resource { ValidateFunc: validateReferenceName, }, - "performance_mode": &schema.Schema{ + "performance_mode": { Type: schema.TypeString, Optional: true, ForceNew: true, @@ -95,8 +95,8 @@ func resourceAwsEfsFileSystemCreate(d *schema.ResourceData, meta interface{}) er return nil, "error", err } - if len(resp.FileSystems) < 1 { - return nil, "not-found", fmt.Errorf("EFS file system %q not found", d.Id()) + if hasEmptyFileSystems(resp) { + return nil, "not-found", fmt.Errorf("EFS file system %q could not be found.", d.Id()) } fs := resp.FileSystems[0] @@ -110,7 +110,7 @@ func resourceAwsEfsFileSystemCreate(d *schema.ResourceData, meta interface{}) er _, err = stateConf.WaitForState() if err != nil { - return fmt.Errorf("Error waiting for EFS file system (%q) to create: %q", + return fmt.Errorf("Error waiting for EFS file system (%q) to create: %s", d.Id(), err.Error()) } log.Printf("[DEBUG] EFS file system %q created.", d.Id()) @@ -122,7 +122,7 @@ func resourceAwsEfsFileSystemUpdate(d *schema.ResourceData, meta interface{}) er conn := meta.(*AWSClient).efsconn err := setTagsEFS(conn, d) if err != nil { - return fmt.Errorf("Error setting EC2 tags for EFS file system (%q): %q", + return fmt.Errorf("Error setting EC2 tags for EFS file system (%q): %s", d.Id(), err.Error()) } @@ -137,25 +137,29 @@ func resourceAwsEfsFileSystemRead(d *schema.ResourceData, meta interface{}) erro }) if err != nil { if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "FileSystemNotFound" { - log.Printf("[WARN] EFS File System (%s) not found, error code (404)", d.Id()) + log.Printf("[WARN] EFS file system (%s) could not be found.", d.Id()) d.SetId("") return nil } return err } - if len(resp.FileSystems) < 1 { - return fmt.Errorf("EFS file system %q not found", d.Id()) + + if hasEmptyFileSystems(resp) { + return fmt.Errorf("EFS file system %q could not be found.", d.Id()) } tagsResp, err := conn.DescribeTags(&efs.DescribeTagsInput{ FileSystemId: aws.String(d.Id()), }) if err != nil { - return fmt.Errorf("Error retrieving EC2 tags for EFS file system (%q): %q", + return fmt.Errorf("Error retrieving EC2 tags for EFS file system (%q): %s", d.Id(), err.Error()) } - d.Set("tags", tagsToMapEFS(tagsResp.Tags)) + err = d.Set("tags", tagsToMapEFS(tagsResp.Tags)) + if err != nil { + return err + } return nil } @@ -182,7 +186,7 @@ func resourceAwsEfsFileSystemDelete(d *schema.ResourceData, meta interface{}) er return nil, "error", err } - if len(resp.FileSystems) < 1 { + if hasEmptyFileSystems(resp) { return nil, "", nil } @@ -197,7 +201,7 @@ func resourceAwsEfsFileSystemDelete(d *schema.ResourceData, meta interface{}) er _, err = stateConf.WaitForState() if err != nil { - return fmt.Errorf("Error waiting for EFS file system (%q) to delete: %q", + return fmt.Errorf("Error waiting for EFS file system (%q) to delete: %s", d.Id(), err.Error()) } @@ -218,10 +222,17 @@ func validateReferenceName(v interface{}, k string) (ws []string, errors []error func validatePerformanceModeType(v interface{}, k string) (ws []string, errors []error) { value := v.(string) - if value != "generalPurpose" && value != "maxIO" { + if value != efs.PerformanceModeGeneralPurpose && value != efs.PerformanceModeMaxIo { errors = append(errors, fmt.Errorf( - "%q contains an invalid Performance Mode %q. Valid modes are either %q or %q", - k, value, "generalPurpose", "maxIO")) + "%q contains an invalid Performance Mode %q. Valid modes are either %q or %q.", + k, value, efs.PerformanceModeGeneralPurpose, efs.PerformanceModeMaxIo)) } return } + +func hasEmptyFileSystems(fs *efs.DescribeFileSystemsOutput) bool { + if fs != nil && len(fs.FileSystems) > 0 { + return false + } + return true +} diff --git a/builtin/providers/aws/resource_aws_efs_file_system_test.go b/builtin/providers/aws/resource_aws_efs_file_system_test.go index bc22c9807..952057f7d 100644 --- a/builtin/providers/aws/resource_aws_efs_file_system_test.go +++ b/builtin/providers/aws/resource_aws_efs_file_system_test.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestResourceAWSEFSReferenceName_validation(t *testing.T) { +func TestResourceAWSEFSFileSystem_validateReferenceName(t *testing.T) { var value string var errors []error @@ -27,35 +27,20 @@ func TestResourceAWSEFSReferenceName_validation(t *testing.T) { value = acctest.RandString(32) _, errors = validateReferenceName(value, "reference_name") if len(errors) != 0 { - t.Fatalf("Expected to trigger a validation error") + t.Fatalf("Expected not to trigger a validation error") } } -func TestResourceAWSEFSPerformanceMode_validation(t *testing.T) { - type testCase struct { +func TestResourceAWSEFSFileSystem_validatePerformanceModeType(t *testing.T) { + _, errors := validatePerformanceModeType("incorrect", "performance_mode") + if len(errors) == 0 { + t.Fatalf("Expected to trigger a validation error") + } + + var testCases = []struct { Value string ErrCount int - } - - invalidCases := []testCase{ - { - Value: "garrusVakarian", - ErrCount: 1, - }, - { - Value: acctest.RandString(80), - ErrCount: 1, - }, - } - - for _, tc := range invalidCases { - _, errors := validatePerformanceModeType(tc.Value, "performance_mode") - if len(errors) != tc.ErrCount { - t.Fatalf("Expected to trigger a validation error") - } - } - - validCases := []testCase{ + }{ { Value: "generalPurpose", ErrCount: 0, @@ -66,14 +51,36 @@ func TestResourceAWSEFSPerformanceMode_validation(t *testing.T) { }, } - for _, tc := range validCases { - _, errors := validatePerformanceModeType(tc.Value, "aws_efs_file_system") + for _, tc := range testCases { + _, errors := validatePerformanceModeType(tc.Value, "performance_mode") if len(errors) != tc.ErrCount { t.Fatalf("Expected not to trigger a validation error") } } } +func TestResourceAWSEFSFileSystem_hasEmptyFileSystems(t *testing.T) { + fs := &efs.DescribeFileSystemsOutput{ + FileSystems: []*efs.FileSystemDescription{}, + } + + var actual bool + + actual = hasEmptyFileSystems(fs) + if !actual { + t.Fatalf("Expected return value to be true, got %t", actual) + } + + // Add an empty file system. + fs.FileSystems = append(fs.FileSystems, &efs.FileSystemDescription{}) + + actual = hasEmptyFileSystems(fs) + if actual { + t.Fatalf("Expected return value to be false, got %t", actual) + } + +} + func TestAccAWSEFSFileSystem_basic(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) },