This repository has been archived by the owner. It is now read-only.

should systemctl do daemon-reload automatically? #191

Closed
oaken-source opened this Issue Oct 16, 2014 · 93 comments

Comments

Projects
None yet
@oaken-source
Copy link

oaken-source commented Oct 16, 2014

Issue Type:

Feature Idea

Ansible Version:

ansible 1.7.2, but tested against the github version

Environment:

host OS: Gentoo Linux
managed OS: Arch Linux

Summary:

On /etc/init.d scripts, the start/stop/enable routines always use the service files on disk, which makes it easy to schedule service restart on redeployment of changed service files.
I feel that systemd should behave similarily from within the service module, that is, when I have a rule like

service: name=foo state=started

I would want service to go ahead and tell systemd to load the latest service files from disk if necessary. At the moment, I would have to invoke systemctl daemon-reload manually, or reboot, which does not feel right.

Steps To Reproduce:

update a .service file via a copy rule, then call:

service: name=foo state=started
Expected Results:

the service should start using the new .service file

Actual Results:

the service files are not reloaded and the old version is used instead

@oaken-source

This comment has been minimized.

Copy link

oaken-source commented Oct 17, 2014

thanks, updated.

@breun

This comment has been minimized.

Copy link

breun commented Feb 12, 2015

I've ran into this as well with directories with versions in their names in systemd service scripts (generated from an Ansible template of course). As a workaround I've had Ansible create a symlink called current pointing to the directory with the version number and referring to the current symlink from the systemd service script, but if the service module would call systemctl daemon-reload then that workaround wouldn't be necessary anymore.

@bcoca

This comment has been minimized.

Copy link
Member

bcoca commented Feb 12, 2015

if you are changing the service file why not call service with 'restarted'?

@breun

This comment has been minimized.

Copy link

breun commented Feb 12, 2015

Before our symlink workaround our Ansible playbook used the service module to stop the service, upgraded the application, updated the systemd service script (using the template module) and started the service again (again using the service module). This did not work, because the old systemd service script was still used by systemd and systemctl daemon-reload was not called, so systemd tried to start the application using the start command from the previous service script.

Instead of using the symlink workaround (which causes the start and stop command paths to not change, thus not requiring a systemctl daemon-reload) we could also of course use the command module to just always call systemctl daemon-reload after the template task which puts in the systemd service script, but not needing to do that is exactly what this issue is about.

@batmat

This comment has been minimized.

Copy link

batmat commented May 19, 2015

+1. We use the service module to restart Docker after we've updated its config (to add things like or internal registry, http proxy conf, etc.) and currently status=restarted will just be insufficient.

IMO, as I read above and agree, this does not feel right. Ansible should just hide that complexity and do the right thing: restart the service (and implicitly: make it usable with its new config, I cannot imagine any other reason I would want to have a service restarted...)

Thanks a lot

@grahamrhay

This comment has been minimized.

Copy link
Contributor

grahamrhay commented May 26, 2015

We've recently started moving to Debian 8, and therefore systemd; and simply restarting the service is no longer sufficient, if the unit file has changed. Some way of forcing daemon-reload, implicitly or explicitly, would be useful.

@holms

This comment has been minimized.

Copy link

holms commented Jun 1, 2015

👍 Voting for this! Centos7 requires daemon-reload

@bcoca

This comment has been minimized.

Copy link
Member

bcoca commented Jun 1, 2015

So to clear up for non systemd folks (as I myself was confused by this), this is not analogous to the reload/reloaded state for most deamons, this has systemd stop the deamon using and old configuration and start it using the new one after the configuration has been updated.

Otherwise you would need to manually stop with current config, change config, start again with new config, which might be tricky with some of the configs that are triggered by other services/messages/time/etc and you run into all types of race conditions.

@Faheetah

This comment has been minimized.

Copy link

Faheetah commented Jun 3, 2015

+1 here, or at least give a way for service to force a daemon-reload similar to how the apt module can force a cache update. This could be through an extra arg daemon_reload=true (default: false, required: no) to have the module push a systemctl daemon-reload before running the service command.

@pieterlexis

This comment has been minimized.

Copy link

pieterlexis commented Jun 24, 2015

👍

pieterlexis added a commit to pieterlexis/ansible-modules-core that referenced this issue Jun 24, 2015

Service: optional daemon-reload for systemd
This adds the `daemon_reload` option to the service module. This is
ofcourse a systemd-only option. When this option is set, we issue a
`systemctl daemon-reload` before we try to manipulate the service.

Closes ansible#191

pieterlexis added a commit to pieterlexis/ansible-modules-core that referenced this issue Jun 24, 2015

Service: optional daemon-reload for systemd
This adds the `daemon_reload` option to the service module. This is
ofcourse a systemd-only option. When this option is set, we issue a
`systemctl daemon-reload` before we try to manipulate the service.

Closes ansible#191
@razvanphp

This comment has been minimized.

Copy link

razvanphp commented Jul 13, 2015

+1 for this, run into it today with Jessie.

I don't even know how to do it elegantly, since from my task I call notify, and I don't want every time to use daemon-reload. Maybe 2 handlers?

@holms

This comment has been minimized.

Copy link

holms commented Jul 14, 2015

If anybody could point me about @pieterlexis pull request state: if it's valid or invalid, should be rewritten, or what's the delay. I'd PR something in here then... @bcoca maybe you could do a code review please for pull-request above?

@caylorme

This comment has been minimized.

Copy link

caylorme commented Jul 28, 2015

+1 for this

@tcwalther

This comment has been minimized.

Copy link

tcwalther commented Jul 30, 2015

Another +1, also on jessie.

@sapun

This comment has been minimized.

Copy link

sapun commented Aug 6, 2015

+1

@schms

This comment has been minimized.

Copy link

schms commented Aug 12, 2015

+1,

@AlexanderPavlenko

This comment has been minimized.

Copy link

AlexanderPavlenko commented Aug 17, 2015

+1

2 similar comments
@efine

This comment has been minimized.

Copy link

efine commented Aug 17, 2015

+1

@domibarton

This comment has been minimized.

Copy link
Contributor

domibarton commented Aug 23, 2015

+1

@pstauffer

This comment has been minimized.

Copy link

pstauffer commented Aug 23, 2015

1+

@mxmo0rhuhn

This comment has been minimized.

Copy link

mxmo0rhuhn commented Aug 23, 2015

+1

2 similar comments
@ptqa

This comment has been minimized.

Copy link

ptqa commented Aug 23, 2015

+1

@knobli

This comment has been minimized.

Copy link

knobli commented Aug 25, 2015

+1

@corebug

This comment has been minimized.

Copy link

corebug commented Aug 28, 2015

+1

@SuperQ

This comment has been minimized.

Copy link

SuperQ commented Jan 30, 2016

+1

2 similar comments
@farazhussain

This comment has been minimized.

Copy link

farazhussain commented Feb 2, 2016

+1

@geerlingguy

This comment has been minimized.

Copy link
Contributor

geerlingguy commented Feb 16, 2016

👍

@4everinbeta

This comment has been minimized.

Copy link

4everinbeta commented Feb 16, 2016

+1

1 similar comment
@ninthnails

This comment has been minimized.

Copy link

ninthnails commented Feb 17, 2016

+1

@nemesisdesign

This comment has been minimized.

Copy link

nemesisdesign commented Feb 21, 2016

Here's a workaround if anybody needs it:
nemesisdesign/Stouts.openvpn@fe8ae77

@matthewhartstonge

This comment has been minimized.

Copy link

matthewhartstonge commented Feb 24, 2016

👍

@ringxw

This comment has been minimized.

Copy link

ringxw commented Mar 3, 2016

+1

@raqqun

This comment has been minimized.

Copy link

raqqun commented Mar 6, 2016

i use something like this in my playbook if the varnish.service unit file has been changed.

- name: Ensure varnish.service is latest
  template: src=varnish.service dest=/etc/systemd/system/varnish.service
  register: result
  notify:
    - restart varnish

- name: Ensure systemd is reloaded if varnish.service has changed
  shell: systemctl daemon-reload
  when: result|changed
@Zyphrax

This comment has been minimized.

Copy link

Zyphrax commented Mar 20, 2016

Why not simply use notify here too?
Side-note: Ansible recommends to use command instead of shell whenever possible.

- name: Create systemd scripts
  template: src=test.service dest=/etc/systemd/system/test.service
  notify: Reload systemd

- name: Reload systemd
  command: systemctl daemon-reload
@holms

This comment has been minimized.

Copy link

holms commented Mar 20, 2016

@Zyphrax because that's a hack, and it's fine to use it as hack, but not as solution. Normally you need to twist your brain the way that this "command" wouldn't violate Idempotence.. and that's wrong. This should be in a module itself, and it's about time to fix this.. cause that's basic functionality of Debian system which must be done after copying configuration. Why use hacking solution each time?

@Zyphrax

This comment has been minimized.

Copy link

Zyphrax commented Mar 21, 2016

@holms I agree, the module should handle this. My goal was to provide a simple workaround. I saw raqqun's solution which works fine, but is a bit more complex than necessary and won't work in combinations with with_items and a few other constructs.

@muirdok

This comment has been minimized.

Copy link

muirdok commented Apr 1, 2016

+1 to add additional action for "reloaded" or "started" state.

@kennethmyers

This comment has been minimized.

Copy link

kennethmyers commented Apr 12, 2016

+1

@holms

This comment has been minimized.

Copy link

holms commented Apr 13, 2016

Guys, there's a thumb up vote on original post.. no need to do +1 anymore. This was created intensely to avoid voting flood.

@hho

This comment has been minimized.

Copy link

hho commented Apr 20, 2016

@Zyphrax Unfortunately, your solution doesn't really work if I want to add, enable and start a service in the same playbook. The handler reloading the systemd daemon would only be executed after the playbook – and by then it is too late.

@nemesisdesign

This comment has been minimized.

Copy link

nemesisdesign commented Apr 25, 2016

comments with +1 are useless, either write some code, a workaround or anything other interesting or refrain from commenting please!

@akostadinov

This comment has been minimized.

Copy link

akostadinov commented Apr 25, 2016

+1 are useful, please add them especially now you can just use a reaction icon under on a post. They can help project owners give priority to particular features/bugs. Especially with issue like this, where solution is not as hard as having project owners agree on how things should work.
Everybody is free to manage his/her own projects as he/she desires, but advising on a generic practice to refrain from giving feedback (even if it is simple +/-1) is not doing a favor to anybody.

@nervo

This comment has been minimized.

Copy link

nervo commented Apr 25, 2016

@nemesisdesign 👍

@nemesisdesign

This comment has been minimized.

Copy link

nemesisdesign commented Apr 25, 2016

@akostadinov all this +1 one comments without any meaningful comment just make it hard for people who want to know about solutions employed or opened pull request to follow the discussion. Congratulations! Write some more hundreds of +1 comments and it will be fixed faster!

@akostadinov

This comment has been minimized.

Copy link

akostadinov commented Apr 25, 2016

@nemesisdesign , agreed, I think @holms addressed that concern though.

@nemesisdesign

This comment has been minimized.

Copy link

nemesisdesign commented Apr 29, 2016

Running into this on a fresh Ubuntu 16 system. It's painful and ugly to apply the workaround to generic roles. I do not understand if it's possible to fix it in core and if there's somebody who is working on it.

@ghost

This comment has been minimized.

Copy link

ghost commented May 4, 2016

Well a PR was submitted and closed on this issue. I believe it's rather strange now that systemd is on most major systems that we don't have some way to do a systemctl daemon-reload (a critical function) inside the service module.

@akostadinov

This comment has been minimized.

Copy link

akostadinov commented May 4, 2016

I think @bcoca working on this with #1616 but not sure what the status is. Can priority of this issue be raised a little bit (P4 sounds somehow low)?

@flavius

This comment has been minimized.

Copy link

flavius commented May 14, 2016

+1

@bcoca

This comment has been minimized.

Copy link
Member

bcoca commented May 16, 2016

fixed via #3660

@bcoca bcoca closed this May 16, 2016

@ansible ansible locked and limited conversation to collaborators May 16, 2016

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