-
Notifications
You must be signed in to change notification settings - Fork 196
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
Bug: Found an issue with Azure SQL Server where it doesnt recognize existing SQL servers #782
Bug: Found an issue with Azure SQL Server where it doesnt recognize existing SQL servers #782
Conversation
@@ -106,22 +106,21 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, | |||
instance.Status.SpecHash = hash | |||
} | |||
|
|||
if instance.Status.Provisioning { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line was blocking the reconciler from "attaching" to existing SQL Servers - that was the desired functionality at one time, is it not anymore @jananivMS ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frodopwns fixed this as part of his PR here - #741 and I tested this. This scenario worked fine, so wondering what was the scenario that you saw the failure in, @WilliamMortlMicrosoft ? Status.Message would tell you if this reconcile doesnt happen. One of the cases where this happens is if the SQL server is present but the secret is not present in kube/keyvault. This is because we dont know the admin creds of an existing server and for it to work correctly we need to have the person externally creating the SQL server to also populate the secret.
The scenario #2 only works if you create a secret with the username and password for the sql server you created manually. Also, when you say "update your yaml" are you talking about patching the first sql server you created or changing the yaml and then deploying a new server? |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
What SQL server wasn't doing is recognizing that server existed... This used to be an important criteria... happy to pull the PR @frodopwns if you think it isnt needed |
@WilliamMortlMicrosoft reviewer should just verify they can repro |
@WilliamMortlMicrosoft is this on master? I can give this a try as I did the testing for the different scenarios in Erin’s PR and see if it reproduces and then work with you on if we need this PR. |
@WilliamMortlMicrosoft Is this basically what you were seeing?
|
yes |
@jananivMS said working as expected - so closing the PR |
closes #783
The Azure SQL Server operator should detect if a SQL server exists in the selected resource group and then complete reconciliation. The code was blocking that functionality. This bug fix addresses that issue.
To test: