azurerm_kubernetes_cluster_node_pool: fix subnet lock to use resource ID instead of name#32001
Open
Reasonably wants to merge 1 commit intohashicorp:mainfrom
Open
azurerm_kubernetes_cluster_node_pool: fix subnet lock to use resource ID instead of name#32001Reasonably wants to merge 1 commit intohashicorp:mainfrom
azurerm_kubernetes_cluster_node_pool: fix subnet lock to use resource ID instead of name#32001Reasonably wants to merge 1 commit intohashicorp:mainfrom
Conversation
… ID instead of name The subnet mutex in nodepool creation used `locks.MultipleByName` with just the subnet name as the lock key. This caused false positive lock contention when two nodepools in different VNets/clusters used subnets with the same name (e.g., "nodesubnet"), serializing operations that could safely run in parallel. Switch to `locks.MultipleByID` which uses the full Azure resource ID as the lock key, ensuring that only operations on the same actual subnet are serialized. This is consistent with the approach already used in `container_group_resource.go`. Add acceptance test `TestAccKubernetesClusterNodePool_parallelCrossVNetSameSubnetName` to verify parallel nodepool creation across different VNets with identically-named subnets.
azurerm_kubernetes_cluster_node_pool: fix subnet lock to use resource ID instead of name
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Community Note
Description
The subnet mutex in
azurerm_kubernetes_cluster_node_poolduring nodepool creation useslocks.MultipleByNamewith just the subnet name (e.g.,"nodesubnet") as the lock key. This causes false-positive lock contention when two nodepools in different VNets or clusters use subnets with the same name, serializing operations that could safely run in parallel.Root Cause
In
kubernetes_cluster_node_pool_resource.go(lines 652-665), the lock key is constructed from the subnet name only:Since
locks.MultipleByNamecreates a global mutex key of"azurerm_subnet." + subnetName, two completely independent subnets in different VNets with the same name (e.g.,"nodesubnet") will contend on the same mutex. The lock is held for the entire duration of the nodepool creation API call + polling (5-20 minutes), causing the second operation to wait unnecessarily.Fix
Replace
locks.MultipleByNamewithlocks.MultipleByID, which uses the full Azure resource ID as the lock key. This ensures:This approach is consistent with the existing pattern in
container_group_resource.go(line 758), which already useslocks.ByID(subnet.ID()).Related Issues / PRs
azurerm_kubernetes_cluster_node_pool- Subnet name-based mutex causes false serialization across different VNets/regions #32002azurerm_kubernetes_cluster_node_pool- lock subnet ID instead of subnet name #26939 — Previously proposed the same fix (ByName → ByID), closed without mergeazurerm_kubernetes_cluster_node_pool- prevent race by polling pod subnet provisioning state during node pool creation #29537 — Reintroduced subnet name locks (including pod subnet), reintroducing the regressionChanges
internal/services/containers/kubernetes_cluster_node_pool_resource.go: Changedlocks.MultipleByName(&subnetsToLock, network.SubnetResourceName)tolocks.MultipleByID(&subnetIDsToLock), using full subnet resource IDs as lock keysinternal/services/containers/kubernetes_cluster_node_pool_resource_test.go: AddedTestAccKubernetesClusterNodePool_parallelCrossVNetSameSubnetNameacceptance test that creates two AKS clusters with nodepools in different VNets using identically-named subnetsTesting
go build ./internal/services/containers/...passesgo vet ./internal/services/containers/...passesTestAccKubernetesClusterNodePool_parallelCrossVNetSameSubnetName(requires Azure environment)TestAccKubernetesClusterNodePool_parallelPodSubnetstill passesNew Test:
TestAccKubernetesClusterNodePool_parallelCrossVNetSameSubnetNameCreates:
"nodesubnet") in different VNets