fix: return error when GetServiceProperties in account search

fix
k8s-infra-cherrypick-robot · Jul 12, 2024 · 4a31ecf · 4a31ecf
1 parent aec602a
commit 4a31ecf
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 24 deletions.
diff --git a/pkg/provider/azure_storageaccount.go b/pkg/provider/azure_storageaccount.go
@@ -120,13 +120,32 @@ func (az *Cloud) getStorageAccounts(ctx context.Context, accountOptions *Account
 				isRequireInfrastructureEncryptionEqual(acct, accountOptions) &&
 				isAllowSharedKeyAccessEqual(acct, accountOptions) &&
 				isAccessTierEqual(acct, accountOptions) &&
-				az.isMultichannelEnabledEqual(ctx, acct, accountOptions) &&
-				az.isDisableFileServiceDeleteRetentionPolicyEqual(ctx, acct, accountOptions) &&
-				az.isEnableBlobDataProtectionEqual(ctx, acct, accountOptions) &&
 				isPrivateEndpointAsExpected(acct, accountOptions)) {
 				continue
 			}
 
+			equal, err := az.isMultichannelEnabledEqual(ctx, acct, accountOptions)
+			if err != nil {
+				return nil, err
+			}
+			if !equal {
+				continue
+			}
+
+			if equal, err = az.isDisableFileServiceDeleteRetentionPolicyEqual(ctx, acct, accountOptions); err != nil {
+				return nil, err
+			}
+			if !equal {
+				continue
+			}
+
+			if equal, err = az.isEnableBlobDataProtectionEqual(ctx, acct, accountOptions); err != nil {
+				return nil, err
+			}
+			if !equal {
+				continue
+			}
+
 			accounts = append(accounts, accountWithLocation{Name: *acct.Name, StorageType: string((*acct.Sku).Name), Location: *acct.Location})
 			if !accountOptions.PickRandomMatchingAccount {
 				// return the first matching account if it's not required to pick a random one
@@ -933,74 +952,71 @@ func isAccessTierEqual(account storage.Account, accountOptions *AccountOptions)
 	return accountOptions.AccessTier == string(account.AccessTier)
 }
 
-func (az *Cloud) isMultichannelEnabledEqual(ctx context.Context, account storage.Account, accountOptions *AccountOptions) bool {
+func (az *Cloud) isMultichannelEnabledEqual(ctx context.Context, account storage.Account, accountOptions *AccountOptions) (bool, error) {
 	if accountOptions.IsMultichannelEnabled == nil {
-		return true
+		return true, nil
 	}
 
 	if account.Name == nil {
 		klog.Warningf("account.Name under resource group(%s) is nil", accountOptions.ResourceGroup)
-		return false
+		return false, nil
 	}
 
 	prop, err := az.getFileServicePropertiesCache(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, *account.Name)
 	if err != nil {
-		klog.Warningf("GetServiceProperties(%s) under resource group(%s) failed with %v", *account.Name, accountOptions.ResourceGroup, err)
-		return false
+		return false, err
 	}
 
 	if prop.FileServicePropertiesProperties == nil ||
 		prop.FileServicePropertiesProperties.ProtocolSettings == nil ||
 		prop.FileServicePropertiesProperties.ProtocolSettings.Smb == nil ||
 		prop.FileServicePropertiesProperties.ProtocolSettings.Smb.Multichannel == nil {
-		return !*accountOptions.IsMultichannelEnabled
+		return !*accountOptions.IsMultichannelEnabled, nil
 	}
 
-	return *accountOptions.IsMultichannelEnabled == pointer.BoolDeref(prop.FileServicePropertiesProperties.ProtocolSettings.Smb.Multichannel.Enabled, false)
+	return *accountOptions.IsMultichannelEnabled == pointer.BoolDeref(prop.FileServicePropertiesProperties.ProtocolSettings.Smb.Multichannel.Enabled, false), nil
 }
 
-func (az *Cloud) isDisableFileServiceDeleteRetentionPolicyEqual(ctx context.Context, account storage.Account, accountOptions *AccountOptions) bool {
+func (az *Cloud) isDisableFileServiceDeleteRetentionPolicyEqual(ctx context.Context, account storage.Account, accountOptions *AccountOptions) (bool, error) {
 	if accountOptions.DisableFileServiceDeleteRetentionPolicy == nil {
-		return true
+		return true, nil
 	}
 
 	if account.Name == nil {
 		klog.Warningf("account.Name under resource group(%s) is nil", accountOptions.ResourceGroup)
-		return false
+		return false, nil
 	}
 
 	prop, err := az.FileClient.WithSubscriptionID(accountOptions.SubscriptionID).GetServiceProperties(ctx, accountOptions.ResourceGroup, *account.Name)
 	if err != nil {
-		klog.Warningf("GetServiceProperties(%s) under resource group(%s) failed with %v", *account.Name, accountOptions.ResourceGroup, err)
-		return false
+		return false, err
 	}
 
 	if prop.FileServicePropertiesProperties == nil ||
 		prop.FileServicePropertiesProperties.ShareDeleteRetentionPolicy == nil ||
 		prop.FileServicePropertiesProperties.ShareDeleteRetentionPolicy.Enabled == nil {
 		// by default, ShareDeleteRetentionPolicy.Enabled is true if it's nil
-		return !*accountOptions.DisableFileServiceDeleteRetentionPolicy
+		return !*accountOptions.DisableFileServiceDeleteRetentionPolicy, nil
 	}
 
-	return *accountOptions.DisableFileServiceDeleteRetentionPolicy != *prop.FileServicePropertiesProperties.ShareDeleteRetentionPolicy.Enabled
+	return *accountOptions.DisableFileServiceDeleteRetentionPolicy != *prop.FileServicePropertiesProperties.ShareDeleteRetentionPolicy.Enabled, nil
 }
 
-func (az *Cloud) isEnableBlobDataProtectionEqual(ctx context.Context, account storage.Account, accountOptions *AccountOptions) bool {
+func (az *Cloud) isEnableBlobDataProtectionEqual(ctx context.Context, account storage.Account, accountOptions *AccountOptions) (bool, error) {
 	if accountOptions.SoftDeleteBlobs == 0 &&
 		accountOptions.SoftDeleteContainers == 0 &&
 		accountOptions.EnableBlobVersioning == nil {
-		return true
+		return true, nil
 	}
 
 	property, err := az.BlobClient.GetServiceProperties(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, *account.Name)
 	if err != nil {
-		klog.Warningf("GetServiceProperties failed for account %s, err: %v", *account.Name, err)
-		return false
+		return false, err
 	}
 
 	return isSoftDeleteBlobsEqual(property, accountOptions) &&
 		isSoftDeleteContainersEqual(property, accountOptions) &&
-		isEnableBlobVersioningEqual(property, accountOptions)
+		isEnableBlobVersioningEqual(property, accountOptions), nil
 }
 
 func isSoftDeleteBlobsEqual(property storage.BlobServiceProperties, accountOptions *AccountOptions) bool {

diff --git a/pkg/provider/azure_storageaccount_test.go b/pkg/provider/azure_storageaccount_test.go
@@ -1692,7 +1692,7 @@ func TestIsMultichannelEnabledEqual(t *testing.T) {
 			mockFileClient.EXPECT().GetServiceProperties(gomock.Any(), gomock.Any(), gomock.Any()).Return(*test.serviceProperties, test.servicePropertiesRetError).Times(1)
 		}
 
-		result := cloud.isMultichannelEnabledEqual(ctx, test.account, test.accountOptions)
+		result, _ := cloud.isMultichannelEnabledEqual(ctx, test.account, test.accountOptions)
 		assert.Equal(t, test.expectedResult, result, test.desc)
 	}
 }
@@ -1837,7 +1837,7 @@ func TestIsDisableFileServiceDeleteRetentionPolicyEqual(t *testing.T) {
 			mockFileClient.EXPECT().GetServiceProperties(gomock.Any(), gomock.Any(), gomock.Any()).Return(*test.serviceProperties, test.servicePropertiesRetError).Times(1)
 		}
 
-		result := cloud.isDisableFileServiceDeleteRetentionPolicyEqual(ctx, test.account, test.accountOptions)
+		result, _ := cloud.isDisableFileServiceDeleteRetentionPolicyEqual(ctx, test.account, test.accountOptions)
 		assert.Equal(t, test.expectedResult, result, test.desc)
 	}
 }