-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
New Resource: azurerm_sql_managed_instance_active_directory_administrator
#14104
New Resource: azurerm_sql_managed_instance_active_directory_administrator
#14104
Conversation
378e9b7
to
92ce45a
Compare
92ce45a
to
a654c1e
Compare
9ca37ad
to
164b0a9
Compare
Hmm, looks like we need to increase the timeouts even more?
|
@katbyte which timeouts can we possibly change here? This seems like a timeout of the Azure API, not a resource creation timeout AFAIK. The resource create timeout for a Managed Instance is 24hrs, which is much more than current the 20 minutes timeout we're facing..? This looks like an Azure API instability issue |
Currently waiting for response on Azure/azure-rest-api-specs#16838 Edit: found a workaround, first tests are promising: // azurerm_sql_managed_instance is working again
❯ TF_ACC=1 go test -v ./internal/services/sql -run=TestAccAzureRMSqlMiServer_basic -timeout 1800m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccAzureRMSqlMiServer_basic
=== PAUSE TestAccAzureRMSqlMiServer_basic
=== CONT TestAccAzureRMSqlMiServer_basic
testcase.go:110: Step 1/2 is completed for TestAccAzureRMSqlMiServer_basic
testcase.go:110: Step 2/2 is completed for TestAccAzureRMSqlMiServer_basic
--- PASS: TestAccAzureRMSqlMiServer_basic (16103.77s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/sql 16106.193s // Identity works as expected
❯ TF_ACC=1 go test -v ./internal/services/sql -run=TestAccAzureRMSqlMiServer_identity -timeout 1800m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccAzureRMSqlMiServer_identity
=== PAUSE TestAccAzureRMSqlMiServer_identity
=== CONT TestAccAzureRMSqlMiServer_identity
--- PASS: TestAccAzureRMSqlMiServer_identity (18194.80s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/sql 18194.833s |
azurerm_sql_managed_identity_active_directory_administrator
: New resourceazurerm_sql_managed_instance_active_directory_administrator
: New resource
internal/services/sql/sql_managed_instance_administrator_resource.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another error on import:
--- PASS: TestAccSqlMiAdministrator_basic (17654.32s)
=== CONT TestAccSqlMiAdministrator_requiresImport
testcase.go:110: Step 3/3, expected an error with pattern, no match on: Error running pre-apply refresh: exit status 1
Error: Reference to undeclared resource
internal/services/sql/sql_managed_instance_administrator_resource_test.go
Outdated
Show resolved
Hide resolved
d410e8a
to
ebaa249
Compare
And last test fixed: --- PASS: TestAccSqlMiAdministrator_requiresImport (15679.28s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @aristosvo! This PR looks good with just a couple callouts
internal/services/sql/sql_managed_instance_administrator_resource.go
Outdated
Show resolved
Hide resolved
website/docs/r/sql_managed_instance_active_directory_administrator.html.markdown
Show resolved
Hide resolved
ebaa249
to
c7a3d91
Compare
Rebased and squashed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @aristosvo
azurerm_sql_managed_instance_active_directory_administrator
: New resourceazurerm_sql_managed_instance_active_directory_administrator
This functionality has been released in v2.89.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #13957
Needs
azuread
v2 via #14105Needs merge of #14052
Acceptance Tests