From f6f847be8f9760eeacba8fa64bb9a6e12571f46f Mon Sep 17 00:00:00 2001
From: Christian Muirhead <christian.muirhead@microsoft.com>
Date: Mon, 21 Dec 2020 16:02:58 +1300
Subject: [PATCH] Don't ignore errors checking whether a being-deleted DB
 exists (#1338)

This can mean that if there's a communication error when checking with
Azure then we just remove the k8s database resource without trying to
delete the Azure one.
---
 controllers/mysql_combined_test.go            |  2 +-
 pkg/helpers/sqlrole.go                        |  2 +-
 .../azuresql/azuresqldb/azuresqldb.go         | 36 +++++++++++++++++--
 .../azuresqldb/azuresqldb_reconcile.go        | 13 ++-----
 4 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/controllers/mysql_combined_test.go b/controllers/mysql_combined_test.go
index 492e15c1cd..09a620c683 100644
--- a/controllers/mysql_combined_test.go
+++ b/controllers/mysql_combined_test.go
@@ -99,7 +99,7 @@ func TestMySQLHappyPath(t *testing.T) {
 	EnsureInstance(ctx, t, tc, ruleInstance)
 
 	// Create user and ensure it can be updated
-	RunMySQLUserHappyPath(ctx, t, mySQLServerName,mySQLDBName, rgName)
+	RunMySQLUserHappyPath(ctx, t, mySQLServerName, mySQLDBName, rgName)
 
 	// Create VNet and VNetRules -----
 	RunMySqlVNetRuleHappyPath(t, mySQLServerName, rgLocation)
diff --git a/pkg/helpers/sqlrole.go b/pkg/helpers/sqlrole.go
index 3eb5e948aa..1330e391d6 100644
--- a/pkg/helpers/sqlrole.go
+++ b/pkg/helpers/sqlrole.go
@@ -7,7 +7,7 @@ type SQLRoleDelta struct {
 
 func DiffCurrentAndExpectedSQLRoles(currentRoles map[string]struct{}, expectedRoles map[string]struct{}) SQLRoleDelta {
 	result := SQLRoleDelta{
-		AddedRoles: make(map[string]struct{}),
+		AddedRoles:   make(map[string]struct{}),
 		DeletedRoles: make(map[string]struct{}),
 	}
 
diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go
index 937657cd35..322017256c 100644
--- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go
+++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go
@@ -10,6 +10,8 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql"
 	sql3 "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql"
+	"github.com/Azure/azure-service-operator/pkg/errhelp"
+	"github.com/Azure/azure-service-operator/pkg/helpers"
 	azuresqlshared "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 
@@ -66,12 +68,16 @@ func (m *AzureSqlDbManager) DeleteDB(
 	// check to see if the server exists, if it doesn't then short-circuit
 	server, err := m.GetServer(ctx, resourceGroupName, serverName)
 	if err != nil || *server.State != "Ready" {
-		return nil, nil
+		return nil, ignoreNotFound(err)
 	}
 
 	// check to see if the db exists, if it doesn't then short-circuit
-	_, err = m.GetDB(ctx, resourceGroupName, serverName, databaseName)
+	db, err := m.GetDB(ctx, resourceGroupName, serverName, databaseName)
 	if err != nil {
+		return nil, ignoreNotFound(err)
+	}
+	if db.Name == nil {
+		// The database doesn't exist, we don't need to delete it.
 		return nil, nil
 	}
 
@@ -196,3 +202,29 @@ func (m *AzureSqlDbManager) AddLongTermRetention(ctx context.Context, resourceGr
 
 	return future.Response(), err
 }
+
+var goneCodes = []string{
+	errhelp.ResourceGroupNotFoundErrorCode,
+	errhelp.ParentNotFoundErrorCode,
+	errhelp.NotFoundErrorCode,
+	errhelp.ResourceNotFound,
+}
+
+func isNotFound(azerr *errhelp.AzureError) bool {
+	return helpers.ContainsString(goneCodes, azerr.Type)
+}
+
+var incompleteCodes = []string{
+	errhelp.AsyncOpIncompleteError,
+}
+
+func isIncompleteOp(azerr *errhelp.AzureError) bool {
+	return helpers.ContainsString(incompleteCodes, azerr.Type)
+}
+
+func ignoreNotFound(err error) error {
+	if isNotFound(errhelp.NewAzureError(err)) {
+		return nil
+	}
+	return err
+}
diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go
index 6110dda207..330e3d073a 100644
--- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go
+++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go
@@ -218,19 +218,10 @@ func (db *AzureSqlDbManager) Delete(ctx context.Context, obj runtime.Object, opt
 
 	_, err = db.DeleteDB(ctx, groupName, server, dbName)
 	if err != nil {
-		catch := []string{
-			errhelp.AsyncOpIncompleteError,
-		}
-		gone := []string{
-			errhelp.ResourceGroupNotFoundErrorCode,
-			errhelp.ParentNotFoundErrorCode,
-			errhelp.NotFoundErrorCode,
-			errhelp.ResourceNotFound,
-		}
 		azerr := errhelp.NewAzureError(err)
-		if helpers.ContainsString(catch, azerr.Type) {
+		if isIncompleteOp(azerr) {
 			return true, nil
-		} else if helpers.ContainsString(gone, azerr.Type) {
+		} else if isNotFound(azerr) {
 			return false, nil
 		}
 		return true, fmt.Errorf("AzureSqlDb delete error %v", err)