Add support for SKU Database to support updates (Azure#1242)

* Add support for updating SQL DBs

* Improve test resource cleanup

* Improve usage of pollclient constants

Makefile

@@ -97,6 +97,11 @@ test-cleanup-azure-resources:
 		az group delete --name $$rgname --no-wait --yes; \
 	done
 
+	for rgname in `az group list --query "[*].[name]" -o table | grep 'rg-prime$$' `; do \
+		echo "$$rgname will be deleted"; \
+		az group delete --name $$rgname --no-wait --yes; \
+	done
+
 # Build the docker image
 docker-build:
 	docker build . -t ${IMG} ${ARGS}

controllers/azuresql_combined_test.go

@@ -7,6 +7,7 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -112,7 +113,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 			EnsureInstance(ctx, t, tc, sqlDatabaseInstance1)
 		})
 
-		t.Run("set up second database in primary server using sku with maxsizebytes", func(t *testing.T) {
+		t.Run("set up second database in primary server using sku with maxsizebytes, then update it to use a different SKU", func(t *testing.T) {
 			t.Parallel()
 
 			maxSize := resource.MustParse("500Mi")
@@ -135,6 +136,48 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 			}
 
 			EnsureInstance(ctx, t, tc, sqlDatabaseInstance2)
+
+			namespacedName := types.NamespacedName{Name: sqlDatabaseName2, Namespace: "default"}
+			err = tc.k8sClient.Get(ctx, namespacedName, sqlDatabaseInstance2)
+			assert.Equal(nil, err, "get sql database in k8s")
+
+			originalHash := sqlDatabaseInstance2.Status.SpecHash
+
+			sqlDatabaseInstance2.Spec.Sku = &v1beta1.SqlDatabaseSku{
+				Name: "Basic",
+				Tier: "Basic",
+			}
+			maxSizeMb := 100
+			maxSize = resource.MustParse(fmt.Sprintf("%dMi", maxSizeMb))
+			sqlDatabaseInstance2.Spec.MaxSize = &maxSize
+
+			err = tc.k8sClient.Update(ctx, sqlDatabaseInstance2)
+			assert.Equal(nil, err, "updating sql database in k8s")
+
+			assert.Eventually(func() bool {
+				db := &v1beta1.AzureSqlDatabase{}
+				err = tc.k8sClient.Get(ctx, namespacedName, db)
+				assert.Equal(nil, err, "err getting DB from k8s")
+				return originalHash != db.Status.SpecHash
+			}, tc.timeout, tc.retry, "wait for sql database to be updated in k8s")
+
+			assert.Eventually(func() bool {
+				db, err := tc.sqlDbManager.GetDB(ctx, rgName, sqlServerName, sqlDatabaseName2)
+				assert.Equal(nil, err, "err getting DB fromAzure")
+				return db.Sku.Name != nil && *db.Sku.Name == "Basic"
+			}, tc.timeout, tc.retry, "wait for sql database Sku.Name to be updated in azure")
+
+			assert.Eventually(func() bool {
+				db, err := tc.sqlDbManager.GetDB(ctx, rgName, sqlServerName, sqlDatabaseName2)
+				assert.Equal(nil, err, "err getting DB fromAzure")
+				return db.Sku.Tier != nil && *db.Sku.Tier == "Basic"
+			}, tc.timeout, tc.retry, "wait for sql database Sku.Tier to be updated in azure")
+
+			assert.Eventually(func() bool {
+				db, err := tc.sqlDbManager.GetDB(ctx, rgName, sqlServerName, sqlDatabaseName2)
+				assert.Equal(nil, err, "err getting DB fromAzure")
+				return db.MaxSizeBytes != nil && *db.MaxSizeBytes == int64(maxSizeMb)*int64(1024)*int64(1024)
+			}, tc.timeout, tc.retry, "wait for sql database MaxSizeBytes to be updated in azure")
 		})
 
 		// Create FirewallRules ---------------------------------------

controllers/suite_test.go

@@ -6,6 +6,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	resourcemanagersqldb "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqldb"
 	"log"
 	"net/http"
 	"os"
@@ -24,7 +25,6 @@ import (
 	resourcemanagerapimgmt "github.com/Azure/azure-service-operator/pkg/resourcemanager/apim/apimgmt"
 	resourcemanagerappinsights "github.com/Azure/azure-service-operator/pkg/resourcemanager/appinsights"
 	resourcemanagersqlaction "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlaction"
-	resourcemanagersqldb "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqldb"
 	resourcemanagersqlfailovergroup "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlfailovergroup"
 	resourcemanagersqlfirewallrule "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlfirewallrule"
 	resourcemanagersqlmanageduser "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlmanageduser"
@@ -151,6 +151,7 @@ func setup() error {
 	keyVaultManager := resourcemanagerkeyvaults.NewAzureKeyVaultManager(k8sManager.GetScheme())
 	eventhubClient := resourcemanagereventhub.NewEventhubClient(secretClient, scheme.Scheme)
 	consumerGroupClient := resourcemanagereventhub.NewConsumerGroupClient()
+	azureSqlDatabaseManager := resourcemanagersqldb.NewAzureSqlDbManager()
 
 	timeout = time.Second * 780
 
@@ -398,7 +399,7 @@ func setup() error {
 	err = (&AzureSqlDatabaseReconciler{
 		Reconciler: &AsyncReconciler{
 			Client:      k8sManager.GetClient(),
-			AzureClient: resourcemanagersqldb.NewAzureSqlDbManager(),
+			AzureClient: azureSqlDatabaseManager,
 			Telemetry: telemetry.InitializeTelemetryDefault(
 				"AzureSqlDb",
 				ctrl.Log.WithName("controllers").WithName("AzureSqlDb"),
@@ -856,6 +857,7 @@ func setup() error {
 		eventhubClient:        eventhubClient,
 		resourceGroupManager:  resourceGroupManager,
 		keyVaultManager:       keyVaultManager,
+		sqlDbManager:          azureSqlDatabaseManager,
 		timeout:               timeout,
 		timeoutFast:           time.Minute * 3,
 		retry:                 time.Second * 3,

pkg/errhelp/errors.go

@@ -58,6 +58,7 @@ const (
 	NameNotAvailable                               = "NameNotAvailable"
 	PublicIPIdleTimeoutIsOutOfRange                = "PublicIPIdleTimeoutIsOutOfRange"
 	InvalidRequestContent                          = "InvalidRequestContent"
+	InvalidMaxSizeTierCombination                  = "InvalidMaxSizeTierCombination"
 	InternalServerError                            = "InternalServerError"
 	NetworkAclsValidationFailure                   = "NetworkAclsValidationFailure"
 	SubnetHasServiceEndpointWithInvalidServiceName = "SubnetHasServiceEndpointWithInvalidServiceName"

pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go

@@ -18,6 +18,9 @@ import (
 type AzureSqlDbManager struct {
 }
 
+// Ensure we implement the interface we expect
+var _ SqlDbManager = &AzureSqlDbManager{}
+
 func NewAzureSqlDbManager() *AzureSqlDbManager {
 	return &AzureSqlDbManager{}
 }
@@ -52,54 +55,55 @@ func (_ *AzureSqlDbManager) GetDB(ctx context.Context, resourceGroupName string,
 }
 
 // DeleteDB deletes a DB
-func (sdk *AzureSqlDbManager) DeleteDB(ctx context.Context, resourceGroupName string, serverName string, databaseName string) (result *http.Response, err error) {
-	// TODO: Probably shouldn't return a response at all in the err case here (all through this function)
-	result = &http.Response{
-		StatusCode: 200,
-	}
+func (sdk *AzureSqlDbManager) DeleteDB(
+	ctx context.Context,
+	resourceGroupName string,
+	serverName string,
+	databaseName string) (future *sql.DatabasesDeleteFuture, err error) {
 
 	// check to see if the server exists, if it doesn't then short-circuit
 	server, err := sdk.GetServer(ctx, resourceGroupName, serverName)
 	if err != nil || *server.State != "Ready" {
-		return result, nil
+		return nil, nil
 	}
 
 	// check to see if the db exists, if it doesn't then short-circuit
 	_, err = sdk.GetDB(ctx, resourceGroupName, serverName, databaseName)
 	if err != nil {
-		return result, nil
+		return nil, nil
 	}
 
 	dbClient, err := azuresqlshared.GetGoDbClient()
 	if err != nil {
-		return result, err
+		return nil, err
 	}
 
-	future, err := dbClient.Delete(
+	result, err := dbClient.Delete(
 		ctx,
 		resourceGroupName,
 		serverName,
 		databaseName,
 	)
 
 	if err != nil {
-		return result, err
+		return nil, err
 	}
 
-	return future.Response(), err
+	return &result, err
 }
 
 // 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) {
-
-	// TODO: Probably shouldn't return a response at all in the err case here (all through this function)
-	result := &http.Response{
-		StatusCode: 0,
-	}
+func (_ *AzureSqlDbManager) CreateOrUpdateDB(
+	ctx context.Context,
+	resourceGroupName string,
+	location string,
+	serverName string,
+	tags map[string]*string,
+	properties azuresqlshared.SQLDatabaseProperties) (string, *sql.Database, error) {
 
 	dbClient, err := azuresqlshared.GetGoDbClient()
 	if err != nil {
-		return result, err
+		return "", nil, err
 	}
 
 	dbProp := azuresqlshared.SQLDatabasePropertiesToDatabase(properties)
@@ -118,10 +122,12 @@ func (_ *AzureSqlDbManager) CreateOrUpdateDB(ctx context.Context, resourceGroupN
 		})
 
 	if err != nil {
-		return result, err
+		return "", nil, err
 	}
 
-	return future.Response(), err
+	result, err := future.Result(dbClient)
+
+	return future.PollingURL(), &result, err
 }
 
 // AddLongTermRetention enables / disables long term retention

pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_manager.go

@@ -11,7 +11,6 @@ import (
 	azuresqlshared "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared"
 
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager"
-	"github.com/Azure/go-autorest/autorest"
 )
 
 // SqlDbManager is the client for the resource manager for SQL databases
@@ -21,12 +20,12 @@ type SqlDbManager interface {
 		location string,
 		serverName string,
 		tags map[string]*string,
-		properties azuresqlshared.SQLDatabaseProperties) (*http.Response, error)
+		properties azuresqlshared.SQLDatabaseProperties) (pollingUrl string, db *sql.Database, err error)
 
 	DeleteDB(ctx context.Context,
 		resourceGroupName string,
 		serverName string,
-		databaseName string) (result autorest.Response, err error)
+		databaseName string) (future *sql.DatabasesDeleteFuture, err error)
 
 	GetDB(ctx context.Context,
 		resourceGroupName string,