resource/aws_elasticache_cluster: Introduce initial CustomizeDiff

[bflad]

Closes #1702
Closes #2421

• Support plan time validation of az_mode
• Support plan time validation of node_type requiring VPC for cache.t2 instances
• Support plan time validation of num_cache_nodes > 1 for redis
• ForceNew memcached on node_type changes
• ForceNew on engine_version downgrades

[paultyng]

Approved, but some style comments. The early return vs nesting thing is somewhat opinion based, but I think in Go with the error handling it somewhat favors early return style:

• https://golang.org/doc/effective_go.html#if
• https://arne-mertz.de/2016/12/early-return/
• http://www.danp.net/2015/11/02/reducing-go-nesting.html

etc.

[paultyng]

Not something to hold up merging, and probably leaning more towards preference, but you could rewrite this a little so you could just reuse the standard ok and short circuiting a little earlier (also helping the line length a little):

if azMode, ok := diff.GetOk("az_mode"); !ok || azMode.(string) != elasticache.AZModeCrossAz {
  return nil
}

if numCacheNodes, ok := diff.GetOk("num_cache_nodes"); !ok || numCacheNodes.(int) != 1 {
  return nil
}

return errors.New(...)

[bflad]

Makes sense, I'll update this. (Personally I prefer early returns too)

[paultyng]

Similar to the other short circuiting one, this flattens out if you just do the easy return first:

if diff.Id() == "" || !diff.HasChange("engine_version") {
  return nil
}

// rest of it, but flatter indentation

[bflad]

Short-circuit logic is passing acceptance testing:

13 tests passed (all tests)
=== RUN   TestAccAWSElasticacheCluster_NumCacheNodes_Redis_Ec2Classic
--- PASS: TestAccAWSElasticacheCluster_NumCacheNodes_Redis_Ec2Classic (3.06s)
=== RUN   TestAccAWSElasticacheCluster_AZMode_Redis_Ec2Classic
--- PASS: TestAccAWSElasticacheCluster_AZMode_Redis_Ec2Classic (492.09s)
=== RUN   TestAccAWSElasticacheCluster_AZMode_Memcached_Ec2Classic
--- PASS: TestAccAWSElasticacheCluster_AZMode_Memcached_Ec2Classic (504.87s)
=== RUN   TestAccAWSElasticacheCluster_basic
--- PASS: TestAccAWSElasticacheCluster_basic (529.05s)
=== RUN   TestAccAWSElasticacheCluster_importBasic
--- PASS: TestAccAWSElasticacheCluster_importBasic (544.75s)
=== RUN   TestAccAWSElasticacheCluster_multiAZInVpc
--- PASS: TestAccAWSElasticacheCluster_multiAZInVpc (627.27s)
=== RUN   TestAccAWSElasticacheCluster_vpc
--- PASS: TestAccAWSElasticacheCluster_vpc (678.90s)
=== RUN   TestAccAWSElasticacheCluster_NodeTypeResize_Redis_Ec2Classic
--- PASS: TestAccAWSElasticacheCluster_NodeTypeResize_Redis_Ec2Classic (816.04s)
=== RUN   TestAccAWSElasticacheCluster_NodeTypeResize_Memcached_Ec2Classic
--- PASS: TestAccAWSElasticacheCluster_NodeTypeResize_Memcached_Ec2Classic (918.65s)
=== RUN   TestAccAWSElasticacheCluster_EngineVersion_Memcached_Ec2Classic
--- PASS: TestAccAWSElasticacheCluster_EngineVersion_Memcached_Ec2Classic (1071.12s)
=== RUN   TestAccAWSElasticacheCluster_decreasingCacheNodes
--- PASS: TestAccAWSElasticacheCluster_decreasingCacheNodes (1133.37s)
=== RUN   TestAccAWSElasticacheCluster_EngineVersion_Redis_Ec2Classic
--- PASS: TestAccAWSElasticacheCluster_EngineVersion_Redis_Ec2Classic (1145.27s)
=== RUN   TestAccAWSElasticacheCluster_snapshotsWithUpdates
--- PASS: TestAccAWSElasticacheCluster_snapshotsWithUpdates (1352.50s)

Merging!

[bflad]

This has been released in version 1.12.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!