From 0b19e4f09a04af09c67bab37d9807d69ced39256 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 27 Jun 2021 13:13:57 +0000 Subject: [PATCH] fix: reduce GetDisk in AttachDisk fix ut --- go.mod | 2 +- go.sum | 4 +- pkg/azuredisk/azuredisk.go | 17 +++--- pkg/azuredisk/azuredisk_test.go | 2 +- pkg/azuredisk/azuredisk_v1_test.go | 2 +- pkg/azuredisk/azuredisk_v2.go | 13 ++--- pkg/azuredisk/controllerserver.go | 10 ++-- pkg/azuredisk/controllerserver_v2.go | 9 ++-- pkg/azuredisk/fake_azuredisk.go | 3 +- vendor/modules.txt | 4 +- .../pkg/auth/azure_auth.go | 14 +++-- .../cloud-provider-azure/pkg/consts/consts.go | 7 +++ .../pkg/provider/azure.go | 52 ++++++++++++------- .../pkg/provider/azure_controller_common.go | 39 ++++---------- .../pkg/provider/azure_routes.go | 37 ++++++++++++- .../pkg/provider/azure_storageaccount.go | 1 + .../pkg/provider/azure_utils.go | 32 +++++++++--- 17 files changed, 156 insertions(+), 92 deletions(-) diff --git a/go.mod b/go.mod index 3fc1c1b2a5..cff1a3b360 100644 --- a/go.mod +++ b/go.mod @@ -68,5 +68,5 @@ replace ( k8s.io/sample-cli-plugin => k8s.io/sample-cli-plugin v0.21.0 k8s.io/sample-controller => k8s.io/sample-controller v0.21.0 - sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d + sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a ) diff --git a/go.sum b/go.sum index cd4da1376b..975c45396f 100644 --- a/go.sum +++ b/go.sum @@ -1206,8 +1206,8 @@ rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.15 h1:4uqm9Mv+w2MmBYD+F4qf/v6tDFUdPOk29C095RbU5mY= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.15/go.mod h1:LEScyzhFmoF5pso/YSeBstl57mOzx9xlU9n85RGrDQg= -sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d h1:4rGP3LN7nL6cVClNAWZzkFdhXoj6kTK3XD+ZXMgZhBQ= -sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d/go.mod h1:Y5dSIj1lXNzGSRePDD/WpB0uZb1aCwdoHew+Oqt6M90= +sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a h1:CNyZKx+bEZe8Y4s1w6yazvMFdLZfyDGyu5HfZCqADFE= +sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a/go.mod h1:Y5dSIj1lXNzGSRePDD/WpB0uZb1aCwdoHew+Oqt6M90= sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU= sigs.k8s.io/kustomize/api v0.8.5/go.mod h1:M377apnKT5ZHJS++6H4rQoCHmWtt6qTpp3mbe7p6OLY= sigs.k8s.io/kustomize/cmd/config v0.9.7/go.mod h1:MvXCpHs77cfyxRmCNUQjIqCmZyYsbn5PyQpWiq44nW0= diff --git a/pkg/azuredisk/azuredisk.go b/pkg/azuredisk/azuredisk.go index f78a60ffad..94e88241d8 100644 --- a/pkg/azuredisk/azuredisk.go +++ b/pkg/azuredisk/azuredisk.go @@ -259,32 +259,33 @@ func (d *Driver) isGetDiskThrottled() bool { return cache != nil } -func (d *Driver) checkDiskExists(ctx context.Context, diskURI string) error { +func (d *Driver) checkDiskExists(ctx context.Context, diskURI string) (*compute.Disk, error) { diskName, err := GetDiskName(diskURI) if err != nil { - return err + return nil, err } resourceGroup, err := GetResourceGroupFromURI(diskURI) if err != nil { - return err + return nil, err } if d.isGetDiskThrottled() { klog.Warningf("skip checkDiskExists(%s) since it's still in throttling", diskURI) - return nil + return nil, nil } - if _, rerr := d.cloud.DisksClient.Get(ctx, resourceGroup, diskName); rerr != nil { + disk, rerr := d.cloud.DisksClient.Get(ctx, resourceGroup, diskName) + if rerr != nil { if strings.Contains(rerr.RawError.Error(), rateLimited) { klog.Warningf("checkDiskExists(%s) is throttled with error: %v", diskURI, rerr.Error()) d.getDiskThrottlingCache.Set(throttlingKey, "") - return nil + return &disk, nil } - return rerr.Error() + return nil, rerr.Error() } - return nil + return &disk, nil } func (d *Driver) checkDiskCapacity(ctx context.Context, resourceGroup, diskName string, requestGiB int) (bool, error) { diff --git a/pkg/azuredisk/azuredisk_test.go b/pkg/azuredisk/azuredisk_test.go index c38c48135a..692d4718d8 100644 --- a/pkg/azuredisk/azuredisk_test.go +++ b/pkg/azuredisk/azuredisk_test.go @@ -695,6 +695,6 @@ func TestRun(t *testing.T) { func TestDriver_checkDiskExists(t *testing.T) { d, _ := NewFakeDriver(t) - err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/providers/Microsoft.Compute/disks/name") + _, err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/providers/Microsoft.Compute/disks/name") assert.NotEqual(t, err, nil) } diff --git a/pkg/azuredisk/azuredisk_v1_test.go b/pkg/azuredisk/azuredisk_v1_test.go index 5f3c5c32be..a2ff382a01 100644 --- a/pkg/azuredisk/azuredisk_v1_test.go +++ b/pkg/azuredisk/azuredisk_v1_test.go @@ -48,6 +48,6 @@ func TestCheckDiskCapacity_V1(t *testing.T) { func TestDriver_checkDiskExists_V1(t *testing.T) { d, _ := NewFakeDriver(t) d.setDiskThrottlingCache(throttlingKey, "") - err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/resourceGroups/23/providers/Microsoft.Compute/disks/name") + _, err := d.checkDiskExists(context.TODO(), "testurl/subscriptions/12/resourceGroups/23/providers/Microsoft.Compute/disks/name") assert.Equal(t, err, nil) } diff --git a/pkg/azuredisk/azuredisk_v2.go b/pkg/azuredisk/azuredisk_v2.go index 027b35bfda..1d94103ccd 100644 --- a/pkg/azuredisk/azuredisk_v2.go +++ b/pkg/azuredisk/azuredisk_v2.go @@ -135,22 +135,23 @@ func (d *DriverV2) Run(endpoint, kubeconfig string, disableAVSetNodes, testingMo s.Wait() } -func (d *DriverV2) checkDiskExists(ctx context.Context, diskURI string) error { +func (d *DriverV2) checkDiskExists(ctx context.Context, diskURI string) (*compute.Disk, error) { diskName, err := GetDiskName(diskURI) if err != nil { - return err + return nil, err } resourceGroup, err := GetResourceGroupFromURI(diskURI) if err != nil { - return err + return nil, err } - if _, rerr := d.cloud.DisksClient.Get(ctx, resourceGroup, diskName); rerr != nil { - return rerr.Error() + disk, rerr := d.cloud.DisksClient.Get(ctx, resourceGroup, diskName) + if rerr != nil { + return nil, rerr.Error() } - return nil + return &disk, nil } func (d *DriverV2) checkDiskCapacity(ctx context.Context, resourceGroup, diskName string, requestGiB int) (bool, error) { diff --git a/pkg/azuredisk/controllerserver.go b/pkg/azuredisk/controllerserver.go index 5de7479219..72665f0ed4 100644 --- a/pkg/azuredisk/controllerserver.go +++ b/pkg/azuredisk/controllerserver.go @@ -414,7 +414,8 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle return nil, status.Error(codes.InvalidArgument, "Volume capability not supported") } - if err := d.checkDiskExists(ctx, diskURI); err != nil { + disk, err := d.checkDiskExists(ctx, diskURI) + if err != nil { return nil, status.Error(codes.NotFound, fmt.Sprintf("Volume not found, failed with error: %v", err)) } @@ -458,7 +459,7 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle } klog.V(2).Infof("Trying to attach volume %q to node %q", diskURI, nodeName) - lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode) + lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode, disk) if err == nil { klog.V(2).Infof("Attach operation successful: volume %q attached to node %q.", diskURI, nodeName) } else { @@ -473,7 +474,7 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle return nil, status.Errorf(codes.Internal, "Could not detach volume %q from node %q: %v", diskURI, derr.CurrentNode, err) } klog.V(2).Infof("Trying to attach volume %q to node %q again", diskURI, nodeName) - lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode) + lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode, disk) } if err != nil { klog.Errorf("Attach volume %q to instance %q failed with %v", diskURI, nodeName, err) @@ -537,8 +538,7 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida return nil, status.Error(codes.InvalidArgument, "Volume capabilities missing in request") } - err := d.checkDiskExists(ctx, diskURI) - if err != nil { + if _, err := d.checkDiskExists(ctx, diskURI); err != nil { return nil, status.Error(codes.NotFound, fmt.Sprintf("Volume not found, failed with error: %v", err)) } diff --git a/pkg/azuredisk/controllerserver_v2.go b/pkg/azuredisk/controllerserver_v2.go index 6af3644670..35adaed73a 100644 --- a/pkg/azuredisk/controllerserver_v2.go +++ b/pkg/azuredisk/controllerserver_v2.go @@ -351,7 +351,7 @@ func (d *DriverV2) ControllerPublishVolume(ctx context.Context, req *csi.Control return nil, status.Error(codes.InvalidArgument, "Volume capability not supported") } - err := d.checkDiskExists(ctx, diskURI) + disk, err := d.checkDiskExists(ctx, diskURI) if err != nil { return nil, status.Error(codes.NotFound, fmt.Sprintf("Volume not found, failed with error: %v", err)) } @@ -396,7 +396,7 @@ func (d *DriverV2) ControllerPublishVolume(ctx context.Context, req *csi.Control } klog.V(2).Infof("Trying to attach volume %q to node %q", diskURI, nodeName) - lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode) + lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode, disk) if err == nil { klog.V(2).Infof("Attach operation successful: volume %q attached to node %q.", diskURI, nodeName) } else { @@ -406,7 +406,7 @@ func (d *DriverV2) ControllerPublishVolume(ctx context.Context, req *csi.Control return nil, status.Errorf(codes.Internal, "Could not detach volume %q from node %q: %v", diskURI, derr.CurrentNode, err) } klog.V(2).Infof("Trying to attach volume %q to node %q again", diskURI, nodeName) - lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode) + lun, err = d.cloud.AttachDisk(true, diskName, diskURI, nodeName, cachingMode, disk) } if err != nil { klog.Errorf("Attach volume %q to instance %q failed with %v", diskURI, nodeName, err) @@ -470,8 +470,7 @@ func (d *DriverV2) ValidateVolumeCapabilities(ctx context.Context, req *csi.Vali return nil, status.Error(codes.InvalidArgument, "Volume capabilities missing in request") } - err := d.checkDiskExists(ctx, diskURI) - if err != nil { + if _, err := d.checkDiskExists(ctx, diskURI); err != nil { return nil, status.Error(codes.NotFound, fmt.Sprintf("Volume not found, failed with error: %v", err)) } diff --git a/pkg/azuredisk/fake_azuredisk.go b/pkg/azuredisk/fake_azuredisk.go index 7cce34b3e6..c220dc7393 100644 --- a/pkg/azuredisk/fake_azuredisk.go +++ b/pkg/azuredisk/fake_azuredisk.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-12-01/compute" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -77,7 +78,7 @@ type FakeDriver interface { setMounter(*mount.SafeFormatAndMount) checkDiskCapacity(context.Context, string, string, int) (bool, error) - checkDiskExists(ctx context.Context, diskURI string) error + checkDiskExists(ctx context.Context, diskURI string) (*compute.Disk, error) getSnapshotInfo(string) (string, string, error) getSnapshotByID(context.Context, string, string, string) (*csi.Snapshot, error) ensureMountPoint(string) (bool, error) diff --git a/vendor/modules.txt b/vendor/modules.txt index 6c9b8fa886..32b167233a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -867,7 +867,7 @@ k8s.io/utils/trace # sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.15 sigs.k8s.io/apiserver-network-proxy/konnectivity-client/pkg/client sigs.k8s.io/apiserver-network-proxy/konnectivity-client/proto/client -# sigs.k8s.io/cloud-provider-azure v0.7.4 => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d +# sigs.k8s.io/cloud-provider-azure v0.7.4 => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a ## explicit sigs.k8s.io/cloud-provider-azure/pkg/auth sigs.k8s.io/cloud-provider-azure/pkg/azureclients @@ -947,4 +947,4 @@ sigs.k8s.io/yaml # k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.21.0 # k8s.io/sample-cli-plugin => k8s.io/sample-cli-plugin v0.21.0 # k8s.io/sample-controller => k8s.io/sample-controller v0.21.0 -# sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210611045613-f7491146d89d +# sigs.k8s.io/cloud-provider-azure => sigs.k8s.io/cloud-provider-azure v0.7.4-0.20210627125724-fb498b8d847a diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/auth/azure_auth.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/auth/azure_auth.go index 785696867a..2dfa5d3fe2 100644 --- a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/auth/azure_auth.go +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/auth/azure_auth.go @@ -80,7 +80,7 @@ type AzureAuthConfig struct { // If NetworkResourceTenantID and NetworkResourceSubscriptionID are specified to have different values than TenantID and SubscriptionID, network resources are deployed in different AAD Tenant and Subscription than those for the cluster, // than only azure clients except VM/VMSS and network resource ones use this method to fetch Token. // For tokens for VM/VMSS and network resource ones, please check GetMultiTenantServicePrincipalToken and GetNetworkResourceServicePrincipalToken. -func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) (*adal.ServicePrincipalToken, error) { +func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment, resource string) (*adal.ServicePrincipalToken, error) { var tenantID string if strings.EqualFold(config.IdentitySystem, consts.ADFSIdentitySystem) { tenantID = consts.ADFSIdentitySystem @@ -88,6 +88,10 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) ( tenantID = config.TenantID } + if resource == "" { + resource = env.ServiceManagementEndpoint + } + if config.UseManagedIdentityExtension { klog.V(2).Infoln("azure: using managed identity extension to retrieve access token") msiEndpoint, err := adal.GetMSIVMEndpoint() @@ -97,13 +101,13 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) ( if len(config.UserAssignedIdentityID) > 0 { klog.V(4).Info("azure: using User Assigned MSI ID to retrieve access token") return adal.NewServicePrincipalTokenFromMSIWithUserAssignedID(msiEndpoint, - env.ServiceManagementEndpoint, + resource, config.UserAssignedIdentityID) } klog.V(4).Info("azure: using System Assigned MSI to retrieve access token") return adal.NewServicePrincipalTokenFromMSI( msiEndpoint, - env.ServiceManagementEndpoint) + resource) } oauthConfig, err := adal.NewOAuthConfigWithAPIVersion(env.ActiveDirectoryEndpoint, tenantID, nil) @@ -117,7 +121,7 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) ( *oauthConfig, config.AADClientID, config.AADClientSecret, - env.ServiceManagementEndpoint) + resource) } if len(config.AADClientCertPath) > 0 && len(config.AADClientCertPassword) > 0 { @@ -135,7 +139,7 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) ( config.AADClientID, certificate, privateKey, - env.ServiceManagementEndpoint) + resource) } return nil, ErrorNoAuth diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/consts/consts.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/consts/consts.go index ecd02d4ed5..af1f983bff 100644 --- a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/consts/consts.go +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/consts/consts.go @@ -340,3 +340,10 @@ const ( RouteNameFmt = "%s____%s" RouteNameSeparator = "____" ) + +// cloud provider config secret +const ( + DefaultCloudProviderConfigSecName = "azure-cloud-provider" + DefaultCloudProviderConfigSecNamespace = "kube-system" + DefaultCloudProviderConfigSecKey = "cloud-config" +) diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure.go index 7cc69b6528..166b903be3 100644 --- a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure.go +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure.go @@ -330,8 +330,8 @@ func init() { } // NewCloud returns a Cloud with initialized clients -func NewCloud(configReader io.Reader, syncZones bool) (cloudprovider.Interface, error) { - az, err := NewCloudWithoutFeatureGates(configReader, syncZones) +func NewCloud(configReader io.Reader, callFromCCM bool) (cloudprovider.Interface, error) { + az, err := NewCloudWithoutFeatureGates(configReader, callFromCCM) if err != nil { return nil, err } @@ -340,7 +340,7 @@ func NewCloud(configReader io.Reader, syncZones bool) (cloudprovider.Interface, return az, nil } -func NewCloudFromConfigFile(configFilePath string, syncZones bool) (cloudprovider.Interface, error) { +func NewCloudFromConfigFile(configFilePath string, calFromCCM bool) (cloudprovider.Interface, error) { var ( cloud cloudprovider.Interface err error @@ -355,7 +355,7 @@ func NewCloudFromConfigFile(configFilePath string, syncZones bool) (cloudprovide } defer config.Close() - cloud, err = NewCloud(config, syncZones) + cloud, err = NewCloud(config, calFromCCM) } else { // Pass explicit nil so plugins can actually check for nil. See // "Why is my nil error value not equal to nil?" in golang.org/doc/faq. @@ -372,6 +372,24 @@ func NewCloudFromConfigFile(configFilePath string, syncZones bool) (cloudprovide return cloud, nil } +func (az *Cloud) configSecretMetadata(secretName, secretNamespace, cloudConfigKey string) { + if secretName == "" { + secretName = consts.DefaultCloudProviderConfigSecName + } + if secretNamespace == "" { + secretNamespace = consts.DefaultCloudProviderConfigSecNamespace + } + if cloudConfigKey == "" { + cloudConfigKey = consts.DefaultCloudProviderConfigSecKey + } + + az.InitSecretConfig = InitSecretConfig{ + SecretName: secretName, + SecretNamespace: secretNamespace, + CloudConfigKey: cloudConfigKey, + } +} + func NewCloudFromSecret(clientBuilder cloudprovider.ControllerClientBuilder, secretName, secretNamespace, cloudConfigKey string) (cloudprovider.Interface, error) { az := &Cloud{ nodeNames: sets.NewString(), @@ -379,13 +397,10 @@ func NewCloudFromSecret(clientBuilder cloudprovider.ControllerClientBuilder, sec nodeResourceGroups: map[string]string{}, unmanagedNodes: sets.NewString(), routeCIDRs: map[string]string{}, - InitSecretConfig: InitSecretConfig{ - SecretName: secretName, - SecretNamespace: secretNamespace, - CloudConfigKey: cloudConfigKey, - }, } + az.configSecretMetadata(secretName, secretNamespace, cloudConfigKey) + az.Initialize(clientBuilder, wait.NeverStop) err := az.InitializeCloudFromSecret() @@ -400,7 +415,7 @@ func NewCloudFromSecret(clientBuilder cloudprovider.ControllerClientBuilder, sec // NewCloudWithoutFeatureGates returns a Cloud without trying to wire the feature gates. This is used by the unit tests // that don't load the actual features being used in the cluster. -func NewCloudWithoutFeatureGates(configReader io.Reader, syncZones bool) (*Cloud, error) { +func NewCloudWithoutFeatureGates(configReader io.Reader, callFromCCM bool) (*Cloud, error) { config, err := parseConfig(configReader) if err != nil { return nil, err @@ -414,7 +429,7 @@ func NewCloudWithoutFeatureGates(configReader io.Reader, syncZones bool) (*Cloud routeCIDRs: map[string]string{}, } - err = az.InitializeCloudFromConfig(config, false, syncZones) + err = az.InitializeCloudFromConfig(config, false, callFromCCM) if err != nil { return nil, err } @@ -423,7 +438,7 @@ func NewCloudWithoutFeatureGates(configReader io.Reader, syncZones bool) (*Cloud } // InitializeCloudFromConfig initializes the Cloud from config. -func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret, syncZones bool) error { +func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret, callFromCCM bool) error { if config == nil { // should not reach here return fmt.Errorf("InitializeCloudFromConfig: cannot initialize from nil config") @@ -468,7 +483,7 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret, syncZones return err } - servicePrincipalToken, err := auth.GetServicePrincipalToken(&config.AzureAuthConfig, env) + servicePrincipalToken, err := auth.GetServicePrincipalToken(&config.AzureAuthConfig, env, env.ServiceManagementEndpoint) if errors.Is(err, auth.ErrorNoAuth) { // Only controller-manager would lazy-initialize from secret, and credentials are required for such case. if fromSecret { @@ -544,15 +559,16 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret, syncZones return err } - // start delayed route updater. - az.routeUpdater = newDelayedRouteUpdater(az, routeUpdateInterval) - go az.routeUpdater.run() + // updating routes and syncing zones only in CCM + if callFromCCM { + // start delayed route updater. + az.routeUpdater = newDelayedRouteUpdater(az, routeUpdateInterval) + go az.routeUpdater.run() - if syncZones { // wait for the success first time of syncing zones err = az.syncRegionZonesMap() if err != nil { - klog.Errorf("InitializeCloudFromConfig: failed eto sync regional zones map for the first time: %s", err.Error()) + klog.Errorf("InitializeCloudFromConfig: failed to sync regional zones map for the first time: %s", err.Error()) return err } diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_controller_common.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_controller_common.go index 8c0142bfcd..154dbed1db 100644 --- a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_controller_common.go +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_controller_common.go @@ -37,7 +37,6 @@ 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/retry" ) const ( @@ -145,25 +144,14 @@ func (c *controllerCommon) getNodeVMSet(nodeName types.NodeName, crt azcache.Azu // AttachDisk attaches a vhd to vm. The vhd must exist, can be identified by diskName, diskURI. // return (lun, error) -func (c *controllerCommon) AttachDisk(isManagedDisk bool, diskName, diskURI string, nodeName types.NodeName, cachingMode compute.CachingTypes) (int32, error) { +func (c *controllerCommon) AttachDisk(isManagedDisk bool, diskName, diskURI string, nodeName types.NodeName, + cachingMode compute.CachingTypes, disk *compute.Disk) (int32, error) { diskEncryptionSetID := "" writeAcceleratorEnabled := false - if isManagedDisk { - diskName := path.Base(diskURI) - resourceGroup, err := getResourceGroupFromDiskURI(diskURI) - if err != nil { - return -1, err - } - - ctx, cancel := getContextWithCancel() - defer cancel() - - disk, rerr := c.cloud.DisksClient.Get(ctx, resourceGroup, diskName) - if rerr != nil { - return -1, rerr.Error() - } - + // there is possibility that disk is nil when GetDisk is throttled + // don't check disk state when GetDisk is throttled + if disk != nil { if disk.ManagedBy != nil && (disk.MaxShares == nil || *disk.MaxShares <= 1) { vmset, err := c.getNodeVMSet(nodeName, azcache.CacheReadTypeUnsafe) if err != nil { @@ -225,8 +213,8 @@ func (c *controllerCommon) AttachDisk(isManagedDisk bool, diskName, diskURI stri writeAcceleratorEnabled: writeAcceleratorEnabled, } node := strings.ToLower(string(nodeName)) - disk := strings.ToLower(diskURI) - if err := c.insertAttachDiskRequest(disk, node, &options); err != nil { + diskuri := strings.ToLower(diskURI) + if err := c.insertAttachDiskRequest(diskuri, node, &options); err != nil { return -1, err } @@ -237,7 +225,7 @@ func (c *controllerCommon) AttachDisk(isManagedDisk bool, diskName, diskURI stri return -1, err } - lun, err := c.SetDiskLun(nodeName, disk, diskMap) + lun, err := c.SetDiskLun(nodeName, diskuri, diskMap) if err != nil { return -1, err } @@ -341,18 +329,11 @@ func (c *controllerCommon) DetachDisk(diskName, diskURI string, nodeName types.N err, diskURI) return nil } - if retry.IsErrorRetriable(err) && c.cloud.CloudProviderBackoff { - klog.Warningf("azureDisk - update backing off: detach disk(%s, %s), err: %w", diskName, diskURI, err) - err = vmset.DetachDisk(nodeName, diskMap) - } + klog.Errorf("azureDisk - detach disk(%s, %s) failed, err: %v", diskName, diskURI, err) + return err } } - if err != nil { - klog.Errorf("azureDisk - detach disk(%s, %s) failed, err: %v", diskName, diskURI, err) - return err - } - klog.V(2).Infof("azureDisk - detach disk(%s, %s) succeeded", diskName, diskURI) return nil } diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_routes.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_routes.go index ea025e6ff9..6b2b32747b 100644 --- a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_routes.go +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_routes.go @@ -146,12 +146,17 @@ func (d *delayedRouteUpdater) updateRoutes() { } // reconcile routes. - dirty := false + dirty, onlyUpdateTags := false, true routes := []network.Route{} if routeTable.RouteTablePropertiesFormat != nil && routeTable.RouteTablePropertiesFormat.Routes != nil { routes = *routeTable.Routes } - onlyUpdateTags := true + + routes, dirty = d.cleanupOutdatedRoutes(routes) + if dirty { + onlyUpdateTags = false + } + for _, rt := range d.routesToUpdate { if rt.operation == routeTableOperationUpdateTags { routeTable.Tags = rt.routeTableTags @@ -204,6 +209,34 @@ func (d *delayedRouteUpdater) updateRoutes() { } } +// cleanupOutdatedRoutes deletes all non-dualstack routes when dualstack is enabled, +// and deletes all dualstack routes when dualstack is not enabled. +func (d *delayedRouteUpdater) cleanupOutdatedRoutes(existingRoutes []network.Route) (routes []network.Route, changed bool) { + for i := len(existingRoutes) - 1; i >= 0; i-- { + existingRouteName := to.String(existingRoutes[i].Name) + split := strings.Split(existingRouteName, consts.RouteNameSeparator) + + // filter out unmanaged routes + deleteRoute := false + if d.az.nodeNames.Has(split[0]) { + if d.az.ipv6DualStackEnabled && len(split) == 1 { + klog.V(2).Infof("cleanupOutdatedRoutes: deleting outdated non-dualstack route %s", existingRouteName) + deleteRoute = true + } else if !d.az.ipv6DualStackEnabled && len(split) == 2 { + klog.V(2).Infof("cleanupOutdatedRoutes: deleting outdated dualstack route %s", existingRouteName) + deleteRoute = true + } + + if deleteRoute { + existingRoutes = append(existingRoutes[:i], existingRoutes[i+1:]...) + changed = true + } + } + } + + return existingRoutes, changed +} + // addRouteOperation adds the routeOperation to delayedRouteUpdater and returns a delayedRouteOperation. func (d *delayedRouteUpdater) addRouteOperation(operation routeOperation, route network.Route) (*delayedRouteOperation, error) { d.lock.Lock() diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_storageaccount.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_storageaccount.go index 7e132af699..e7df8a5c41 100644 --- a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_storageaccount.go +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_storageaccount.go @@ -230,6 +230,7 @@ func (az *Cloud) EnsureStorageAccount(accountOptions *AccountOptions, genAccount NetworkRuleSet: networkRuleSet, IsHnsEnabled: accountOptions.IsHnsEnabled, EnableNfsV3: accountOptions.EnableNfsV3, + MinimumTLSVersion: storage.MinimumTLSVersionTLS12, }, Tags: tags, Location: &location} diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_utils.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_utils.go index 4e4a6c9cba..27ddc3a3cf 100644 --- a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_utils.go +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_utils.go @@ -115,32 +115,52 @@ func parseTags(tags string) map[string]*string { return formatted } +func findKeyInMapCaseInsensitive(targetMap map[string]*string, key string) (bool, string) { + for k := range targetMap { + if strings.EqualFold(k, key) { + return true, k + } + } + + return false, "" +} + func (az *Cloud) reconcileTags(currentTagsOnResource, newTags map[string]*string) (reconciledTags map[string]*string, changed bool) { var systemTags []string - var systemTagsMap sets.String + systemTagsMap := make(map[string]*string) if az.SystemTags != "" { systemTags = strings.Split(az.SystemTags, consts.TagsDelimiter) for i := 0; i < len(systemTags); i++ { systemTags[i] = strings.TrimSpace(systemTags[i]) } - systemTagsMap = sets.NewString(systemTags...) + + for _, systemTag := range systemTags { + systemTagsMap[systemTag] = to.StringPtr("") + } } // if the systemTags is not set, just add/update new currentTagsOnResource and not delete old currentTagsOnResource for k, v := range newTags { - if vv, ok := currentTagsOnResource[k]; !ok || !strings.EqualFold(to.String(v), to.String(vv)) { + found, key := findKeyInMapCaseInsensitive(currentTagsOnResource, k) + + if !found { currentTagsOnResource[k] = v changed = true + } else if !strings.EqualFold(to.String(v), to.String(currentTagsOnResource[key])) { + currentTagsOnResource[key] = v + changed = true } } // if the systemTags is set, delete the old currentTagsOnResource if len(systemTagsMap) > 0 { for k := range currentTagsOnResource { - if _, ok := newTags[k]; !ok && !systemTagsMap.Has(k) { - delete(currentTagsOnResource, k) - changed = true + if _, ok := newTags[k]; !ok { + if found, _ := findKeyInMapCaseInsensitive(systemTagsMap, k); !found { + delete(currentTagsOnResource, k) + changed = true + } } } }