diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD index 8ef496f757c..409ad725022 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD @@ -98,6 +98,7 @@ go_test( "azure_storage_test.go", "azure_storageaccount_test.go", "azure_test.go", + "azure_utils_test.go", "azure_vmss_cache_test.go", "azure_vmss_test.go", "azure_wrap_test.go", diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go index 9c466898d42..d8fc3b6798f 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go @@ -142,8 +142,8 @@ func (c *controllerCommon) AttachDisk(isManagedDisk bool, diskName, diskURI stri return -1, fmt.Errorf("failed to get azure instance id for node %q (%v)", nodeName, err) } - c.vmLockMap.LockEntry(string(nodeName)) - defer c.vmLockMap.UnlockEntry(string(nodeName)) + c.vmLockMap.LockEntry(strings.ToLower(string(nodeName))) + defer c.vmLockMap.UnlockEntry(strings.ToLower(string(nodeName))) lun, err := c.GetNextDiskLun(nodeName) if err != nil { @@ -179,20 +179,20 @@ func (c *controllerCommon) DetachDisk(diskName, diskURI string, nodeName types.N klog.V(2).Infof("detach %v from node %q", diskURI, nodeName) // make the lock here as small as possible - c.vmLockMap.LockEntry(string(nodeName)) + c.vmLockMap.LockEntry(strings.ToLower(string(nodeName))) c.diskAttachDetachMap.Store(strings.ToLower(diskURI), "detaching") resp, err := vmset.DetachDisk(diskName, diskURI, nodeName) c.diskAttachDetachMap.Delete(strings.ToLower(diskURI)) - c.vmLockMap.UnlockEntry(string(nodeName)) + c.vmLockMap.UnlockEntry(strings.ToLower(string(nodeName))) if c.cloud.CloudProviderBackoff && shouldRetryHTTPRequest(resp, err) { klog.V(2).Infof("azureDisk - update backing off: detach disk(%s, %s), err: %v", diskName, diskURI, err) retryErr := kwait.ExponentialBackoff(c.cloud.RequestBackoff(), func() (bool, error) { - c.vmLockMap.LockEntry(string(nodeName)) + c.vmLockMap.LockEntry(strings.ToLower(string(nodeName))) c.diskAttachDetachMap.Store(strings.ToLower(diskURI), "detaching") resp, err := vmset.DetachDisk(diskName, diskURI, nodeName) c.diskAttachDetachMap.Delete(strings.ToLower(diskURI)) - c.vmLockMap.UnlockEntry(string(nodeName)) + c.vmLockMap.UnlockEntry(strings.ToLower(string(nodeName))) return c.cloud.processHTTPRetryResponse(nil, "", resp, err) }) if retryErr != nil { diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go index bef369c83cb..d4b5aba8015 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go @@ -1,7 +1,7 @@ // +build !providerless /* -Copyright 2018 The Kubernetes Authors. +Copyright 2019 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. diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go new file mode 100644 index 00000000000..cb527f37dd2 --- /dev/null +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go @@ -0,0 +1,85 @@ +// +build !providerless + +/* +Copyright 2019 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 azure + +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(t *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 + } +}