From 4e7ccc35a31c925e721aa37641952772f2930db6 Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Mon, 23 Jan 2017 18:12:34 -0500 Subject: [PATCH 1/2] provider/aws: Add diff suppress function for aws_db_instance Adds a diff suppress function for the `engine_version` attribute of the `db_instance` AWS resource. The function only supresses the state diff, if the attribute key `auto_minor_version_upgrade` is set, and if the returned `engine_version` from the running RDS instance shares the same prefix as the configured `engine_version`. ``` $ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSDBInstance_MinorVersion' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/01/23 17:59:14 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSDBInstance_MinorVersion -timeout 120m === RUN TestAccAWSDBInstance_MinorVersion --- PASS: TestAccAWSDBInstance_MinorVersion (503.48s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 503.518s ``` --- builtin/providers/aws/diff_suppress_funcs.go | 18 ++++++ .../providers/aws/resource_aws_db_instance.go | 7 ++- .../aws/resource_aws_db_instance_test.go | 59 ++++++++++++++----- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/builtin/providers/aws/diff_suppress_funcs.go b/builtin/providers/aws/diff_suppress_funcs.go index 408063e26..909f4d477 100644 --- a/builtin/providers/aws/diff_suppress_funcs.go +++ b/builtin/providers/aws/diff_suppress_funcs.go @@ -1,6 +1,9 @@ package aws import ( + "log" + "strings" + "github.com/hashicorp/terraform/helper/schema" "github.com/jen20/awspolicyequivalence" ) @@ -13,3 +16,18 @@ func suppressEquivalentAwsPolicyDiffs(k, old, new string, d *schema.ResourceData return equivalent } + +// Suppresses minor version changes to the db_instance engine_version attribute +func suppressAwsDbEngineVersionDiffs(k, old, new string, d *schema.ResourceData) bool { + if d.Get("auto_minor_version_upgrade").(bool) { + // If we're set to auto upgrade minor versions + // ignore a minor version diff between versions + if strings.HasPrefix(old, new) { + log.Printf("[DEBUG] Ignoring minor version diff") + return true + } + } + + // Throw a diff by default + return false +} diff --git a/builtin/providers/aws/resource_aws_db_instance.go b/builtin/providers/aws/resource_aws_db_instance.go index 964628bdc..b1635f540 100644 --- a/builtin/providers/aws/resource_aws_db_instance.go +++ b/builtin/providers/aws/resource_aws_db_instance.go @@ -63,9 +63,10 @@ func resourceAwsDbInstance() *schema.Resource { }, "engine_version": { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + DiffSuppressFunc: suppressAwsDbEngineVersionDiffs, }, "character_set_name": { diff --git a/builtin/providers/aws/resource_aws_db_instance_test.go b/builtin/providers/aws/resource_aws_db_instance_test.go index 5107d9542..b6ad634d6 100644 --- a/builtin/providers/aws/resource_aws_db_instance_test.go +++ b/builtin/providers/aws/resource_aws_db_instance_test.go @@ -27,7 +27,7 @@ func TestAccAWSDBInstance_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSDBInstanceConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), @@ -65,7 +65,7 @@ func TestAccAWSDBInstance_kmsKey(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: config, Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), @@ -115,7 +115,7 @@ func TestAccAWSDBInstance_optionGroup(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSDBInstanceConfigWithOptionGroup, Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), @@ -136,7 +136,7 @@ func TestAccAWSDBInstanceReplica(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccReplicaInstanceConfig(rand.New(rand.NewSource(time.Now().UnixNano())).Int()), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &s), @@ -158,7 +158,7 @@ func TestAccAWSDBInstanceSnapshot(t *testing.T) { // created, and subequently deletes it CheckDestroy: testAccCheckAWSDBInstanceSnapshot, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccSnapshotInstanceConfig(), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.snapshot", &snap), @@ -176,7 +176,7 @@ func TestAccAWSDBInstanceNoSnapshot(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceNoSnapshot, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccNoSnapshotInstanceConfig(), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.no_snapshot", &nosnap), @@ -195,7 +195,7 @@ func TestAccAWSDBInstance_enhancedMonitoring(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceNoSnapshot, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccSnapshotInstanceConfig_enhancedMonitoring(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.enhanced_monitoring", &dbInstance), @@ -220,7 +220,7 @@ func TestAccAWS_separate_DBInstance_iops_update(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccSnapshotInstanceConfig_iopsUpdate(rName, 1000), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), @@ -228,7 +228,7 @@ func TestAccAWS_separate_DBInstance_iops_update(t *testing.T) { ), }, - resource.TestStep{ + { Config: testAccSnapshotInstanceConfig_iopsUpdate(rName, 2000), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), @@ -249,7 +249,7 @@ func TestAccAWSDBInstance_portUpdate(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccSnapshotInstanceConfig_mysqlPort(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), @@ -258,7 +258,7 @@ func TestAccAWSDBInstance_portUpdate(t *testing.T) { ), }, - resource.TestStep{ + { Config: testAccSnapshotInstanceConfig_updateMysqlPort(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), @@ -278,7 +278,7 @@ func TestAccAWSDBInstance_MSSQL_TZ(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSDBMSSQL_timezone, Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), @@ -290,7 +290,7 @@ func TestAccAWSDBInstance_MSSQL_TZ(t *testing.T) { ), }, - resource.TestStep{ + { Config: testAccAWSDBMSSQL_timezone_AKST, Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), @@ -305,6 +305,24 @@ func TestAccAWSDBInstance_MSSQL_TZ(t *testing.T) { }) } +func TestAccAWSDBInstance_MinorVersion(t *testing.T) { + var v rds.DBInstance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDBInstanceConfigAutoMinorVersion, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v), + ), + }, + }, + }) +} + func testAccCheckAWSDBInstanceDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).rdsconn @@ -1088,7 +1106,7 @@ resource "aws_db_instance" "mssql" { username = "somecrazyusername" password = "somecrazypassword" engine = "sqlserver-ex" - backup_retention_period = 0 + backup_retention_period = #publicly_accessible = true @@ -1113,3 +1131,16 @@ resource "aws_security_group_rule" "rds-mssql-1" { security_group_id = "${aws_security_group.rds-mssql.id}" } ` + +var testAccAWSDBInstanceConfigAutoMinorVersion = fmt.Sprintf(` +resource "aws_db_instance" "bar" { + identifier = "foobarbaz-test-terraform-%d" + allocated_storage = 10 + engine = "MySQL" + engine_version = "5.6" + instance_class = "db.t1.micro" + name = "baz" + password = "barbarbarbar" + username = "foo" +} +`, acctest.RandInt()) From 4c5a08cae71386ffb34d89bb83b87ff7ebfa1bd3 Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Mon, 23 Jan 2017 18:28:16 -0500 Subject: [PATCH 2/2] fix fat-fingered change in test --- builtin/providers/aws/resource_aws_db_instance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_db_instance_test.go b/builtin/providers/aws/resource_aws_db_instance_test.go index b6ad634d6..0223ff68d 100644 --- a/builtin/providers/aws/resource_aws_db_instance_test.go +++ b/builtin/providers/aws/resource_aws_db_instance_test.go @@ -1106,7 +1106,7 @@ resource "aws_db_instance" "mssql" { username = "somecrazyusername" password = "somecrazypassword" engine = "sqlserver-ex" - backup_retention_period = + backup_retention_period = 0 #publicly_accessible = true