fix: refresh service only based on drop-in file changes #406
Conversation
Since multiple drop-in files of the same unit notify the same Systemd::Daemon_reload resource, changing a drop in file without notify_service still triggers a service refresh if at least one other drop-in file has notify_service set (default) regardless of it being changed. Therefore the Systemd::Daemon_reload should only be ordered before the Service but not notify it.
|
I am quite uncertain on how to test these changes. Especially since tests for absence of certain relationships seem to be asked for in this case. |
There was a problem hiding this comment.
This probably impossible to test due to rspec-puppet having different transitive dependencies than real Puppet: puppetlabs/rspec-puppet#35 and also missing matchers (puppetlabs/rspec-puppet#28).
| @@ -81,15 +81,15 @@ | |||
| File[$full_filename] ~> Service <| title == $unit or name == $unit |> | |||
|
|
|||
| if $daemon_reload { | |||
| Systemd::Daemon_reload[$unit] ~> Service <| title == $unit or name == $unit |> | |||
| Systemd::Daemon_reload[$unit] -> Service <| title == $unit or name == $unit |> | |||
There was a problem hiding this comment.
If this isn't a notify anymore, should this be around line 77 instead? I think the ordering is still correct. Same for the other case.
There was a problem hiding this comment.
Thanks for taking a look at this @ekohl. The conditions stay the same. The ordering of the Daemon_reload in relation to the Service still only needs to happen when both $notify_service and $daemon_reload are true.
shieldwed
force-pushed
the
fix_service_dependency
branch
2 times, most recently
from
d8a93f0
to
c798ae1
Compare
shieldwed
force-pushed
the
fix_service_dependency
branch
from
c798ae1
to
d47d3e9
Compare
I've implemented some tests for the changed cases. Can you please look at them again? |
|
@ekohl can you please have another look at this PR? Thank you |
|
Can someone please have a look at this again (e.g. @bastelfreak or @TheMeier perhaps)? Thank you. |
Pull Request (PR) description
Since multiple drop-in files of the same unit notify the same
Systemd::Daemon_reloadresource, changing a drop in file withoutnotify_servicestill triggers a service refresh if at least one other drop-in file hasnotify_serviceset (default) regardless of it being changed.Therefore, the
Systemd::Daemon_reloadshould only be ordered before theServicebut not notify it. Theservicewill already be refreshed if the drop-in file changes andnotify_serviceis set (default).This Pull Request (PR) addresses the following issues
Consider the following Puppet code snippet with two drop-in files for the service
nginx.service:systemd::dropin_file { default: unit => 'nginx.service', ; "restart.conf": notify_service => false, content => @(EOT) [Service] Restart=on-failure | EOT , ; "ulimit.conf": content => @(EOT) [Service] LimitNOFILE=4096 | EOT , }On the first Puppet-run, this code will correctly refresh the service, as the
ulimit.conf-file will be written. Once the drop-in files are in sync with this configuration, no more SystemD daemon reloads and service refreshes happen.Now, changing the
Restartconfiguration ofnginx.servicedoesn't need the service itself to be restarted.However, due to the second drop-in file specifying
notify_service => true(default), this will lead toSystemd::Daemon_reload[$unit]being triggered (expected due todaemon_reload => true(default)), which then in turn triggersService[nginx.service]andService[nginx]if they exist.