Repository: kubernetes-sigs/cloud-provider-azure
Pull request #3106 (2023-01-11 09:01:10)

[release-1.24] Use TimedCache.Get() for read-only resources
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide/first-contribution.md#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/sig-release/release.md#issuepr-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/sig-testing/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

#### What type of PR is this?

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
-->

/kind bug

#### What this PR does / why we need it:

Cherry pick of #2949 against release-1.24: Use TimedCache.Get() for read-only resources

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

#### Special notes for your reviewer:

#### Does this PR introduce a user-facing change?
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

For more information on release notes see: https://git.k8s.io/community/contributors/guide/release-notes.md
-->
```release-note
fix: Use TimedCache.Get() for read-only resources
```

#### Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

<!--
This section can be blank if this pull request does not require a release note.

When adding links which point to resources within git repositories, like
KEPs or supporting documentation, please reference a specific commit and avoid
linking directly to the master branch. This ensures that links reference a
specific point in time, rather than a document that may change over time.

See here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files

Please use the following format for linking documentation:
- [KEP]: <link>
- [Usage]: <link>
- [Other doc]: <link>
-->
```docs

```



Patch:
diff --git a/pkg/cache/azure_cache.go b/pkg/cache/azure_cache.go
index d88041de6..1ac799588 100644
--- a/pkg/cache/azure_cache.go
+++ b/pkg/cache/azure_cache.go
@@ -122,8 +122,13 @@ func (t *TimedCache) getInternal(key string) (*AzureCacheEntry, error) {
 	return newEntry, nil
 }
 
-// Get returns the requested item by key with deep copy.
+// Get returns the requested item by key.
 func (t *TimedCache) Get(key string, crt AzureCacheReadType) (interface{}, error) {
+	return t.get(key, crt)
+}
+
+// Get returns the requested item by key with deep copy.
+func (t *TimedCache) GetWithDeepCopy(key string, crt AzureCacheReadType) (interface{}, error) {
 	data, err := t.get(key, crt)
 	copied := deepcopy.Copy(data)
 	return copied, err
diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go
index ab9d286b9..f5a22dc21 100644
--- a/pkg/provider/azure_vmss.go
+++ b/pkg/provider/azure_vmss.go
@@ -172,7 +172,7 @@ func (ss *ScaleSet) getVmssVMByNodeIdentity(node *nodeIdentity, crt azcache.Azur
 
 		virtualMachines := cached.(*sync.Map)
 		if entry, ok := virtualMachines.Load(nodeName); ok {
-			result := entry.(*VMSSVirtualMachinesEntry)
+			result := entry.(*VMSSVirtualMachineEntry)
 			if result.VirtualMachine == nil {
 				klog.Warningf("failed to get VM with vmssVirtualMachinesEntry on Node %q", nodeName)
 				return nil, false, nil
@@ -309,7 +309,7 @@ func (ss *ScaleSet) getVmssVMByInstanceID(resourceGroup, scaleSetName, instanceI
 
 		virtualMachines := cached.(*sync.Map)
 		virtualMachines.Range(func(key, value interface{}) bool {
-			vmEntry := value.(*VMSSVirtualMachinesEntry)
+			vmEntry := value.(*VMSSVirtualMachineEntry)
 			if strings.EqualFold(vmEntry.ResourceGroup, resourceGroup) &&
 				strings.EqualFold(vmEntry.VMSSName, scaleSetName) &&
 				strings.EqualFold(vmEntry.InstanceID, instanceID) {
diff --git a/pkg/provider/azure_vmss_cache.go b/pkg/provider/azure_vmss_cache.go
index d12a857cf..a1a622b0c 100644
--- a/pkg/provider/azure_vmss_cache.go
+++ b/pkg/provider/azure_vmss_cache.go
@@ -33,7 +33,7 @@ import (
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 )
 
-type VMSSVirtualMachinesEntry struct {
+type VMSSVirtualMachineEntry struct {
 	ResourceGroup  string
 	VMSSName       string
 	InstanceID     string
@@ -157,7 +157,7 @@ func (ss *ScaleSet) newVMSSVirtualMachinesCache(resourceGroupName, vmssName, cac
 	getter := func(key string) (interface{}, error) {
 		localCache := &sync.Map{} // [nodeName]*vmssVirtualMachinesEntry
 
-		oldCache := make(map[string]VMSSVirtualMachinesEntry)
+		oldCache := make(map[string]VMSSVirtualMachineEntry)
 
 		if vmssCache, ok := ss.vmssVMCache.Load(cacheKey); ok {
 			// get old cache before refreshing the cache
@@ -171,7 +171,7 @@ func (ss *ScaleSet) newVMSSVirtualMachinesCache(resourceGroupName, vmssName, cac
 				if cached != nil {
 					virtualMachines := cached.(*sync.Map)
 					virtualMachines.Range(func(key, value interface{}) bool {
-						oldCache[key.(string)] = *value.(*VMSSVirtualMachinesEntry)
+						oldCache[key.(string)] = *value.(*VMSSVirtualMachineEntry)
 						return true
 					})
 				}
@@ -196,7 +196,7 @@ func (ss *ScaleSet) newVMSSVirtualMachinesCache(resourceGroupName, vmssName, cac
 				continue
 			}
 
-			vmssVMCacheEntry := &VMSSVirtualMachinesEntry{
+			vmssVMCacheEntry := &VMSSVirtualMachineEntry{
 				ResourceGroup:  resourceGroupName,
 				VMSSName:       vmssName,
 				InstanceID:     to.String(vm.InstanceID),
@@ -231,7 +231,7 @@ func (ss *ScaleSet) newVMSSVirtualMachinesCache(resourceGroupName, vmssName, cac
 			}
 
 			klog.V(5).Infof("adding old entries to new cache for %s", name)
-			localCache.Store(name, &VMSSVirtualMachinesEntry{
+			localCache.Store(name, &VMSSVirtualMachineEntry{
 				ResourceGroup:  vmEntry.ResourceGroup,
 				VMSSName:       vmEntry.VMSSName,
 				InstanceID:     vmEntry.InstanceID,
@@ -280,7 +280,6 @@ func (ss *ScaleSet) DeleteCacheForNode(nodeName string) error {
 		return err
 	}
 
-	// Delete in VMSS VM cache
 	if err := ss.gcVMSSVMCache(); err != nil {
 		klog.Errorf("DeleteCacheForNode(%s) failed to gc stale vmss caches: %v", nodeName, err)
 	}
diff --git a/pkg/provider/azure_wrap.go b/pkg/provider/azure_wrap.go
index 10a6e2714..c004556c1 100644
--- a/pkg/provider/azure_wrap.go
+++ b/pkg/provider/azure_wrap.go
@@ -86,7 +86,7 @@ func (az *Cloud) getRouteTable(crt azcache.AzureCacheReadType) (routeTable netwo
 		return routeTable, false, fmt.Errorf("Route table name is not configured")
 	}
 
-	cachedRt, err := az.rtCache.Get(az.RouteTableName, crt)
+	cachedRt, err := az.rtCache.GetWithDeepCopy(az.RouteTableName, crt)
 	if err != nil {
 		return routeTable, false, err
 	}
@@ -109,7 +109,7 @@ func (az *Cloud) getPIPCacheKey(pipResourceGroup string, pipName string) string
 func (az *Cloud) getPublicIPAddress(pipResourceGroup string, pipName string, crt azcache.AzureCacheReadType) (network.PublicIPAddress, bool, error) {
 	pip := network.PublicIPAddress{}
 	cacheKey := az.getPIPCacheKey(pipResourceGroup, pipName)
-	cachedPIP, err := az.pipCache.Get(cacheKey, crt)
+	cachedPIP, err := az.pipCache.GetWithDeepCopy(cacheKey, crt)
 	if err != nil {
 		return pip, false, err
 	}
@@ -146,7 +146,7 @@ func (az *Cloud) getSubnet(virtualNetworkName string, subnetName string) (networ
 }
 
 func (az *Cloud) getAzureLoadBalancer(name string, crt azcache.AzureCacheReadType) (lb *network.LoadBalancer, exists bool, err error) {
-	cachedLB, err := az.lbCache.Get(name, crt)
+	cachedLB, err := az.lbCache.GetWithDeepCopy(name, crt)
 	if err != nil {
 		return lb, false, err
 	}
@@ -164,7 +164,7 @@ func (az *Cloud) getSecurityGroup(crt azcache.AzureCacheReadType) (network.Secur
 		return nsg, fmt.Errorf("securityGroupName is not configured")
 	}
 
-	securityGroup, err := az.nsgCache.Get(az.SecurityGroupName, crt)
+	securityGroup, err := az.nsgCache.GetWithDeepCopy(az.SecurityGroupName, crt)
 	if err != nil {
 		return nsg, err
 	}
@@ -177,7 +177,7 @@ func (az *Cloud) getSecurityGroup(crt azcache.AzureCacheReadType) (network.Secur
 }
 
 func (az *Cloud) getPrivateLinkService(frontendIPConfigID *string, crt azcache.AzureCacheReadType) (pls network.PrivateLinkService, err error) {
-	cachedPLS, err := az.plsCache.Get(*frontendIPConfigID, crt)
+	cachedPLS, err := az.plsCache.GetWithDeepCopy(*frontendIPConfigID, crt)
 	if err != nil {
 		return pls, err
 	}


Test patch:
diff --git a/pkg/cache/azure_cache_test.go b/pkg/cache/azure_cache_test.go
index 50b110d59..82e30d0e6 100644
--- a/pkg/cache/azure_cache_test.go
+++ b/pkg/cache/azure_cache_test.go
@@ -98,7 +98,7 @@ func TestCacheGet(t *testing.T) {
 	for _, c := range cases {
 		dataSource, cache := newFakeCache(t)
 		dataSource.set(c.data)
-		val, err := cache.Get(c.key, CacheReadTypeDefault)
+		val, err := cache.GetWithDeepCopy(c.key, CacheReadTypeDefault)
 		assert.NoError(t, err, c.name)
 		assert.Equal(t, c.expected, val, c.name)
 	}
@@ -112,7 +112,7 @@ func TestCacheGetError(t *testing.T) {
 	cache, err := NewTimedcache(fakeCacheTTL, getter)
 	assert.NoError(t, err)
 
-	val, err := cache.Get("key", CacheReadTypeDefault)
+	val, err := cache.GetWithDeepCopy("key", CacheReadTypeDefault)
 	assert.Error(t, err)
 	assert.Equal(t, getError, err)
 	assert.Nil(t, val)
@@ -140,7 +140,7 @@ func TestCacheGetWithDeepCopy(t *testing.T) {
 			dataSource, cache := newFakeCache(t)
 			dataSource.set(c.data)
 			cache.Set(c.key, valFake)
-			val, err := cache.Get(c.key, CacheReadTypeDefault)
+			val, err := cache.GetWithDeepCopy(c.key, CacheReadTypeDefault)
 			assert.NoError(t, err)
 			assert.Equal(t, c.expected, val.(*fakeDataObj).Data)
 
@@ -160,13 +160,13 @@ func TestCacheDelete(t *testing.T) {
 	dataSource, cache := newFakeCache(t)
 	dataSource.set(data)
 
-	v, err := cache.Get(testKey, CacheReadTypeDefault)
+	v, err := cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 	assert.NoError(t, err)
 	assert.Equal(t, val, v, "cache should get correct data")
 
 	dataSource.set(nil)
 	_ = cache.Delete(testKey)
-	v, err = cache.Get(testKey, CacheReadTypeDefault)
+	v, err = cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 	assert.NoError(t, err)
 	assert.Equal(t, 1, dataSource.called)
 	assert.Equal(t, nil, v, "cache should get nil after data is removed")
@@ -180,13 +180,13 @@ func TestCacheExpired(t *testing.T) {
 	dataSource, cache := newFakeCache(t)
 	dataSource.set(data)
 
-	v, err := cache.Get(testKey, CacheReadTypeDefault)
+	v, err := cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 	assert.NoError(t, err)
 	assert.Equal(t, 1, dataSource.called)
 	assert.Equal(t, val, v, "cache should get correct data")
 
 	time.Sleep(fakeCacheTTL)
-	v, err = cache.Get(testKey, CacheReadTypeDefault)
+	v, err = cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 	assert.NoError(t, err)
 	assert.Equal(t, 2, dataSource.called)
 	assert.Equal(t, val, v, "cache should get correct data even after expired")
@@ -200,13 +200,13 @@ func TestCacheAllowUnsafeRead(t *testing.T) {
 	dataSource, cache := newFakeCache(t)
 	dataSource.set(data)
 
-	v, err := cache.Get(testKey, CacheReadTypeDefault)
+	v, err := cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 	assert.NoError(t, err)
 	assert.Equal(t, 1, dataSource.called)
 	assert.Equal(t, val, v, "cache should get correct data")
 
 	time.Sleep(fakeCacheTTL)
-	v, err = cache.Get(testKey, CacheReadTypeUnsafe)
+	v, err = cache.GetWithDeepCopy(testKey, CacheReadTypeUnsafe)
 	assert.NoError(t, err)
 	assert.Equal(t, 1, dataSource.called)
 	assert.Equal(t, val, v, "cache should return expired as allow unsafe read is allowed")
@@ -226,10 +226,10 @@ func TestCacheNoConcurrentGet(t *testing.T) {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			_, _ = cache.Get(testKey, CacheReadTypeDefault)
+			_, _ = cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 		}()
 	}
-	v, err := cache.Get(testKey, CacheReadTypeDefault)
+	v, err := cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 	wg.Wait()
 	assert.NoError(t, err)
 	assert.Equal(t, 1, dataSource.called)
@@ -244,12 +244,12 @@ func TestCacheForceRefresh(t *testing.T) {
 	dataSource, cache := newFakeCache(t)
 	dataSource.set(data)
 
-	v, err := cache.Get(testKey, CacheReadTypeDefault)
+	v, err := cache.GetWithDeepCopy(testKey, CacheReadTypeDefault)
 	assert.NoError(t, err)
 	assert.Equal(t, 1, dataSource.called)
 	assert.Equal(t, val, v, "cache should get correct data")
 
-	v, err = cache.Get(testKey, CacheReadTypeForceRefresh)
+	v, err = cache.GetWithDeepCopy(testKey, CacheReadTypeForceRefresh)
 	assert.NoError(t, err)
 	assert.Equal(t, 2, dataSource.called)
 	assert.Equal(t, val, v, "should refetch unexpired data as forced refresh")
diff --git a/pkg/provider/azure_backoff_test.go b/pkg/provider/azure_backoff_test.go
index a661b05b0..76db3e1b7 100644
--- a/pkg/provider/azure_backoff_test.go
+++ b/pkg/provider/azure_backoff_test.go
@@ -234,7 +234,7 @@ func TestCreateOrUpdateSecurityGroupCanceled(t *testing.T) {
 	assert.EqualError(t, fmt.Errorf("Retriable: false, RetryAfter: 0s, HTTPStatusCode: 0, RawError: %w", fmt.Errorf("canceledandsupersededduetoanotheroperation")), err.Error())
 
 	// security group should be removed from cache if the operation is canceled
-	shouldBeEmpty, err := az.nsgCache.Get("sg", cache.CacheReadTypeDefault)
+	shouldBeEmpty, err := az.nsgCache.GetWithDeepCopy("sg", cache.CacheReadTypeDefault)
 	assert.NoError(t, err)
 	assert.Empty(t, shouldBeEmpty)
 }
@@ -287,12 +287,12 @@ func TestCreateOrUpdateLB(t *testing.T) {
 		assert.EqualError(t, test.expectedErr, err.Error())
 
 		// loadbalancer should be removed from cache if the etag is mismatch or the operation is canceled
-		shouldBeEmpty, err := az.lbCache.Get("lb", cache.CacheReadTypeDefault)
+		shouldBeEmpty, err := az.lbCache.GetWithDeepCopy("lb", cache.CacheReadTypeDefault)
 		assert.NoError(t, err)
 		assert.Empty(t, shouldBeEmpty)
 
 		// public ip cache should be populated since there's GetPIP
-		shouldNotBeEmpty, err := az.pipCache.Get(az.getPIPCacheKey(az.ResourceGroup, "pip"), cache.CacheReadTypeDefault)
+		shouldNotBeEmpty, err := az.pipCache.GetWithDeepCopy(az.getPIPCacheKey(az.ResourceGroup, "pip"), cache.CacheReadTypeDefault)
 		assert.NoError(t, err)
 		assert.NotEmpty(t, shouldNotBeEmpty)
 	}
@@ -417,7 +417,7 @@ func TestCreateOrUpdatePIP(t *testing.T) {
 		err := az.CreateOrUpdatePIP(&v1.Service{}, az.ResourceGroup, network.PublicIPAddress{Name: to.StringPtr("nic")})
 		assert.EqualError(t, test.expectedErr, err.Error())
 
-		cachedPIP, err := az.pipCache.Get(az.getPIPCacheKey(az.ResourceGroup, "nic"), cache.CacheReadTypeDefault)
+		cachedPIP, err := az.pipCache.GetWithDeepCopy(az.getPIPCacheKey(az.ResourceGroup, "nic"), cache.CacheReadTypeDefault)
 		assert.NoError(t, err)
 		if test.cacheExpectedEmpty {
 			assert.Empty(t, cachedPIP)
@@ -496,7 +496,7 @@ func TestCreateOrUpdateRouteTable(t *testing.T) {
 		assert.EqualError(t, test.expectedErr, err.Error())
 
 		// route table should be removed from cache if the etag is mismatch or the operation is canceled
-		shouldBeEmpty, err := az.rtCache.Get("rt", cache.CacheReadTypeDefault)
+		shouldBeEmpty, err := az.rtCache.GetWithDeepCopy("rt", cache.CacheReadTypeDefault)
 		assert.NoError(t, err)
 		assert.Empty(t, shouldBeEmpty)
 	}
@@ -542,7 +542,7 @@ func TestCreateOrUpdateRoute(t *testing.T) {
 			assert.EqualError(t, test.expectedErr, err.Error())
 		}
 
-		shouldBeEmpty, err := az.rtCache.Get("rt", cache.CacheReadTypeDefault)
+		shouldBeEmpty, err := az.rtCache.GetWithDeepCopy("rt", cache.CacheReadTypeDefault)
 		assert.NoError(t, err)
 		assert.Empty(t, shouldBeEmpty)
 	}
diff --git a/pkg/provider/azure_vmss_test.go b/pkg/provider/azure_vmss_test.go
index e5f2ec545..304781c6f 100644
--- a/pkg/provider/azure_vmss_test.go
+++ b/pkg/provider/azure_vmss_test.go
@@ -958,7 +958,7 @@ func TestGetVmssVMByNodeIdentity(t *testing.T) {
 			assert.Nil(t, err)
 			virtualMachines := cached.(*sync.Map)
 			for _, vm := range test.goneVMList {
-				entry := VMSSVirtualMachinesEntry{
+				entry := VMSSVirtualMachineEntry{
 					ResourceGroup: ss.ResourceGroup,
 					VMSSName:      testVMSSName,
 				}
diff --git a/tests/e2e/network/node.go b/tests/e2e/network/node.go
index 88b7064e8..01d7473da 100644
--- a/tests/e2e/network/node.go
+++ b/tests/e2e/network/node.go
@@ -320,11 +320,11 @@ var _ = Describe("Azure nodes", func() {
 
 		utils.Logf("scaling VMSS")
 		count := *vmss.Sku.Capacity
-		err = utils.ScaleVMSS(tc, *vmss.Name, tc.GetResourceGroup(), int64(vmssScaleUpCelling))
+		err = utils.ScaleVMSS(tc, *vmss.Name, int64(vmssScaleUpCelling))
 
 		defer func() {
 			utils.Logf("restoring VMSS")
-			err = utils.ScaleVMSS(tc, *vmss.Name, tc.GetResourceGroup(), count)
+			err = utils.ScaleVMSS(tc, *vmss.Name, count)
 			Expect(err).NotTo(HaveOccurred())
 		}()
 
diff --git a/tests/e2e/node/vmss.go b/tests/e2e/node/vmss.go
index 2ba894842..1b7cd4f59 100644
--- a/tests/e2e/node/vmss.go
+++ b/tests/e2e/node/vmss.go
@@ -17,9 +17,6 @@ limitations under the License.
 package node
 
 import (
-	"os"
-	"strings"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
@@ -29,7 +26,7 @@ import (
 	"sigs.k8s.io/cloud-provider-azure/tests/e2e/utils"
 )
 
-var _ = Describe("Lifecycle of VMSS", Label(utils.TestSuiteLabelVMSS), func() {
+var _ = Describe("Lifecycle of VMSS", Label(utils.TestSuiteLabelVMSS, utils.TestSuiteLabelVMSSScale), func() {
 	var (
 		ns     *v1.Namespace
 		k8sCli kubernetes.Interface
@@ -72,21 +69,13 @@ var _ = Describe("Lifecycle of VMSS", Label(utils.TestSuiteLabelVMSS), func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		By("deallocate VMSS instance")
-		if strings.EqualFold(os.Getenv(utils.CAPZTestCCM), "true") {
-			err = utils.ScaleMachinePool(*vmss.Name, numInstance-1)
-		} else {
-			err = utils.ScaleVMSS(azCli, *vmss.Name, azCli.GetResourceGroup(), numInstance-1)
-		}
+		err = utils.Scale(azCli, *vmss.Name, numInstance-1)
 		Expect(err).NotTo(HaveOccurred())
 		expectedCap[*vmss.Name] = numInstance - 1
 
 		defer func() {
 			By("reset VMSS instance")
-			if strings.EqualFold(os.Getenv(utils.CAPZTestCCM), "true") {
-				err = utils.ScaleMachinePool(*vmss.Name, numInstance)
-			} else {
-				err = utils.ScaleVMSS(azCli, *vmss.Name, azCli.GetResourceGroup(), numInstance)
-			}
+			err = utils.Scale(azCli, *vmss.Name, numInstance)
 			Expect(err).NotTo(HaveOccurred())
 			expectedCap[*vmss.Name] = numInstance
 
@@ -116,21 +105,13 @@ var _ = Describe("Lifecycle of VMSS", Label(utils.TestSuiteLabelVMSS), func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		By("allocate VMSS instance")
-		if strings.EqualFold(os.Getenv(utils.CAPZTestCCM), "true") {
-			err = utils.ScaleMachinePool(*vmss.Name, numInstance+1)
-		} else {
-			err = utils.ScaleVMSS(azCli, *vmss.Name, azCli.GetResourceGroup(), numInstance+1)
-		}
+		err = utils.Scale(azCli, *vmss.Name, numInstance+1)
 		Expect(err).NotTo(HaveOccurred())
 		expectedCap[*vmss.Name] = numInstance + 1
 
 		defer func() {
 			By("reset VMSS instance")
-			if strings.EqualFold(os.Getenv(utils.CAPZTestCCM), "true") {
-				err = utils.ScaleMachinePool(*vmss.Name, numInstance)
-			} else {
-				err = utils.ScaleVMSS(azCli, *vmss.Name, azCli.GetResourceGroup(), numInstance)
-			}
+			err = utils.Scale(azCli, *vmss.Name, numInstance)
 			Expect(err).NotTo(HaveOccurred())
 			expectedCap[*vmss.Name] = numInstance
 
diff --git a/tests/e2e/utils/consts.go b/tests/e2e/utils/consts.go
index ab9b60985..f85a125fe 100644
--- a/tests/e2e/utils/consts.go
+++ b/tests/e2e/utils/consts.go
@@ -24,6 +24,7 @@ const (
 	TestSuiteLabelMultiNodePools     = "Multi-Nodepool"
 	TestSuiteLabelSingleNodePool     = "Single-Nodepool"
 	TestSuiteLabelVMSS               = "VMSS"
+	TestSuiteLabelVMSSScale          = "VMSS-Scale"
 	TestSuiteLabelSpotVM             = "Spot-VM"
 	TestSuiteLabelKubenet            = "Kubenet"
 	TestSuiteLabelMultiGroup         = "Multi-Group"
diff --git a/tests/e2e/utils/vmss_utils.go b/tests/e2e/utils/vmss_utils.go
index d8649c9b5..525b3b724 100644
--- a/tests/e2e/utils/vmss_utils.go
+++ b/tests/e2e/utils/vmss_utils.go
@@ -19,6 +19,7 @@ package utils
 import (
 	"context"
 	"fmt"
+	"os"
 	"regexp"
 	"strings"
 
@@ -53,9 +54,17 @@ func FindTestVMSS(tc *AzureTestClient, rgName string) (*azcompute.VirtualMachine
 	return &vmssList[0], nil
 }
 
+func Scale(tc *AzureTestClient, vmssName string, instanceCount int64) error {
+	if strings.EqualFold(os.Getenv(CAPZTestCCM), "true") {
+		return ScaleMachinePool(vmssName, instanceCount)
+	}
+	return ScaleVMSS(tc, vmssName, instanceCount)
+}
+
 // ScaleVMSS scales the given VMSS
-func ScaleVMSS(tc *AzureTestClient, vmssName, rgName string, instanceCount int64) (err error) {
+func ScaleVMSS(tc *AzureTestClient, vmssName string, instanceCount int64) (err error) {
 	Logf("ScaleVMSS: start")
+	rgName := tc.GetResourceGroup()
 
 	vmssClient := tc.createVMSSClient()
 
@@ -178,10 +187,11 @@ func ValidateClusterNodesMatchVMSSInstances(tc *AzureTestClient, expectedCap map
 				Logf("Failed to validate VMSS instances: %s", err)
 			}
 
-			Logf("Matching cluster nodes[%s] with VMSS instances[%s]",
+			Logf("Matching cluster nodes[%s] with VMSS instances[%s]\nExpected capacity: %v, actual capacity: %v",
 				strings.Join(nodeSet.List(), ","),
-				strings.Join(instanceSet.List(), ","))
-			Logf("Expected capacity: %v, actual one: %v", expectedCap, actualCap)
+				strings.Join(instanceSet.List(), ","),
+				expectedCap,
+				actualCap)
 		}()
 
 		nodes, err = GetAgentNodes(k8sCli)
@@ -196,6 +206,10 @@ func ValidateClusterNodesMatchVMSSInstances(tc *AzureTestClient, expectedCap map
 		vmssList, _ := ListVMSSes(tc)
 		capMatch := true
 		for _, vmss := range vmssList {
+			cap, ok := expectedCap[*vmss.Name]
+			if !ok {
+				continue
+			}
 			vms, err := ListVMSSVMs(tc, *vmss.Name)
 			if err != nil {
 				return false, err
@@ -210,15 +224,23 @@ func ValidateClusterNodesMatchVMSSInstances(tc *AzureTestClient, expectedCap map
 				vmssInstanceSet.Insert(strings.ToLower(nodeName))
 				instanceSet.Insert(strings.ToLower(nodeName))
 			}
-			cap, ok := expectedCap[*vmss.Name]
-			if ok {
-				actualCap[*vmss.Name] = *vmss.Sku.Capacity
-				// For autoscaling cluster, simply comparing the capacity may not work since if the number of current nodes is lower than the "minCount", a new node may be created after scaling down.
-				// In this situation, we compare the expected capacity with the length of intersection between original nodes and current nodes.
-				if cap != *vmss.Sku.Capacity && cap != int64(originalNodeSet.Intersection(vmssInstanceSet).Len()) {
+
+			actualCap[*vmss.Name] = *vmss.Sku.Capacity
+			if cap != *vmss.Sku.Capacity {
+				if !strings.Contains(os.Getenv(AKSClusterType), "autoscaling") {
 					capMatch = false
+					break
+				}
+				if cap != int64(originalNodeSet.Intersection(vmssInstanceSet).Len()) {
+					// For autoscaling cluster, simply comparing the capacity may not work since if the number of current nodes is lower than the "minCount", a new node may be created after scaling down.
+					// In this situation, we compare the expected capacity with the length of intersection between original nodes and current nodes.
 					Logf("VMSS %q sku capacity is expected to be %d, but actually %d", *vmss.Name, cap, *vmss.Sku.Capacity)
+					capMatch = false
+					break
 				}
+			} else if int64(len(nodeSet)) != cap {
+				capMatch = false
+				break
 			}
 		}