Merge branch 'master' into storage

controllers/azuresql_combined_test.go

@@ -44,7 +44,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 	sqlServerInstance2 := azurev1alpha1.NewAzureSQLServer(sqlServerNamespacedName2, rgName, rgLocation2)
 
 	// create and wait
-	EnsureInstance(ctx, t, tc, sqlServerInstance)
+	RequireInstance(ctx, t, tc, sqlServerInstance)
 
 	//verify secret exists in k8s for server 1 ---------------------------------
 	secret := &v1.Secret{}

controllers/helpers.go

@@ -223,17 +223,50 @@ func EnsureInstanceWithResult(ctx context.Context, t *testing.T, tc TestContext,
 
 	names := types.NamespacedName{Name: res.GetName(), Namespace: res.GetNamespace()}
 
-	// Wait for first sql server to resolve
-	assert.Eventually(func() bool {
-		_ = tc.k8sClient.Get(ctx, names, instance)
-		return HasFinalizer(res, finalizerName)
-	}, tc.timeoutFast, tc.retry, "error waiting for %s to have finalizer", typeOf)
+	// Wait for finalizer
+	err = helpers.Retry(tc.timeoutFast, tc.retry, func() error {
+		err := tc.k8sClient.Get(ctx, names, instance)
+		if err != nil {
+			return err
+		}
+
+		if !HasFinalizer(res, finalizerName) {
+			return fmt.Errorf("resource '%s' (%s) does not have finalizer '%s'", names.Name, typeOf, finalizerName)
+		}
+		return nil
+	})
+	assert.Nil(err, "error waiting for %s to have finalizer", typeOf)
+
+	// wait for provisioned and message to be as expected
+	err = helpers.Retry(tc.timeout, tc.retry, func() error {
+		err := tc.k8sClient.Get(ctx, names, instance)
+		if err != nil {
+			return err
+		}
 
-	assert.Eventually(func() bool {
-		_ = tc.k8sClient.Get(ctx, names, instance)
 		statused := ConvertToStatus(instance)
-		return strings.Contains(statused.Status.Message, message) && statused.Status.Provisioned == provisioned
-	}, tc.timeout, tc.retry, "wait for %s to provision", typeOf)
+		// if we expect this resource to end up with provisioned == true then failedProvisioning == true is unrecoverable
+		if provisioned && statused.Status.FailedProvisioning {
+			return helpers.NewStop(fmt.Errorf("Failed provisioning: %s", statused.Status.Message))
+		}
+		if !strings.Contains(statused.Status.Message, message) || statused.Status.Provisioned != provisioned {
+			return fmt.Errorf(
+				`Expected: 
+					Status.Message to contain %s
+					Status.Provisioned to be %t
+				Actual:
+					Message: '%s'
+					Provisioned: %t
+				`,
+				message,
+				provisioned,
+				statused.Status.Message,
+				statused.Status.Provisioned,
+			)
+		}
+		return nil
+	})
+	assert.Nil(err, "wait for %s to provision", typeOf)
 
 }
 
@@ -273,17 +306,49 @@ func RequireInstanceWithResult(ctx context.Context, t *testing.T, tc TestContext
 
 	names := types.NamespacedName{Name: res.GetName(), Namespace: res.GetNamespace()}
 
-	// Wait for first sql server to resolve
-	require.Eventually(func() bool {
-		_ = tc.k8sClient.Get(ctx, names, instance)
-		return HasFinalizer(res, finalizerName)
-	}, tc.timeoutFast, tc.retry, "error waiting for %s to have finalizer", typeOf)
+	// Wait for finalizer
+	err = helpers.Retry(tc.timeoutFast, tc.retry, func() error {
+		err := tc.k8sClient.Get(ctx, names, instance)
+		if err != nil {
+			return err
+		}
+
+		if !HasFinalizer(res, finalizerName) {
+			return fmt.Errorf("resource '%s' (%s) does not have finalizer '%s'", names.Name, typeOf, finalizerName)
+		}
+		return nil
+	})
+	require.Nil(err, "error waiting for %s to have finalizer", typeOf)
+
+	// wait for provisioned state and message to be as expected
+	err = helpers.Retry(tc.timeout, tc.retry, func() error {
+		err := tc.k8sClient.Get(ctx, names, instance)
+		if err != nil {
+			return err
+		}
 
-	require.Eventually(func() bool {
-		_ = tc.k8sClient.Get(ctx, names, instance)
 		statused := ConvertToStatus(instance)
-		return strings.Contains(statused.Status.Message, message) && statused.Status.Provisioned == provisioned
-	}, tc.timeout, tc.retry, "wait for %s to provision", typeOf)
+		if provisioned && statused.Status.FailedProvisioning {
+			return helpers.NewStop(fmt.Errorf("Failed provisioning: %s", statused.Status.Message))
+		}
+		if !strings.Contains(statused.Status.Message, message) || statused.Status.Provisioned != provisioned {
+			return fmt.Errorf(
+				`Expected: 
+					Status.Message to contain %s
+					Status.Provisioned to be %t
+				Actual:
+					Message: '%s'
+					Provisioned: %t
+				`,
+				message,
+				provisioned,
+				statused.Status.Message,
+				statused.Status.Provisioned,
+			)
+		}
+		return nil
+	})
+	require.Nil(err, "wait for %s to provision", typeOf)
 
 }
 

controllers/keyvault_controller_test.go

@@ -30,7 +30,7 @@ func TestKeyvaultControllerHappyPath(t *testing.T) {
 	ctx := context.Background()
 	assert := assert.New(t)
 
-	keyVaultName := helpers.FillWithRandom(GenerateTestResourceName("kv"), 24)
+	keyVaultName := GenerateTestResourceNameWithRandom("kv", 6)
 	const poll = time.Second * 10
 
 	keyVaultLocation := tc.resourceGroupLocation
@@ -72,7 +72,7 @@ func TestKeyvaultControllerWithAccessPolicies(t *testing.T) {
 	ctx := context.Background()
 	assert := assert.New(t)
 
-	keyVaultName := helpers.FillWithRandom(GenerateTestResourceName("kv"), 24)
+	keyVaultName := GenerateTestResourceNameWithRandom("kv", 6)
 	const poll = time.Second * 10
 	keyVaultLocation := tc.resourceGroupLocation
 	accessPolicies := []azurev1alpha1.AccessPolicyEntry{
@@ -150,7 +150,7 @@ func TestKeyvaultControllerWithLimitedAccessPoliciesAndUpdate(t *testing.T) {
 	defer PanicRecover(t)
 	ctx := context.Background()
 	assert := assert.New(t)
-	keyVaultName := helpers.FillWithRandom(GenerateTestResourceName("kv"), 24)
+	keyVaultName := GenerateTestResourceNameWithRandom("kv", 6)
 	const poll = time.Second * 10
 	keyVaultLocation := tc.resourceGroupLocation
 	limitedPermissions := []string{"backup"}
@@ -346,7 +346,7 @@ func TestKeyvaultControllerWithVirtualNetworkRulesAndUpdate(t *testing.T) {
 	ctx := context.Background()
 	assert := assert.New(t)
 
-	keyVaultName := helpers.FillWithRandom(GenerateTestResourceName("kv"), 24)
+	keyVaultName := GenerateTestResourceNameWithRandom("kv", 6)
 	const poll = time.Second * 10
 	keyVaultLocation := tc.resourceGroupLocation
 	accessPolicies := []azurev1alpha1.AccessPolicyEntry{

pkg/helpers/retry.go

@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package helpers
+
+import "time"
+
+func NewStop(e error) *StopErr {
+	return &StopErr{e}
+}
+
+type StopErr struct {
+	Err error
+}
+
+func (s *StopErr) Error() string {
+	return s.Err.Error()
+}
+
+func Retry(timeout time.Duration, sleep time.Duration, fn func() error) error {
+	if err := fn(); err != nil {
+		// allow early exit
+		if v, ok := err.(*StopErr); ok {
+			return v.Err
+		}
+		if timeout > 0 {
+			time.Sleep(sleep)
+			return Retry(timeout-sleep, sleep, fn)
+		}
+		return err
+	}
+	return nil
+}

pkg/helpers/retry_test.go

@@ -0,0 +1,31 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package helpers
+
+import (
+	"fmt"
+	"testing"
+	"time"
+)
+
+func TestRetryTimeout(t *testing.T) {
+	start := time.Now()
+	_ = Retry(5*time.Second, 1*time.Second, func() error {
+		return fmt.Errorf("test")
+	})
+	stop := time.Now().Sub(start)
+	if stop < 5*time.Second {
+		t.Errorf("retry ended too soon: %v", stop)
+	}
+}
+func TestRetryStopErr(t *testing.T) {
+	start := time.Now()
+	_ = Retry(5*time.Second, 1*time.Second, func() error {
+		return NewStop(fmt.Errorf("test"))
+	})
+	stop := time.Now().Sub(start)
+	if stop > 1*time.Second {
+		t.Errorf("retry with stop should not take so long: %v", stop)
+	}
+}

pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go

@@ -34,7 +34,10 @@ func (_ *AzureSqlDbManager) GetServer(ctx context.Context, resourceGroupName str
 
 // GetDB retrieves a database
 func (_ *AzureSqlDbManager) GetDB(ctx context.Context, resourceGroupName string, serverName string, databaseName string) (sql.Database, error) {
-	dbClient := azuresqlshared.GetGoDbClient()
+	dbClient, err := azuresqlshared.GetGoDbClient()
+	if err != nil {
+		return sql.Database{}, err
+	}
 
 	return dbClient.Get(
 		ctx,
@@ -65,7 +68,11 @@ func (sdk *AzureSqlDbManager) DeleteDB(ctx context.Context, resourceGroupName st
 		return result, nil
 	}
 
-	dbClient := azuresqlshared.GetGoDbClient()
+	dbClient, err := azuresqlshared.GetGoDbClient()
+	if err != nil {
+		return result, err
+	}
+
 	result, err = dbClient.Delete(
 		ctx,
 		resourceGroupName,
@@ -78,7 +85,13 @@ func (sdk *AzureSqlDbManager) DeleteDB(ctx context.Context, resourceGroupName st
 
 // CreateOrUpdateDB creates or updates a DB in Azure
 func (_ *AzureSqlDbManager) CreateOrUpdateDB(ctx context.Context, resourceGroupName string, location string, serverName string, tags map[string]*string, properties azuresqlshared.SQLDatabaseProperties) (*http.Response, error) {
-	dbClient := azuresqlshared.GetGoDbClient()
+	dbClient, err := azuresqlshared.GetGoDbClient()
+	if err != nil {
+		return &http.Response{
+			StatusCode: 0,
+		}, err
+	}
+
 	dbProp := azuresqlshared.SQLDatabasePropertiesToDatabase(properties)
 
 	future, err := dbClient.CreateOrUpdate(

pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup.go

@@ -42,7 +42,10 @@ func (f *AzureSqlFailoverGroupManager) GetServer(ctx context.Context, resourceGr
 
 // GetDB retrieves a database
 func (f *AzureSqlFailoverGroupManager) GetDB(ctx context.Context, resourceGroupName string, serverName string, databaseName string) (sql.Database, error) {
-	dbClient := azuresqlshared.GetGoDbClient()
+	dbClient, err := azuresqlshared.GetGoDbClient()
+	if err != nil {
+		return sql.Database{}, err
+	}
 
 	return dbClient.Get(
 		ctx,

pkg/resourcemanager/azuresql/azuresqlshared/getgoclients.go

@@ -11,12 +11,15 @@ import (
 )
 
 // GetGoDbClient retrieves a DatabasesClient
-func GetGoDbClient() sql.DatabasesClient {
+func GetGoDbClient() (sql.DatabasesClient, error) {
 	dbClient := sql.NewDatabasesClientWithBaseURI(config.BaseURI(), config.SubscriptionID())
-	a, _ := iam.GetResourceManagementAuthorizer()
+	a, err := iam.GetResourceManagementAuthorizer()
+	if err != nil {
+		return sql.DatabasesClient{}, err
+	}
 	dbClient.Authorizer = a
 	dbClient.AddToUserAgent(config.UserAgent())
-	return dbClient
+	return dbClient, nil
 }
 
 // GetGoServersClient retrieves a ServersClient