From a57d18ac1e2af729f6b0a178f8300eb1c6fe634a Mon Sep 17 00:00:00 2001 From: William Mortl Date: Mon, 6 Apr 2020 17:29:46 -0700 Subject: [PATCH 1/4] first --- .../azuresqlserver_reconcile.go | 66 +++++++++---------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go index 7220d213cb1..b2d21c41a69 100644 --- a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go @@ -103,50 +103,44 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, instance.Status.SpecHash = hash } - if instance.Status.Provisioning { - - serv, err := s.GetServer(ctx, instance.Spec.ResourceGroup, instance.Name) - if err != nil { - azerr := errhelp.NewAzureErrorAzureError(err) - - // handle failures in the async operation - if instance.Status.PollingURL != "" { - pClient := pollclient.NewPollClient() - res, err := pClient.Get(ctx, instance.Status.PollingURL) - if err != nil { - return false, err - } - - if res.Status == "Failed" { - instance.Status.Message = res.Error.Error() - instance.Status.Provisioning = false - return true, nil - } - } + serv, err := s.GetServer(ctx, instance.Spec.ResourceGroup, instance.Name) + if err != nil { + azerr := errhelp.NewAzureErrorAzureError(err) - // @Todo: ResourceNotFound should be handled if the time since the last PUT is unreasonable - if azerr.Type != errhelp.ResourceNotFound { + // handle failures in the async operation + if instance.Status.PollingURL != "" { + pClient := pollclient.NewPollClient() + res, err := pClient.Get(ctx, instance.Status.PollingURL) + if err != nil { return false, err } - // the first minute or so after a PUT to create a server will result in failed GETs - instance.Status.State = "NotReady" - } else { - instance.Status.State = *serv.State + if res.Status == "Failed" { + instance.Status.Message = res.Error.Error() + instance.Status.Provisioning = false + return true, nil + } } - if instance.Status.State == "Ready" { - instance.Status.Message = resourcemanager.SuccessMsg - instance.Status.Provisioned = true - instance.Status.Provisioning = false - instance.Status.ResourceId = *serv.ID - instance.Status.SpecHash = hash - instance.Status.PollingURL = "" - return true, nil + // @Todo: ResourceNotFound should be handled if the time since the last PUT is unreasonable + if azerr.Type != errhelp.ResourceNotFound { + return false, err } - // server not done provisioning - return false, nil + // the first minute or so after a PUT to create a server will result in failed GETs + instance.Status.State = "NotReady" + } else { + instance.Status.State = *serv.State + } + + if instance.Status.State == "Ready" { + instance.Status.Message = resourcemanager.SuccessMsg + instance.Status.Provisioned = true + instance.Status.Provisioning = false + instance.Status.ResourceId = *serv.ID + instance.Status.SpecHash = hash + instance.Status.PollingURL = "" + return true, nil } // create the sql server From b28c342093762734254b5120a89dd8887e4efc7f Mon Sep 17 00:00:00 2001 From: William Mortl Date: Mon, 6 Apr 2020 17:32:14 -0700 Subject: [PATCH 2/4] forgot to reset failed provisioning --- .../azuresql/azuresqlserver/azuresqlserver_reconcile.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go index b2d21c41a69..68e5e8ec9bb 100644 --- a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go @@ -137,6 +137,7 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, instance.Status.Message = resourcemanager.SuccessMsg instance.Status.Provisioned = true instance.Status.Provisioning = false + instance.Status.FailedProvisioning = false instance.Status.ResourceId = *serv.ID instance.Status.SpecHash = hash instance.Status.PollingURL = "" From 46652e6bb90e65ecd3bb258881bb815694d2cb6f Mon Sep 17 00:00:00 2001 From: William Mortl Date: Tue, 7 Apr 2020 14:43:37 -0700 Subject: [PATCH 3/4] erin refactor --- .../azuresqlserver_reconcile.go | 82 +++++++++++-------- 1 file changed, 48 insertions(+), 34 deletions(-) diff --git a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go index 68e5e8ec9bb..6f8c8cd7fc1 100644 --- a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go @@ -94,54 +94,68 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, // set a spec hash if one hasn't been set hash := helpers.Hash256(instance.Spec) + specHashWasEmpty := false + if instance.Status.SpecHash == "" { + instance.Status.SpecHash = hash + specHashWasEmpty = true + } + + // early exit if hashes match and this server has been provisioned / failed provisioning if instance.Status.SpecHash == hash && (instance.Status.Provisioned || instance.Status.FailedProvisioning) { instance.Status.RequestedAt = nil return true, nil } - if instance.Status.SpecHash == "" { - instance.Status.SpecHash = hash - } + // if we've already started provisioning then try to get the server, + // or if the hash wasnt empty and the Spec Hash matches the calculated hash + // this indicates that we've already issued and update and its worth + // checking to see if the server is there + if instance.Status.Provisioning || + (!specHashWasEmpty && instance.Status.SpecHash == hash) { - serv, err := s.GetServer(ctx, instance.Spec.ResourceGroup, instance.Name) - if err != nil { - azerr := errhelp.NewAzureErrorAzureError(err) + serv, err := s.GetServer(ctx, instance.Spec.ResourceGroup, instance.Name) + if err != nil { + azerr := errhelp.NewAzureErrorAzureError(err) + + // handle failures in the async operation + if instance.Status.PollingURL != "" { + pClient := pollclient.NewPollClient() + res, err := pClient.Get(ctx, instance.Status.PollingURL) + if err != nil { + return false, err + } - // handle failures in the async operation - if instance.Status.PollingURL != "" { - pClient := pollclient.NewPollClient() - res, err := pClient.Get(ctx, instance.Status.PollingURL) - if err != nil { - return false, err + if res.Status == "Failed" { + instance.Status.Message = res.Error.Error() + instance.Status.Provisioning = false + return true, nil + } } - if res.Status == "Failed" { - instance.Status.Message = res.Error.Error() - instance.Status.Provisioning = false - return true, nil + // @Todo: ResourceNotFound should be handled if the time since the last PUT is unreasonable + if azerr.Type != errhelp.ResourceNotFound { + return false, err } - } - // @Todo: ResourceNotFound should be handled if the time since the last PUT is unreasonable - if azerr.Type != errhelp.ResourceNotFound { - return false, err + // the first minute or so after a PUT to create a server will result in failed GETs + instance.Status.State = "NotReady" + } else { + instance.Status.State = *serv.State } - // the first minute or so after a PUT to create a server will result in failed GETs - instance.Status.State = "NotReady" - } else { - instance.Status.State = *serv.State - } + if instance.Status.State == "Ready" { + instance.Status.Message = resourcemanager.SuccessMsg + instance.Status.Provisioned = true + instance.Status.Provisioning = false + instance.Status.FailedProvisioning = false + instance.Status.ResourceId = *serv.ID + instance.Status.SpecHash = hash + instance.Status.PollingURL = "" + return true, nil + } - if instance.Status.State == "Ready" { - instance.Status.Message = resourcemanager.SuccessMsg - instance.Status.Provisioned = true - instance.Status.Provisioning = false - instance.Status.FailedProvisioning = false - instance.Status.ResourceId = *serv.ID - instance.Status.SpecHash = hash - instance.Status.PollingURL = "" - return true, nil + // server not done provisioning + return false, nil } // create the sql server From 5d3e8312252cc5264ef0e92c8f9f04b4191c7492 Mon Sep 17 00:00:00 2001 From: William Mortl Date: Thu, 9 Apr 2020 12:44:21 -0700 Subject: [PATCH 4/4] small update --- .../azuresql/azuresqlserver/azuresqlserver_reconcile.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go index 6f8c8cd7fc1..ed0bfaebbfb 100644 --- a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go @@ -101,7 +101,9 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, } // early exit if hashes match and this server has been provisioned / failed provisioning - if instance.Status.SpecHash == hash && (instance.Status.Provisioned || instance.Status.FailedProvisioning) { + if !specHashWasEmpty && + instance.Status.SpecHash == hash && + (instance.Status.Provisioned || instance.Status.FailedProvisioning) { instance.Status.RequestedAt = nil return true, nil }