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

[COOK-2353] split out config from enable action #20

Merged
merged 5 commits into from Feb 28, 2013

Conversation

Projects
None yet
3 participants
@dwradcliffe
Contributor

dwradcliffe commented Feb 19, 2013

This will ensure templates are changed when service is already running. Seems to be working for me but definitely needs to be looked over. There might be a better way to get this done. :)

@@ -250,6 +270,7 @@ def run_script
@run_script.source("sv-#{new_resource.run_template_name}-run.erb")
@run_script.cookbook(template_cookbook)
@run_script.mode(00755)
@run_script.notifies(:restart, "service[#{new_resource.service_name}]")

This comment has been minimized.

@dwradcliffe

dwradcliffe Feb 19, 2013

Contributor

This doesn't seem to be working for me.

This comment has been minimized.

@jtimberman

jtimberman Feb 20, 2013

Member

From Seth, how about something like this at the end of the action_enable method?

scripts_updated = [ run_script, log_run_script, finish_script, control_signal_files ].flatten.any? do |r| 
  r.updated_by_last_action?
end

restart_service if scripts_updated

This comment has been minimized.

@dwradcliffe

dwradcliffe Feb 20, 2013

Contributor

The finish and control scripts shouldn't need a restart, since they are run from disk at the time they are called.
I'm still on the fence about the log script, and the more I think about it I'd really like to be able to NOT restart the service in either case.

Example: We use unicorn, and do zero-downtime deploys using USR2 to restart. I really want to control when a restart or a full restart happens, and it's almost never going to be at the time of a chef run.

This comment has been minimized.

@jtimberman

jtimberman Feb 20, 2013

Member

yeah, untested code and all that :-). So really:

restart_service if run-script.updated_by_last_action?

This comment has been minimized.

@dwradcliffe

dwradcliffe Feb 20, 2013

Contributor

How about this:

restart_service if new_resource.restart_on_update and run_script.updated_by_last_action?

This comment has been minimized.

@jtimberman

jtimberman Feb 21, 2013

Member

Where is restart_on_update? Is that a new attribute you're proposing? Would it be true by default so existing recipes wouldn't need to be updated?

This comment has been minimized.

@dwradcliffe

dwradcliffe Feb 21, 2013

Contributor

Yes and yes.

This comment has been minimized.

@jtimberman

jtimberman Feb 22, 2013

Member

👍 - make sure we have test coverage. :)

@dwradcliffe

This comment has been minimized.

Contributor

dwradcliffe commented Feb 24, 2013

Tests added for restart_on_update attribute. All tests passing. Might be good to add a test for the new restart action.

resource.restart_on_update.should be_true
end
it 'hash a restart_on_update parameter that controls whether a the service is restarted when the run script is updated' do

This comment has been minimized.

@stevendanna

This comment has been minimized.

@dwradcliffe

dwradcliffe Feb 26, 2013

Contributor

Fixed.

jtimberman added a commit that referenced this pull request Feb 28, 2013

Merge pull request #20 from dwradcliffe/COOK-2353
[COOK-2353] split out config from enable action

@jtimberman jtimberman merged commit cfa5cfc into chef-cookbooks:master Feb 28, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment