Skip to content

Commit

Permalink
#issue858#StorageControllertest#HandleAsyncError
Browse files Browse the repository at this point in the history
  • Loading branch information
buhongw7583c committed Apr 10, 2020
1 parent d5a30c2 commit 4a89de5
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 17 deletions.
9 changes: 5 additions & 4 deletions controllers/storage_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
config "github.com/Azure/azure-service-operator/pkg/resourcemanager/config"

"github.com/Azure/azure-service-operator/pkg/errhelp"
"github.com/Azure/go-autorest/autorest/to"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -43,7 +44,7 @@ func TestStorageControllerHappyPathWithoutNetworkRule(t *testing.T) {
// delete rg
EnsureDelete(ctx, t, tc, saInstance)
}
func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) {
func TestStorageControllerFailurePathWithNetworkRule(t *testing.T) {
t.Parallel()
defer PanicRecover(t)
ctx := context.Background()
Expand Down Expand Up @@ -75,7 +76,7 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) {
Subnets: []azurev1alpha1.VNetSubnets{VNetSubNetInstance},
},
}

//Create VNet
EnsureInstance(ctx, t, tc, VNetInstance)

subnetID := "/subscriptions/" + config.SubscriptionID() + "/resourceGroups/" + rgName + "/providers/Microsoft.Network/virtualNetworks/" + VNetName + "/subnets/" + subnetName
Expand Down Expand Up @@ -119,10 +120,10 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) {
},
}

EnsureInstance(ctx, t, tc, cnInstance)
//Due to the service endpoint cannot be automatically created, can only verify the failure result
EnsureInstanceWithResult(ctx, t, tc, cnInstance, errhelp.NetworkAclsValidationFailure, false)

// Delete instance

EnsureDelete(ctx, t, tc, cnInstance)

}
2 changes: 1 addition & 1 deletion controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ func setup() error {

log.Println("Creating SA:", storageAccountName)
// Create the Storage Account and Container
_, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageSku{
_, _, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageSku{
Name: "Standard_LRS",
}, "Storage", map[string]*string{}, "", nil, nil, nil)

Expand Down
1 change: 1 addition & 0 deletions pkg/errhelp/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const (
RequestDisallowedByPolicy = "RequestDisallowedByPolicy"
ServiceBusy = "ServiceBusy"
NameNotAvailable = "NameNotAvailable"
NetworkAclsValidationFailure = "NetworkAclsValidationFailure"
)

func NewAzureError(err error) error {
Expand Down
20 changes: 11 additions & 9 deletions pkg/resourcemanager/storages/storageaccount/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"errors"
"log"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-04-01/storage"
"github.com/Azure/azure-service-operator/api/v1alpha1"
Expand Down Expand Up @@ -91,7 +90,7 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context,
kind azurev1alpha1.StorageKind,
tags map[string]*string,
accessTier azurev1alpha1.StorageAccessTier,
enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (result storage.Account, err error) {
enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (pollingURL string, result storage.Account, err error) {

storagesClient := getStoragesClient()

Expand All @@ -100,16 +99,19 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context,
checkAccountParams := storage.AccountCheckNameAvailabilityParameters{Name: &storageAccountName, Type: &storageType}
checkNameResult, err := storagesClient.CheckNameAvailability(ctx, checkAccountParams)
if err != nil {
return result, err
return "", result, err
}
if dataLakeEnabled == to.BoolPtr(true) && kind != "StorageV2" {
return result, errors.New("unable to create datalake enabled storage account")
err = errors.New("unable to create datalake enabled storage account")
return
}
if *checkNameResult.NameAvailable == false {
if checkNameResult.Reason == storage.AccountNameInvalid {
return result, errors.New("AccountNameInvalid")
err = errors.New("AccountNameInvalid")
return
} else if checkNameResult.Reason == storage.AlreadyExists {
return result, errors.New("AlreadyExists")
err = errors.New("AlreadyExists")
return
}
}

Expand All @@ -134,12 +136,12 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context,
//log.Println(fmt.Sprintf("creating storage '%s' in resource group '%s' and location: %v", storageAccountName, groupName, location))
future, err := storagesClient.Create(ctx, groupName, storageAccountName, params)
if err != nil {
return result, err
return "", result, err
}

time.Sleep(2 * time.Second)
result, err = future.Result(storagesClient)

return future.Result(storagesClient)
return future.PollingURL(), result, err

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type StorageManager interface {
kind azurev1alpha1.StorageKind,
tags map[string]*string,
accessTier azurev1alpha1.StorageAccessTier,
enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (result storage.Account, err error)
enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (pollingURL string, result storage.Account, err error)

// Get gets the description of the specified storage account.
// Parameters:
Expand Down
25 changes: 23 additions & 2 deletions pkg/resourcemanager/storages/storageaccount/storage_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/Azure/azure-service-operator/pkg/errhelp"
"github.com/Azure/azure-service-operator/pkg/helpers"
"github.com/Azure/azure-service-operator/pkg/resourcemanager"
"github.com/Azure/azure-service-operator/pkg/resourcemanager/pollclient"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -33,6 +34,7 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
accessTier := instance.Spec.AccessTier
enableHTTPSTrafficOnly := instance.Spec.EnableHTTPSTrafficOnly
dataLakeEnabled := instance.Spec.DataLakeEnabled
pollURL := instance.Status.PollingURL

networkAcls := storage.NetworkRuleSet{}

Expand All @@ -51,11 +53,26 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
if err != nil {
instance.Status.Message = err.Error()
instance.Status.State = "NotReady"

// handle failures in the async operation
if pollURL != "" {
pClient := pollclient.NewPollClient()
res, err := pClient.Get(ctx, pollURL)
if err != nil {
return false, err
}

if res.Status == "Failed" {
instance.Status.Message = res.Error.Error()
instance.Status.Provisioning = false
return true, nil
}
}
} else {
instance.Status.State = string(stor.ProvisioningState)

hash = helpers.Hash256(instance.Spec)
if instance.Status.SpecHash == hash && instance.Status.Provisioned {
if instance.Status.SpecHash == hash && (instance.Status.Provisioned || instance.Status.FailedProvisioning) {
instance.Status.RequestedAt = nil
return true, nil
}
Expand All @@ -67,13 +84,14 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
instance.Status.Provisioning = false
instance.Status.SpecHash = hash
instance.Status.ResourceId = *stor.ID
instance.Status.PollingURL = ""
return true, nil
}

instance.Status.Provisioning = true
instance.Status.Provisioned = false

_, err = sa.CreateStorage(ctx, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled, &networkAcls)
pollURL, _, err = sa.CreateStorage(ctx, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled, &networkAcls)
if err != nil {
instance.Status.Message = err.Error()
fmt.Println(err.Error())
Expand Down Expand Up @@ -113,6 +131,7 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o

if azerr.Type == errhelp.AsyncOpIncompleteError {
instance.Status.Provisioning = true
instance.Status.PollingURL = pollURL
}
return false, nil
}
Expand All @@ -122,7 +141,9 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
errhelp.NetworkAclsValidationFailure,
}
if helpers.ContainsString(stop, azerr.Type) {
instance.Status.Message = "Unable to provision Azure Storage Account due to error: " + errhelp.StripErrorIDs(err)
instance.Status.Provisioning = false
instance.Status.Provisioned = false
return true, nil
}

Expand Down

0 comments on commit 4a89de5

Please sign in to comment.