diff --git a/api/v1alpha1/aso_types.go b/api/v1alpha1/aso_types.go index fbf3eb6c127..10723e9aeb4 100644 --- a/api/v1alpha1/aso_types.go +++ b/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"` diff --git a/api/v1alpha2/mysqlserver_types.go b/api/v1alpha2/mysqlserver_types.go index 26409ed58ef..32f4f9b14d5 100644 --- a/api/v1alpha2/mysqlserver_types.go +++ b/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", }, }, diff --git a/api/v1beta1/aso_types.go b/api/v1beta1/aso_types.go index 8ff8d9b7f32..6803c585c70 100644 --- a/api/v1beta1/aso_types.go +++ b/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"` diff --git a/controllers/async_controller.go b/controllers/async_controller.go index f0e239ef902..87900cbb369 100644 --- a/controllers/async_controller.go +++ b/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 diff --git a/controllers/azuresql_combined_test.go b/controllers/azuresql_combined_test.go index a1732280be1..76fcbecb000 100644 --- a/controllers/azuresql_combined_test.go +++ b/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 --------------------------------------- diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go index e6f5a38faab..3414013c4e4 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go +++ b/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,9 +124,8 @@ 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 @@ -141,11 +133,8 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt } // 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,6 +176,7 @@ 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 } @@ -195,18 +184,17 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt // 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 } diff --git a/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go index a5d3e858037..435437d53b8 100644 --- a/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go +++ b/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")