Chef

Service can't depend on config file that restarts service

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 0.7.14
  • Fix Version/s: 0.9.10
  • Component/s: Chef Client, Chef Solo
  • Labels:
    None
  • Environment:

    centos

  • Triage Status:
    Triaged

Description

I have a simple task: start a service with a custom config file. I have very a simple recipe:

service "syslog-ng" do
supports :start => true, :stop => true, :restart => true, :status => true
action [ :enable, :start ]
end

template "/etc/syslog-ng/syslog-ng.conf" do
source "syslog-ng.conf.erb"
notifies :restart, resources(:service => "syslog-ng")
end

But chef will start the service before putting in the config file. So I can make Chef fail simply by doing this:
$ service collectd stop
$ echo xx > /etc/collectd.conf
$ chef-solo
INFO: Starting Chef Solo Run
ERROR: service[collectd] (/home/chef/cookbooks/collectd/recipes/default.rb line 4) had an error:
/sbin/service collectd start returned 1, expected 0
---- Begin output of /sbin/service collectd start ----
STDOUT: Starting collectd: [FAILED]STDERR: Parse error in file `/etc/collectd.conf', line 2 near `<newline>': syntax error, unexpected EOL
yyparse returned error #1
Error: Reading the config file failed!
Read the syslog for details.
---- End output of /sbin/service collectd start ----

Ideally, the order of the entries in the recipe would be reversed, since the collectd service clearly depends on the collectd.conf config file. But if I try switching the stanzas, it gives me an error:

/usr/lib/ruby/gems/1.8/gems/chef-0.7.14/lib/chef/resource_collection.rb:107:in `lookup': Cannot find a resource matching service[collectd] (did you define it first?) (ArgumentError)

Looking at the opscode-cookbook examples, I can see 2 camps:

  • munin and postgres don't attempt to start the service at all, but rely on the change in config files to restart it. (real bad if you already have the right config, or if chef crashes after the config file is installed)
  • nginx starts the service, but doesn't attempt to restart the service when the config file is changed.

Neither one is "correct" as far as I'm concerned. Chef should be happy reversing the stanzas, because that's the correct dependency order.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Dan DeMaggio added a comment - 27/Jan/10 7:07 PM

oops, pasted wrong recipe vs error text. But you get what I mean.

Show
Dan DeMaggio added a comment - 27/Jan/10 7:07 PM oops, pasted wrong recipe vs error text. But you get what I mean.
Hide
Permalink
Joshua Timberman added a comment - 28/Jan/10 9:13 AM

You can reverse the resources, and use subscribes instead of notifies.

template "/etc/collectd.conf" do
source "collectd.conf.erb"
end

service "collectd" do
supports :start => true, :stop => true, :restart => true, :status => true
action [:enable, :start]
subscribes :restart, resources(:template, "/etc/collectd"), :immediately
end

What happens when chef runs is these two resources are added to the collection, and chef knows that during convergence when the specified actions are taken, that when the template is created or updated, to immediately restart the service. If the template doesn't change, then the service is enabled and started, though not restarted.

Show
Joshua Timberman added a comment - 28/Jan/10 9:13 AM You can reverse the resources, and use subscribes instead of notifies. template "/etc/collectd.conf" do source "collectd.conf.erb" end service "collectd" do supports :start => true, :stop => true, :restart => true, :status => true action [:enable, :start] subscribes :restart, resources(:template, "/etc/collectd"), :immediately end What happens when chef runs is these two resources are added to the collection, and chef knows that during convergence when the specified actions are taken, that when the template is created or updated, to immediately restart the service. If the template doesn't change, then the service is enabled and started, though not restarted.
Hide
Permalink
Dan DeMaggio added a comment - 28/Jan/10 2:56 PM

Ah, yes. That works fine for 1 config file. But if I have more than one, it's still broken:

INFO: Creating template[/etc/collectd.conf1] at /etc/collectd.conf1
INFO: template[/etc/collectd.conf1] sending restart action to service[collectd] (immediate)
INFO: service[collectd]: restarted successfully
INFO: Creating template[/etc/collectd.conf2] at /etc/collectd.conf2
INFO: template[/etc/collectd.conf2] sending restart action to service[collectd] (immediate)
INFO: service[collectd]: restarted successfully
INFO: Creating template[/etc/collectd.conf3] at /etc/collectd.conf3
INFO: template[/etc/collectd.conf3] sending restart action to service[collectd] (immediate)
INFO: service[collectd]: restarted successfully

For now, I'm going with this, but it's pretty ugly:

service "syslog-ng" do
supports :start => true, :stop => true, :restart => true, :status => true
action :none
end

template "/etc/syslog-ng/syslog-ng.conf" do
source "syslog-ng.conf.erb"
notifies :restart, resources(:service => "syslog-ng")
end

service "syslog-ng" do
action [ :enable, :start ]
end

On a philosophical note:

  • The fact that every example Chef recipe gets this wrong (and I was bitten by this) violates the "Simple things should be Simple" rule.
  • The docs say subscribes is "the opposite of notifies", so I expected them to be the inverse of each other.
  • the "subscribes .. resource(:template .." line makes it even more clear that :remote_file and :template should be merged into a single :file – The service shouldn't have to care if the config file uses ERB or not.
  • This may not be a "major" bug, but I think it brings up a major issue.
Show
Dan DeMaggio added a comment - 28/Jan/10 2:56 PM Ah, yes. That works fine for 1 config file. But if I have more than one, it's still broken: INFO: Creating template[/etc/collectd.conf1] at /etc/collectd.conf1 INFO: template[/etc/collectd.conf1] sending restart action to service[collectd] (immediate) INFO: service[collectd]: restarted successfully INFO: Creating template[/etc/collectd.conf2] at /etc/collectd.conf2 INFO: template[/etc/collectd.conf2] sending restart action to service[collectd] (immediate) INFO: service[collectd]: restarted successfully INFO: Creating template[/etc/collectd.conf3] at /etc/collectd.conf3 INFO: template[/etc/collectd.conf3] sending restart action to service[collectd] (immediate) INFO: service[collectd]: restarted successfully For now, I'm going with this, but it's pretty ugly: service "syslog-ng" do supports :start => true, :stop => true, :restart => true, :status => true action :none end template "/etc/syslog-ng/syslog-ng.conf" do source "syslog-ng.conf.erb" notifies :restart, resources(:service => "syslog-ng") end service "syslog-ng" do action [ :enable, :start ] end On a philosophical note:
  • The fact that every example Chef recipe gets this wrong (and I was bitten by this) violates the "Simple things should be Simple" rule.
  • The docs say subscribes is "the opposite of notifies", so I expected them to be the inverse of each other.
  • the "subscribes .. resource(:template .." line makes it even more clear that :remote_file and :template should be merged into a single :file – The service shouldn't have to care if the config file uses ERB or not.
  • This may not be a "major" bug, but I think it brings up a major issue.
Hide
Permalink
Toomas Pelberg added a comment - 20/Aug/10 7:03 PM

Maybe this should be done via LWRP's. One LWRP would include several configuration files and then fire only one event if any of its members change after running them all.

Show
Toomas Pelberg added a comment - 20/Aug/10 7:03 PM Maybe this should be done via LWRP's. One LWRP would include several configuration files and then fire only one event if any of its members change after running them all.
Hide
Permalink
Dan DeMaggio added a comment - 21/Aug/10 3:51 AM

I would like to see that code, but I'd argue that's the wrong path in the long term.

I thought the whole point of Chef was that I could make a random change to my config files (or accidentally delete them), and/or stop services, and Chef will 'correct' things for me. That's the mental model users have. But it takes extra-careful (undocumented) design of the recipes to achieve this. Even the OpsCode cookbook gets it wrong more often than not.

So what can we do? I don't know, but I/we can brainstorm:

1) Document the 3-stanza solution ("ugly" solution above). Hit people with rules like "always start your service at the bottom of the file" and "pull out your 'service start' so it comes after all possible recipes that modify the service config."

2) Same as above, but allow a 2 stanza solution (config + notify, then service :start). This requires Chef to support forward references for services.

3) Allow a resource (such as "service action :start") to be run the LAST time it appears in the resource list, instead of the first. (this is a hack which only fixes simple cases)

4) Allow some sort of "multi-pass" on resources

5) I'm sure there are other ideas?

Show
Dan DeMaggio added a comment - 21/Aug/10 3:51 AM I would like to see that code, but I'd argue that's the wrong path in the long term. I thought the whole point of Chef was that I could make a random change to my config files (or accidentally delete them), and/or stop services, and Chef will 'correct' things for me. That's the mental model users have. But it takes extra-careful (undocumented) design of the recipes to achieve this. Even the OpsCode cookbook gets it wrong more often than not. So what can we do? I don't know, but I/we can brainstorm: 1) Document the 3-stanza solution ("ugly" solution above). Hit people with rules like "always start your service at the bottom of the file" and "pull out your 'service start' so it comes after all possible recipes that modify the service config." 2) Same as above, but allow a 2 stanza solution (config + notify, then service :start). This requires Chef to support forward references for services. 3) Allow a resource (such as "service action :start") to be run the LAST time it appears in the resource list, instead of the first. (this is a hack which only fixes simple cases) 4) Allow some sort of "multi-pass" on resources 5) I'm sure there are other ideas?
Hide
Permalink
Toomas Pelberg added a comment - 24/Aug/10 6:38 AM

When users make changes to configuration files - and there are many such interdependent configuration files, they know that they're in the "progress of making changes" and should only restart the service afterwards.

It doesn't have to be LWRP's, but something that allows to group various resources in an similar way.

Show
Toomas Pelberg added a comment - 24/Aug/10 6:38 AM When users make changes to configuration files - and there are many such interdependent configuration files, they know that they're in the "progress of making changes" and should only restart the service afterwards. It doesn't have to be LWRP's, but something that allows to group various resources in an similar way.
Hide
Permalink
Daniel DeLeo added a comment - 27/Sep/10 11:40 PM

fixed and merged: http://github.com/opscode/chef/commit/348a2f34e1595a7262e09ae9dbcb866a676860f6

with the afore-linked patch, you can now refer to resources that haven't yet been defined, so you can do something like:

template("first-config.conf") do
  # other stuff
  notifies(:restart, "service[apache]")
end

template("second-config.conf") do
  #other stuff
  notifies(:restart, "service[apache]")
end

service("apache") do
  # properties of the service
end

The references to the apache service will be resolved in between the eval/compilation phase and execution phase. I could've let them be completely lazy, i.e., resolved when the resource is executed, but that would've led to sub-optimal failure scenarios.

Show
Daniel DeLeo added a comment - 27/Sep/10 11:40 PM fixed and merged: http://github.com/opscode/chef/commit/348a2f34e1595a7262e09ae9dbcb866a676860f6 with the afore-linked patch, you can now refer to resources that haven't yet been defined, so you can do something like: 
template("first-config.conf") do
  # other stuff
  notifies(:restart, "service[apache]")
end

template("second-config.conf") do
  #other stuff
  notifies(:restart, "service[apache]")
end

service("apache") do
  # properties of the service
end
The references to the apache service will be resolved in between the eval/compilation phase and execution phase. I could've let them be completely lazy, i.e., resolved when the resource is executed, but that would've led to sub-optimal failure scenarios.
Hide
Permalink
Daniel DeLeo added a comment - 27/Sep/10 11:40 PM

Fixed by the patch as noted above.

Show
Daniel DeLeo added a comment - 27/Sep/10 11:40 PM Fixed by the patch as noted above.

People

Vote (1)
Watch (1)

Dates

  • Created:
    27/Jan/10 7:05 PM
    Updated:
    27/Sep/10 11:40 PM
    Resolved:
    27/Sep/10 11:40 PM
Atlassian JIRA (v4.3#614-r144677) | Report a problem
Powered by a free Atlassian JIRA open source license for Chef and Ohai. Try JIRA - bug tracking software for your team.