Skip to content
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

add upstart init #71

Merged
merged 3 commits into from
Nov 14, 2014
Merged

add upstart init #71

merged 3 commits into from
Nov 14, 2014

Conversation

wilreichert
Copy link

Native upstart service for Ubuntu, it behaves identically to the sysvinit script. Anyone upgrading will get blessed with 2 init files which might cause a bit of confusion.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ca34cf9 on wilreichert:upstart into 39940d0 on johnbellone:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f9a5d6d on wilreichert:upstart into 39940d0 on johnbellone:master.

@lyrixx
Copy link
Contributor

lyrixx commented Oct 31, 2014

Hi.

I think it's a good idea to add upstart support.

But I don't like the implementation, because the restart command really restart the process.

I think it's better to check everywhere in the code where we call restart or reload and replace it with something custom to leverage consul's hot reload

@johnbellone
Copy link
Contributor

The init script runs kill -INT and then just runs a normal start on the daemon. We should be able to do the same using upstart.

@wilreichert
Copy link
Author

it look like upstart sends the appropriate SIGHUP for the reload
http://upstart.ubuntu.com/cookbook/#reload-signal

The restart is a SIGTERM followed by a start & that can be reconfigured as well.
http://upstart.ubuntu.com/cookbook/#kill-signal

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c666027 on wilreichert:upstart into 0d96fc4 on johnbellone:master.

@johnbellone johnbellone added this to the 0.5.0 milestone Nov 6, 2014
@@ -149,6 +158,7 @@
end

service 'consul' do
provider Chef::Provider::Service::Upstart if platform?("ubuntu")

This comment was marked as outdated.

This comment was marked as outdated.

johnbellone added a commit that referenced this pull request Nov 14, 2014
@johnbellone johnbellone merged commit 111a581 into sous-chefs:master Nov 14, 2014
@lyrixx
Copy link
Contributor

lyrixx commented Dec 30, 2014

This PR is not BC. More often, it's a big BC break, because on existing node, I had consul with init.d and now, I have both init and upstart. And so chef failed:

 * service[consul] action restart[2014-12-30T14:04:00+00:00] INFO: Processing service[consul] action restart (consul::_service line 211)


    ================================================================================
    Error executing action `restart` on resource 'service[consul]'
    ================================================================================

    Chef::Exceptions::Exec
    ----------------------
    /sbin/start consul returned 1, expected 0

    Resource Declaration:
    ---------------------
    # In /var/chef/cache/cookbooks/consul/recipes/_service.rb

    211:   service 'consul' do
    212:     provider Chef::Provider::Service::Upstart if platform?("ubuntu")
    213:     supports status: true, restart: true, reload: true
    214:     action [:enable, :start]
    215:     subscribes :restart, "file[#{consul_config_filename}", :delayed
    216:   end
    217: when 'runit'

@lyrixx
Copy link
Contributor

lyrixx commented Feb 2, 2015

Hi, Sorry to "spam" this thread, but what id the right procedure to migrate ?

@lock
Copy link

lock bot commented Apr 25, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants