add systemd support #98
Conversation
|
I am assuming this is why I am having service restart uses after my upgrade to Ubuntu 16.04. I would love to see this merged. |
|
Yes, the current gem doesn't work at all on 16.04. It is unlikely that this will be merged anytime soon(as there are a number of other modifications competing to expand support of the plugin). If you want to use my branch you can just put the following in your gemfile: |
|
@BenoitHiller thanks much. I will give it a try. I have some crazy hacks in place right now to force restarting of my unicorn workers. If you saw it you would laugh. |
lib/capistrano/tasks/unicorn.rake
Outdated
| execute :chmod, '+x', unicorn_initd_file | ||
| sudo 'update-rc.d', '-f', fetch(:unicorn_service), 'defaults' | ||
| when :systemd | ||
| sudo_upload! template('unicorn_service.erb'), fetch(:unicorn_service) |
There was a problem hiding this comment.
@BenoitHiller shouldn't this be the full path and not just the service file name? It appears that if you just fetch(:unicorn_service) the script uploads to the home directly on the server and not to the correct services directory. I think you intended to call your unicorn_systemd_file method defined in lib/capistrano/dsl/unicorn_paths.rb, but instead called fetch(:unicorn_service) which only returns the filename and not the full path.
sudo_upload! template('unicorn_service.erb'), unicorn_systemd_fileIf I am correct, please update your pull request. I would love to merge your code.
There was a problem hiding this comment.
You are correct I have updated the path(I had already done so locally to make it work).
|
@BenoitHiller thanks so much for your input. I will read up on the documentation you included and work towards implementing the correct solution. I mean to merge all your changes when final into the parent master. I was just using my fork to experiment with my own production app. I think once we resolve these issues you can add the code to your branch and we can merge it. This is mostly your work anyway. I am just trying to make sure it works for me and that it will be a benefit to the community as a whole. |
|
@BenoitHiller, you mentioned me being added as a contributor. Honestly I have no idea why I was invited. But, I have always wanted a chance to work on something open source that I also used and I guess this is my chance. You will have to excuse me as I am very new to the process. Any help or advice you have for my is greatly appreciated. |
|
@griffithac I would assume that the owner of the repository has added you because they read one of the articles floating around that talked about how just adding people with pull requests as collaborators had turned out well on projects they had little time for. As for advice I would suggest being cautious. There appear to be a fairly sizeable number of people using this gem in its current form, so the number one priority in merging this code in is making sure nothing breaks for the people on ubuntu 14.04. Also I would strongly suggest contacting @rhomeister directly to get a better idea of what they were hoping for in adding you. They are also likely the best source of advice on addressing the existing issues and pull requests for the project so long as you are willing to do the legwork. As for this pull request now that I have had a lot more time to think about how this should be structured I think that instead of switching directly between different behaviour in a case statement the correct approach is to extract different sub-tasks for the different init system and run those conditionally based on the target environment you specify. If you want to do more work on this I can change the target branch to be something like |
|
@BenoitHiller thanks for the words of caution. I intend to be careful. I will also reach out to @rhomeister, so far I have not been able to contact him, but I will give it another shot. I think you are right about changing your approach to sub-tasks. Are you interested and writing that code, if not I can probably work on it. And yes, please change the target branch to |
|
K. I have merged it into |
|
Will do, thanks. |
This fixes #70 by adding an
:init_systemvariable which takes values of either:sysvor:systemd.The code should not produce any changes in behaviour when run without setting
:init_systemexcept that I called publish action for unicornrun_latest, so the print will be different.I'm not sure if that is a change you approve of, however the semantics of systemd are different and we have to be careful to actually start the services. Also supporting the reload action with a signal is apparently very much not recommended(it wouldn't be that hard to do though if we really wanted to).
I see that there are other pull requests like #45 that are having trouble getting merged that do similar tasks. It might make sense to have separate flags for init system, service installer, and service reloader. That seems like it would cover all of the different cases if you combined it with making more of the paths configurable. Then the whole thing could be wrapped up in some presets for common configurations. That way you would have decent support for both common OS's and arbitrary OS's.
This code is tested on my arch system right now. I should be putting it through its paces on ubuntu 16.04 soon and could probably give 14.04 a try at the same time. Seems like we are going to need something to run automated tests on different images though or it is going to be impossible to maintain extensions like this in the future.