Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sql location issue #890

Merged
merged 14 commits into from
Apr 10, 2020
Merged

Sql location issue #890

merged 14 commits into from
Apr 10, 2020

Conversation

WilliamMortlMicrosoft
Copy link
Contributor

@WilliamMortlMicrosoft WilliamMortlMicrosoft commented Apr 7, 2020

closes #809

Describe the bug
If a SQL server already exists and you want to take it over via the operator you would first create a secret with the credentials for that server then deploy a kube manifest for the server.

If the location in the manifest does not match the location of the exist server the kube resource will fail to provision (status.failedProvision=true).

If the user update the location to be correct the resource should then recover and provision.

what actually happens is an unrecoverable loop

 status:
  failedProvisioning: true
  message: successfully provisioned
  provisioned: true
  requested: "2020-03-24T19:30:58Z"
  resourceId: /subscriptions/70/....
  specHash: 8b2cd8e07d4e98404008c51b8ea629c9605ec0f9b6f73c1c7caa8df33864db60
  state: Ready

To Reproduce
Steps to reproduce the behavior:

  • create sql server via operator,
  • save the secret it creates
  • clear local kube cluster
  • change location of sql server request and redeploy it to kube cluster...should see failed state
  • update sql server request to have correct location
  • now works!!!!

New Expected behavior

after updating the location to be correct, the server should no longer say failedProvisioning and the operator should end reconciliation successfully

@WilliamMortlMicrosoft WilliamMortlMicrosoft added bug 🪲 Something isn't working azure-sql high-priority Issues we intend to prioritize (security, outage, blocking bug) labels Apr 7, 2020
@WilliamMortlMicrosoft
Copy link
Contributor Author

I just need to verify... I'm like 80% sure this will work

@WilliamMortlMicrosoft WilliamMortlMicrosoft self-assigned this Apr 7, 2020
@frodopwns
Copy link
Contributor

doesn't this just need failedProvisioning to be reset to false?

@WilliamMortlMicrosoft
Copy link
Contributor Author

WilliamMortlMicrosoft commented Apr 7, 2020

doesn't this just need failedProvisioning to be reset to false?

@frodopwns I dont believe so, because on here:

https://github.com/Azure/azure-service-operator/blob/master/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go#L194

provisioning is set to false (its also set to false in a slew of other false cases), thus here:

https://github.com/Azure/azure-service-operator/blob/master/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go#L106

will never trigger the true condition... I believe this is your infinite loop.

@WilliamMortlMicrosoft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@frodopwns
Copy link
Contributor

doesn't this just need failedProvisioning to be reset to false?

@frodopwns I dont believe so, because on here:

https://github.com/Azure/azure-service-operator/blob/master/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go#L194

provisioning is set to false (its also set to false in a slew of other false cases), thus here:

https://github.com/Azure/azure-service-operator/blob/master/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go#L106

will never trigger the true condition... I believe this is your infinite loop.

ah yes...that is probably why I punted on it...the refactor was getting a bit long in the tooth and then this bug showed up

@WilliamMortlMicrosoft
Copy link
Contributor Author

@frodopwns @jananivMS I'm going to test this, but its such a corner case that I was wondering if you could both look and give me some early thoughts...

@WilliamMortlMicrosoft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@WilliamMortlMicrosoft WilliamMortlMicrosoft changed the title WIP: Sql location issue Sql location issue Apr 9, 2020
Copy link
Contributor

@frodopwns frodopwns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@WilliamMortlMicrosoft WilliamMortlMicrosoft removed the request for review from jananivMS April 9, 2020 23:09
@WilliamMortlMicrosoft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@WilliamMortlMicrosoft WilliamMortlMicrosoft merged commit 2c99f3d into Azure:master Apr 10, 2020
@WilliamMortlMicrosoft WilliamMortlMicrosoft deleted the sqlLocation branch April 10, 2020 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working high-priority Issues we intend to prioritize (security, outage, blocking bug)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: updating location in sql server when in failed state due to bad location
2 participants