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

Added resource for SQL MI "azurerm_sql_managed_instance" #2727

Closed
wants to merge 3 commits into from
Closed

Added resource for SQL MI "azurerm_sql_managed_instance" #2727

wants to merge 3 commits into from

Conversation

joranm
Copy link
Contributor

@joranm joranm commented Jan 21, 2019

New resource provider for SQL Managed Instance
https://docs.microsoft.com/en-us/azure/sql-database/sql-database-managed-instance

Known issues

  • Destroys fails due to platform lease on Virtual Server Object (12 hours before it is deleted)

Fixes #1747

@joranm joranm changed the title Added resource for SQL MI instance Added resource for SQL MI "azurerm_sql_managed_instance" Jan 21, 2019
@joranm
Copy link
Contributor Author

joranm commented Jan 22, 2019

@katbyte , what is the issue with the megalinter? I'm seeing the same issue with PR #2716

@katbyte
Copy link
Collaborator

katbyte commented Jan 23, 2019

@joranm,

gometalinter removed some linters (megacheck and unused) as they were merged into staticcheck and we didn't pin the tooling version. As a result all linting was failing. It has been resolved in #2722 so merge/rebase on master and it should pass 🙂

@joranm
Copy link
Contributor Author

joranm commented Jan 23, 2019

Hi @katbyte, @tombuildsstuff , did a rebase, and tests have passed successfully.
Regarding the known issue. What would be the best approach?

1) Timeout issue on create (6 hours): Is it possible to implement the Timeout on these resources? I didn't found this feature implemented at any other resource.
2) Lock on dependend resources The creation of SQL Managed Instance does also create a Virtual Server Object and a Network Intent Policy, which locks the subnet and routetable. This object is automatically cleaned after 12 hours after the SQL MI instance removal.

@katbyte
Copy link
Collaborator

katbyte commented Feb 19, 2019

@joranm,

wrt 1 - at the moment we don't have timeouts implemented, that is something we plan to implement for v2.0 so until then tested this will be.. difficult. We have a few ideas but none are idea (an ENV for a pre existing subnet where a MI has already been provisioned for example)

@archmangler
Copy link

There is a suggestion on #1747 that the creation time has been greatly reduced on the AzureRM side. Has anyone tested if this improves the situation sufficiently to make this provider usable?

@pkaminski-degree
Copy link

Is this feature planned to be merged any time soon?

@jacobm3
Copy link

jacobm3 commented Apr 30, 2019

Any idea when this resource will be merged? Would love to see this available. Thanks!

@sarayho1
Copy link

sarayho1 commented May 1, 2019

Will this resource be merged and released anytime soon? Received a time-out error running the code with ARM JSON.

@TraGicCode
Copy link

Would be awesome to see more progress on getting this pushed through!

@WascawyWabbit
Copy link

Any news on when this will be merged?

@katbyte
Copy link
Collaborator

katbyte commented May 10, 2019

@joranm,

I went to run these tests today and got the following build error (as we have updated the azure go sdk)

==> Checking that code complies with gofmt requirements...
go install
# github.com/terraform-providers/terraform-provider-azurerm/azurerm
azurerm/resource_arm_sql_managed_instance.go:143:4: cannot use utils.String(licenseType) (type *string) as type "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/2015-05-01-preview/sql".ManagedInstanceLicenseType in field value
make: *** [build] Error 2

Went to merge master and push but it seems I can't? any chance you could update this se we could see if the instance provisions any faster now

@kim0
Copy link

kim0 commented Jun 4, 2019

I had opened #3593 and #3587 but apparently this is still WIP. If you have any questions, let me know though

@ghost ghost removed the waiting-response label Jun 4, 2019
@katbyte
Copy link
Collaborator

katbyte commented Jun 4, 2019

hi @kim0, the new PR is #3575 as this one seems to have been abandoned. Currently it is blocked with:

------- Stdout: -------
=== RUN   TestAccAzureRMSqlMiServer_basic
=== PAUSE TestAccAzureRMSqlMiServer_basic
=== CONT  TestAccAzureRMSqlMiServer_basic
--- FAIL: TestAccAzureRMSqlMiServer_basic (741.07s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: Code="VnetSubnetConflictWithIntendedPolicy" Message="Route Table. (https://go.microsoft.com/fwlink/?linkid=871071)"
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test102924249/main.tf line 40:
          (source code not available)
        
        
FAIL

so any help on how to resolve that would be fantastic as i am working on other things at the moment.

@joranm
Copy link
Contributor Author

joranm commented Jun 5, 2019

@joranm,
I went to run these tests today and got the following build error (as we have updated the azure go sdk)
==> Checking that code complies with gofmt requirements...
go install

github.com/terraform-providers/terraform-provider-azurerm/azurerm

azurerm/resource_arm_sql_managed_instance.go:143:4: cannot use utils.String(licenseType) (type *string) as type "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/2015-05-01-preview/sql".ManagedInstanceLicenseType in field value
make: *** [build] Error 2

Went to merge master and push but it seems I can't? any chance you could update this se we could see if the instance provisions any faster now

Hi @katbyte,
Provisioning of the first SQL MI in a vnet still takes between 6 and 12 hours. A lot of plumbing is done at the Azure side with regard to building a 5 node cluster, sql data gateway, loadbalancers and so on. I don't expect that the provisioning will be faster in the near future. Is it possible to implement the "timeout" property for Azure resources?

@katbyte
Copy link
Collaborator

katbyte commented Jun 6, 2019

@joranm, I believe that is in the works for v2.0 of the provider

@katbyte katbyte removed this from the v1.30.0 milestone Jun 6, 2019
@katbyte
Copy link
Collaborator

katbyte commented Jun 6, 2019

If you could pull in the changes from the PR i opened and or grant us push access to this branch we can take it from here

@tombuildsstuff
Copy link
Contributor

tombuildsstuff commented Jun 7, 2019

Provisioning of the first SQL MI in a vnet still takes between 6 and 12 hours.

@joranm

Indeed, it's better than it was (36h) but it still takes a while; chatting with the SQL Team a while back it turns out there's multiple conditions where the Create/Update call could take this long:

  • when the cluster this is using under the hood within the subnet is initially built (as you're seeing)
  • when the cluster is scaled up (for every 5 or so instances/databases within the same subnet)
  • when the cluster is scaled down (for every 5 or so instances/databases within the same subnet)

Is it possible to implement the "timeout" property for Azure resources?

As @katbyte has mentioned this is something we're doing as a part of the 2.0 release - at this point in time we're working on the dependencies to make that possible (we're working on switching out the Azure Storage SDK at the moment) at which point we should be able to enable Timeout support. However since this requires the Timeout support/this is going to ship in the 2.0 timeframe - I'm going to add this to the 2.0 milestone.

@tombuildsstuff tombuildsstuff added this to the v2.0.0 milestone Jun 7, 2019
@kim0

This comment has been minimized.

@kim0

This comment has been minimized.

@tombuildsstuff

This comment has been minimized.

@tombuildsstuff tombuildsstuff removed their assignment Jun 17, 2019
Optional: true,
Default: 8,
ValidateFunc: validate.IntInSlice([]int{
8,
Copy link

Choose a reason for hiding this comment

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

A few weeks ago a smaller number of vcores (4) was introduced:
https://docs.microsoft.com/en-us/azure/sql-database/sql-database-managed-instance-resource-limits#instance-level-resource-limits

It seems worthwhile to have this new resource up to date.

@katbyte
Copy link
Collaborator

katbyte commented Jan 25, 2020

Hi @joranm, i'm going to close this as @mbfrahry has started to move this forward in #5399

@katbyte katbyte closed this Jan 25, 2020
@ghost
Copy link

ghost commented Feb 24, 2020

This has been released in version 2.0.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.0.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Feb 25, 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!

@ghost ghost locked and limited conversation to collaborators Feb 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Managed SQL database instance