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

(MODULES-6022) Make SQLServer Instance Idempotent Again #250

Merged
merged 2 commits into from Nov 28, 2017
Merged

(MODULES-6022) Make SQLServer Instance Idempotent Again #250

merged 2 commits into from Nov 28, 2017

Conversation

michaeltlombardi
Copy link
Contributor

@michaeltlombardi michaeltlombardi commented Nov 21, 2017

Fix issues with idempotency as a result of the changes in 2.0.1 to retrieve facts information from registry instead of WMI. Also updates acceptance tests to validate idempotency. Increases the test tier for changed tests to high per @ThoughtCrhyme's suggestion.

Supersedes #249 which was accidentally based on a branch on upstream instead of my fork. 🤦‍♂️

Prior to this commit, applying a manifest that ensures a SQL
instance is installed with any features will always attempt to
uninstall and reinstall those features on subsequent applies.
This was due to a bug in the code returning discovered features
nested inside of a needless array. This commit rewrites the logic
for retrieving the instance features to concatenate them into an
array instead of mapping them, preventing the array-of-arrays
problem entirely.
Prior to this commit, the acceptance tests for sqlserver instances
do not verify idempotencies for the applied manifests. This allowed
us to ship revisions of the code which passed the acceptance suite
but were not idempotent, including critical failures such as causing
each run to attempt to uninstall and reinstall all instance features.

This commit updates the logic of the ensure_sqlserver_instance
helper function to execute the specified manifest twice; once to
test for failures when applying and a second time to ensure no
changes are attempted when the manifest is re-applied. It also
raises the test tier for tests which rely on this function to
the high tier until after the next release.
@michaeltlombardi
Copy link
Contributor Author

This should be in a reviewable state now, but I'm considering adding a MAINT commit to update the other acceptance tests to check for idempotency as well. Kicking off this version in adhoc though.

@michaeltlombardi michaeltlombardi changed the title Ticket/master/modules 6022 fix instance discovery (MODULES-6022) Make SQLServer Instance Idempotent Again Nov 21, 2017
Copy link
Contributor

@glennsarti glennsarti left a comment

Choose a reason for hiding this comment

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

:shipit:

@michaeltlombardi
Copy link
Contributor Author

Passed adhoc last week, forgot to update here.

@jpogran jpogran merged commit fea5ce4 into puppetlabs:master Nov 28, 2017
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.

None yet

3 participants