Skip to content

Commit

Permalink
fix: return error when GetServiceProperties in account search
Browse files Browse the repository at this point in the history
fix
  • Loading branch information
andyzhangx authored and k8s-infra-cherrypick-robot committed Jul 12, 2024
1 parent c919111 commit ff8a243
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 24 deletions.
60 changes: 38 additions & 22 deletions pkg/provider/azure_storageaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,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
Expand Down Expand Up @@ -930,74 +949,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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/provider/azure_storageaccount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down

0 comments on commit ff8a243

Please sign in to comment.