Repository: kubernetes-sigs/cloud-provider-azure
Pull request #7722 (2024-12-03 04:40:15)

[release-1.30] Chores: move lockmap to util package
This is an automated cherry-pick of #7714

/assign MartinForReal

```release-note
NONE
```

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/cloud-provider-azure/pull/7722#" title="Author self-approved">k8s-infra-cherrypick-robot</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign [andyzhangx](https://github.com/andyzhangx) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcloud-provider-azure).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/cloud-provider-azure/blob/release-1.30/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["andyzhangx"]} -->
coveralls: 
[![Coverage Status](https://coveralls.io/builds/71158169/badge)](https://coveralls.io/builds/71158169)

coverage: 76.308%. remained the same
when pulling **c7ba15e057872657c15a233be463e93d8d8884e5 on k8s-infra-cherrypick-robot:cherry-pick-7714-to-release-1.30**
into **d3677bd36db6c02dbda68eb7340558e225fb0317 on kubernetes-sigs:release-1.30**.

MartinForReal: /kind cleanup
MartinForReal: /lgtm
/approve
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/cloud-provider-azure/pull/7722#" title="Author self-approved">k8s-infra-cherrypick-robot</a>*, *<a href="https://github.com/kubernetes-sigs/cloud-provider-azure/pull/7722#issuecomment-2513564647" title="Approved">MartinForReal</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcloud-provider-azure).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/cloud-provider-azure/blob/release-1.30/OWNERS)~~ [MartinForReal]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/provider/azure.go b/pkg/provider/azure.go
index c19a72e72..d6b9a029e 100644
--- a/pkg/provider/azure.go
+++ b/pkg/provider/azure.go
@@ -81,6 +81,7 @@ import (
 	ratelimitconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
 	"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
 	"sigs.k8s.io/cloud-provider-azure/pkg/retry"
+	"sigs.k8s.io/cloud-provider-azure/pkg/util/lockmap"
 	utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
 	"sigs.k8s.io/cloud-provider-azure/pkg/util/taints"
 )
@@ -399,7 +400,7 @@ type Cloud struct {
 	// node-sync-loop routine and service-reconcile routine should not update LoadBalancer at the same time
 	serviceReconcileLock sync.Mutex
 
-	lockMap *LockMap
+	lockMap *lockmap.LockMap
 	// multipleStandardLoadBalancerConfigurationsSynced make sure the `reconcileMultipleStandardLoadBalancerConfigurations`
 	// runs only once every time the cloud provide restarts.
 	multipleStandardLoadBalancerConfigurationsSynced bool
@@ -433,7 +434,7 @@ func NewCloud(ctx context.Context, clientBuilder cloudprovider.ControllerClientB
 
 	az.ipv6DualStackEnabled = true
 	if az.lockMap == nil {
-		az.lockMap = newLockMap()
+		az.lockMap = lockmap.NewLockMap()
 	}
 
 	if clientBuilder != nil {
@@ -595,7 +596,7 @@ func (az *Cloud) InitializeCloudFromConfig(ctx context.Context, config *Config,
 		return err
 	}
 
-	az.lockMap = newLockMap()
+	az.lockMap = lockmap.NewLockMap()
 	az.Config = *config
 	az.Environment = *env
 	az.ResourceRequestBackoff = resourceRequestBackoff
diff --git a/pkg/provider/azure_fakes.go b/pkg/provider/azure_fakes.go
index 763fa52fa..12b8e3d84 100644
--- a/pkg/provider/azure_fakes.go
+++ b/pkg/provider/azure_fakes.go
@@ -47,6 +47,7 @@ import (
 	"sigs.k8s.io/cloud-provider-azure/pkg/provider/privatelinkservice"
 	"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
 	"sigs.k8s.io/cloud-provider-azure/pkg/provider/subnet"
+	"sigs.k8s.io/cloud-provider-azure/pkg/util/lockmap"
 	utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
 )
 
@@ -110,7 +111,7 @@ func GetTestCloud(ctrl *gomock.Controller) (az *Cloud) {
 		nodePrivateIPs:           map[string]*utilsets.IgnoreCaseSet{},
 		routeCIDRs:               map[string]string{},
 		eventRecorder:            &record.FakeRecorder{},
-		lockMap:                  newLockMap(),
+		lockMap:                  lockmap.NewLockMap(),
 	}
 	az.DisksClient = mockdiskclient.NewMockInterface(ctrl)
 	az.InterfacesClient = mockinterfaceclient.NewMockInterface(ctrl)
diff --git a/pkg/provider/azure_utils.go b/pkg/provider/azure_utils.go
index cd1c74621..66ff23e48 100644
--- a/pkg/provider/azure_utils.go
+++ b/pkg/provider/azure_utils.go
@@ -47,44 +47,6 @@ var strToExtendedLocationType = map[string]armnetwork.ExtendedLocationTypes{
 	"edgezone": armnetwork.ExtendedLocationTypesEdgeZone,
 }
 
-// LockMap used to lock on entries
-type LockMap struct {
-	sync.Mutex
-	mutexMap map[string]*sync.Mutex
-}
-
-// NewLockMap returns a new lock map
-func newLockMap() *LockMap {
-	return &LockMap{
-		mutexMap: make(map[string]*sync.Mutex),
-	}
-}
-
-// LockEntry acquires a lock associated with the specific entry
-func (lm *LockMap) LockEntry(entry string) {
-	lm.Lock()
-	// check if entry does not exists, then add entry
-	mutex, exists := lm.mutexMap[entry]
-	if !exists {
-		mutex = &sync.Mutex{}
-		lm.mutexMap[entry] = mutex
-	}
-	lm.Unlock()
-	mutex.Lock()
-}
-
-// UnlockEntry release the lock associated with the specific entry
-func (lm *LockMap) UnlockEntry(entry string) {
-	lm.Lock()
-	defer lm.Unlock()
-
-	mutex, exists := lm.mutexMap[entry]
-	if !exists {
-		return
-	}
-	mutex.Unlock()
-}
-
 func getContextWithCancel() (context.Context, context.CancelFunc) {
 	return context.WithCancel(context.Background())
 }
diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go
index dcdbf43fe..52248cc71 100644
--- a/pkg/provider/azure_vmss.go
+++ b/pkg/provider/azure_vmss.go
@@ -41,6 +41,7 @@ import (
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 	"sigs.k8s.io/cloud-provider-azure/pkg/metrics"
 	"sigs.k8s.io/cloud-provider-azure/pkg/provider/virtualmachine"
+	"sigs.k8s.io/cloud-provider-azure/pkg/util/lockmap"
 	vmutil "sigs.k8s.io/cloud-provider-azure/pkg/util/vm"
 )
 
@@ -103,7 +104,7 @@ type ScaleSet struct {
 	nonVmssUniformNodesCache azcache.Resource
 
 	// lockMap in cache refresh
-	lockMap *LockMap
+	lockMap *lockmap.LockMap
 }
 
 // RefreshCaches invalidates and renew all related caches.
@@ -154,14 +155,14 @@ func newScaleSet(az *Cloud) (VMSet, error) {
 		Cloud:           az,
 		availabilitySet: as,
 		flexScaleSet:    fs,
-		lockMap:         newLockMap(),
+		lockMap:         lockmap.NewLockMap(),
 	}
 
 	if err := ss.RefreshCaches(); err != nil {
 		return nil, err
 	}
 
-	ss.lockMap = newLockMap()
+	ss.lockMap = lockmap.NewLockMap()
 	return ss, nil
 }
 
diff --git a/pkg/provider/azure_vmssflex.go b/pkg/provider/azure_vmssflex.go
index a96abf3e7..b7f2ae400 100644
--- a/pkg/provider/azure_vmssflex.go
+++ b/pkg/provider/azure_vmssflex.go
@@ -39,6 +39,7 @@ import (
 	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 	"sigs.k8s.io/cloud-provider-azure/pkg/metrics"
+	"sigs.k8s.io/cloud-provider-azure/pkg/util/lockmap"
 	vmutil "sigs.k8s.io/cloud-provider-azure/pkg/util/vm"
 )
 
@@ -55,7 +56,7 @@ type FlexScaleSet struct {
 	vmssFlexVMCache          azcache.Resource
 
 	// lockMap in cache refresh
-	lockMap *LockMap
+	lockMap *lockmap.LockMap
 }
 
 // RefreshCaches invalidates and renew all related caches.
@@ -80,7 +81,7 @@ func newFlexScaleSet(az *Cloud) (VMSet, error) {
 		Cloud:                    az,
 		vmssFlexVMNameToVmssID:   &sync.Map{},
 		vmssFlexVMNameToNodeName: &sync.Map{},
-		lockMap:                  newLockMap(),
+		lockMap:                  lockmap.NewLockMap(),
 	}
 
 	if err := fs.RefreshCaches(); err != nil {
diff --git a/pkg/util/lockmap/lockmap.go b/pkg/util/lockmap/lockmap.go
new file mode 100644
index 000000000..57f2205fe
--- /dev/null
+++ b/pkg/util/lockmap/lockmap.go
@@ -0,0 +1,57 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package lockmap
+
+import "sync"
+
+// LockMap used to lock on entries
+type LockMap struct {
+	sync.Mutex
+	mutexMap map[string]*sync.Mutex
+}
+
+// NewLockMap returns a new lock map
+func NewLockMap() *LockMap {
+	return &LockMap{
+		mutexMap: make(map[string]*sync.Mutex),
+	}
+}
+
+// LockEntry acquires a lock associated with the specific entry
+func (lm *LockMap) LockEntry(entry string) {
+	lm.Lock()
+	// check if entry does not exists, then add entry
+	mutex, exists := lm.mutexMap[entry]
+	if !exists {
+		mutex = &sync.Mutex{}
+		lm.mutexMap[entry] = mutex
+	}
+	lm.Unlock()
+	mutex.Lock()
+}
+
+// UnlockEntry release the lock associated with the specific entry
+func (lm *LockMap) UnlockEntry(entry string) {
+	lm.Lock()
+	defer lm.Unlock()
+
+	mutex, exists := lm.mutexMap[entry]
+	if !exists {
+		return
+	}
+	mutex.Unlock()
+}


Test patch:
diff --git a/pkg/provider/azure_storageaccount_test.go b/pkg/provider/azure_storageaccount_test.go
index f5fd283e6..fa799c609 100644
--- a/pkg/provider/azure_storageaccount_test.go
+++ b/pkg/provider/azure_storageaccount_test.go
@@ -43,6 +43,7 @@ import (
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/subnetclient/mocksubnetclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/cache"
 	"sigs.k8s.io/cloud-provider-azure/pkg/retry"
+	"sigs.k8s.io/cloud-provider-azure/pkg/util/lockmap"
 )
 
 const TestLocation = "testLocation"
@@ -678,7 +679,7 @@ func TestAddStorageAccountTags(t *testing.T) {
 	defer cancel()
 
 	cloud := &Cloud{}
-	cloud.lockMap = newLockMap()
+	cloud.lockMap = lockmap.NewLockMap()
 	tests := []struct {
 		name           string
 		subsID         string
@@ -782,7 +783,7 @@ func TestRemoveStorageAccountTags(t *testing.T) {
 
 	getter := func(_ context.Context, _ string) (interface{}, error) { return nil, nil }
 	cloud.storageAccountCache, _ = cache.NewTimedCache(time.Minute, getter, false)
-	cloud.lockMap = newLockMap()
+	cloud.lockMap = lockmap.NewLockMap()
 	for _, test := range tests {
 		mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl)
 		cloud.StorageAccountClient = mockStorageAccountsClient
diff --git a/pkg/provider/azure_utils_test.go b/pkg/provider/azure_utils_test.go
index 43b389ebb..9756ac6ea 100644
--- a/pkg/provider/azure_utils_test.go
+++ b/pkg/provider/azure_utils_test.go
@@ -21,7 +21,6 @@ import (
 	"reflect"
 	"sync"
 	"testing"
-	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v6"
@@ -37,67 +36,6 @@ import (
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 )
 
-func TestSimpleLockEntry(t *testing.T) {
-	testLockMap := newLockMap()
-
-	callbackChan1 := make(chan interface{})
-	go testLockMap.lockAndCallback(t, "entry1", callbackChan1)
-	ensureCallbackHappens(t, callbackChan1)
-}
-
-func TestSimpleLockUnlockEntry(t *testing.T) {
-	testLockMap := newLockMap()
-
-	callbackChan1 := make(chan interface{})
-	go testLockMap.lockAndCallback(t, "entry1", callbackChan1)
-	ensureCallbackHappens(t, callbackChan1)
-	testLockMap.UnlockEntry("entry1")
-}
-
-func TestConcurrentLockEntry(t *testing.T) {
-	testLockMap := newLockMap()
-
-	callbackChan1 := make(chan interface{})
-	callbackChan2 := make(chan interface{})
-
-	go testLockMap.lockAndCallback(t, "entry1", callbackChan1)
-	ensureCallbackHappens(t, callbackChan1)
-
-	go testLockMap.lockAndCallback(t, "entry1", callbackChan2)
-	ensureNoCallback(t, callbackChan2)
-
-	testLockMap.UnlockEntry("entry1")
-	ensureCallbackHappens(t, callbackChan2)
-	testLockMap.UnlockEntry("entry1")
-}
-
-func (lm *LockMap) lockAndCallback(_ *testing.T, entry string, callbackChan chan<- interface{}) {
-	lm.LockEntry(entry)
-	callbackChan <- true
-}
-
-var callbackTimeout = 2 * time.Second
-
-func ensureCallbackHappens(t *testing.T, callbackChan <-chan interface{}) bool {
-	select {
-	case <-callbackChan:
-		return true
-	case <-time.After(callbackTimeout):
-		t.Fatalf("timed out waiting for callback")
-		return false
-	}
-}
-
-func ensureNoCallback(t *testing.T, callbackChan <-chan interface{}) bool {
-	select {
-	case <-callbackChan:
-		t.Fatalf("unexpected callback")
-		return false
-	case <-time.After(callbackTimeout):
-		return true
-	}
-}
-
 func TestReconcileTags(t *testing.T) {
 	for _, testCase := range []struct {
 		description, systemTags                      string
diff --git a/pkg/util/lockmap/lockmap_test.go b/pkg/util/lockmap/lockmap_test.go
new file mode 100644
index 000000000..8d45967a8
--- /dev/null
+++ b/pkg/util/lockmap/lockmap_test.go
@@ -0,0 +1,83 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package lockmap
+
+import (
+	"testing"
+	"time"
+)
+
+func TestSimpleLockEntry(t *testing.T) {
+	testLockMap := NewLockMap()
+
+	callbackChan1 := make(chan interface{})
+	go testLockMap.lockAndCallback(t, "entry1", callbackChan1)
+	ensureCallbackHappens(t, callbackChan1)
+}
+
+func TestSimpleLockUnlockEntry(t *testing.T) {
+	testLockMap := NewLockMap()
+
+	callbackChan1 := make(chan interface{})
+	go testLockMap.lockAndCallback(t, "entry1", callbackChan1)
+	ensureCallbackHappens(t, callbackChan1)
+	testLockMap.UnlockEntry("entry1")
+}
+
+func TestConcurrentLockEntry(t *testing.T) {
+	testLockMap := NewLockMap()
+
+	callbackChan1 := make(chan interface{})
+	callbackChan2 := make(chan interface{})
+
+	go testLockMap.lockAndCallback(t, "entry1", callbackChan1)
+	ensureCallbackHappens(t, callbackChan1)
+
+	go testLockMap.lockAndCallback(t, "entry1", callbackChan2)
+	ensureNoCallback(t, callbackChan2)
+
+	testLockMap.UnlockEntry("entry1")
+	ensureCallbackHappens(t, callbackChan2)
+	testLockMap.UnlockEntry("entry1")
+}
+
+func (lm *LockMap) lockAndCallback(_ *testing.T, entry string, callbackChan chan<- interface{}) {
+	lm.LockEntry(entry)
+	callbackChan <- true
+}
+
+var callbackTimeout = 2 * time.Second
+
+func ensureCallbackHappens(t *testing.T, callbackChan <-chan interface{}) bool {
+	select {
+	case <-callbackChan:
+		return true
+	case <-time.After(callbackTimeout):
+		t.Fatalf("timed out waiting for callback")
+		return false
+	}
+}
+
+func ensureNoCallback(t *testing.T, callbackChan <-chan interface{}) bool {
+	select {
+	case <-callbackChan:
+		t.Fatalf("unexpected callback")
+		return false
+	case <-time.After(callbackTimeout):
+		return true
+	}
+}