criteo-cookbooks / wsus-client

Chef Cookbook to install and configure client for Windows Server Update Services (WSUS)
Apache License 2.0
15 stars 19 forks source link

Updated force update detction #17

Closed jboeshart closed 8 years ago

jboeshart commented 9 years ago

Changed the 'Force Windows update detection cycle' resource from an execute to a powershell_script as it was failing with -2147024891(access denied) which I'm guessing is due to the execute resource running in ruby instead of a native shell. Also removed the c:\windows\sysnative prefix, as it should get resolved properly when ran through a Powershell shell.

hh commented 9 years ago

This will be awesome. Thank you.

Annih commented 9 years ago

Hello @jboeshart Are you sure it fixes the probleme, or just hides the faulty exit code? :) I'm not really confortable with this solution, so I'll try to reproduce your issue and investigate this error code.

Sauraus commented 9 years ago

IMHO the actual problem with the original implementation is the hard-coded path to wuauclt.exe.

On 2012R2 wuauclt.exe resides in c:\windows\system32 and the folder c:\windows\sysnative does not even exist on the system

jboeshart commented 9 years ago

@Sauraus see my comment in issue 16 [1] with details on the sysnative directory alias.

[1] https://github.com/criteo-cookbooks/wsus-client/issues/16#issuecomment-144817450

Annih commented 8 years ago

Hello @jboeshart

Can you confirm that you only encounter your issue when running chef via winrm?

Thanks!

jboeshart commented 8 years ago

I'm having a few issues getting it to run via chef-solo from the command line, but when I run chef-solo locally on the system in Powershell it does not throw the error. Seems like it may be related to the way things are handled via WinRM, but I'd still say that it's a better process to use the powershell script resource, as it also has the benefit of resolving the wuauclt.exe path properly without hardcoding it.

Annih commented 8 years ago

I'ld disagree on your last point, and close this PR. Just to give you more explanation:

jboeshart commented 8 years ago

So what's your suggestion to get this to work when using WinRM in any capacity?

I don't think that the Powershell script resource works because of the computed path, rather I think it's because you get a more functional shell than you get with Ruby shellout. It may not be better, but it works.

Do you have specific concerns around using the powershell script to run that command? This PR really gets past some basic functional errors and allows the usage of standard tools (knife winrm, Test Kitchen). I don't quite get the objections to fixing these issues.

Annih commented 8 years ago

Hello @jboeshart

Sorry for delay! I think the frameworks using WinRM as transport should take care of its limitation, and provide a solution to avoid that kind of error. That's why I support the following issues:

BTW for test-kitchen as mentionned in test-kitchen#876, there is a solution working quite fine yet: the chef-zero-scheduled-task provisioner

Sorry for not providing you a better solution at the moment!,

Regards,

Annih

jboeshart commented 8 years ago

Not to be difficult here, but I still disagree. Why would you want to rely on a separate Ruby gem to work around an issue that can be properly handled (and arguably a more reliable solution) with native Chef client functionality? This introduces the risk of adding more dependencies and more opportunities for defects to impact the functionality of this cookbook.

This issue can be solved with some very minor and functionally equivalent changes to this cookbook, I really don't understand the objections to this PR. Are there technical issues that you don't feel this suggested methodology should be used? If there aren't, can we get this merged so that we can get a functional cookbook for all use cases?