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

Incorrect or misleading documentation for azurerm_sql_failover_group resource #8778

Closed
rossmobi opened this issue Oct 7, 2020 · 5 comments · Fixed by #8845
Closed

Incorrect or misleading documentation for azurerm_sql_failover_group resource #8778

rossmobi opened this issue Oct 7, 2020 · 5 comments · Fixed by #8845

Comments

@rossmobi
Copy link

rossmobi commented Oct 7, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureRM Provider) Version

+ provider registry.terraform.io/hashicorp/azurerm v2.30.0
+ provider registry.terraform.io/hashicorp/random v2.3.0

Affected Resource(s)

  • azurerm_mssql_database
  • azurerm_sql_failover_group

Terraform Configuration Files

# Primary database
resource "azurerm_mssql_database" "test" {
  name            = "test"
  server_id       = var.sql_server_id
  elastic_pool_id = var.sql_elastic_pool_id
  collation       = "SQL_Latin1_General_CP1_CI_AS"
  max_size_gb     = var.mssql_database["max_size_gb"]
  read_scale      = var.mssql_database["read_scale"]
  zone_redundant  = var.mssql_database["zone_redundant"]
}

# Secondary database
resource "azurerm_mssql_database" "test_secondary" {
  count           = var.secondary_sql_server_id != null && var.secondary_sql_elastic_pool_id != null ? 1 : 0
  name            = "test"
  server_id       = var.secondary_sql_server_id
  elastic_pool_id = var.secondary_sql_elastic_pool_id
  collation       = "SQL_Latin1_General_CP1_CI_AS"
  max_size_gb     = var.mssql_database["max_size_gb"]
  read_scale      = var.mssql_database["read_scale"]
  zone_redundant  = var.mssql_database["zone_redundant"]
}

# Failover Group to replicate from Primary to Secondary
resource "azurerm_sql_failover_group" "test" {
  count               = var.secondary_sql_server_id != null && var.secondary_sql_elastic_pool_id != null ? 1 : 0
  name                = "test-failover-group"
  resource_group_name = split("/", var.sql_server_id)[4] 
  server_name         = split("/", var.sql_server_id)[8] 
  databases           = [azurerm_mssql_database.test.id]

  partner_servers {
    id = var.secondary_sql_server_id
  }

  read_write_endpoint_failover_policy {
    mode          = "Automatic"
    grace_minutes = 60
  }

  depends_on = [
    azurerm_mssql_database.test,
    azurerm_mssql_database.test_secondary[0]
  ]
}

Debug Output

Panic Output

Expected Behavior

Terraform should create the primary and secondary databases, then create a Failover Group between them.

Actual Behavior

Terraform successfully creates the primary and secondary databases, then the Azure REST API returned a 409 RemoteDatabaseExists - The destination database name already exists on the destination server error when trying to create the failover group:

Error: Error issuing create/update request for SQL Failover Group "test-failover-group" (Resource Group "test", Server "test"): sql.FailoverGroupsClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: autorest/azure: Service returned an error. Status=<nil> Code="FailoverGroupUnableToPerformGroupOperationOnDatabases" Message="The operation cannot be performed due to multiple errors." Details=[{"code":"45138","message":"The destination database name 'test' already exists on the server 'test-secondary'."}]

  on mssql.tf line 29, in resource "azurerm_sql_failover_group" "test":
  29: resource "azurerm_sql_failover_group" "test" {

After having looked the Azure REST API reference, this appears to be the expected behaviour. Thus, I consider the AzureRM Provider documentation to be quite wrong:

NOTE: The failover group will create a secondary database for each database listed in databases. If the secondary databases need to be managed through Terraform, they should be defined as resources and a dependency added to the failover group to ensure the secondary databases are created first.

The secondary database is defined as a resource and a dependency is added to the failover group. This does not work to create the failover group.

I think what would be needed it:

  • apply Terraform with a primary database resource and a failover group resource
  • write a secondary database resource
  • import the secondary database to the Terraform state

Steps to Reproduce

Run Terraform similar to the above.

References

azurerm_sql_failover_group documentation

@WodansSon WodansSon added documentation service/mssql Microsoft SQL Server labels Oct 9, 2020
@yupwei68
Copy link
Contributor

yupwei68 commented Oct 9, 2020

Hi @rossdotpink , thanks for opening this issue. I think what the document means is that you need to create a secondary database before creating a azurerm_sql_failover_group. The following example illustrates the document and is applied with success, you could refer it:

provider "azurerm" {
  features {}
}

resource "azurerm_resource_group" "test" {
  name     = "acctestRG-yup2"
  location = "eastus"
}

resource "azurerm_sql_server" "test1" {
  name                         = "acctestmssqlyup2-primary"
  resource_group_name          = azurerm_resource_group.test.name
  location                     = azurerm_resource_group.test.location
  version                      = "12.0"
  administrator_login          = "mradministrator"
  administrator_login_password = "thisIsDog11"
}

resource "azurerm_sql_server" "test2" {
  name                         = "acctestmssqlyup2-secondary"
  resource_group_name          = azurerm_resource_group.test.name
  location                     = "eastus2"
  version                      = "12.0"
  administrator_login          = "mradministrator"
  administrator_login_password = "thisIsDog11"
}

resource "azurerm_mssql_database" "test" {
  name         = "acctest-db-yup2"
  server_id    = azurerm_sql_server.test1.id
  collation    = "SQL_AltDiction_CP850_CI_AI"
  license_type = "BasePrice"
  sku_name     = "GP_Gen5_2"
}

resource "azurerm_mssql_database" "secondary" {
  name                        = "acctest-db-yup2"
  server_id                   = azurerm_sql_server.test2.id
  create_mode                 = "Secondary"
  creation_source_database_id = azurerm_mssql_database.test.id

}

resource "azurerm_sql_failover_group" "test" {
  name                = "acctestsfgyup2"
  resource_group_name = azurerm_resource_group.test.name
  server_name         = azurerm_sql_server.test1.name
  databases           = [azurerm_mssql_database.test.id]

  partner_servers {
    id = azurerm_sql_server.test2.id
  }

  read_write_endpoint_failover_policy {
    mode          = "Automatic"
    grace_minutes = 60
  }

  depends_on = [azurerm_mssql_database.secondary]
}

@rossmobi
Copy link
Author

rossmobi commented Oct 9, 2020

@yupwei68 Ahhh! 谢谢您.

This is mostly a case of my ignorance then 🐱 do you think this should be specified in the documentation?

For example:

If the secondary databases need to be managed through Terraform, they should be defined as resources and a dependency added to the failover group to ensure the secondary databases are created first.

could become:

If the secondary databases need to be managed through Terraform, they should first be defined as resources with create_mode and creation_source_database_id configured accordingly, with a dependency then added to the failover group to ensure the secondary databases are created first.

@rossmobi
Copy link
Author

@yupwei68 Thanks!

@ghost
Copy link

ghost commented Oct 15, 2020

This has been released in version 2.32.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.32.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Nov 14, 2020

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. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@hashicorp hashicorp locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants