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

windows_task will not 'notify' other resources #271

Closed
spuder opened this Issue Aug 24, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@spuder
Contributor

spuder commented Aug 24, 2015

If you have a scheduled task, and you attempt to delete it with ':delayed' it will never be deleted.

Example

  windows_task 'foobar' do
    user "example.com\\bob"
    command  "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe -noprofile -noexit -executionpolicy Bypass -File c:\foo.ps1 '"
    action [:create,:run]
    notifies :end, 'windows_task[foobar]', :delayed  #TODO This strangely never gets deleted
    notifies :delete, 'windows_task[foobar]', :delayed #TODO This strangely never gets deleted
    run_level :highest
    force true
  end

Windows 2012 R2
Chef 12.4.1
Windows cookbook 1.38.1

@spuder

This comment has been minimized.

Contributor

spuder commented Aug 24, 2015

Two work arounds:

If the script that the task runs is extremely quick, you can do this.

    action [:create,:run,:delete]

However this has a huge limitation that if your script takes more than a second or two to run, then it will be deleted before it finishes.

A better workaround is this:

windows_task 'foobar' do
  command "echo hello"
  action [:create,:run]
#  notifies :delete, 'windows_task[foobar]', :immediately
end

ruby_block 'null' do
  guard_interpreter :powershell_script
  block do
    puts "doing nothing"
  end
  action :run
  #notifies :end, 'windows_task[foobar]', :immediately
  notifies :delete, 'windows_task[foobar]', :delayed
  only_if "schtasks.exe /Query /TN foobar"
end

It seems that :create and :run on a scheduled task don't set updated_by_last_action to true. This causes any notify actions on the resource to not run. By putting the notify on another resource that is running every time, it will properly get executed.

@chrisgit

This comment has been minimized.

chrisgit commented Feb 18, 2016

Hi, my response is late but I hope this helps someone out.

If the windows_task resource task_name attribute is not specified then the task_name is defaulted to the resource name. Therefore in your example windows_task 'foobar' is the name of the resource and also the task_name.

In load_current_resource the task_name is changed (see below)
pathed_task_name = @new_resource.task_name[0, 1] == '\\' ? @new_resource.task_name : @new_resource.task_name.prepend('\\')

The @new_resource.task_name.prepend('') is adding two slashes to the @resource.task_name. Normally in Ruby when the underlying object is changed a method is marked with a bang to signify warning, it's not obvious but prepend appears to change the underlying object.

When the windows_task LWRP has finished executing if there were any actions the Chef::Runner will try and locate the resource in the resource queue to establish whether it should also action any notifications. My belief is that the name of the resource has changed and now includes two backslashes (), the resource cannot be found in the resource queue, which is why the notifications are failing.

Can you try your code specifying the task_name and making it different from the resource name, e.g
windows_task 'install_foobar' do task_name 'foobar' user "example.com\\bob" command "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe -noprofile -noexit -executionpolicy Bypass -File c:\foo.ps1 '" action [:create,:run] notifies :end, 'windows_task[install_foobar]', :delayed notifies :delete, 'windows_task[install_foobar]', :delayed run_level :highest force true end

@spuder

This comment has been minimized.

Contributor

spuder commented Feb 22, 2016

Thanks for the response. I see now that the reason this happens is because the resource name is not the same as the task name.

So basically the following will fail to send notifications

windows_task 'chef ad-join' do
  user 'SYSTEM'
  command 'chef-client'
  notifies :create, 'file[c:/notifytest.txt]', :immediately # Notification fails
  action :create
end

Where as this will work properly

windows_task 'chef ad-join' do
  task_name 'foobar'
  user 'SYSTEM'
  command 'chef-client'
  notifies :create, 'file[c:/notifytest.txt]', :immediately # Notification works
  action :create
end

I still think a change needs to be made to the cookbook since this is an easy trap to fall into (It bit me twice). The task_name parameter needs to be made mandatory.

@chrisgit

This comment has been minimized.

chrisgit commented Feb 22, 2016

Thanks for confirming. It's a bug in the windows_taks resource, the resource name gets changed in load_current_resource hence why Chef::Runner can't locate resources in the notification queue. I can't make any fixes at the moment as we don't have a CLA.

The error is to do with the use of the prepend method, this changes the underlying object, e.g

x = 'hello'
y = x.prepend('')
x #=> \hello
y #=> \hello

By the way I've only spotted some of the more involved resource issues thanks to pry or by applying a monkey patch with break points.

@spuder spuder changed the title from Windows task wont delete if delayed to windows_task will not 'notify' other resources Feb 23, 2016

@spuder

This comment has been minimized.

Contributor

spuder commented Mar 4, 2016

If i'm understanding correctly, String.prepend in ruby creates another object. I don't quite see that.

irb
2.2.0 :013 > puts foo.object_id
70161858505360
2.2.0 :014 > puts foo.prepend("\\")
\\abcd
2.2.0 :015 > puts foo.object_id
70161858505360

I"m guessing a solution might be to change this line:

 pathed_task_name = @new_resource.task_name[0, 1] == '\\' ? @new_resource.task_name : @new_resource.task_name.prepend('\\')

to something like this?

if @new_resource.task_name[0, 1] == '\\' do
  @new_resource.task_name
else
  new_name = "\\#{@new_resource.task_name}"
  @new_resource.task_name = new_name
end

or perhaps this

 pathed_task_name = @new_resource.task_name[0, 1] == '\\' ? @new_resource.task_name : @new_resource.task_name.insert(0,'\\')

This is just a stab in the dark. I'll need to do more testing. I do have an CLA so if this works, i'll open a pull request.

@mwrock

This comment has been minimized.

Member

mwrock commented Mar 4, 2016

wow. You'd think prepend should be prepend!.

mwrock added a commit that referenced this issue Mar 4, 2016

@spuder

This comment has been minimized.

Contributor

spuder commented Mar 4, 2016

👍 Manually verified against edee5a7 confirmed working

@mwrock mwrock closed this in edee5a7 Mar 9, 2016

mwrock added a commit that referenced this issue Mar 9, 2016

Merge pull request #348 from chef-cookbooks/task
fixes #271 making notify work for windows_task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment