From c849760a550573c661d646d75acd078b6c8e7c7c Mon Sep 17 00:00:00 2001 From: Denis Kisselev Date: Wed, 25 Mar 2020 14:19:18 -0700 Subject: [PATCH 1/5] Make: Extract build tags out to a make parameter --- Makefile | 7 +++++-- docs/test.md | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 37eac79ffe1..8a3368f17d6 100644 --- a/Makefile +++ b/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/ diff --git a/docs/test.md b/docs/test.md index 0298869beae..0dd280c6990 100644 --- a/docs/test.md +++ b/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. From 49b951e5e84e0b6251d6303159c0c5e83bd0f2fa Mon Sep 17 00:00:00 2001 From: jpflueger Date: Wed, 25 Mar 2020 17:05:18 -0600 Subject: [PATCH 2/5] updating provisioning flags --- .../azuresql/azuresqldb/azuresqldb_reconcile.go | 2 ++ .../azuresqlvnetrule_reconcile.go | 16 +++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go index 637569122a5..07a0c73a304 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go +++ b/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) } } @@ -100,6 +101,7 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt return false, nil } + instance.Status.Provisioning = false return true, fmt.Errorf("AzureSqlDb CreateOrUpdate error %v", err) } diff --git a/pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go index 2aa1c31ab2d..ee4745cd0d1 100644 --- a/pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go @@ -34,14 +34,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 } @@ -54,6 +52,7 @@ func (vr *AzureSqlVNetRuleManager) Ensure(ctx context.Context, obj runtime.Objec azerr := errhelp.NewAzureErrorAzureError(err) if strings.Contains(azerr.Type, errhelp.AsyncOpIncompleteError) { + instance.Status.Provisioning = false instance.Status.Message = "Resource request submitted to Azure successfully" return false, nil } @@ -63,7 +62,6 @@ 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 From 02cbc9adb2d7bbb1da8ce184d5fba70fc5a4002a Mon Sep 17 00:00:00 2001 From: jpflueger Date: Thu, 26 Mar 2020 18:59:15 -0600 Subject: [PATCH 3/5] addressing pr feedback --- .../azuresql/azuresqldb/azuresqldb_reconcile.go | 8 +++++++- .../azuresqlvnetrule/azuresqlvnetrule_reconcile.go | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go index 07a0c73a304..4faa118d253 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go @@ -85,13 +85,19 @@ 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 } diff --git a/pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go index ee4745cd0d1..dc64bfb88df 100644 --- a/pkg/resourcemanager/azuresql/azuresqlvnetrule/azuresqlvnetrule_reconcile.go +++ b/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" @@ -51,8 +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) { - instance.Status.Provisioning = false + if azerr.Type == errhelp.AsyncOpIncompleteError { + instance.Status.Provisioning = true instance.Status.Message = "Resource request submitted to Azure successfully" return false, nil } @@ -66,6 +65,7 @@ func (vr *AzureSqlVNetRuleManager) Ensure(ctx context.Context, obj runtime.Objec instance.Status.Provisioning = false return false, nil } + return false, err } From 8db7a11f7f61fb4ffa481a45a2a89e4b8c91359d Mon Sep 17 00:00:00 2001 From: jpflueger Date: Thu, 26 Mar 2020 19:03:34 -0600 Subject: [PATCH 4/5] remove extra false provisioning --- pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go index 4faa118d253..23796d97a86 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go @@ -107,7 +107,6 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt return false, nil } - instance.Status.Provisioning = false return true, fmt.Errorf("AzureSqlDb CreateOrUpdate error %v", err) } From 10cd7b3d79eddb5112c689f79398f396d1d0d0ef Mon Sep 17 00:00:00 2001 From: jpflueger Date: Fri, 27 Mar 2020 09:21:57 -0600 Subject: [PATCH 5/5] not provisioning on sqldb 404 --- pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go index 23796d97a86..fae66455458 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go @@ -104,6 +104,7 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt // 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 }