voxpupuli / puppet-windowsfeature

Library that uses ServerAdministration api that comes with Windows Server 2008 and Windows Server 2012 to add / remove windows features
https://forge.puppet.com/puppet/windowsfeature
MIT License
30 stars 49 forks source link

Restart Parameter causing Corruption #36

Closed ricou84 closed 7 years ago

ricou84 commented 9 years ago

Hello. I had an issue with windowsfeature and the restart parameter. i would like to install SMTP-Server features which depends to IIS. For some reason IIS dont install (some corrupted files In Crypto/machineskeys) manually either automatically. Manually, the windows server manager want a reboot to undo the changes. Automatically, puppet reboot the server and few minutes after the reboot, try to redo the installation of IIS. This leads to a infinite loop reboot.

So what is the behaviour of restart => true| false parameter about undo the changes? if i put restart => false parameter, will the server reboot in case of undo changes ask by servermanager?

cyberious commented 9 years ago

I actually prefer to use the reboot module for these instances as it is a little easier to detect, however it can get into this same scenario if the needs reboot flag in the registry does not get cleared out. If it is not behaving properly I don't believe this is an issue we can take care of in the module but possibly an issue with the Powershell Cmdlet that it is using.

TraGicCode commented 7 years ago

I Honestly think this parameter should be removed and a recommendation should be made to use the reboot module instead. Since the restart parameter essentially allows powershell to reboot the machine if a reboot from a feature install/uninstall is needed. This is not very 'graceful' compared to the reboot module. The reboot module has smarted in it to actually tell the puppet run hey a reboot is needed i need to gracefully halt and postphone application of the catalog until the next run.

Can you provide any input on this @glennsarti or @jpogran

TraGicCode commented 7 years ago

@bastelfreak @cyberious I would really like to deprecate the restart parameter in favor of using the reboot module based on my above comment. Can you provide feedback please.

jpogran commented 7 years ago

I agree with @TraGicCode that the parameter does not help the user, as it does not participate in the graph and won't schedule a reboot in accordance with the manifest since it is outside of Puppet. Removing the parameter would constitute a breaking change, deprecation and a warning in the help text might go a long way.

TraGicCode commented 7 years ago

Thanks for the input @jpogran I think i can get this rolling with a semver compliant deprecation notice + future removal.