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

Deadlock creating azurerm_subnet #15204

Closed
cberner opened this issue Jun 8, 2017 · 4 comments
Closed

Deadlock creating azurerm_subnet #15204

cberner opened this issue Jun 8, 2017 · 4 comments

Comments

@cberner
Copy link

cberner commented Jun 8, 2017

Terraform Version

0.9.6

Affected Resource(s)

azurerm_subnet

Terraform Configuration Files

https://github.com/cberner/terraform-hang

Expected Behavior

All resources should be created

Actual Behavior

Terraform hangs (I waited over 10min) creating module.etcd_junk.azurerm_subnet.etcd

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. git clone https://github.com/cberner/terraform-hang
  2. cd terraform-hang/main
  3. terraform apply

References

I suspect this is a regression caused by one of the below PRs, as configs like this used to work for us, and reverting the locks that were added in those fixes the problem:

@tombuildsstuff
Copy link
Contributor

Hey @cberner

Thanks for reporting this issue :)

I identified this earlier in the week too - and, as you've identified I believe it was introduced in #13637. I've got a fix which resolves this and will confirm this solves the deadlock issue with this configuration too.

Thanks!

@echuvyrov
Copy link
Contributor

@tombuildsstuff @cberner I spent some time yesterday and today looking at the issue and I believe the collision/deadlock between mutex for subnet name and the mutex for network security group are the cause here. In the code above, the names of subnet and nsg are the same, resulting in two mutexes using the same key. Not sure if that's the same fix you've come up with @tombuildsstuff, but I changed my code to be something like the below, which eliminated the issue with @cberner code for me. I can submit a PR but did not want to collide nor assume that this fully resolves it.

uniqueMtxName := fmt.Sprint("sub-", name)
armMutexKV.Lock(uniqueMtxName)
defer armMutexKV.Unlock(uniqueMtxName)

uniqueMtxVNet := fmt.Sprint("vnet-", vnetName)
armMutexKV.Lock(uniqueMtxVNet)
defer armMutexKV.Unlock(uniqueMtxVNet)

properties := network.SubnetPropertiesFormat{
	AddressPrefix: &addressPrefix,
}

if v, ok := d.GetOk("network_security_group_id"); ok {
            [...]
	uniqueMtxNsg := fmt.Sprint("nsg-", networkSecurityGroupName)
	armMutexKV.Lock(uniqueMtxNsg)
	defer armMutexKV.Unlock(uniqueMtxNsg)
}

if v, ok := d.GetOk("route_table_id"); ok {
            [...]
	uniqueMtxRtTbl := fmt.Sprint("rt-", routeTableName)
	armMutexKV.Lock(uniqueMtxRtTbl)
	defer armMutexKV.Unlock(uniqueMtxRtTbl)
}

@tombuildsstuff
Copy link
Contributor

Hey @echuvyrov

Indeed - it appears we've gone down a similar path - however I ended up updating all of the Locks by Name in the AzureRM Provider to be prefixed by the resource type, so we didn't accidentally bring this up in the future.

I've validated this against this repository and have a few test-cases for this - however since the 0.10 announcement / repository split - I've pushed my changes to the new AzureRM Provider Repository in PR #6. I'm going to validate this PR against all 4 test-cases - and I'll post an update here when that's done.

Thanks!

@ghost
Copy link

ghost commented Apr 9, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

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

No branches or pull requests

4 participants