Skip to content

MODULES-7734 Detect installed features with value greater than 1 #274

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

Merged
merged 1 commit into from
Jan 7, 2019

Conversation

GrammatonKlaric
Copy link
Contributor

Add logical OR to registry subkey check for installed features

@glennsarti
Copy link
Contributor

Thanks @GrammatonKlaric for the PR. These numbers look suspiciously like a bit mask. I don't suppose you have any information about how to install SQL Server and get the DWORD of 4.

@GrammatonKlaric
Copy link
Contributor Author

GrammatonKlaric commented Sep 10, 2018

@glennsarti I do not know how to get the SQL install to do this, but in our organization lately it seems that 1 out of 4 at a minimum will present this error in at least one of the features. I do know that you can manually change the registry to 4 and replicate the exact error I am seeing. You can make this change on any or all of the instance features at "HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Microsoft SQL Server\MSSQL12.MSSQLSERVER\ConfigurationState"

This will reliably create the condition that I am seeing. SQL will be fully functional, but the sqlserver_instance resource will not read the values (and facterdb will show an incorrect features array).

Also, you can correct the condition by simply changing the 4 back to a 1 and then the features array will report correctly and the resource will behave as expected.

@GrammatonKlaric
Copy link
Contributor Author

GrammatonKlaric commented Sep 11, 2018

I have just discovered a server with values of 2 in the registry (I suspected it would be more than 1 or 4, this is confirmation). Since this registry value is stored as a string, perhaps we should convert to integer and then compare for values greater than 0).
I am conducting research on the server with values of 2 and seeing if the instance is still operational and installed and if the same resolution will work.

@GrammatonKlaric
Copy link
Contributor Author

I just noticed that this occurred on a server that had an updated version of SQL 2014 installed first, then and older version with older patches were pushed up via hieradata. This would not happen in our production environment, so I'm not sure if the value of 2 is relevant or not.

@GrammatonKlaric
Copy link
Contributor Author

GrammatonKlaric commented Sep 12, 2018

FYI, I am going to submit a new Pull Request with better code. This is my first contribution to GitHub, so I'm not sure if I should close this PR or not (for comments/history).
Update: I amended the code instead of submitting a new PR. Instead of a logical OR, I have tested converting to integer and doing a numerical comparison. Please review.

  * Any DWORD value greater than 0 is acceptable to verify installation.
@glennsarti
Copy link
Contributor

Sorry @GrammatonKlaric I've been out for a while. You can close this PR but you can still reference it in the comments of the new one.

@GrammatonKlaric
Copy link
Contributor Author

Instead of making a new PR, I amended this commit with the new code. I figured this was okay since no one is using it yet. Typically I don't code this way, but I am dealing with an operational issue, so the first code was a quick patch, but the new code is ideal and has been tested.

Please use this PR.

@GrammatonKlaric
Copy link
Contributor Author

@glennsarti Hi Glenn, looks like we lost touch for a while. Is there any reason why this PR cannot be accepted? I have this code running in my organization on hundreds of SQL servers now with no issue.

@glennsarti glennsarti merged commit cb0b912 into puppetlabs:master Jan 7, 2019
@glennsarti
Copy link
Contributor

Thanks @GrammatonKlaric Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants