Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_mysql_flexible_server - read create_mode from config #25179

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ func resourceMysqlFlexibleServerRead(d *pluginsdk.ResourceData, meta interface{}
d.Set("zone", props.AvailabilityZone)
d.Set("version", string(pointer.From(props.Version)))
d.Set("fqdn", props.FullyQualifiedDomainName)
d.Set("create_mode", d.Get("create_mode").(string))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other SQL related resources we set this field during Import, since this is an Import specific issue - can we do that here too?

Copy link
Contributor Author

@neil-yechenwei neil-yechenwei Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tombuildsstuff , I remember previously setting "d.Set("create_mode", d.Get("create_mode").(string))" in read func can set the property with the value in the tf config to handle the issue of not returning the value by service API but now seems it still sets the value as null in state file while importing the resource. I tried below fixes but all they don't work. Is there any suggestion about how to fix the issue of API doesn't return the value while importing the resource?

Fix # 1. I tried to add "d.Set("create_mode", d.Get("create_mode").(string))" in read func but TF still always sets it as "null" in state file when importing the resource.

Fix # 2. I tried to add d.set() with d.Get() to importer but TF still always sets it as "null" in state file when importing the resource.

func mysqlFlexibleServerResourceImporter(ctx context.Context, d *pluginsdk.ResourceData, meta interface{}) ([]*pluginsdk.ResourceData, error) {
	d.Set("create_mode", d.Get("create_mode").(string))

	return []*pluginsdk.ResourceData{d}, nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this kind of issue, seems only ignore_changes can workaround this issue for now. So I will close this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neil-yechenwei as I mentioned in my original comment, if you refer to the other places where we set create_mode at import time we set a value for this field:

d.Set("create_mode", string(databases.CreateModeDefault))
return []*pluginsdk.ResourceData{d}, nil
}
(and override it in some cases depending on the import type, although it appears the ordering is a bug here).

As such this is possible to provide a default value for this field, and then override it from the config as needed.

Copy link
Contributor Author

@neil-yechenwei neil-yechenwei Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neil-yechenwei as I mentioned in my original comment, if you refer to the other places where we set create_mode at import time we set a value for this field:

d.Set("create_mode", string(databases.CreateModeDefault))
return []*pluginsdk.ResourceData{d}, nil
}

(and override it in some cases depending on the import type, although it appears the ordering is a bug here).
As such this is possible to provide a default value for this field, and then override it from the config as needed.

@tombuildsstuff , Here "the config" you mentioned points to the tf config file or the configurations returned by service API?

If here "the config" you mentioned points to the tf config file, I tried to use d.Get() in the custom importer but it can't get the value from the tf config when importing the resource.

If here "the config" you mentioned points to the configurations returned by service API, I assume that we can't differentiate create modes "PointInTimeRestore" and "Default" and "GeoRestore" for the PITR resource and the Default resource and the GeoRestore resource since API always returns replicationRole "None" for them. (Note: replicationRole is the unique property to distinguish create mode.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neil-yechenwei

I assume

Rather than assuming, please validate the API behaviour for each type of CreateMode.

Whilst you're correct insofar as PointInTimeRestore and GeoRestore are going to be similar, the API responses mentioned above should allow us to do something similar to this:

			// CreateMode defaults to `Default` but can be overridden, as such we need to check the
			// API Response to determine which to set.
			createMode := servers.CreateModeDefault
			if model := resp.Model; model != nil {
				if props := model.Properties; props != nil {
					if props.RestorePointInTime != nil {
						createMode = servers.CreateModePointInTimeRestore
					}

					if props.ReplicationRole != nil && *props.ReplicationRole == servers.ReplicationRoleReplica {
						createMode = servers.CreateModeReplica
					}
				}
			}
			d.Set("create_mode", string(createMode))

As such would you be able to log the API responses in each case here, so that we can determine how to proceed?

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tombuildsstuff , Below is the test result. Except "create_mode = Replica", seems we can't distinguish other create modes from the service configuration.

Below is the response when creating mysql fs with create_mode = "Default" or not set create_mode.

{
  "sku": {
    "name": "Standard_D4ds_v4",
    "tier": "GeneralPurpose"
  },
  "properties": {
    "administratorLogin": "adminTerraform",
    "storage": {
      "storageSizeGB": 20,
      "iops": 360,
      "autoGrow": "Enabled",
      "autoIoScaling": "Disabled",
      "storageSku": "Premium_LRS",
      "logOnDisk": "Disabled"
    },
    "version": "8.0.21",
    "state": "Ready",
    "fullyQualifiedDomainName": "acctest-fs-test20.mysql.database.azure.com",
    "availabilityZone": "2",
    "maintenanceWindow": {
      "customWindow": "Disabled",
      "dayOfWeek": 0,
      "startHour": 0,
      "startMinute": 0
    },
    "replicationRole": "None",
    "replicaCapacity": 10,
    "network": {
      "publicNetworkAccess": "Enabled"
    },
    "backup": {
      "backupRetentionDays": 7,
      "geoRedundantBackup": "Disabled",
      "earliestRestoreDate": "2024-03-14T02:27:47.0636752+00:00"
    },
    "highAvailability": {
      "mode": "Disabled",
      "state": "NotEnabled",
      "standbyAvailabilityZone": ""
    },
    "privateEndpointConnections": []
  },
  "location": "West Europe",
  "tags": {},
  "id": "/subscriptions/xx-xx-xxx-xx/resourceGroups/acctestRG-mysql-test20/providers/Microsoft.DBforMySQL/flexibleServers/acctest-fs-test20",
  "name": "acctest-fs-test20",
  "type": "Microsoft.DBforMySQL/flexibleServers"
}

Below is the response when creating mysql fs with create_mode = "PointInTimeRestore".

{
  "sku": {
    "name": "Standard_D4ds_v4",
    "tier": "GeneralPurpose"
  },
  "properties": {
    "administratorLogin": "adminTerraform",
    "storage": {
      "storageSizeGB": 20,
      "iops": 360,
      "autoGrow": "Enabled",
      "autoIoScaling": "Disabled",
      "storageSku": "Premium_LRS",
      "logOnDisk": "Disabled"
    },
    "version": "8.0.21",
    "state": "Ready",
    "fullyQualifiedDomainName": "acctest-fs-pitr-test20.mysql.database.azure.com",
    "availabilityZone": "2",
    "maintenanceWindow": {
      "customWindow": "Disabled",
      "dayOfWeek": 0,
      "startHour": 0,
      "startMinute": 0
    },
    "replicationRole": "None",
    "replicaCapacity": 10,
    "network": {
      "publicNetworkAccess": "Enabled"
    },
    "backup": {
      "backupRetentionDays": 7,
      "geoRedundantBackup": "Disabled",
      "earliestRestoreDate": "2024-03-14T02:48:05.905809+00:00"
    },
    "highAvailability": {
      "mode": "Disabled",
      "state": "NotEnabled",
      "standbyAvailabilityZone": ""
    },
    "privateEndpointConnections": []
  },
  "location": "West Europe",
  "tags": {},
  "id": "/subscriptions/xx-xx-xxx-xx/resourceGroups/acctestRG-mysql-test20/providers/Microsoft.DBforMySQL/flexibleServers/acctest-fs-pitr-test20",
  "name": "acctest-fs-pitr-test20",
  "type": "Microsoft.DBforMySQL/flexibleServers"
}

Below is the response when creating mysql fs with create_mode = "Replica".

{
  "sku": {
    "name": "Standard_D4ds_v4",
    "tier": "GeneralPurpose"
  },
  "properties": {
    "administratorLogin": "adminTerraform",
    "storage": {
      "storageSizeGB": 20,
      "iops": 360,
      "autoGrow": "Enabled",
      "autoIoScaling": "Disabled",
      "storageSku": "Premium_LRS",
      "logOnDisk": "Disabled"
    },
    "version": "8.0.21",
    "state": "Ready",
    "fullyQualifiedDomainName": "acctest-fs-replica-test20.mysql.database.azure.com",
    "sourceServerResourceId": "/subscriptions/xx-xx-xxx-xx/resourceGroups/acctestRG-mysql-test20/providers/Microsoft.DBforMySQL/flexibleServers/acctest-fs-test20",
    "availabilityZone": "2",
    "maintenanceWindow": {
      "customWindow": "Disabled",
      "dayOfWeek": 0,
      "startHour": 0,
      "startMinute": 0
    },
    "replicationRole": "Replica",
    "replicaCapacity": 0,
    "network": {
      "publicNetworkAccess": "Enabled"
    },
    "backup": {
      "backupRetentionDays": 7,
      "geoRedundantBackup": "Disabled",
      "earliestRestoreDate": "2024-03-14T03:10:34.5234034+00:00"
    },
    "highAvailability": {
      "mode": "Disabled",
      "state": "NotEnabled",
      "standbyAvailabilityZone": ""
    },
    "privateEndpointConnections": []
  },
  "location": "West Europe",
  "tags": {},
  "id": "/subscriptions/xx-xx-xxx-xx/resourceGroups/acctestRG-mysql-test20/providers/Microsoft.DBforMySQL/flexibleServers/acctest-fs-replica-test20",
  "name": "acctest-fs-replica-test20",
  "type": "Microsoft.DBforMySQL/flexibleServers"
}

Below is the response when creating mysql fs with create_mode = "GeoRestore".

{
  "sku": {
    "name": "Standard_D4ds_v4",
    "tier": "GeneralPurpose"
  },
  "properties": {
    "administratorLogin": "adminTerraform",
    "storage": {
      "storageSizeGB": 20,
      "iops": 360,
      "autoGrow": "Enabled",
      "autoIoScaling": "Disabled",
      "storageSku": "Premium_LRS",
      "logOnDisk": "Disabled"
    },
    "version": "8.0.21",
    "state": "Ready",
    "fullyQualifiedDomainName": "acctest-fs-restore-test20.mysql.database.azure.com",
    "availabilityZone": "2",
    "maintenanceWindow": {
      "customWindow": "Disabled",
      "dayOfWeek": 0,
      "startHour": 0,
      "startMinute": 0
    },
    "replicationRole": "None",
    "replicaCapacity": 10,
    "network": {
      "publicNetworkAccess": "Enabled"
    },
    "backup": {
      "backupRetentionDays": 7,
      "geoRedundantBackup": "Disabled",
      "earliestRestoreDate": "2024-03-14T03:43:34.9680531+00:00"
    },
    "highAvailability": {
      "mode": "Disabled",
      "state": "NotEnabled",
      "standbyAvailabilityZone": ""
    },
    "privateEndpointConnections": []
  },
  "location": "North Europe",
  "tags": {},
  "id": "/subscriptions/xx-xx-xxx-xx/resourceGroups/acctestRG-mysql-test20/providers/Microsoft.DBforMySQL/flexibleServers/acctest-fs-restore-test20",
  "name": "acctest-fs-restore-test20",
  "type": "Microsoft.DBforMySQL/flexibleServers"
}


if network := props.Network; network != nil {
d.Set("public_network_access_enabled", *network.PublicNetworkAccess == servers.EnableStatusEnumEnabled)
Expand Down
Loading