Merge branch 'master' into issue-797-redis-vnet

Makefile

@@ -17,6 +17,9 @@ BUILD_ID ?= $(shell git rev-parse --short HEAD)
 # best to keep the prefix as short as possible to not exceed naming limits for things like keyvault (24 chars)
 TEST_RESOURCE_PREFIX ?= aso-$(BUILD_ID)
 
+# Some parts of the test suite use Go Build Tags to ignore certain tests. Default to all tests but allow the user to pass custom tags.
+BUILD_TAGS ?= all
+
 all: manager
 
 # Generate test certs for development
@@ -40,7 +43,7 @@ api-test: generate fmt vet manifests
 # Run tests
 test: generate fmt vet manifests 
 	TEST_USE_EXISTING_CLUSTER=false TEST_CONTROLLER_WITH_MOCKS=true REQUEUE_AFTER=20 \
-	go test -tags all -parallel 3 -v -coverprofile=coverage.txt -covermode count \
+	go test -tags "$(BUILD_TAGS)" -parallel 3 -v -coverprofile=coverage.txt -covermode count \
 	./api/... \
 	./controllers/... \
 	-timeout 10m 2>&1 | tee testlogs.txt
@@ -49,7 +52,7 @@ test: generate fmt vet manifests
 
 # Run tests with existing cluster
 test-existing-controllers: generate fmt vet manifests
-	TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=true REQUEUE_AFTER=20 go test -tags all -parallel 4 -v ./controllers/... -timeout 45m
+	TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=true REQUEUE_AFTER=20 go test -tags "$(BUILD_TAGS)" -parallel 4 -v ./controllers/... -timeout 45m
 
 unit-tests:
 	go test ./pkg/resourcemanager/keyvaults/unittest/

docs/test.md

@@ -5,6 +5,6 @@ Testing the full project can be done in two ways:
 - `make api-test` - Runs the Kubernetes API tests against the CRDs
 - `make test` - Test against the Kubernetes integration testing framework.
 - `make test-existing-managers` - Test the resource managers against an existing cluster. This is currently the easiest way to run tests against a kind cluster setup.
-- `make test-existing-controllers` - Test the controllers against an existing cluster. By default this will run every controller test, but you can edit the `tags` parameter in the makefile to specify individual test suites. Each controller test file declares its tags in the `// +build` comment at the top of the file.
-
+- `make test-existing-controllers` - Test the controllers against an existing cluster. 
 
+Some test suites (primarily the controller suite) have included Go Build Tags to allow selectively running tests. Users can specify individual test suites by setting the `BUILD_TAGS` parameter as an environment variable or before the make target: `make BUILD_TAGS=azuresqlservercombined test-existing-controllers`. Test files declare their tags in the `// +build` comment at the top of the file.

pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go

@@ -75,6 +75,7 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 		}
 		if !helpers.ContainsString(ignore, azerr.Type) {
 			instance.Status.Message = err.Error()
+			instance.Status.Provisioning = false
 			return false, fmt.Errorf("AzureSqlDb GetDB error %v", err)
 		}
 	}
@@ -84,19 +85,26 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 		instance.Status.Message = err.Error()
 		azerr := errhelp.NewAzureErrorAzureError(err)
 
+		// resource request has been sent to ARM
+		if azerr.Type == errhelp.AsyncOpIncompleteError {
+			instance.Status.Provisioning = true
+			return false, nil
+		}
+
 		// the errors that can arise during reconcilliation where we simply requeue
 		catch := []string{
 			errhelp.ResourceGroupNotFoundErrorCode,
 			errhelp.ParentNotFoundErrorCode,
-			errhelp.AsyncOpIncompleteError,
 		}
 		if helpers.ContainsString(catch, azerr.Type) {
+			instance.Status.Provisioning = false
 			return false, nil
 		}
 
 		// assertion that a 404 error implies that the Azure SQL server hasn't been provisioned yet
 		if resp != nil && resp.StatusCode == 404 {
 			instance.Status.Message = fmt.Sprintf("Waiting for SQL Server %s to provision", server)
+			instance.Status.Provisioning = false
 			return false, nil
 		}
 

pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go

@@ -6,7 +6,6 @@ package azuresqlvnetrule
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/2015-05-01-preview/sql"
 	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
@@ -34,14 +33,12 @@ func (vr *AzureSqlVNetRuleManager) Ensure(ctx context.Context, obj runtime.Objec
 
 	vnetrule, err := vr.GetSQLVNetRule(ctx, groupName, server, ruleName)
 	if err == nil {
-		if vnetrule.VirtualNetworkRuleProperties != nil {
-			if vnetrule.VirtualNetworkRuleProperties.State == sql.Ready {
-				instance.Status.Provisioning = false
-				instance.Status.Provisioned = true
-				instance.Status.Message = resourcemanager.SuccessMsg
-				instance.Status.ResourceId = *vnetrule.ID
-				return true, nil
-			}
+		if vnetrule.VirtualNetworkRuleProperties != nil && vnetrule.VirtualNetworkRuleProperties.State == sql.Ready {
+			instance.Status.Provisioning = false
+			instance.Status.Provisioned = true
+			instance.Status.Message = resourcemanager.SuccessMsg
+			instance.Status.ResourceId = *vnetrule.ID
+			return true, nil
 		}
 		return false, nil
 	}
@@ -53,7 +50,8 @@ func (vr *AzureSqlVNetRuleManager) Ensure(ctx context.Context, obj runtime.Objec
 		instance.Status.Message = err.Error()
 		azerr := errhelp.NewAzureErrorAzureError(err)
 
-		if strings.Contains(azerr.Type, errhelp.AsyncOpIncompleteError) {
+		if azerr.Type == errhelp.AsyncOpIncompleteError {
+			instance.Status.Provisioning = true
 			instance.Status.Message = "Resource request submitted to Azure successfully"
 			return false, nil
 		}
@@ -63,11 +61,11 @@ func (vr *AzureSqlVNetRuleManager) Ensure(ctx context.Context, obj runtime.Objec
 			errhelp.ParentNotFoundErrorCode,
 			errhelp.ResourceNotFound,
 		}
-
 		if helpers.ContainsString(ignorableErrors, azerr.Type) {
 			instance.Status.Provisioning = false
 			return false, nil
 		}
+
 		return false, err
 	}