From 7550c00dd57a8d2a16258f404c380f96f153ae3d Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 19 Mar 2018 09:26:40 -0400 Subject: [PATCH 1/2] resource/aws_elasticache_*: Allow port to be optional --- aws/import_aws_elasticache_cluster_test.go | 36 ----- aws/resource_aws_elasticache_cluster.go | 17 +- aws/resource_aws_elasticache_cluster_test.go | 149 ++++++++++++++++-- ...ource_aws_elasticache_replication_group.go | 5 +- .../docs/r/elasticache_cluster.html.markdown | 3 +- ...lasticache_replication_group.html.markdown | 2 +- 6 files changed, 156 insertions(+), 56 deletions(-) delete mode 100644 aws/import_aws_elasticache_cluster_test.go diff --git a/aws/import_aws_elasticache_cluster_test.go b/aws/import_aws_elasticache_cluster_test.go deleted file mode 100644 index 6128ddf9503e..000000000000 --- a/aws/import_aws_elasticache_cluster_test.go +++ /dev/null @@ -1,36 +0,0 @@ -package aws - -import ( - "os" - "testing" - - "github.com/hashicorp/terraform/helper/acctest" - "github.com/hashicorp/terraform/helper/resource" -) - -func TestAccAWSElasticacheCluster_importBasic(t *testing.T) { - oldvar := os.Getenv("AWS_DEFAULT_REGION") - os.Setenv("AWS_DEFAULT_REGION", "us-east-1") - defer os.Setenv("AWS_DEFAULT_REGION", oldvar) - - name := acctest.RandString(10) - - resourceName := "aws_elasticache_cluster.bar" - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSElasticacheClusterDestroy, - Steps: []resource.TestStep{ - resource.TestStep{ - Config: testAccAWSElasticacheClusterConfigBasic(name), - }, - - resource.TestStep{ - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, - }, - }) -} diff --git a/aws/resource_aws_elasticache_cluster.go b/aws/resource_aws_elasticache_cluster.go index 9e778189b49b..9f3f3c368b49 100644 --- a/aws/resource_aws_elasticache_cluster.go +++ b/aws/resource_aws_elasticache_cluster.go @@ -102,8 +102,15 @@ func resourceAwsElastiCacheCommonSchema() map[string]*schema.Schema { }, "port": { Type: schema.TypeInt, - Required: true, + Optional: true, ForceNew: true, + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + // Supress default memcached/redis ports when not defined + if !d.IsNewResource() && new == "0" && (old == "6379" || old == "11211") { + return true + } + return false + }, }, "notification_topic_arn": { Type: schema.TypeString, @@ -221,7 +228,6 @@ func resourceAwsElasticacheClusterCreate(d *schema.ResourceData, meta interface{ numNodes := int64(d.Get("num_cache_nodes").(int)) // 2 engine := d.Get("engine").(string) // memcached engineVersion := d.Get("engine_version").(string) // 1.4.14 - port := int64(d.Get("port").(int)) // e.g) 11211 subnetGroupName := d.Get("subnet_group_name").(string) securityNameSet := d.Get("security_group_names").(*schema.Set) securityIdSet := d.Get("security_group_ids").(*schema.Set) @@ -236,7 +242,6 @@ func resourceAwsElasticacheClusterCreate(d *schema.ResourceData, meta interface{ NumCacheNodes: aws.Int64(numNodes), Engine: aws.String(engine), EngineVersion: aws.String(engineVersion), - Port: aws.Int64(port), CacheSubnetGroupName: aws.String(subnetGroupName), CacheSecurityGroupNames: securityNames, SecurityGroupIds: securityIds, @@ -248,6 +253,10 @@ func resourceAwsElasticacheClusterCreate(d *schema.ResourceData, meta interface{ req.CacheParameterGroupName = aws.String(v.(string)) } + if v, ok := d.GetOk("port"); ok { + req.Port = aws.Int64(int64(v.(int))) + } + if v, ok := d.GetOk("snapshot_retention_limit"); ok { req.SnapshotRetentionLimit = aws.Int64(int64(v.(int))) } @@ -352,6 +361,8 @@ func resourceAwsElasticacheClusterRead(d *schema.ResourceData, meta interface{}) d.Set("port", c.ConfigurationEndpoint.Port) d.Set("configuration_endpoint", aws.String(fmt.Sprintf("%s:%d", *c.ConfigurationEndpoint.Address, *c.ConfigurationEndpoint.Port))) d.Set("cluster_address", aws.String(fmt.Sprintf("%s", *c.ConfigurationEndpoint.Address))) + } else if len(c.CacheNodes) > 0 { + d.Set("port", int(aws.Int64Value(c.CacheNodes[0].Endpoint.Port))) } if c.ReplicationGroupId != nil { diff --git a/aws/resource_aws_elasticache_cluster_test.go b/aws/resource_aws_elasticache_cluster_test.go index 8dbe2da6d32a..29565686ab8d 100644 --- a/aws/resource_aws_elasticache_cluster_test.go +++ b/aws/resource_aws_elasticache_cluster_test.go @@ -2,6 +2,8 @@ package aws import ( "fmt" + "os" + "strconv" "strings" "testing" @@ -13,7 +15,108 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestAccAWSElasticacheCluster_basic(t *testing.T) { +func TestAccAWSElasticacheCluster_Engine_Memcached_Ec2Classic(t *testing.T) { + oldvar := os.Getenv("AWS_DEFAULT_REGION") + os.Setenv("AWS_DEFAULT_REGION", "us-east-1") + defer os.Setenv("AWS_DEFAULT_REGION", oldvar) + + var ec elasticache.CacheCluster + rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(8)) + resourceName := "aws_elasticache_cluster.bar" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSElasticacheClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheClusterConfig_Engine_Memcached(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheClusterExists(resourceName, &ec), + resource.TestCheckResourceAttr(resourceName, "cache_nodes.0.id", "0001"), + resource.TestCheckResourceAttrSet(resourceName, "configuration_endpoint"), + resource.TestCheckResourceAttrSet(resourceName, "cluster_address"), + resource.TestCheckResourceAttr(resourceName, "engine", "memcached"), + resource.TestCheckResourceAttr(resourceName, "port", "11211"), + ), + }, + resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAWSElasticacheCluster_Engine_Redis_Ec2Classic(t *testing.T) { + oldvar := os.Getenv("AWS_DEFAULT_REGION") + os.Setenv("AWS_DEFAULT_REGION", "us-east-1") + defer os.Setenv("AWS_DEFAULT_REGION", oldvar) + + var ec elasticache.CacheCluster + rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(8)) + resourceName := "aws_elasticache_cluster.bar" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSElasticacheClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheClusterConfig_Engine_Redis(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheClusterExists(resourceName, &ec), + resource.TestCheckResourceAttr(resourceName, "cache_nodes.0.id", "0001"), + resource.TestCheckResourceAttr(resourceName, "engine", "redis"), + resource.TestCheckResourceAttr(resourceName, "port", "6379"), + ), + }, + resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAWSElasticacheCluster_Port_Ec2Classic(t *testing.T) { + oldvar := os.Getenv("AWS_DEFAULT_REGION") + os.Setenv("AWS_DEFAULT_REGION", "us-east-1") + defer os.Setenv("AWS_DEFAULT_REGION", oldvar) + + var ec elasticache.CacheCluster + port := 11212 + rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(8)) + resourceName := "aws_elasticache_cluster.bar" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSElasticacheClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheClusterConfig_Port(rName, port), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheClusterExists(resourceName, &ec), + resource.TestCheckResourceAttr(resourceName, "cache_nodes.0.id", "0001"), + resource.TestCheckResourceAttrSet(resourceName, "configuration_endpoint"), + resource.TestCheckResourceAttrSet(resourceName, "cluster_address"), + resource.TestCheckResourceAttr(resourceName, "engine", "memcached"), + resource.TestCheckResourceAttr(resourceName, "port", strconv.Itoa(port)), + ), + }, + resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAWSElasticacheCluster_SecurityGroup(t *testing.T) { var ec elasticache.CacheCluster resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -21,7 +124,7 @@ func TestAccAWSElasticacheCluster_basic(t *testing.T) { CheckDestroy: testAccCheckAWSElasticacheClusterDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSElasticacheClusterConfig, + Config: testAccAWSElasticacheClusterConfig_SecurityGroup, Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheSecurityGroupExists("aws_elasticache_security_group.bar"), testAccCheckAWSElasticacheClusterExists("aws_elasticache_cluster.bar", &ec), @@ -219,24 +322,44 @@ func testAccCheckAWSElasticacheClusterExists(n string, v *elasticache.CacheClust } } -func testAccAWSElasticacheClusterConfigBasic(clusterId string) string { +func testAccAWSElasticacheClusterConfig_Engine_Memcached(rName string) string { return fmt.Sprintf(` -provider "aws" { - region = "us-east-1" +resource "aws_elasticache_cluster" "bar" { + cluster_id = "%s" + engine = "memcached" + node_type = "cache.m1.small" + num_cache_nodes = 1 + parameter_group_name = "default.memcached1.4" +} +`, rName) } +func testAccAWSElasticacheClusterConfig_Engine_Redis(rName string) string { + return fmt.Sprintf(` resource "aws_elasticache_cluster" "bar" { - cluster_id = "tf-%s" - engine = "memcached" - node_type = "cache.m1.small" - num_cache_nodes = 1 - port = 11211 - parameter_group_name = "default.memcached1.4" + cluster_id = "%s" + engine = "redis" + node_type = "cache.m1.small" + num_cache_nodes = 1 + parameter_group_name = "default.redis3.2" +} +`, rName) +} + +func testAccAWSElasticacheClusterConfig_Port(rName string, port int) string { + return fmt.Sprintf(` +resource "aws_elasticache_cluster" "bar" { + cluster_id = "%s" + engine = "memcached" + node_type = "cache.m1.small" + num_cache_nodes = 1 + parameter_group_name = "default.memcached1.4" + port = %d } -`, clusterId) +`, rName, port) } -var testAccAWSElasticacheClusterConfig = fmt.Sprintf(` +var testAccAWSElasticacheClusterConfig_SecurityGroup = fmt.Sprintf(` provider "aws" { region = "us-east-1" } diff --git a/aws/resource_aws_elasticache_replication_group.go b/aws/resource_aws_elasticache_replication_group.go index 03d387e3c61e..55f75d70912a 100644 --- a/aws/resource_aws_elasticache_replication_group.go +++ b/aws/resource_aws_elasticache_replication_group.go @@ -133,7 +133,6 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i AutoMinorVersionUpgrade: aws.Bool(d.Get("auto_minor_version_upgrade").(bool)), CacheNodeType: aws.String(d.Get("node_type").(string)), Engine: aws.String(d.Get("engine").(string)), - Port: aws.Int64(int64(d.Get("port").(int))), Tags: tags, } @@ -151,6 +150,10 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i params.CacheParameterGroupName = aws.String(v.(string)) } + if v, ok := d.GetOk("port"); ok { + params.Port = aws.Int64(int64(v.(int))) + } + if v, ok := d.GetOk("subnet_group_name"); ok { params.CacheSubnetGroupName = aws.String(v.(string)) } diff --git a/website/docs/r/elasticache_cluster.html.markdown b/website/docs/r/elasticache_cluster.html.markdown index ac9ae93529d9..efcad38dcc6e 100644 --- a/website/docs/r/elasticache_cluster.html.markdown +++ b/website/docs/r/elasticache_cluster.html.markdown @@ -64,8 +64,7 @@ the highest numbered nodes will be removed. * `parameter_group_name` – (Required) Name of the parameter group to associate with this cache cluster -* `port` – (Required) The port number on which each of the cache nodes will -accept connections. For Memcache the default is 11211, and for Redis the default port is 6379. +* `port` – (Optional) The port number on which each of the cache nodes will accept connections. For Memcache the default is 11211, and for Redis the default port is 6379. * `subnet_group_name` – (Optional, VPC only) Name of the subnet group to be used for the cache cluster. diff --git a/website/docs/r/elasticache_replication_group.html.markdown b/website/docs/r/elasticache_replication_group.html.markdown index 4603c9a58893..e8fc983a8e4d 100644 --- a/website/docs/r/elasticache_replication_group.html.markdown +++ b/website/docs/r/elasticache_replication_group.html.markdown @@ -69,7 +69,7 @@ The following arguments are supported: * `auth_token` - (Optional) The password used to access a password protected server. Can be specified only if `transit_encryption_enabled = true`. * `engine_version` - (Optional) The version number of the cache engine to be used for the cache clusters in this replication group. * `parameter_group_name` - (Optional) The name of the parameter group to associate with this replication group. If this argument is omitted, the default cache parameter group for the specified engine is used. -* `port` – (Required) The port number on which each of the cache nodes will accept connections. For Memcache the default is 11211, and for Redis the default port is 6379. +* `port` – (Optional) The port number on which each of the cache nodes will accept connections. For Memcache the default is 11211, and for Redis the default port is 6379. * `subnet_group_name` - (Optional) The name of the cache subnet group to be used for the replication group. * `security_group_names` - (Optional) A list of cache security group names to associate with this replication group. * `security_group_ids` - (Optional) One or more Amazon VPC security groups associated with this replication group. Use this parameter only when you are creating a replication group in an Amazon Virtual Private Cloud From 5e113259418e8fb2a1f66077474dd96bb0216087 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 21 Mar 2018 11:09:36 -0400 Subject: [PATCH 2/2] tests/resource/aws_elasticache_cluster: Remove redundant resource.TestStep type declarations in slices --- aws/resource_aws_elasticache_cluster_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aws/resource_aws_elasticache_cluster_test.go b/aws/resource_aws_elasticache_cluster_test.go index 29565686ab8d..da3f9b4d9c32 100644 --- a/aws/resource_aws_elasticache_cluster_test.go +++ b/aws/resource_aws_elasticache_cluster_test.go @@ -40,7 +40,7 @@ func TestAccAWSElasticacheCluster_Engine_Memcached_Ec2Classic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "port", "11211"), ), }, - resource.TestStep{ + { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, @@ -72,7 +72,7 @@ func TestAccAWSElasticacheCluster_Engine_Redis_Ec2Classic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "port", "6379"), ), }, - resource.TestStep{ + { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, @@ -107,7 +107,7 @@ func TestAccAWSElasticacheCluster_Port_Ec2Classic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "port", strconv.Itoa(port)), ), }, - resource.TestStep{ + { ResourceName: resourceName, ImportState: true, ImportStateVerify: true,