From 63f0ea1214e3727628784a0e4b834d6778c6127f Mon Sep 17 00:00:00 2001 From: kanwarudaysingh Date: Thu, 4 Aug 2022 04:51:43 +0000 Subject: [PATCH 1/7] Adding support for major version upgrade in cloud sql instance. --- .../resource_sql_database_instance.go.erb | 69 ++++++++++++------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb index 92841d9e1761..f8e9569ce5bb 100644 --- a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb +++ b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb @@ -477,7 +477,6 @@ is set to true.`, "database_version": { Type: schema.TypeString, Required: true, - ForceNew: true, Description: `The MySQL, PostgreSQL or SQL Server (beta) version to use. Supported values include MYSQL_5_6, MYSQL_5_7, MYSQL_8_0, POSTGRES_9_6, POSTGRES_10, POSTGRES_11, POSTGRES_12, POSTGRES_13, POSTGRES_14, SQLSERVER_2017_STANDARD, SQLSERVER_2017_ENTERPRISE, SQLSERVER_2017_EXPRESS, SQLSERVER_2017_WEB. Database Version Policies includes an up-to-date reference of supported versions.`, }, @@ -1303,33 +1302,55 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) return err } - // Update only updates the settings, so they are all we need to set. - instance := &sqladmin.DatabaseInstance{ - Settings: expandSqlDatabaseInstanceSettings(d.Get("settings").([]interface{})), - } - - // Collation cannot be included in the update request - instance.Settings.Collation = "" - - // Lock on the master_instance_name just in case updating any replica - // settings causes operations on the master. - if v, ok := d.GetOk("master_instance_name"); ok { - mutexKV.Lock(instanceMutexKey(project, v.(string))) - defer mutexKV.Unlock(instanceMutexKey(project, v.(string))) - } + desiredSetting := d.Get("settings") var op *sqladmin.Operation - err = retryTimeDuration(func() (rerr error) { - op, rerr = config.NewSqlAdminClient(userAgent).Instances.Update(project, d.Get("name").(string), instance).Do() - return rerr - }, d.Timeout(schema.TimeoutUpdate), isSqlOperationInProgressError) - if err != nil { - return fmt.Errorf("Error, failed to update instance settings for %s: %s", instance.Name, err) + var instance *sqladmin.DatabaseInstance + if v, ok := d.GetOk("database_version"); ok { + instance = &sqladmin.DatabaseInstance{DatabaseVersion: v.(string)} + err = retryTimeDuration(func() (rerr error) { + op, rerr = config.NewSqlAdminClient(userAgent).Instances.Patch(project, d.Get("name").(string), instance).Do() + return rerr + }, d.Timeout(schema.TimeoutUpdate), isSqlOperationInProgressError) + if err != nil { + return fmt.Errorf("error, failed to patch instance settings for %s: %s", instance.Name, err) + } + err = sqlAdminOperationWaitTime(config, op, project, "Update Instance", userAgent, d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return err + } + err = resourceSqlDatabaseInstanceRead(d, meta) + if err != nil { + return err + } } - err = sqlAdminOperationWaitTime(config, op, project, "Update Instance", userAgent, d.Timeout(schema.TimeoutUpdate)) - if err != nil { - return err + if len(desiredSetting.([]interface{})) != 0 { + s := d.Get("settings") + instance = &sqladmin.DatabaseInstance{ + Settings: expandSqlDatabaseInstanceSettings(desiredSetting.([]interface{})), + } + _settings := s.([]interface{})[0].(map[string]interface{}) + instance.Settings.SettingsVersion = int64(_settings["version"].(int)) + // Collation cannot be included in the update request + instance.Settings.Collation = "" + // Lock on the master_instance_name just in case updating any replica + // settings causes operations on the master. + if v, ok := d.GetOk("master_instance_name"); ok { + mutexKV.Lock(instanceMutexKey(project, v.(string))) + defer mutexKV.Unlock(instanceMutexKey(project, v.(string))) + } + err = retryTimeDuration(func() (rerr error) { + op, rerr = config.NewSqlAdminClient(userAgent).Instances.Update(project, d.Get("name").(string), instance).Do() + return rerr + }, d.Timeout(schema.TimeoutUpdate), isSqlOperationInProgressError) + if err != nil { + return fmt.Errorf("Error, failed to update instance settings for %s: %s", instance.Name, err) + } + err = sqlAdminOperationWaitTime(config, op, project, "Update Instance", userAgent, d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return err + } } // Perform a backup restore if the backup context exists and has changed From da1c4caf78331215d67a43a59d33e5382c902423 Mon Sep 17 00:00:00 2001 From: kanwarudaysingh Date: Thu, 4 Aug 2022 06:32:37 +0000 Subject: [PATCH 2/7] Resolving Lint errors --- .../resources/resource_sql_database_instance.go.erb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb index f8e9569ce5bb..317800a600e7 100644 --- a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb +++ b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb @@ -1306,6 +1306,7 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) var op *sqladmin.Operation var instance *sqladmin.DatabaseInstance + // Major version upgrade if v, ok := d.GetOk("database_version"); ok { instance = &sqladmin.DatabaseInstance{DatabaseVersion: v.(string)} err = retryTimeDuration(func() (rerr error) { @@ -1313,9 +1314,9 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) return rerr }, d.Timeout(schema.TimeoutUpdate), isSqlOperationInProgressError) if err != nil { - return fmt.Errorf("error, failed to patch instance settings for %s: %s", instance.Name, err) + return fmt.Errorf("Error, failed to patch instance settings for %s: %s", instance.Name, err) } - err = sqlAdminOperationWaitTime(config, op, project, "Update Instance", userAgent, d.Timeout(schema.TimeoutUpdate)) + err = sqlAdminOperationWaitTime(config, op, project, "Patch Instance", userAgent, d.Timeout(schema.TimeoutUpdate)) if err != nil { return err } From 4e12c5606a3b3f1057cffd94a4cab596d59b4337 Mon Sep 17 00:00:00 2001 From: kanwarudaysingh Date: Thu, 4 Aug 2022 07:36:48 +0000 Subject: [PATCH 3/7] Minor changes to improve readability --- .../resource_sql_database_instance.go.erb | 54 ++++++++++--------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb index 317800a600e7..868e1f0456f9 100644 --- a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb +++ b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb @@ -1326,32 +1326,34 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) } } - if len(desiredSetting.([]interface{})) != 0 { - s := d.Get("settings") - instance = &sqladmin.DatabaseInstance{ - Settings: expandSqlDatabaseInstanceSettings(desiredSetting.([]interface{})), - } - _settings := s.([]interface{})[0].(map[string]interface{}) - instance.Settings.SettingsVersion = int64(_settings["version"].(int)) - // Collation cannot be included in the update request - instance.Settings.Collation = "" - // Lock on the master_instance_name just in case updating any replica - // settings causes operations on the master. - if v, ok := d.GetOk("master_instance_name"); ok { - mutexKV.Lock(instanceMutexKey(project, v.(string))) - defer mutexKV.Unlock(instanceMutexKey(project, v.(string))) - } - err = retryTimeDuration(func() (rerr error) { - op, rerr = config.NewSqlAdminClient(userAgent).Instances.Update(project, d.Get("name").(string), instance).Do() - return rerr - }, d.Timeout(schema.TimeoutUpdate), isSqlOperationInProgressError) - if err != nil { - return fmt.Errorf("Error, failed to update instance settings for %s: %s", instance.Name, err) - } - err = sqlAdminOperationWaitTime(config, op, project, "Update Instance", userAgent, d.Timeout(schema.TimeoutUpdate)) - if err != nil { - return err - } + s := d.Get("settings") + instance = &sqladmin.DatabaseInstance{ + Settings: expandSqlDatabaseInstanceSettings(desiredSetting.([]interface{})), + } + _settings := s.([]interface{})[0].(map[string]interface{}) + instance.Settings.SettingsVersion = int64(_settings["version"].(int)) + // Collation cannot be included in the update request + instance.Settings.Collation = "" + + // Lock on the master_instance_name just in case updating any replica + // settings causes operations on the master. + if v, ok := d.GetOk("master_instance_name"); ok { + mutexKV.Lock(instanceMutexKey(project, v.(string))) + defer mutexKV.Unlock(instanceMutexKey(project, v.(string))) + } + + var op *sqladmin.Operation + err = retryTimeDuration(func() (rerr error) { + op, rerr = config.NewSqlAdminClient(userAgent).Instances.Update(project, d.Get("name").(string), instance).Do() + return rerr + }, d.Timeout(schema.TimeoutUpdate), isSqlOperationInProgressError) + if err != nil { + return fmt.Errorf("Error, failed to update instance settings for %s: %s", instance.Name, err) + } + + err = sqlAdminOperationWaitTime(config, op, project, "Update Instance", userAgent, d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return err } // Perform a backup restore if the backup context exists and has changed From 3c9a8c6b22d10d29e6a11f0e2272e3d6fd9e9472 Mon Sep 17 00:00:00 2001 From: kanwarudaysingh Date: Thu, 4 Aug 2022 07:42:12 +0000 Subject: [PATCH 4/7] Op redeclared in the file --- .../terraform/resources/resource_sql_database_instance.go.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb index 868e1f0456f9..39ecaa7dd4d4 100644 --- a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb +++ b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb @@ -1342,7 +1342,6 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) defer mutexKV.Unlock(instanceMutexKey(project, v.(string))) } - var op *sqladmin.Operation err = retryTimeDuration(func() (rerr error) { op, rerr = config.NewSqlAdminClient(userAgent).Instances.Update(project, d.Get("name").(string), instance).Do() return rerr From b7a43e3c5614771620a5ce4fdc93522b8f639658 Mon Sep 17 00:00:00 2001 From: Uday Singh Date: Thu, 4 Aug 2022 16:59:27 +0530 Subject: [PATCH 5/7] Update resource_sql_database_instance.go.erb --- .../terraform/resources/resource_sql_database_instance.go.erb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb index 39ecaa7dd4d4..fc86a7fa76d7 100644 --- a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb +++ b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb @@ -824,7 +824,7 @@ func resourceSqlDatabaseInstanceCreate(d *schema.ResourceData, meta interface{}) cloneContext, cloneSource := expandCloneContext(d.Get("clone").([]interface{})) - s, ok := d.GetOk("settings") + s, ok := d.GetOk("settings") desiredSettings := expandSqlDatabaseInstanceSettings(s.([]interface{})) if ok { instance.Settings = desiredSettings @@ -1303,7 +1303,6 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) } desiredSetting := d.Get("settings") - var op *sqladmin.Operation var instance *sqladmin.DatabaseInstance // Major version upgrade From 5da381740dddcb3757521703d1dc057510339c68 Mon Sep 17 00:00:00 2001 From: usingh83 Date: Tue, 9 Aug 2022 20:50:12 +0000 Subject: [PATCH 6/7] Added test for the database version upgrade and added appropriate comments. --- .../resource_sql_database_instance.go.erb | 6 ++- ...resource_sql_database_instance_test.go.erb | 41 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb index fc86a7fa76d7..6e7bdc3ce9b3 100644 --- a/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb +++ b/mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb @@ -1305,7 +1305,8 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) desiredSetting := d.Get("settings") var op *sqladmin.Operation var instance *sqladmin.DatabaseInstance - // Major version upgrade + // Check if the database version is being updated, because patching database version is an atomic operation and can not be + // performed with other fields, we first patch database version before updating the rest of the fields. if v, ok := d.GetOk("database_version"); ok { instance = &sqladmin.DatabaseInstance{DatabaseVersion: v.(string)} err = retryTimeDuration(func() (rerr error) { @@ -1330,6 +1331,9 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{}) Settings: expandSqlDatabaseInstanceSettings(desiredSetting.([]interface{})), } _settings := s.([]interface{})[0].(map[string]interface{}) + // Instance.Patch operation on completion updates the settings proto version by +8. As terraform does not know this it tries + // to make an update call with the proto version before patch and fails. To resolve this issue we update the setting version + // before making the update call. instance.Settings.SettingsVersion = int64(_settings["version"].(int)) // Collation cannot be included in the update request instance.Settings.Collation = "" diff --git a/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb b/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb index af2e92b5a56d..0d60718a864c 100644 --- a/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb @@ -1197,6 +1197,36 @@ func TestAccSqlDatabaseInstance_SqlServerAuditConfig(t *testing.T) { }) } +func TestAccSqlDatabaseInstance_mysqlMVU(t *testing.T) { + t.Parallel() + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccSqlDatabaseInstanceDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testGoogleSqlDatabaseInstance_basic2, + }, + { + ResourceName: "google_sql_database_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"root_password", "deletion_protection"}, + }, + { + Config: testGoogleSqlDatabaseInstance_basic2_update, + }, + { + ResourceName: "google_sql_database_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"root_password", "deletion_protection"}, + }, + }, + }) +} + var testGoogleSqlDatabaseInstance_basic2 = ` resource "google_sql_database_instance" "instance" { region = "us-central1" @@ -1208,6 +1238,17 @@ resource "google_sql_database_instance" "instance" { } ` +var testGoogleSqlDatabaseInstance_basic2_update = ` +resource "google_sql_database_instance" "instance" { + region = "us-central1" + database_version = "MYSQL_8_0" + deletion_protection = false + settings { + tier = "db-f1-micro" + } +} +` + var testGoogleSqlDatabaseInstance_basic3 = ` resource "google_sql_database_instance" "instance" { name = "%s" From 0d13a091afea3e118e2eec4e0698c6a527a4fee8 Mon Sep 17 00:00:00 2001 From: Uday Singh Date: Fri, 12 Aug 2022 12:19:14 +0530 Subject: [PATCH 7/7] Update mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb Co-authored-by: Stephen Lewis (Burrows) --- .../terraform/tests/resource_sql_database_instance_test.go.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb b/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb index 0d60718a864c..1bc036f0ffc7 100644 --- a/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb @@ -1197,7 +1197,7 @@ func TestAccSqlDatabaseInstance_SqlServerAuditConfig(t *testing.T) { }) } -func TestAccSqlDatabaseInstance_mysqlMVU(t *testing.T) { +func TestAccSqlDatabaseInstance_mysqlMajorVersionUpgrade(t *testing.T) { t.Parallel() vcrTest(t, resource.TestCase{