Merge branch 'master' into patch-2

api/v1alpha1/aso_types.go

@@ -25,6 +25,27 @@ type ASOStatus struct {
 	Output             string       `json:"output,omitempty"`
 }
 
+func (s *ASOStatus) SetProvisioned(msg string) {
+	s.Provisioned = true
+	s.Provisioning = false
+	s.FailedProvisioning = false
+	s.Message = msg
+}
+
+func (s *ASOStatus) SetProvisioning(msg string) {
+	s.Provisioned = false
+	s.Provisioning = true
+	s.FailedProvisioning = false
+	s.Message = msg
+}
+
+func (s *ASOStatus) SetFailedProvisioning(msg string) {
+	s.Provisioned = false
+	s.Provisioning = false
+	s.FailedProvisioning = true
+	s.Message = msg
+}
+
 // GenericSpec is a struct to help get the KeyVaultName from the Spec
 type GenericSpec struct {
 	KeyVaultToStoreSecrets string `json:"keyVaultToStoreSecrets,omitempty"`

api/v1alpha2/mysqlserver_types.go

@@ -69,7 +69,7 @@ func NewDefaultMySQLServer(name, resourceGroup, location string) *MySQLServer {
 				Name:     "GP_Gen5_4",
 				Tier:     SkuTier("GeneralPurpose"),
 				Family:   "Gen5",
-				Size:     "51200",
+				Size:     "107374182400",
 				Capacity: 4,
 			},
 			ServerVersion:  ServerVersion("8.0"),
@@ -78,7 +78,7 @@ func NewDefaultMySQLServer(name, resourceGroup, location string) *MySQLServer {
 			StorageProfile: &MySQLStorageProfile{
 				BackupRetentionDays: to.Int32Ptr(10),
 				GeoRedundantBackup:  "Disabled",
-				StorageMB:           to.Int32Ptr(5120),
+				StorageMB:           to.Int32Ptr(100 * 1024),
 				StorageAutogrow:     "Disabled",
 			},
 		},

api/v1beta1/aso_types.go

@@ -25,6 +25,27 @@ type ASOStatus struct {
 	Output             string       `json:"output,omitempty"`
 }
 
+func (s *ASOStatus) SetProvisioned(msg string) {
+	s.Provisioned = true
+	s.Provisioning = false
+	s.FailedProvisioning = false
+	s.Message = msg
+}
+
+func (s *ASOStatus) SetProvisioning(msg string) {
+	s.Provisioned = false
+	s.Provisioning = true
+	s.FailedProvisioning = false
+	s.Message = msg
+}
+
+func (s *ASOStatus) SetFailedProvisioning(msg string) {
+	s.Provisioned = false
+	s.Provisioning = false
+	s.FailedProvisioning = true
+	s.Message = msg
+}
+
 // GenericSpec is a struct to help get the KeyVaultName from the Spec
 type GenericSpec struct {
 	KeyVaultToStoreSecrets string `json:"keyVaultToStoreSecrets,omitempty"`

controllers/async_controller.go

@@ -169,7 +169,7 @@ func (r *AsyncReconciler) Reconcile(req ctrl.Request, obj runtime.Object) (resul
 		status.RequestedAt = nil
 	}
 	if done && !status.Provisioned && ensureErr == nil {
-		status.FailedProvisioning = true
+		status.SetFailedProvisioning(status.Message) // Keep the same message
 	}
 
 	// update the status of the resource in kubernetes

controllers/azuresql_combined_test.go

@@ -178,6 +178,15 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 				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")
+
+			// At this point the DB should be in a stable state, ensure that the right status is set
+			db := &v1beta1.AzureSqlDatabase{}
+			err = tc.k8sClient.Get(ctx, namespacedName, db)
+			assert.Equal(nil, err, "err getting DB from k8s")
+
+			assert.Equal(false, db.Status.Provisioning)
+			assert.Equal(false, db.Status.FailedProvisioning)
+			assert.Equal(true, db.Status.Provisioned)
 		})
 
 		// Create FirewallRules ---------------------------------------

pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go

@@ -47,10 +47,7 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 	// TODO: Ideally we would catch this earlier -- if/when we update the storage version
 	// TODO: to not include Edition, we can remove this
 	if err != nil {
-		instance.Status.Provisioning = false
-		instance.Status.Provisioned = false
-		instance.Status.FailedProvisioning = true
-		instance.Status.Message = fmt.Sprintf("Azure DB error: %s", errhelp.StripErrorIDs(err))
+		instance.Status.SetFailedProvisioning(fmt.Sprintf("Azure DB error: %s", errhelp.StripErrorIDs(err)))
 		return true, nil
 	}
 
@@ -63,8 +60,7 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 		Sku:          dbSku,
 	}
 
-	instance.Status.Provisioning = true
-	instance.Status.Provisioned = false
+	instance.Status.SetProvisioning("")
 
 	// Before we attempt to issue a new update, check if there is a previously ongoing update
 	if instance.Status.PollingURL != "" {
@@ -81,17 +77,15 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 		}
 
 		if res.Status == pollclient.LongRunningOperationPollStatusFailed {
-			instance.Status.Message = res.Error.Error()
-
 			// TODO: Unfortunate that this is duplicated below... this stems from a race condition where
 			// TODO: depending on when the LRO status is updated, we either notice it below or right here
 			if res.Error.Code == errhelp.InvalidMaxSizeTierCombination {
-				instance.Status.FailedProvisioning = true
-				instance.Status.Provisioning = false
+				instance.Status.SetFailedProvisioning(res.Error.Error())
 				instance.Status.PollingURL = ""
 				return true, nil
 			}
 
+			instance.Status.Message = res.Error.Error()
 			// There can be intermediate errors and various other things that cause requests to fail, so we need to try again.
 			instance.Status.PollingURL = "" // Clear URL to force retry
 			return false, nil
@@ -104,7 +98,6 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 
 		// Previous operation was a success
 		if res.Status == pollclient.LongRunningOperationPollStatusSucceeded {
-			instance.Status.Provisioning = false
 			instance.Status.SpecHash = hash
 			instance.Status.PollingURL = ""
 		}
@@ -131,21 +124,17 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 				instance.Status.Message = fmt.Sprintf("Azure DB long-term retention policy error: %s", errhelp.StripErrorIDs(err))
 				azerr := errhelp.NewAzureError(err)
 				if helpers.ContainsString(failureErrors, azerr.Type) {
-					instance.Status.Provisioning = false
-					instance.Status.Provisioned = false
-					instance.Status.FailedProvisioning = true
+					// Leave message the same as above
+					instance.Status.SetFailedProvisioning(instance.Status.Message)
 					return true, nil
 				} else {
 					return false, err
 				}
 			}
 
 			// db exists, we have successfully provisioned everything
-			instance.Status.Provisioning = false
-			instance.Status.Provisioned = true
-			instance.Status.FailedProvisioning = false
+			instance.Status.SetProvisioned(resourcemanager.SuccessMsg)
 			instance.Status.State = string(dbGet.Status)
-			instance.Status.Message = resourcemanager.SuccessMsg
 			instance.Status.ResourceId = *dbGet.ID
 			return true, nil
 		} else {
@@ -156,6 +145,7 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 				errhelp.ResourceGroupNotFoundErrorCode,
 			}
 			if helpers.ContainsString(requeuErrors, azerr.Type) {
+				// TODO: Doing this seems pointless, but leaving it for now
 				instance.Status.Provisioning = false
 				return false, nil
 			}
@@ -170,15 +160,13 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 		// handle errors
 		// resource request has been sent to ARM
 		if azerr.Type == errhelp.AsyncOpIncompleteError {
-			instance.Status.Message = "Resource request successfully submitted to Azure"
-			instance.Status.Provisioning = true
+			instance.Status.SetProvisioning("Resource request successfully submitted to Azure")
 			instance.Status.PollingURL = pollingUrl
 			return false, nil
 		}
 
 		if azerr.Type == errhelp.InvalidMaxSizeTierCombination {
-			instance.Status.FailedProvisioning = true
-			instance.Status.Provisioning = false
+			instance.Status.SetFailedProvisioning(instance.Status.Message)
 			return true, nil
 		}
 
@@ -188,25 +176,25 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt
 			errhelp.ParentNotFoundErrorCode,
 		}
 		if helpers.ContainsString(catch, azerr.Type) {
+			// TODO: Doing this seems pointless, but leaving it for now
 			instance.Status.Provisioning = false
 			return false, nil
 		}
 
 		// if the database is busy, requeue
 		errorString := err.Error()
 		if strings.Contains(errorString, "Try again later") {
+			// TODO: Doing this seems pointless, but leaving it for now
 			instance.Status.Provisioning = false
 			return false, nil
 		}
 
 		switch azerr.Code {
 		case http.StatusBadRequest:
-			instance.Status.FailedProvisioning = true
-			instance.Status.Provisioning = false
+			instance.Status.SetFailedProvisioning(instance.Status.Message)
 			return true, nil
 		case http.StatusNotFound:
-			instance.Status.Message = fmt.Sprintf("Waiting for SQL DB %s to provision", dbName)
-			instance.Status.Provisioning = false
+			instance.Status.SetFailedProvisioning(fmt.Sprintf("Waiting for SQL DB %s to provision", dbName))
 			return false, nil
 		}
 

pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go

@@ -43,15 +43,14 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj
 	_, err = s.GetDB(ctx, instance.Spec.ResourceGroup, instance.Spec.Server, instance.Spec.DbName)
 	if err != nil {
 		instance.Status.Message = errhelp.StripErrorIDs(err)
-		instance.Status.Provisioning = false
 
-		requeuErrors := []string{
+		requeueErrors := []string{
 			errhelp.ResourceNotFound,
 			errhelp.ParentNotFoundErrorCode,
 			errhelp.ResourceGroupNotFoundErrorCode,
 		}
 		azerr := errhelp.NewAzureError(err)
-		if helpers.ContainsString(requeuErrors, azerr.Type) {
+		if helpers.ContainsString(requeueErrors, azerr.Type) {
 			return false, nil
 		}
 
@@ -61,7 +60,7 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj
 			return false, nil
 		}
 
-		// if this is an unmarshall error - igmore and continue, otherwise report error and requeue
+		// if this is an unmarshall error - ignore and continue, otherwise report error and requeue
 		if !strings.Contains(errorString, "cannot unmarshal array into Go struct field serviceError2.details") {
 			return false, err
 		}
@@ -70,7 +69,6 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj
 	db, err := s.ConnectToSqlDbAsCurrentUser(ctx, instance.Spec.Server, instance.Spec.DbName)
 	if err != nil {
 		instance.Status.Message = errhelp.StripErrorIDs(err)
-		instance.Status.Provisioning = false
 
 		// catch firewall issue - keep cycling until it clears up
 		if strings.Contains(err.Error(), "create a firewall rule for this IP address") {
@@ -83,6 +81,8 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj
 			return false, nil
 		}
 
+		// Other failures are terminal
+		instance.Status.SetFailedProvisioning(instance.Status.Message)
 		return true, nil
 	}
 
@@ -93,10 +93,13 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj
 	}
 
 	if !userExists {
+		instance.Status.SetProvisioning("")
+
 		err = s.EnableUser(ctx, requestedUsername, instance.Spec.ManagedIdentityClientId, db)
 		if err != nil {
-			instance.Status.Message = "failed enabling managed identity user, err: " + err.Error()
+			instance.Status.Message = fmt.Sprintf("failed enabling managed identity user, err: %v", err)
 			if strings.Contains(err.Error(), "The login already has an account under a different user name") {
+				instance.Status.SetFailedProvisioning(instance.Status.Message)
 				return true, nil
 			}
 			return false, err
@@ -106,25 +109,23 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj
 	// apply roles to user
 	if len(instance.Spec.Roles) == 0 {
 		instance.Status.Message = "No roles specified for user"
-		return false, fmt.Errorf("No roles specified for database user")
+		return false, fmt.Errorf("no roles specified for database user")
 	}
 
 	err = s.GrantUserRoles(ctx, requestedUsername, instance.Spec.Roles, db)
 	if err != nil {
-		instance.Status.Message = "GrantUserRoles failed"
-		return false, fmt.Errorf("GrantUserRoles failed")
+		instance.Status.Message = fmt.Sprintf("GrantUserRoles failed: %v", err)
+		return false, fmt.Errorf("GrantUserRoles failed: %v", err)
 	}
 
 	err = s.UpdateSecret(ctx, instance, s.SecretClient)
 	if err != nil {
-		instance.Status.Message = "Updating secret failed " + err.Error()
-		return false, fmt.Errorf("Updating secret failed")
+		instance.Status.Message = fmt.Sprintf("Updating secret failed: %v", err)
+		return false, fmt.Errorf("updating secret failed")
 	}
 
-	instance.Status.Provisioned = true
+	instance.Status.SetProvisioned(resourcemanager.SuccessMsg)
 	instance.Status.State = "Succeeded"
-	instance.Status.Message = resourcemanager.SuccessMsg
-
 	return true, nil
 }
 
@@ -159,11 +160,7 @@ func (s *AzureSqlManagedUserManager) Delete(ctx context.Context, obj runtime.Obj
 		azerr := errhelp.NewAzureError(err)
 		if helpers.ContainsString(catch, azerr.Type) {
 			// Best case deletion of secrets
-			err2 := s.DeleteSecrets(ctx, instance, s.SecretClient)
-			if err2 != nil {
-				instance.Status.Message = "failed to delete secrets for managed identity user, err: " + err2.Error()
-				return false, err2
-			}
+			s.DeleteSecrets(ctx, instance, s.SecretClient)
 
 			return false, nil
 		}
@@ -173,7 +170,6 @@ func (s *AzureSqlManagedUserManager) Delete(ctx context.Context, obj runtime.Obj
 	db, err := s.ConnectToSqlDbAsCurrentUser(ctx, instance.Spec.Server, instance.Spec.DbName)
 	if err != nil {
 		instance.Status.Message = errhelp.StripErrorIDs(err)
-		instance.Status.Provisioning = false
 
 		// catch firewall issue - keep cycling until it clears up
 		if strings.Contains(err.Error(), "create a firewall rule for this IP address") {
@@ -204,11 +200,7 @@ func (s *AzureSqlManagedUserManager) Delete(ctx context.Context, obj runtime.Obj
 	}
 
 	// Best case deletion of secrets
-	err = s.DeleteSecrets(ctx, instance, s.SecretClient)
-	if err != nil {
-		instance.Status.Message = "failed to delete secrets for managed identity user, err: " + err.Error()
-		return false, err
-	}
+	s.DeleteSecrets(ctx, instance, s.SecretClient)
 
 	instance.Status.Message = fmt.Sprintf("Delete AzureSqlManagedUser succeeded")