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

SqlSetup: Sets a lot of settings on the sql resource but doesn't enforce consistency #37

Open
TravisEz13 opened this issue Feb 20, 2016 · 11 comments
Labels
enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community.

Comments

@TravisEz13
Copy link
Contributor

xSqlSetup sets a lot of settings on the sql resource but doesn't enforce consistency. In test, it checks if all the features are there and if they are it says it's good.

@kwirkykat kwirkykat added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. enhancement The issue is an enhancement request. and removed bug The issue is a bug. labels Aug 2, 2016
@johlju johlju changed the title xSqlSetup sets a lot of settings on the sql resource but doesn't enforce consistency xSQLServerSetup: Sets a lot of settings on the sql resource but doesn't enforce consistency Dec 23, 2016
@johlju
Copy link
Member

johlju commented Sep 19, 2017

From discussions in different issues in this repository, the conclusion seems to have come to consensus (but not written in stone) that SqlSetup will go against normal DSC resource praxis in the sense it will not enforce consistency of properties set during setup. SqlSetup will only make sure that each component is installed. It will not validate that the component is installed correctly (but I can see the resource being extended to do that). Neither will it validate it the component is working correctly, because if it doesn't, then there are more serious problems, which might only be resolved by reinstall (from the perspective of the resource). That could lead to even worse problems like loss of data.

Properties set by SqlSetup resource should be considered as default values, regardless if they were set by the user or by setup.exe, and will be managed by separate resources in this module. Those other resources will enforce consistency.
In essens SqlSetup will only do what is possible to do with SQL Server setup.exe.
If this resource would enforce consistency on all the properties is can set or touch, I think it would become an mammoth of a resource, making most other resource obsolete.

We should reflect this in the documentation.

@johlju johlju changed the title xSQLServerSetup: Sets a lot of settings on the sql resource but doesn't enforce consistency SqlSetup: Sets a lot of settings on the sql resource but doesn't enforce consistency Dec 22, 2017
@Francois-Rousseau
Copy link

I @johlju . I would like to ask you about an issue that I have with consistency in setting my SQL with DSC, and, based on what you said in this post, I would like to know how you think we can get rid of this issue the "right way". Actually, it is pretty easy to "get DSC to crash" for SQL Server. Here's the steps :
1- Install SQL via DSC
2- Stop the SQL server service (or SQL Browser either)
3- When the service is stopped, try to launch the DSC config again. It will fail and here's what I think may be an issue:
I can see in the logs that the SqlSetup ressource is restarting the service, but in my lab, the installation fails:
PowerShell DSC resource MSFT_SqlSetup failed to execute Test-TargetResource functionality with error message: System.InvalidOperationException: Failed to connect to SQL instance 'DSC\SQL'.
Immediatly after I get this error, I can see that the service is started. So I know that the resource is restarting the service. But, it fails to connect to the instance, and I think it's just because it's missing a few seconds between the command that start the service and the command that try to connect to the instance. If I launch it a third time, it passes because the service is running (issue?)

Next, I tried it with the SQL service stopped, but Disabled:
PowerShell DSC resource MSFT_ServiceResource failed to execute Set-TargetResource functionality with error message: Failure starting service 'SQLAgent$SQL'. Please check the path '"D:\Program
Files\Microsoft SQL Server\MSSQL11.SQL\MSSQL\Binn\SQLAGENT.EXE" -i SQL' provided for the service. Message: 'Exception calling "Start" with "0" argument(s): "Cannot start service
SQLAgent$SQL on computer '.'."' (another issue?)

In terms of consistency, is SqlSetup ressource should set the service to Manual or Automatic and restart it? (the information about the services startup type should be in the configuration data, and are currently in process to be added to the SqlSetup ressource (issue #1165)

Same thing if the service is Paused. Nothing work because when a service is paused, it can not be started. It should be resumed.

The only way that I have right now to face this issue is to start the services a Service ressource before launching SqlSetup, but I causes me another issue when SQL is not installed. So right now the real only way I have is to have a SetScript checking if the services are present, and if they are, start them. That's... not cool.

Do you think they're real issues and that SqlSetup should be ajusted to face them?
Thank you!

@johlju
Copy link
Member

johlju commented Jul 5, 2018

I wonder if we can use Service resource in PSDSCResources or xService in xPSDesiredStateConfiguration to set the state of the service prior to running SqlSetup (or any other SqlServerDsc resources). But if the service does not exist Service resource will probably fail 🤔

So I could see that optional parameters to control the state of the services could be added to SqlSetup, or a SqlService resource that will not fail if the services does not exist yet. 🤔

After discussions in their repo it was concluded that SqlSetup should only do what setup.exe can do. So to continue with that approach then we should create a new resource like SqlService. But this could be an exception to the rule, to have a ServiceState optional parameter in the SqlSetup resources.

@mdaniou
Copy link
Contributor

mdaniou commented Jul 5, 2018

Fyi, I am indeed working on the issue #1165 .

I am at the testing step and I encountered the error you are reporting with the new parameters SqlSvcStartupType and AsSvcStartuptype when set to "Disabled" (still have to test other combinations though). I have decided to not allow the "Disabled" value for the two parameters. only "Manual" and "Automatic" are allowed.
I thought that it was not the goal of this issue to address this flaw. Hope you will agree.

@Francois-Rousseau
Copy link

I @mdaniou I totally agree with you. The issue #1165 you are working on have not the goal of addressing this flaw. I also agree that you do not permit "Disabled" state as it does not make sense to me to have a SQL Server disabled by DSC.

I @johlju I can already answer to you about the Service from PSDSCResources resource because I tried it and here are the issues that I see with it 👍
1- If the SQL server is not installed, the ressource fails
2- If the SQL instance is on a cluster, the Service resource will fail on all nodes that are not the current active node.
3- I also get a bug with this Service resource because I can't use account with no password (like "NT Service\MsDtsServer110") because we need to pass a credential and we can't create a credential with no password. I tried with dummy password and didn't work either. (I will open an issue in the Service Resource if necessary for this one)

Based on that, I think that a new resource like SqlService should be created, with the possibility of do the work on a cluster (like SqlServiceAccount resource with ServerName property) (maybe this can be added to the project or do you want me to create a new issue for this one?) This New SqlService feature should handle all parameters of a service (State, Start Mode, User Logon.)

@johlju
Copy link
Member

johlju commented Jul 6, 2018

@mdaniou Totally forgot about you were working on this, sorry! 🙂 I suggest that you allow to use the Disabled startup type in SqlSetup, and then we create a new resource that handle the consistency of that property (like all other properties that SqlSetup (setup.exe) sets). In the meantime, until the new resource exist, using 'Disable' will have the same effects as if one would use it with setup.exe, or if someone manually disables a service after it is installed regardless of startup type on the setup.exe argument. Is this inline with what you are thinking, and will that work with what you working on now?

@Francois-Rousseau Could you please add a new issue for a resource proposal for SqlService?

@mdaniou
Copy link
Contributor

mdaniou commented Jul 7, 2018

@johlju I am afraid that this new resource wouldn't created anytime soon.
My idea was to not allow Disable for now and when the new resource would be created, the only change to apply in the resource SqlServerSetup would be to add it in the list of choices for these parameters. It fairly easy.

@johlju
Copy link
Member

johlju commented Jul 9, 2018

@mdaniou and add a unit tests for the option too, but as you say, not to difficult. We can add Disabled later - we should add a separate issue for that though, so it's tracked.

@mdaniou
Copy link
Contributor

mdaniou commented Jul 9, 2018

@johlju Now that I am going forward with testing my changes, it appears that the dsc is failing (well, obviously ...) to performed the final check as long as the SQLENGINE is not on "Automatic" ... Same for AS.

So now it doesn't make sense to have SqlSvcStartupType and AsSvcStartupType available for configuration and have for only choice Automatic.
I guess that I will then allow the three values Disable, Manual and Automatic. The new resource will have to come after to sort this out.

What do you think ?

@johlju
Copy link
Member

johlju commented Jul 9, 2018

It’s enough that SqlSetup add the argument to setup.exe with the value the user set in the configuration of SqlSetup, either if that is Disabled, Manual or Automatic. Agree that a new resource should handle the consistency of that property. Since SqlSetup should only do what setup.exe is able to do.

@mdaniou
Copy link
Contributor

mdaniou commented Jul 9, 2018

Thanks for your answer !
I'll carry on this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community.
Projects
None yet
Development

No branches or pull requests

5 participants