criteo-cookbooks / nssm

Chef cookbook to install the Non-Sucking Service Manager
http://nssm.cc
MIT License
13 stars 15 forks source link

Always trying to start service on every chef run #19

Closed axelrtgs closed 7 years ago

axelrtgs commented 7 years ago

the code

service new_resource.servicename do # ~FC021 action [:start] only_if { new_resource.start } end

in the provider always executes on every run. my assumption is that it would only start the service on create and not every run, basically following the idempotence of the service existing and not the service being started.

i have worked around it by setting start to false and using a notification to follow the bahavior outlined above.

if it was intended to ensure the service was always running then feel free to close this issue with a comment letting me know. i just thought to bring it to your attention

dhoer commented 7 years ago

I build AWS AMIs using the windows cookbooks, so I really haven't tested the Windows cookbooks for idempotency. Do you have a fix in mind for this to make this idempotent?

axelrtgs commented 7 years ago

probably a terrible idea but theres the service installed check and only start it if service is not installed (meaning initial setup) and that should prevent it from running on every chef run. it wouldnt really help in cases where config changed since theres no method to restart the service if configs change.

Only other idea i can think of is take the start out of the main code and include a notification to the service resource only if the start flag is true. that way the notification will only run if the previous resource has changed or done something which would allow for restart on change configs as well. im just not sure you can notify a service resource without knowing the name of the service in advance. I've seen other custom resources do that but the service name is known (ex: Apache2)

Annih commented 7 years ago

I'm currently working on a refacto of the resource, and I definitely think the start property should be removed.

In my temporary branch (soon available as PR) I implemented a basic start (and stop) action to ensure the service is running at every chef run.

IMHO if one want to start/restart only when something change and having this behavior controlled by the a conditional attribute, one can do something like this

service 'my_service'
nssm 'my_service' do
  action :install
  notify :restart, 'service[my_service]' if node['xxx']['nssm_should_restart']
end
axelrtgs commented 7 years ago

so from what i understand you are removing the start from the resource and we should control it using a notification to a service resource?

ill check out your PR when its up and review and add any comments/suggestions if i have any once your work is complete.

thanks for maintaining this cookbook and being very responsive :)

Annih commented 7 years ago

Hum finally I think we could keep the start property in addition to the start action. As you said, the start property is here to indicate the service should be setup as "started".

I'm preparing a PR to fix this behavior.