Increase robustness of disk_autoresize in sql_database_instance (#14952)

Author: same-id

mmv1/third_party/terraform/services/sql/resource_sql_database_instance.go.tmpl

@@ -17,7 +17,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 
-	"github.com/hashicorp/terraform-provider-google/google/services/compute"
 	"github.com/hashicorp/terraform-provider-google/google/services/servicenetworking"
 	"github.com/hashicorp/terraform-provider-google/google/tpgresource"
 	transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
@@ -127,6 +126,41 @@ var (
 	}
 )
 
+func diskSizeCutomizeDiff(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error {
+	key := "settings.0.disk_size"
+
+	old, new := d.GetChange(key)
+	// It's okay to remove size entirely.
+	if old == nil || new == nil {
+		return nil
+	}
+	autoResizeI, exists := d.GetOkExists("settings.0.disk_autoresize")
+	autoResize := !exists || autoResizeI.(bool)
+
+	if old.(int) <= new.(int) {
+		// If a resize up, always allow it - keep the diff.
+		return nil
+	}
+
+	if autoResize {
+		// Allow having disk size larger in the state than in config if auto resize is enabled.
+		// Delete the diff.
+		err := d.Clear(key)
+		if err != nil {
+			return err
+		}
+		return nil
+	}
+
+	// If we are here, we are trying to shrink the disk with auto resize disabled and no ignore changes on disk size.
+	// This will force a new resource.
+	if err := d.ForceNew(key); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func ResourceSqlDatabaseInstance() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceSqlDatabaseInstanceCreate,
@@ -145,7 +179,7 @@ func ResourceSqlDatabaseInstance() *schema.Resource {
 
 		CustomizeDiff: customdiff.All(
 			tpgresource.DefaultProviderProject,
-			customdiff.ForceNewIfChange("settings.0.disk_size", compute.IsDiskShrinkage),
+			diskSizeCutomizeDiff,
 			customdiff.ForceNewIf("master_instance_name", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool {
 				// If we set master but this is not the new master of a switchover, require replacement and warn user.
 				return !isSwitchoverFromOldPrimarySide(d)
@@ -2269,10 +2303,27 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{})
 		}
 	}
 
-	s := d.Get("settings")
 	instance = &sqladmin.DatabaseInstance{
 		Settings: expandSqlDatabaseInstanceSettings(desiredSetting.([]interface{}), databaseVersion),
 	}
+
+	// If there is no change detected in disk size, no need to try and update the disk size, send 0 always
+	instance.Settings.DataDiskSizeGb = 0
+	if d.HasChange("settings.0.disk_size") {
+		autoResize := true
+		_, autoResizeI := d.GetChange("settings.0.disk_autoresize")
+		if autoResizeI != nil {
+			autoResize = autoResizeI.(bool)
+		}
+		oldDiskSizeI, newDiskSizeI := d.GetChange("settings.0.disk_size")
+		if !autoResize || newDiskSizeI.(int) > oldDiskSizeI.(int) {
+			// If auto resize is not enabled - set the disk size as requested, even if it's a decrease - let it fail.
+			// Otherwise, allow increasing even if auto resize is enabled.
+			instance.Settings.DataDiskSizeGb = int64(newDiskSizeI.(int))
+		}
+	}
+
+	s := d.Get("settings")
 	_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

mmv1/third_party/terraform/services/sql/resource_sql_database_instance_test.go.tmpl

@@ -3531,6 +3531,107 @@ func TestAccSqlDatabaseInstance_useCustomerManagedServerCa(t *testing.T) {
 	})
 }
 
+func TestAccSqlDatabaseInstance_DiskSizeAutoResizeWithoutDiskSize(t *testing.T) {
+	t.Parallel()
+
+	project := envvar.GetTestProjectFromEnv()
+	databaseName := "tf-test-" + acctest.RandString(t, 10)
+
+	trueVar := true
+	falseVar := false
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccSqlDatabaseInstanceDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				// Create DB with disk size 10gb (minimal) - no disk size specified in configuration, auto resize enabled
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 0, 100, nil, false, false),
+				// Add additional 2gb outside of TF to simulate increase in disk size
+				Check: testGoogleSqlDatabaseInstanceResizeDisk(t, databaseName, 2),
+			},
+			{
+				// Disk size is now 12gb - requested (original value) 10gb, configuration is empty - should not trigger resize, no errors.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 0, 101, nil, false, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 12),
+			},
+			{
+				// Disk size is now 12gb - requested (original value) 10gb, configuration is empty - should not trigger resize, no errors.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 0, 101, &trueVar, false, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 12),
+			},
+			{
+				// Disk size is now 12gb - requested (original value) 10gb, configuration is empty - disable auto resize - should not error.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 0, 101, &falseVar, false, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 12),
+			},
+			{
+				// Disk size is now 12gb - requested (original value) 10gb, configuration is empty - disable auto resize, but enable deletion protection should not error.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 0, 101, &falseVar, true, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 12),
+			},
+			{
+				// Allow destroy
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 0, 101, &falseVar, true, true),
+			},
+		},
+	})
+}
+
+func TestAccSqlDatabaseInstance_DiskSizeAutoResizeWithDiskSize(t *testing.T) {
+	t.Parallel()
+
+	project := envvar.GetTestProjectFromEnv()
+	databaseName := "tf-test-" + acctest.RandString(t, 10)
+
+	trueVar := true
+	falseVar := false
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccSqlDatabaseInstanceDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				// Create DB with disk size 12gb with auto resize enabled
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 12, 100, nil, false, false),
+				// Add additional 2gb outside of TF to simulate increase in disk size
+				Check: testGoogleSqlDatabaseInstanceResizeDisk(t, databaseName, 2),
+			},
+			{
+				// Disk size is now 14gb - requested (original value) 12gb and auto resize enable - should not trigger resize, no errors.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 12, 101, nil, false, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 14),
+			},
+			{
+				// Disk size is now 14gb - requested 13gb in configuration, still less - should not trigger resize, no errors.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 13, 102, &trueVar, false, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 14),
+			},
+			{
+				// Disk size is now 14gb - requested 15gb in configuration, that's an additional increase should trigger resize to 15gb.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 15, 103, nil, false, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 15),
+			},
+			{
+				// Disk size is now 15gb - requested 14gb, but disabled auto resize - should error because it can't be deleted for replacement.
+				Config:      testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 14, 104, &falseVar, false, false),
+				ExpectError: regexp.MustCompile("Instance cannot be destroyed"),
+			},
+			{
+				// Disk size is now 15gb - requested 14gb, but ignore changes is set - so should ignore the configuration change.
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 14, 105, &falseVar, true, false),
+				Check:  testGoogleSqlDatabaseInstanceCheckDiskSize(t, databaseName, 15),
+			},
+			{
+				// Allow destroy
+				Config: testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, databaseName, 14, 105, &falseVar, true, true),
+			},
+		},
+	})
+}
+
 func testGoogleSqlDatabaseInstance_setCustomSubjectAlternateName(context map[string]interface{}) string {
 	return acctest.Nprintf(`
 data "google_project" "project" {
@@ -7181,3 +7282,102 @@ resource "google_sql_database_instance" "instance" {
 }
 `, instance, databaseVersion, deletionProtection, activationPolicy)
 }
+
+func testGoogleSqlDatabaseInstance_diskSizeAutoResize(project, dbName string, diskSize, maxConnections int, autoResize *bool, ignoreChanges, allowDestroy bool) string {
+	diskSizeStmt := ""
+	if diskSize != 0 {
+		diskSizeStmt = fmt.Sprintf("disk_size = %d", diskSize)
+	}
+	autoResizeStmt := ""
+	if autoResize != nil {
+		if *autoResize {
+			autoResizeStmt = "disk_autoresize = true"
+		} else {
+			autoResizeStmt = "disk_autoresize = false"
+		}
+	}
+	ignoreChangesStmt := ""
+	if ignoreChanges {
+		ignoreChangesStmt = "settings[0].disk_size"
+	}
+
+	preventDestroyStmt := "prevent_destroy = true"
+	if allowDestroy {
+		preventDestroyStmt = ""
+	}
+
+	return fmt.Sprintf(`
+data "google_project" "project" {
+  project_id = "%s"
+}
+
+resource "google_sql_database_instance" "instance" {
+  name                = "%s"
+  region              = "us-central1"
+  database_version    = "POSTGRES_15"
+  deletion_protection = false
+  settings {
+	tier = "db-f1-micro"
+	%s
+	%s
+	database_flags {
+      name  = "max_connections"
+      value = "%d"
+    }
+  }
+  lifecycle {
+    ignore_changes = [%s]
+	%s
+  }
+}
+`, project, dbName, diskSizeStmt, autoResizeStmt, maxConnections, ignoreChangesStmt, preventDestroyStmt)
+}
+
+func testGoogleSqlDatabaseInstanceResizeDisk(t *testing.T, instance string, addGb int64) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		config := acctest.GoogleProviderConfig(t)
+
+		sqlAdminClient := config.NewSqlAdminClient(config.UserAgent)
+
+		inst, err := sqlAdminClient.Instances.Get(config.Project, instance).Do()
+		if err != nil {
+			return fmt.Errorf("Could not get database instance %q: %s", instance, err)
+		}
+
+		operation, err := sqlAdminClient.Instances.Patch(config.Project, instance, &sqladmin.DatabaseInstance{
+			Settings: &sqladmin.Settings{
+				SettingsVersion: inst.Settings.SettingsVersion,
+				DataDiskSizeGb:  inst.Settings.DataDiskSizeGb + addGb,
+			},
+		}).Do()
+		if err != nil {
+			return fmt.Errorf("Could not update database instance %q: %s", instance, err)
+		}
+
+		// Wait for the operation to complete
+		if err := sql.SqlAdminOperationWaitTime(config, operation, config.Project, "Waiting for disk resize", config.UserAgent, 10*time.Minute); err != nil {
+			return fmt.Errorf("Could not wait for operation to complete: %s", err)
+		}
+
+		return nil
+	}
+}
+
+func testGoogleSqlDatabaseInstanceCheckDiskSize(t *testing.T, instance string, size int64) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		config := acctest.GoogleProviderConfig(t)
+
+		sqlAdminClient := config.NewSqlAdminClient(config.UserAgent)
+
+		inst, err := sqlAdminClient.Instances.Get(config.Project, instance).Do()
+		if err != nil {
+			return fmt.Errorf("Could not get database instance %q: %s", instance, err)
+		}
+
+		if inst.Settings.DataDiskSizeGb != size {
+			return fmt.Errorf("Expected disk size %d, got %d", size, inst.Settings.DataDiskSizeGb)
+		}
+
+		return nil
+	}
+}