-
Notifications
You must be signed in to change notification settings - Fork 270
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
commands on path are not available after running windows_path #111
Comments
I have this same problem |
I work around that bug by instead of updating the path I put a .bat file for the command I need in in the location of the chef-client.bat. Which before chef-client converge is already on the path. It is a solution that allows you to move forward unless you are trying to change environment variables other than PATH. My use case is for things like git. Other chef recipes depend on git being on the path and if you are installing git with chef then you are kind of Sol. Unless you want multiple converges required for a successful converge. |
This should be resolved in the latest release v1.34.6 |
+1 Using 1.36.1. The path variable is being updated, but a reboot is required to activate all the changes. Feels like an API call is being missed to notify all other processes like explorer. Hitting this when installing git. and java |
@carpnick what version of chef? |
@jdmundrawala chef-client 11.16.4 |
@carpnick Can you send me a simple recipe that fails and its cookbook dependencies/versions |
@jdmundrawala |
@carpnick a gist is fine. if it's short, just dump it in here and surround it with ``` blocks |
@jdmundrawala , @btm This is on a 2k8 R2 box with SP1 installed. *Installs Java and Git metadata.rb: Actual in use versions: |
I just switched from using setx on the command line via an execute resource to using the windows_path resource and noticed the same behavior. Setx will make the env var change available to all new processes without requiring a reboot (or perhaps just a re-login), while windows_path requires a reboot. So there's a way to do it on Windows. Perhaps I'll fire up Spy++ and see what setx is doing that's different. |
It seems the way Chef handles notifying the system of an env var change works correctly. This code example has the same behavior as setx (for system vars), meaning the new env var is immediately available to new processes without needing to relogon. require 'chef/mixin/windows_env_helper'
require 'win32/registry'
class TestEnv
include Chef::Mixin::WindowsEnvHelper
def set_env
Win32::Registry::HKEY_LOCAL_MACHINE.open(
'SYSTEM\CurrentControlSet\Control\Session Manager\Environment', Win32::Registry::KEY_WRITE) do |reg|
reg.write('CHEF_TEST', Win32::Registry::REG_SZ, 'bar')
end
broadcast_env_change
end
end
TestEnv.new().set_env It feels like the problem is somewhere in the WMI/OLE code we're using. I found the SO question and this MSDN article useful. |
Using 1.36.1. The path variable is being updated, but a reboot is required to activate all the changes. |
@smurawski @adamedx I'm guessing we could use ffi to call SendMessageTimeout with WM_SETTINGCHANGE, and that might help? https://msdn.microsoft.com/en-us/library/windows/desktop/ms644952(v=vs.85).aspx https://msdn.microsoft.com/en-us/library/windows/desktop/ms725497(v=vs.85).aspx |
@btm, I thought we were already doing this. This thread is a little confusing and its hard to get what the gap is between desired and expected behavior. It's pretty normal for apps not to pick up changes like this without a restart from explorer, but maybe the assertion is that we're not even meeting that bar. |
@btm Might need to send the message once for Unicode and once for ANSII. Something to look into anyway. @adamedx I noticed the issue in cmd.exe where env var changes didn't show up even after restarting the process, but did show up after a reboot. My coworker @sweitzel74 and I may have just found a non-obvious bug in the Windows cookbook Path resource if used more than once in a Chef run. |
I have a customer using chef-client to install, say, python or svn, but |
@pburkholder it might be. A workaround is to simply update the current process's environment within the recipe. But I have to say I don't really get why we hit this for anything launched from chef-client itself. Windows_path sets the environment for the current process, so child processes should inherit: https://github.com/opscode-cookbooks/windows/blob/master/providers/path.rb#L40. @jdm, am I missing something obvious as to why the update of the ENV hash by windows_path doesn't prevent the bug? |
@adamedx If I read @pburkholder 's question correctly, he's asking something different than windows_path. In that case, changes to the path via msiexec aren't propagated back to the ruby process running chef, which is by design (of the Windows process model). |
Oh, and this is another reason why chef-client shouldn't run as a service on Windows.. Runs via task scheduler will pick up changes much more quickly (e.g. the next time they run rather than the next time the service gets restarted). |
@smurawski, if the issue is that we're not picking them up in the service context, that's something we could fix -- we could have chef-client explicitly refresh its environment block when a Chef run starts. As far as whether we should run as a service, I am also biased toward task scheduler. But reality is users really wanted a "service." Until we have a way of using chef-client via task scheduler that "looks" like a service, we'll get that feedback. Maybe we could make a service whose job is to enable a scheduled task for chef at startup, and disable it on stop... :) |
There's no reason you couldn't always execute Chef-Client via a scheduled task whether from a Windows service or the command line. Vagrant always executes Chef-Client via a scheduled task, but mostly to get out of the WinRM context. |
I would argue that using the chef service (with or without a scheduled task) should be the default way chef-client is run on Windows. Gets rid of the WinRM context issue, via winrm calls as well. |
The winrm context has nothing to do with it. Running in a service context has its own issues. Chef-client does not operate as a service, it is a task that runs periodically, which is the point of scheduled tasks. Working around it to make a "service" make sense (whether refreshing environmental variables or whatever) is the wrong hard problem. But we are deviating from the core issue here (my fault) of environmental variable refresh. In testing with 11.16.4 and 12.3.0, adding paths seems to work as desired, so if there is a repro of the bad behavior, I'm happy to try to resolve it. |
So on our (ChefCo) internal default-windows-2008r2-standard vagrant image windows_path is suffering from this behavior. I'm not even concerned with future resources in the same chef-client run. I can switch to the GUI and fire up cmd or powershell and the path is wrong. Then when I open the system settings in the GUI the Path setting is correct. After closing the GUI I can open up powershell and cmd and it works. I did not have powershell or cmd shells open when I ran test-kitchen, its not the trivial problem. Just looking at the setting in the GUI tickles and API call which chef is missing. |
WRT:
We are not meeting that bar. Replication:
|
@lamont-granquist Excellent description of the exact problem! |
Not sure if you have to call it twice or not, but the correct string to have shells pick it up is likely one of these two:
|
On Linux using C, L"Environment" turns into this:
That is reproducible with:
But I see references to windows using 16-bit wide characters which would presumably be UTF_16LE, so I think thats the encoding you need to use? |
right, and you would need to call SendMessageW instead of SendMessageA |
So this is worse. I just tried to replicate the bug in the virt just running chef-apply directly and it updates the environment just fine as far as i can see. |
using powershell? |
AFAIK both the ANSI and Unicode methods need to be called to cover all the possible listeners. Just calling one is not enough. BTW just closing the computer properties gui (after looking at the environment variables) seems to call at least the unicode method, which is why new shell instances pick up environment variable changes after that. |
So I can write the patch but I don't know how to test it. Have to inject it into test-kitchen and I don't think the github chef-client provisioner thingy works with windows? |
^ if anyone can test that code on a replication case that'd be useful |
@lamont-granquist can you use the appbundle provisioner and point it at that branch? |
oh, you say it does not work. Hmm, Ill try it out later |
As it stands - Server 2012 and 2012 R2 both pick up the change in new processes. Server 2008 R2 does not. I'll test @lamont-granquist 's PR on Server 2008 R2. |
Seems to be a problem with test-kitchen/kitchen-winrm/winrm and the way that chef-client is getting invoked.
|
@smurawski For me, 2008r2 passes the 'test/integration/path/pester/minimal.path' test. Does that mean the problem is resolved in 2008r2, or that I'm testing wrong, or that the tests are insufficient? |
@pburkholder on the first run through on a fresh machine? or on a second pass? If you do a second verify, it may have picked up the changes. |
The 2008R2 'success' was a false negative. I was running the 'tasks' test suite instead of the 'path' suite since I had some older gems in place. Win2008r2 does indeed fail the paths, as expected. |
Is there any hope of getting this issue fixed in 2008r2 or should we tell people just to work around it? It is past Mainstream Support End Date (https://support.microsoft.com/en-us/lifecycle/search/default.aspx?sort=PN&alpha=Windows%20server&Filter=FilterNO). |
I am incorrect -- it seems Chef supports 2008r2 until end of extended support, per https://github.com/chef/chef-rfc/blob/master/rfc021-platform-support-policy.md, so it would seem we should try to address. |
I was able to reproduce the PATH issue on Windows 2012r2 when chef-client 12.4.1 is running as a service. Repro steps:
You'll see that the chef-client run during bootstrap will succeed. Logging into the box and running chef-client will also succeed, even from a shell that doesn't have Any Chef run triggered by the Chef Client Service will fail with Restarting the chef-client service fixes the problem. |
The root cause of this issue is currently unknown. We're doing WM_SETTINGCHANGE broadcasts correctly now AFAIK. Someone needs to research a solution, my attempt failed to solve the problem. |
In the case of anything hosted by the windows service control manager, seems like those processes may not typically handle system broadcast events. See the answer section of this link for reference: http://stackoverflow.com/questions/22094727/must-reboot-after-changing-path The chef client service process itself may need to setup an event handler to listen for the WM_SETTINGCHANGE broadcast so it can reload the updated environment state (without just stopping and restarting the service, since that would break an in progress chef run, - if restart was required you would need to save the state of the in progress run and restart the run from the point where the service was reset). |
Any resolution to this? |
I have reproduced a similar issue outside of chef with the following:
|
As of the upcoming Chef 13.4 release we've moved the path resource into the chef-client itself. Since we have a much improved codebase included in chef-client now we won't be continuing to work on the resource in this cookbook, and will eventually remove it altogether. At this time I'm going to close out this issue, and I'd encourage you to try the latest Chef 13 release. If you still have issues please file them against chef itself. |
Chef: 11.14.6
Windows Cookbook: 1.34.2
Node : Windows 2012
I’ve encountered a scenario where after the windows_path resource is ran in a Chef run, other resources can't find items currently on the Windows path. For example the batch resource fails with “STDERR: ‘cmd.exe’ is not recognized as an internal or external command.” If you don’t have the windows_path resource in your recipe or if you run the recipe again, the batch resource can find the cmd command on the path.
The text was updated successfully, but these errors were encountered: