Repository: kubernetes-sigs/cloud-provider-azure
Pull request #5073 (2023-11-27 06:34:01)

feat: add occupiedLuns parameter in SetDiskLun func
<!--  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?
/kind feature

#### What this PR does / why we need it:
feat: add occupiedLuns parameter in SetDiskLun func

this PR is for solving https://github.com/kubernetes-sigs/azuredisk-csi-driver/issues/2000

#### 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
feat: add occupiedLuns parameter in SetDiskLun func
```

#### 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
feat: add occupiedLuns parameter in SetDiskLun func
```



Patch:
diff --git a/pkg/azclient/configloader/config_loader.go b/pkg/azclient/configloader/config_loader.go
index 5832ac42c..a2b878f43 100644
--- a/pkg/azclient/configloader/config_loader.go
+++ b/pkg/azclient/configloader/config_loader.go
@@ -20,6 +20,6 @@ import (
 	"context"
 )
 
-type configLoader[Type any] interface {
+type ConfigLoader[Type any] interface {
 	Load(ctx context.Context) (*Type, error)
 }
diff --git a/pkg/azclient/configloader/go.mod b/pkg/azclient/configloader/go.mod
index 17abe7db6..547a848b0 100644
--- a/pkg/azclient/configloader/go.mod
+++ b/pkg/azclient/configloader/go.mod
@@ -3,10 +3,8 @@ module sigs.k8s.io/cloud-provider-azure/pkg/azclient/configloader
 go 1.20
 
 require (
-	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0
 	github.com/onsi/ginkgo/v2 v2.13.1
 	github.com/onsi/gomega v1.30.0
-	k8s.io/api v0.28.4
 	k8s.io/apimachinery v0.28.4
 	k8s.io/client-go v0.28.4
 	sigs.k8s.io/yaml v1.4.0
@@ -15,7 +13,6 @@ require (
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/emicklei/go-restful/v3 v3.10.2 // indirect
-	github.com/evanphx/json-patch v4.12.0+incompatible // indirect
 	github.com/go-logr/logr v1.3.0 // indirect
 	github.com/go-openapi/jsonpointer v0.19.6 // indirect
 	github.com/go-openapi/jsonreference v0.20.2 // indirect
@@ -34,7 +31,7 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
-	github.com/pkg/errors v0.9.1 // indirect
+	github.com/stretchr/testify v1.8.3 // indirect
 	golang.org/x/net v0.17.0 // indirect
 	golang.org/x/oauth2 v0.9.0 // indirect
 	golang.org/x/sys v0.14.0 // indirect
@@ -47,6 +44,7 @@ require (
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
+	k8s.io/api v0.28.4 // indirect
 	k8s.io/klog/v2 v2.100.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20230717233707-2695361300d9 // indirect
 	k8s.io/utils v0.0.0-20230505201702-9f6742963106 // indirect
diff --git a/pkg/azclient/configloader/go.sum b/pkg/azclient/configloader/go.sum
index 5a0294806..7f48999b8 100644
--- a/pkg/azclient/configloader/go.sum
+++ b/pkg/azclient/configloader/go.sum
@@ -1,13 +1,9 @@
-github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0 h1:fb8kj/Dh4CSwgsOzHeZY4Xh68cFVbzXx+ONXGMY//4w=
-github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0/go.mod h1:uReU2sSxZExRPBAg3qKzmAucSi51+SP1OhohieR821Q=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/emicklei/go-restful/v3 v3.10.2 h1:hIovbnmBTLjHXkqEBUz3HGpXZdM7ZrE9fJIZIqlJLqE=
 github.com/emicklei/go-restful/v3 v3.10.2/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
-github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84=
-github.com/evanphx/json-patch v4.12.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
 github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
 github.com/go-logr/logr v1.3.0 h1:2y3SDp0ZXuc6/cjLSZ+Q3ir+QB9T/iG5yYRXqsagWSY=
 github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
@@ -64,8 +60,6 @@ github.com/onsi/ginkgo/v2 v2.13.1 h1:LNGfMbR2OVGBfXjvRZIZ2YCTQdGKtPLvuI1rMCCj3OU
 github.com/onsi/ginkgo/v2 v2.13.1/go.mod h1:XStQ8QcGwLyF4HdfcZB8SFOS/MWCgDuXMSBe6zrvLgM=
 github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8=
 github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ=
-github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
-github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ=
@@ -78,7 +72,8 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
-github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.3 h1:RP3t2pwF7cMEbC1dqtB6poj3niw/9gnV4Cjg5oW5gtY=
+github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
diff --git a/pkg/azclient/configloader/load.go b/pkg/azclient/configloader/load.go
index efa79c52b..350a46d8c 100644
--- a/pkg/azclient/configloader/load.go
+++ b/pkg/azclient/configloader/load.go
@@ -18,12 +18,13 @@ package configloader
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	clientset "k8s.io/client-go/kubernetes"
 )
 
-type decoderFactory[Type any] func(content []byte, loader configLoader[Type]) configLoader[Type]
+type decoderFactory[Type any] func(content []byte, loader ConfigLoader[Type]) ConfigLoader[Type]
 type K8sSecretLoaderConfig struct {
 	K8sSecretConfig
 	KubeClient clientset.Interface
@@ -51,28 +52,31 @@ type ConfigMergeConfig struct {
 }
 
 func Load[Type any](ctx context.Context, secretLoaderConfig *K8sSecretLoaderConfig, fileLoaderConfig *FileLoaderConfig) (*Type, error) {
-	configloader := newEmptyLoader[Type](nil)
-	var loadConfig *ConfigMergeConfig
-	var err error
-	if fileLoaderConfig != nil {
-		loadConfigloader := newEmptyLoader[ConfigMergeConfig](&ConfigMergeConfig{CloudConfigType: CloudConfigTypeMerge})
-		loadConfigloader = newFileLoader(fileLoaderConfig.FilePath, loadConfigloader, newYamlByteLoader[ConfigMergeConfig])
-		//by default the config load type  is merge
-		loadConfig, err = loadConfigloader.Load(ctx)
-		if err != nil {
-			return nil, err
-		}
-		configloader = newFileLoader(fileLoaderConfig.FilePath, nil, newYamlByteLoader[Type])
-		if strings.EqualFold(string(loadConfig.CloudConfigType), string(CloudConfigTypeFile)) {
-			return configloader.Load(ctx)
-		}
+
+	if fileLoaderConfig == nil {
+		return nil, fmt.Errorf("config file load config is nil")
+	}
+	//by default the config load type  is merge
+	loadConfigloader := NewEmptyLoader[ConfigMergeConfig](&ConfigMergeConfig{CloudConfigType: CloudConfigTypeMerge})
+	loadConfigloader = NewFileLoader(fileLoaderConfig.FilePath, loadConfigloader, NewYamlByteLoader[ConfigMergeConfig])
+	loadConfig, err := loadConfigloader.Load(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	configloader := NewFileLoader(fileLoaderConfig.FilePath, nil, NewYamlByteLoader[Type])
+	if strings.EqualFold(string(loadConfig.CloudConfigType), string(CloudConfigTypeFile)) {
+		return configloader.Load(ctx)
 	}
-	if secretLoaderConfig != nil {
-		if loadConfig != nil && strings.EqualFold(string(loadConfig.CloudConfigType), string(CloudConfigTypeSecret)) {
-			configloader = newK8sSecretLoader(&secretLoaderConfig.K8sSecretConfig, secretLoaderConfig.KubeClient, nil, newYamlByteLoader[Type])
-		} else {
-			configloader = newK8sSecretLoader(&secretLoaderConfig.K8sSecretConfig, secretLoaderConfig.KubeClient, configloader, newYamlByteLoader[Type])
-		}
+
+	if secretLoaderConfig == nil {
+		return nil, fmt.Errorf("secret load config is nil")
+	}
+
+	if strings.EqualFold(string(loadConfig.CloudConfigType), string(CloudConfigTypeSecret)) {
+		configloader = NewK8sSecretLoader(&secretLoaderConfig.K8sSecretConfig, secretLoaderConfig.KubeClient, nil, NewYamlByteLoader[Type])
+		return configloader.Load(ctx)
 	}
+	configloader = NewK8sSecretLoader(&secretLoaderConfig.K8sSecretConfig, secretLoaderConfig.KubeClient, configloader, NewYamlByteLoader[Type])
 	return configloader.Load(ctx)
 }
diff --git a/pkg/azclient/configloader/loader_empty.go b/pkg/azclient/configloader/loader_empty.go
index 894490bbb..70b9ee0d5 100644
--- a/pkg/azclient/configloader/loader_empty.go
+++ b/pkg/azclient/configloader/loader_empty.go
@@ -20,19 +20,19 @@ import (
 	"context"
 )
 
-type emptyLoader[Type any] struct {
+type EmptyLoader[Type any] struct {
 	Data *Type
 }
 
-func (e *emptyLoader[Type]) Load(ctx context.Context) (*Type, error) {
+func (e *EmptyLoader[Type]) Load(ctx context.Context) (*Type, error) {
 	return e.Data, nil
 }
 
-func newEmptyLoader[Type any](config *Type) configLoader[Type] {
+func NewEmptyLoader[Type any](config *Type) ConfigLoader[Type] {
 	if config == nil {
 		config = new(Type)
 	}
-	return &emptyLoader[Type]{
+	return &EmptyLoader[Type]{
 		Data: config,
 	}
 }
diff --git a/pkg/azclient/configloader/loader_file.go b/pkg/azclient/configloader/loader_file.go
index 44baddf89..bc33b0f9d 100644
--- a/pkg/azclient/configloader/loader_file.go
+++ b/pkg/azclient/configloader/loader_file.go
@@ -18,35 +18,46 @@ package configloader
 
 import (
 	"context"
+	"io"
 	"os"
 )
 
 type FileLoader[Type any] struct {
 	filePath string
-	configLoader[Type]
+	ConfigLoader[Type]
 	decoderFactory[Type]
 }
 
-// newFileLoader creates a FileLoader with the specified file path and loader.
-// decoderFactory is a function that creates a new loader from the content of the file. it should never be nil.
-func newFileLoader[Type any](filePath string, loader configLoader[Type], decoder decoderFactory[Type]) configLoader[Type] {
+func NewFileLoader[Type any](filePath string, loader ConfigLoader[Type], decoder decoderFactory[Type]) ConfigLoader[Type] {
 	return &FileLoader[Type]{
 		filePath:       filePath,
-		configLoader:   loader,
+		ConfigLoader:   loader,
 		decoderFactory: decoder,
 	}
 }
 
 func (f *FileLoader[Type]) Load(ctx context.Context) (*Type, error) {
-	if f.configLoader == nil {
-		f.configLoader = newEmptyLoader[Type](nil)
+	if f.ConfigLoader == nil {
+		f.ConfigLoader = NewEmptyLoader[Type](nil)
 	}
 
-	content, err := os.ReadFile(f.filePath)
-	if err != nil {
+	var content []byte
+	if err := func() error {
+		file, err := os.Open(f.filePath)
+		if err != nil {
+			return err
+		}
+		defer file.Close()
+
+		content, err = io.ReadAll(file)
+		if err != nil {
+			return err
+		}
+		return nil
+	}(); err != nil {
 		return nil, err
 	}
 
-	loader := f.decoderFactory(content, f.configLoader)
+	loader := f.decoderFactory(content, f.ConfigLoader)
 	return loader.Load(ctx)
 }
diff --git a/pkg/azclient/configloader/loader_secret.go b/pkg/azclient/configloader/loader_secret.go
index 6e114f14c..b8cf9e564 100644
--- a/pkg/azclient/configloader/loader_secret.go
+++ b/pkg/azclient/configloader/loader_secret.go
@@ -25,9 +25,9 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 )
 
-type k8sSecretLoader[Type any] struct {
+type K8sSecretLoader[Type any] struct {
 	KubeClient clientset.Interface
-	configLoader[Type]
+	ConfigLoader[Type]
 	*K8sSecretConfig
 	decoderFactory[Type]
 }
@@ -50,10 +50,7 @@ type K8sSecretConfig struct {
 	CloudConfigKey  string `json:"cloudConfigKey,omitempty" yaml:"cloudConfigKey,omitempty"`
 }
 
-// newK8sSecretLoader returns a config loader which load config from k8s secret.
-// If KubeClient is nil, it will return nil.
-// decoderFactory is a function that creates a new loader from the content of the secret. it should never be nil.
-func newK8sSecretLoader[Type any](config *K8sSecretConfig, KubeClient kubernetes.Interface, loader configLoader[Type], decoder decoderFactory[Type]) configLoader[Type] {
+func NewK8sSecretLoader[Type any](config *K8sSecretConfig, KubeClient kubernetes.Interface, loader ConfigLoader[Type], decoder decoderFactory[Type]) ConfigLoader[Type] {
 	if KubeClient == nil {
 		return nil
 	}
@@ -69,17 +66,17 @@ func newK8sSecretLoader[Type any](config *K8sSecretConfig, KubeClient kubernetes
 	if config.CloudConfigKey == "" {
 		config.CloudConfigKey = DefaultCloudProviderConfigSecKey
 	}
-	return &k8sSecretLoader[Type]{
-		configLoader:    loader,
+	return &K8sSecretLoader[Type]{
+		ConfigLoader:    loader,
 		K8sSecretConfig: config,
 		decoderFactory:  decoder,
 		KubeClient:      KubeClient,
 	}
 }
 
-func (k *k8sSecretLoader[Type]) Load(ctx context.Context) (*Type, error) {
-	if k.configLoader == nil {
-		k.configLoader = newEmptyLoader[Type](nil)
+func (k *K8sSecretLoader[Type]) Load(ctx context.Context) (*Type, error) {
+	if k.ConfigLoader == nil {
+		k.ConfigLoader = NewEmptyLoader[Type](nil)
 	}
 
 	if k.KubeClient == nil {
@@ -98,6 +95,6 @@ func (k *k8sSecretLoader[Type]) Load(ctx context.Context) (*Type, error) {
 	if content, ok = secret.Data[k.CloudConfigKey]; !ok {
 		return nil, ErrNoData
 	}
-	loader := k.decoderFactory([]byte(content), k.configLoader)
+	loader := k.decoderFactory([]byte(content), k.ConfigLoader)
 	return loader.Load(ctx)
 }
diff --git a/pkg/azclient/configloader/loader_yaml.go b/pkg/azclient/configloader/loader_yaml.go
index c5a8a1f30..5de8b4acc 100644
--- a/pkg/azclient/configloader/loader_yaml.go
+++ b/pkg/azclient/configloader/loader_yaml.go
@@ -23,18 +23,18 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
-// yamlByteLoader is a FactoryConfigLoader that loads a YAML file from a byte array.
-type yamlByteLoader[Type any] struct {
+// YamlByteLoader is a FactoryConfigLoader that loads a YAML file from a byte array.
+type YamlByteLoader[Type any] struct {
 	content []byte
-	configLoader[Type]
+	ConfigLoader[Type]
 }
 
 // Load loads the YAML file from the byte array and returns the client factory config.
-func (s *yamlByteLoader[Type]) Load(ctx context.Context) (*Type, error) {
-	if s.configLoader == nil {
-		s.configLoader = newEmptyLoader[Type](nil)
+func (s *YamlByteLoader[Type]) Load(ctx context.Context) (*Type, error) {
+	if s.ConfigLoader == nil {
+		s.ConfigLoader = NewEmptyLoader[Type](nil)
 	}
-	config, err := s.configLoader.Load(ctx)
+	config, err := s.ConfigLoader.Load(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -45,10 +45,10 @@ func (s *yamlByteLoader[Type]) Load(ctx context.Context) (*Type, error) {
 	return config, nil
 }
 
-// newYamlByteLoader creates a YamlByteLoader with the specified content and loader.
-func newYamlByteLoader[Type any](content []byte, loader configLoader[Type]) configLoader[Type] {
-	return &yamlByteLoader[Type]{
+// NewYamlByteLoader creates a YamlByteLoader with the specified content and loader.
+func NewYamlByteLoader[Type any](content []byte, loader ConfigLoader[Type]) ConfigLoader[Type] {
+	return &YamlByteLoader[Type]{
 		content:      content,
-		configLoader: loader,
+		ConfigLoader: loader,
 	}
 }
diff --git a/pkg/provider/azure_controller_common.go b/pkg/provider/azure_controller_common.go
index 9702a8956..7e84e2e30 100644
--- a/pkg/provider/azure_controller_common.go
+++ b/pkg/provider/azure_controller_common.go
@@ -168,9 +168,10 @@ func (c *controllerCommon) getNodeVMSet(nodeName types.NodeName, crt azcache.Azu
 
 // AttachDisk attaches a disk to vm
 // parameter async indicates whether allow multiple batch disk attach on one node in parallel
+// occupiedLuns is used to avoid conflict with other disk attach in k8s VolumeAttachments
 // return (lun, error)
 func (c *controllerCommon) AttachDisk(ctx context.Context, async bool, diskName, diskURI string, nodeName types.NodeName,
-	cachingMode compute.CachingTypes, disk *compute.Disk) (int32, error) {
+	cachingMode compute.CachingTypes, disk *compute.Disk, occupiedLuns []int) (int32, error) {
 	diskEncryptionSetID := ""
 	writeAcceleratorEnabled := false
 
@@ -261,7 +262,7 @@ func (c *controllerCommon) AttachDisk(ctx context.Context, async bool, diskName,
 		return -1, err
 	}
 
-	lun, err := c.SetDiskLun(nodeName, diskuri, diskMap)
+	lun, err := c.SetDiskLun(nodeName, diskuri, diskMap, occupiedLuns)
 	if err != nil {
 		return -1, err
 	}
@@ -527,8 +528,8 @@ func (c *controllerCommon) cleanDetachDiskRequests(nodeName string) (map[string]
 	return diskMap, nil
 }
 
-// getNodeDataDisks invokes vmSet interfaces to get data disks for the node.
-func (c *controllerCommon) getNodeDataDisks(nodeName types.NodeName, crt azcache.AzureCacheReadType) ([]compute.DataDisk, *string, error) {
+// GetNodeDataDisks invokes vmSet interfaces to get data disks for the node.
+func (c *controllerCommon) GetNodeDataDisks(nodeName types.NodeName, crt azcache.AzureCacheReadType) ([]compute.DataDisk, *string, error) {
 	vmset, err := c.getNodeVMSet(nodeName, crt)
 	if err != nil {
 		return nil, nil, err
@@ -539,9 +540,9 @@ func (c *controllerCommon) getNodeDataDisks(nodeName types.NodeName, crt azcache
 
 // GetDiskLun finds the lun on the host that the vhd is attached to, given a vhd's diskName and diskURI.
 func (c *controllerCommon) GetDiskLun(diskName, diskURI string, nodeName types.NodeName) (int32, *string, error) {
-	// getNodeDataDisks need to fetch the cached data/fresh data if cache expired here
+	// GetNodeDataDisks need to fetch the cached data/fresh data if cache expired here
 	// to ensure we get LUN based on latest entry.
-	disks, provisioningState, err := c.getNodeDataDisks(nodeName, azcache.CacheReadTypeDefault)
+	disks, provisioningState, err := c.GetNodeDataDisks(nodeName, azcache.CacheReadTypeDefault)
 	if err != nil {
 		klog.Errorf("error of getting data disks for node %s: %v", nodeName, err)
 		return -1, provisioningState, err
@@ -564,9 +565,10 @@ func (c *controllerCommon) GetDiskLun(diskName, diskURI string, nodeName types.N
 }
 
 // SetDiskLun find unused luns and allocate lun for every disk in diskMap.
+// occupiedLuns is used to avoid conflict with other disk attach in k8s VolumeAttachments
 // Return lun of diskURI, -1 if all luns are used.
-func (c *controllerCommon) SetDiskLun(nodeName types.NodeName, diskURI string, diskMap map[string]*AttachDiskOptions) (int32, error) {
-	disks, _, err := c.getNodeDataDisks(nodeName, azcache.CacheReadTypeDefault)
+func (c *controllerCommon) SetDiskLun(nodeName types.NodeName, diskURI string, diskMap map[string]*AttachDiskOptions, occupiedLuns []int) (int32, error) {
+	disks, _, err := c.GetNodeDataDisks(nodeName, azcache.CacheReadTypeDefault)
 	if err != nil {
 		klog.Errorf("error of getting data disks for node %s: %v", nodeName, err)
 		return -1, err
@@ -594,6 +596,10 @@ func (c *controllerCommon) SetDiskLun(nodeName types.NodeName, diskURI string, d
 		return lun, nil
 	}
 
+	for _, lun := range occupiedLuns {
+		used[int32(lun)] = true
+	}
+
 	// allocate lun for every disk in diskMap
 	var diskLuns []int32
 	count := 0
@@ -636,11 +642,11 @@ func (c *controllerCommon) DisksAreAttached(diskNames []string, nodeName types.N
 		attached[diskName] = false
 	}
 
-	// doing stalled read for getNodeDataDisks to ensure we don't call ARM
+	// doing stalled read for GetNodeDataDisks to ensure we don't call ARM
 	// for every reconcile call. The cache is invalidated after Attach/Detach
 	// disk. So the new entry will be fetched and cached the first time reconcile
 	// loop runs after the Attach/Disk OP which will reflect the latest model.
-	disks, _, err := c.getNodeDataDisks(nodeName, azcache.CacheReadTypeUnsafe)
+	disks, _, err := c.GetNodeDataDisks(nodeName, azcache.CacheReadTypeUnsafe)
 	if err != nil {
 		if errors.Is(err, cloudprovider.InstanceNotFound) {
 			// if host doesn't exist, no need to detach


Test patch:
diff --git a/pkg/azclient/configloader/load_test.go b/pkg/azclient/configloader/load_test.go
deleted file mode 100644
index 4f4539338..000000000
--- a/pkg/azclient/configloader/load_test.go
+++ /dev/null
@@ -1,134 +0,0 @@
-/*
-Copyright 2023 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 configloader
-
-import (
-	"context"
-
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes/fake"
-)
-
-type TestConfig struct {
-	ConfigMergeConfig
-	Value               *string `json:"value,omitempty" yaml:"value,omitempty"`
-	Cloud               *string `json:"cloud,omitempty" yaml:"cloud,omitempty"`                             // configured both in secret and file
-	FromSecret          bool    `json:"fromSecret,omitempty" yaml:"fromSecret,omitempty"`                   // be true only in secret config
-	UseInstanceMetadata bool    `json:"useInstanceMetadata,omitempty" yaml:"useInstanceMetadata,omitempty"` // be true only in file config
-}
-
-var _ = Describe("Load", func() {
-	var fakeKubeClient *fake.Clientset
-	BeforeEach(func() {
-		fakeKubeClient = fake.NewSimpleClientset(&corev1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "azure-cloud-provider",
-
-				Namespace: "kube-system",
-			},
-			Data: map[string][]byte{
-				"cloud-config": []byte(`{"cloud": "AzureCloud","fromSecret": true}`),
-			},
-		})
-	})
-	When("No config is provided", func() {
-		It("should return default config", func() {
-			loader := newEmptyLoader[TestConfig](nil)
-			config, err := loader.Load(context.Background())
-			Expect(err).To(BeNil())
-			Expect(config).NotTo(BeNil())
-			Expect(config.Value).To(BeNil())
-		})
-	})
-	When("file config is provided", func() {
-		var fileLoaderConfig *FileLoaderConfig
-
-		BeforeEach(func() {
-			fileLoaderConfig = &FileLoaderConfig{FilePath: "testdata/azure.json"}
-		})
-		When("config merge type is file", func() {
-			It("should return file config", func() {
-				config, err := Load[TestConfig](context.Background(), nil, fileLoaderConfig)
-				Expect(err).To(BeNil())
-				Expect(config).NotTo(BeNil())
-				Expect(*config.Cloud).To(Equal("AzurePublicCloud"))
-				Expect(config.FromSecret).To(BeFalse())
-				Expect(config.UseInstanceMetadata).To(BeTrue())
-			})
-		})
-		When("secret config is provided", func() {
-			var secretLoaderConfig *K8sSecretLoaderConfig
-			BeforeEach(func() {
-				secretLoaderConfig = &K8sSecretLoaderConfig{
-					K8sSecretConfig: K8sSecretConfig{
-						SecretName:      "azure-cloud-provider",
-						SecretNamespace: "kube-system",
-						CloudConfigKey:  "cloud-config",
-					},
-					KubeClient: fakeKubeClient,
-				}
-			})
-			When("config merge type is secret", func() {
-				It("should return file config", func() {
-					fileLoaderConfig := &FileLoaderConfig{FilePath: "testdata/azure_secret.json"}
-					config, err := Load[TestConfig](context.Background(), secretLoaderConfig, fileLoaderConfig)
-					Expect(err).To(BeNil())
-					Expect(config).NotTo(BeNil())
-					Expect(*config.Cloud).To(Equal("AzureCloud"))
-					Expect(config.FromSecret).To(BeTrue())
-					Expect(config.UseInstanceMetadata).To(BeFalse())
-				})
-			})
-			When("config merge type is merge", func() {
-				It("should return file config", func() {
-					fileLoaderConfig := &FileLoaderConfig{FilePath: "testdata/azure_merge.json"}
-					config, err := Load[TestConfig](context.Background(), secretLoaderConfig, fileLoaderConfig)
-					Expect(err).To(BeNil())
-					Expect(config).NotTo(BeNil())
-					Expect(*config.Cloud).To(Equal("AzureCloud"))
-					Expect(config.FromSecret).To(BeTrue())
-					Expect(config.UseInstanceMetadata).To(BeTrue())
-				})
-			})
-		})
-	})
-	When("only secret config is provided", func() {
-		var secretLoaderConfig *K8sSecretLoaderConfig
-		BeforeEach(func() {
-			secretLoaderConfig = &K8sSecretLoaderConfig{
-				K8sSecretConfig: K8sSecretConfig{
-					SecretName:      "azure-cloud-provider",
-					SecretNamespace: "kube-system",
-					CloudConfigKey:  "cloud-config",
-				},
-				KubeClient: fakeKubeClient,
-			}
-		})
-		It("should return secret config", func() {
-			config, err := Load[TestConfig](context.Background(), secretLoaderConfig, nil)
-			Expect(err).To(BeNil())
-			Expect(config).NotTo(BeNil())
-			Expect(*config.Cloud).To(Equal("AzureCloud"))
-			Expect(config.FromSecret).To(BeTrue())
-			Expect(config.UseInstanceMetadata).To(BeFalse())
-		})
-	})
-
-})
diff --git a/pkg/azclient/configloader/loader_empty_test.go b/pkg/azclient/configloader/loader_empty_test.go
deleted file mode 100644
index 79beb19ea..000000000
--- a/pkg/azclient/configloader/loader_empty_test.go
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
-Copyright 2023 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 configloader
-
-import (
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-)
-
-var _ = Describe("LoaderEmpty", func() {
-	When("default config is not provided", func() {
-		It("should return nil config", func() {
-			loader := newEmptyLoader[TestConfig](nil)
-			config, err := loader.Load(nil)
-			Expect(err).To(BeNil())
-			Expect(config).NotTo(BeNil())
-			Expect(config.Value).To(BeNil())
-		})
-	})
-
-	When("default config is provided", func() {
-		It("should return default config", func() {
-			var value string = "default"
-			loader := newEmptyLoader[TestConfig](&TestConfig{Value: to.Ptr(value)})
-			config, err := loader.Load(nil)
-			Expect(err).To(BeNil())
-			Expect(config).NotTo(BeNil())
-			Expect(*config.Value).To(Equal(value))
-		})
-	})
-})
diff --git a/pkg/azclient/configloader/loader_file_test.go b/pkg/azclient/configloader/loader_file_test.go
deleted file mode 100644
index 9b25ab3f8..000000000
--- a/pkg/azclient/configloader/loader_file_test.go
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
-Copyright 2023 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 configloader
-
-import (
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-)
-
-var _ = Describe("LoaderFile", func() {
-	When("invalid config content is provided", func() {
-		It("should return error", func() {
-			loader := newFileLoader[TestConfig]("error", nil, newYamlByteLoader[TestConfig])
-			config, err := loader.Load(nil)
-			Expect(err).NotTo(BeNil())
-			Expect(config).To(BeNil())
-		})
-	})
-	When("valid config content is provided", func() {
-		It("should return error", func() {
-			loader := newFileLoader[TestConfig]("testdata/azure.json", nil, newYamlByteLoader[TestConfig])
-			config, err := loader.Load(nil)
-			Expect(err).To(BeNil())
-			Expect(config).NotTo(BeNil())
-			Expect(*config.Cloud).To(Equal("AzurePublicCloud"))
-		})
-	})
-})
diff --git a/pkg/azclient/configloader/loader_yaml_test.go b/pkg/azclient/configloader/loader_yaml_test.go
deleted file mode 100644
index f5ef270ca..000000000
--- a/pkg/azclient/configloader/loader_yaml_test.go
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
-Copyright 2023 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 configloader
-
-import (
-	"context"
-
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-)
-
-var _ = Describe("LoaderYaml", func() {
-	When("invalid config content is provided", func() {
-		It("should return error", func() {
-			loader := newYamlByteLoader[TestConfig]([]byte("error"), nil)
-			config, err := loader.Load(context.Background())
-			Expect(err).NotTo(BeNil())
-			Expect(config).To(BeNil())
-		})
-	})
-	When("valid config content is provided", func() {
-		It("should return error", func() {
-			loader := newYamlByteLoader[TestConfig]([]byte(`{"value":"123"}`), nil)
-			config, err := loader.Load(context.Background())
-			Expect(err).To(BeNil())
-			Expect(config).NotTo(BeNil())
-			Expect(*config.Value).To(Equal("123"))
-		})
-	})
-	When("valid config content is provided with default config", func() {
-		It("should return error", func() {
-			loader := newYamlByteLoader[TestConfig]([]byte(`{"value":"123"}`), newFileLoader[TestConfig]("wrong", nil, newYamlByteLoader[TestConfig]))
-			config, err := loader.Load(context.Background())
-			Expect(err).NotTo(BeNil())
-			Expect(config).To(BeNil())
-		})
-	})
-})
diff --git a/pkg/azclient/configloader/testdata/azure_merge.json b/pkg/azclient/configloader/testdata/azure_merge.json
deleted file mode 100644
index 82dbcc393..000000000
--- a/pkg/azclient/configloader/testdata/azure_merge.json
+++ /dev/null
@@ -1,33 +0,0 @@
-{
-    "cloud": "AzurePublicCloud",
-    "tenantId": "<tenant-id>",
-    "subscriptionId": "<subscription-id>",
-    "aadClientId": "<client-id>",
-    "aadClientSecret": "<client-secret>",
-    "resourceGroup": "<resource-group-name>",
-    "location": "<location>",
-    "vmType": "<vm-type>",
-    "subnetName": "<subnet-name>",
-    "securityGroupName": "<security-group-name>",
-    "vnetName": "<vnet-name>",
-    "vnetResourceGroup": "<vnet-resource-group>",
-    "routeTableName": "<route-table-name>",
-    "primaryAvailabilitySetName": "<primary-as-name>",
-    "primaryScaleSetName": "<primary-ss-name>",
-    "cloudProviderBackoff": true,
-    "cloudProviderBackoffRetries": 6,
-    "cloudProviderBackoffExponent": 1.5,
-    "cloudProviderBackoffDuration": 5,
-    "cloudProviderBackoffJitter": 1,
-    "cloudProviderRatelimit": true,
-    "cloudProviderRateLimitQPS": 6,
-    "cloudProviderRateLimitBucket": 20,
-    "useManagedIdentityExtension": false,
-    "userAssignedIdentityID": "",
-    "cloudConfigType":"merge",
-    "useInstanceMetadata": true,
-    "loadBalancerSku": "<loadbalancer-sku>",
-    "excludeMasterFromStandardLB": false,
-    "maximumLoadBalancerRuleCount": 250,
-    "tags": "a=b,c=d"
-}
\ No newline at end of file
diff --git a/pkg/azclient/configloader/testdata/azure_secret.json b/pkg/azclient/configloader/testdata/azure_secret.json
deleted file mode 100644
index dce205399..000000000
--- a/pkg/azclient/configloader/testdata/azure_secret.json
+++ /dev/null
@@ -1,33 +0,0 @@
-{
-    "cloud": "AzurePublicCloud",
-    "tenantId": "<tenant-id>",
-    "subscriptionId": "<subscription-id>",
-    "aadClientId": "<client-id>",
-    "aadClientSecret": "<client-secret>",
-    "resourceGroup": "<resource-group-name>",
-    "location": "<location>",
-    "vmType": "<vm-type>",
-    "subnetName": "<subnet-name>",
-    "securityGroupName": "<security-group-name>",
-    "vnetName": "<vnet-name>",
-    "vnetResourceGroup": "<vnet-resource-group>",
-    "routeTableName": "<route-table-name>",
-    "primaryAvailabilitySetName": "<primary-as-name>",
-    "primaryScaleSetName": "<primary-ss-name>",
-    "cloudProviderBackoff": true,
-    "cloudProviderBackoffRetries": 6,
-    "cloudProviderBackoffExponent": 1.5,
-    "cloudProviderBackoffDuration": 5,
-    "cloudProviderBackoffJitter": 1,
-    "cloudProviderRatelimit": true,
-    "cloudProviderRateLimitQPS": 6,
-    "cloudProviderRateLimitBucket": 20,
-    "useManagedIdentityExtension": false,
-    "userAssignedIdentityID": "",
-    "cloudConfigType":"secret",
-    "useInstanceMetadata": true,
-    "loadBalancerSku": "<loadbalancer-sku>",
-    "excludeMasterFromStandardLB": false,
-    "maximumLoadBalancerRuleCount": 250,
-    "tags": "a=b,c=d"
-}
\ No newline at end of file
diff --git a/pkg/provider/azure_controller_common_test.go b/pkg/provider/azure_controller_common_test.go
index dc7ff9821..01509f0af 100644
--- a/pkg/provider/azure_controller_common_test.go
+++ b/pkg/provider/azure_controller_common_test.go
@@ -294,7 +294,7 @@ func TestCommonAttachDisk(t *testing.T) {
 				}()
 			}
 
-			lun, err := testCloud.AttachDisk(ctx, true, test.diskName, diskURI, tt.nodeName, compute.CachingTypesReadOnly, tt.existedDisk)
+			lun, err := testCloud.AttachDisk(ctx, true, test.diskName, diskURI, tt.nodeName, compute.CachingTypesReadOnly, tt.existedDisk, nil)
 
 			assert.Equal(t, tt.expectedLun, lun, "TestCase[%d]: %s", i, tt.desc)
 			assert.Equal(t, tt.expectErr, err != nil, "TestCase[%d]: %s, return error: %v", i, tt.desc, err)
@@ -515,7 +515,7 @@ func TestCommonAttachDiskWithVMSS(t *testing.T) {
 			mockVMsClient.EXPECT().Update(gomock.Any(), testCloud.ResourceGroup, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
 		}
 
-		lun, err := common.AttachDisk(ctx, true, "test", diskURI, test.nodeName, compute.CachingTypesReadOnly, test.existedDisk)
+		lun, err := common.AttachDisk(ctx, true, "test", diskURI, test.nodeName, compute.CachingTypesReadOnly, test.existedDisk, nil)
 		assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc)
 		assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s, return error: %v", i, test.desc, err)
 	}
@@ -711,17 +711,28 @@ func TestSetDiskLun(t *testing.T) {
 		nodeName        string
 		diskURI         string
 		diskMap         map[string]*AttachDiskOptions
+		occupiedLuns    []int
 		isDataDisksFull bool
 		expectedErr     bool
 		expectedLun     int32
 	}{
 		{
-			desc:        "the minimal LUN shall be returned if there's enough room for extra disks",
-			nodeName:    "nodeName",
-			diskURI:     "diskURI",
-			diskMap:     map[string]*AttachDiskOptions{"diskURI": {}},
-			expectedLun: 3,
-			expectedErr: false,
+			desc:         "the minimal LUN shall be returned if there's enough room for extra disks",
+			nodeName:     "nodeName",
+			diskURI:      "diskURI",
+			occupiedLuns: []int{0, 1, 2},
+			diskMap:      map[string]*AttachDiskOptions{"diskURI": {}},
+			expectedLun:  3,
+			expectedErr:  false,
+		},
+		{
+			desc:         "occupied LUNs shall be skipped",
+			nodeName:     "nodeName",
+			diskURI:      "diskURI",
+			occupiedLuns: []int{0, 1, 2, 3},
+			diskMap:      map[string]*AttachDiskOptions{"diskURI": {}},
+			expectedLun:  4,
+			expectedErr:  false,
 		},
 		{
 			desc:            "LUN -1 and error shall be returned if there's no available LUN",
@@ -755,7 +766,7 @@ func TestSetDiskLun(t *testing.T) {
 			mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, *vm.Name, gomock.Any()).Return(vm, nil).AnyTimes()
 		}
 
-		lun, err := common.SetDiskLun(types.NodeName(test.nodeName), test.diskURI, test.diskMap)
+		lun, err := common.SetDiskLun(types.NodeName(test.nodeName), test.diskURI, test.diskMap, test.occupiedLuns)
 		assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc)
 		assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s", i, test.desc)
 	}