Skip to content

Commit fbcbbc4

Browse files
trevor-vaughantrevor-vaughan
committed
Fix systemctl daemon-reload after file additions
* Add a `systemd::daemon_reload` defined type * Ensure that any file additions to the /etc/systemd/system space are followed by a call to `systemd::daemon_reload` * Allow users to disable the calls to `systemd::daemon_reload` * Allow users to globally disable the `systemctl daemon-reload` exec using a resource collector if necessary * Hook the daemon reload between the file creation and the service as is usually necessary, where possible * Add a 'best effort' optional exec as `systemd::lazy_daemon_reload` to try and clean up systems that were modified betweedn 2.9.0 and this release Fixes #234 Fixes #199
1 parent 56666b2 commit fbcbbc4

File tree

11 files changed

+206
-44
lines changed

11 files changed

+206
-44
lines changed

‎manifests/daemon_reload.pp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# @summary Run systemctl daemon-reload
2+
#
3+
# @api public
4+
#
5+
# @param name
6+
# A globally unique name for the resource
7+
#
8+
# @param enable
9+
# Enable the reload exec
10+
#
11+
# * Added in case users want to disable the reload globally using a resource collector
12+
#
13+
# @param lazy_reload
14+
# Enable a global lazy reload.
15+
#
16+
# * This is meant for cleaning up systems that may have gotten out of sync so no particular
17+
# care is taken in trying to actually "fix" things, that would require a custom type that
18+
# manipulates the running catalog tree.
19+
#
20+
define systemd::daemon_reload (
21+
Boolean $enable = true,
22+
Boolean $lazy_reload = false,
23+
) {
24+
if $enable {
25+
if $lazy_reload {
26+
exec { "${module_name}-${name}-global-systemctl-daemon-check":
27+
command => 'systemctl daemon-reload',
28+
onlyif => 'systemctl show "*" --property=NeedDaemonReload | grep -q "=yes"',
29+
path => $facts['path'],
30+
}
31+
32+
# Give services a fighting chance of coming up properly
33+
Exec["${module_name}-${name}-global-systemctl-daemon-check"] -> Service <||>
34+
}
35+
36+
exec { "${module_name}-${name}-systemctl-daemon-reload":
37+
command => 'systemctl daemon-reload',
38+
refreshonly => true,
39+
path => $facts['path'],
40+
}
41+
}
42+
}

‎manifests/dropin_file.pp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
# @param mode The mode to set on the dropin file
1818
# @param show_diff Whether to show the diff when updating dropin file
1919
# @param notify_service Notify a service for the unit, if it exists
20+
# @param daemon_reload Call systemd::daemon_reload
2021
#
2122
define systemd::dropin_file (
2223
Systemd::Unit $unit,
@@ -32,6 +33,7 @@
3233
String $mode = '0444',
3334
Boolean $show_diff = true,
3435
Boolean $notify_service = false,
36+
Boolean $daemon_reload = true,
3537
) {
3638
include systemd
3739

@@ -69,11 +71,26 @@
6971
show_diff => $show_diff,
7072
}
7173

74+
if $daemon_reload {
75+
ensure_resource('systemd::daemon_reload', $name)
76+
77+
File[$full_filename] ~> Systemd::Daemon_reload[$name]
78+
}
79+
7280
if $notify_service {
7381
File[$full_filename] ~> Service <| title == $unit or name == $unit |>
82+
83+
if $daemon_reload {
84+
Systemd::Daemon_reload[$name] ~> Service <| title == $unit or name == $unit |>
85+
}
86+
7487
if $unit =~ /\.service$/ {
7588
$short_service_name = regsubst($unit, /\.service$/, '')
7689
File[$full_filename] ~> Service <| title == $short_service_name or name == $short_service_name |>
90+
91+
if $daemon_reload {
92+
Systemd::Daemon_reload[$name] ~> Service <| title == $short_service_name or name == $short_service_name |>
93+
}
7794
}
7895
}
7996
}

‎manifests/init.pp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@
174174
# @param oomd_settings
175175
# Hash of systemd-oomd configurations for oomd.conf
176176
#
177+
# @param lazy_daemon_reload
178+
# Perform a check across all potentially overridden service files and trigger a daemon reload if necessary
179+
#
177180
class systemd (
178181
Optional[Pattern['^.+\.target$']] $default_target = undef,
179182
Hash[String,String] $accounting = {},
@@ -227,6 +230,7 @@
227230
Boolean $manage_oomd = false,
228231
Enum['stopped','running'] $oomd_ensure = 'running',
229232
Systemd::OomdSettings $oomd_settings = {},
233+
Boolean $lazy_daemon_reload = false,
230234
) {
231235
contain systemd::install
232236

@@ -316,4 +320,8 @@
316320
* => $resource,
317321
}
318322
}
323+
324+
if $lazy_daemon_reload {
325+
systemd::daemon_reload { 'global-lazy': lazy_reload => $lazy_daemon_reload }
326+
}
319327
}

‎manifests/service_limits.pp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@
7474
command => "systemctl restart ${name}",
7575
path => $facts['path'],
7676
refreshonly => true,
77-
subscribe => File["${path}/${name}.d/90-limits.conf"],
7877
}
78+
79+
Systemd::Dropin_file["${name}-90-limits.conf"] ~> Exec["restart ${name} because limits"]
7980
}
8081
}

‎manifests/timer.pp

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@
5858
# @param ensure
5959
# Defines the desired state of the timer
6060
#
61+
# @param daemon_reload
62+
# Call `systemd::daemon_reload`
63+
#
6164
define systemd::timer (
6265
Enum['present', 'absent', 'file'] $ensure = 'present',
6366
Stdlib::Absolutepath $path = '/etc/systemd/system',
@@ -72,6 +75,7 @@
7275
Boolean $show_diff = true,
7376
Optional[Variant[Boolean, Enum['mask']]] $enable = undef,
7477
Optional[Boolean] $active = undef,
78+
Boolean $daemon_reload = true,
7579
) {
7680
assert_type(Pattern['^.+\.timer$'],$name)
7781

@@ -83,28 +87,30 @@
8387

8488
if $service_content or $service_source {
8589
systemd::unit_file { $_service_unit:
86-
ensure => $ensure,
87-
content => $service_content,
88-
source => $service_source,
89-
path => $path,
90-
owner => $owner,
91-
group => $group,
92-
mode => $mode,
93-
show_diff => $show_diff,
94-
before => Systemd::Unit_File[$name],
90+
ensure => $ensure,
91+
content => $service_content,
92+
source => $service_source,
93+
path => $path,
94+
owner => $owner,
95+
group => $group,
96+
mode => $mode,
97+
show_diff => $show_diff,
98+
before => Systemd::Unit_File[$name],
99+
daemon_reload => $daemon_reload,
95100
}
96101
}
97102

98103
systemd::unit_file { $name:
99-
ensure => $ensure,
100-
content => $timer_content,
101-
source => $timer_source,
102-
path => $path,
103-
owner => $owner,
104-
group => $group,
105-
mode => $mode,
106-
show_diff => $show_diff,
107-
enable => $enable,
108-
active => $active,
104+
ensure => $ensure,
105+
content => $timer_content,
106+
source => $timer_source,
107+
path => $path,
108+
owner => $owner,
109+
group => $group,
110+
mode => $mode,
111+
show_diff => $show_diff,
112+
enable => $enable,
113+
active => $active,
114+
daemon_reload => $daemon_reload,
109115
}
110116
}

‎manifests/unit_file.pp

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@
6161
# @param service_parameters
6262
# hash that will be passed with the splat operator to the service resource
6363
#
64+
# @param daemon_reload
65+
# call `systemd::daemon-reload` to ensure that the modified unit file is loaded
66+
#
6467
# @example manage unit file + service
6568
# systemd::unit_file { 'foo.service':
6669
# content => file("${module_name}/foo.service"),
@@ -85,6 +88,7 @@
8588
Optional[Boolean] $hasstatus = undef,
8689
Boolean $selinux_ignore_defaults = false,
8790
Hash[String[1], Any] $service_parameters = {},
91+
Boolean $daemon_reload = true
8892
) {
8993
include systemd
9094

@@ -125,6 +129,12 @@
125129
selinux_ignore_defaults => $selinux_ignore_defaults,
126130
}
127131

132+
if $daemon_reload {
133+
ensure_resource('systemd::daemon_reload', $name)
134+
135+
File["${path}/${name}"] ~> Systemd::Daemon_reload[$name]
136+
}
137+
128138
if $enable != undef or $active != undef {
129139
service { $name:
130140
ensure => $active,
@@ -143,15 +153,10 @@
143153
Service[$name] -> File["${path}/${name}"]
144154
} else {
145155
File["${path}/${name}"] ~> Service[$name]
146-
}
147-
} else {
148-
# Work around https://tickets.puppetlabs.com/browse/PUP-9473
149-
# and react to changes on static unit files (ie: .service triggered by .timer)
150-
exec { "${name}-systemctl-daemon-reload":
151-
command => 'systemctl daemon-reload',
152-
refreshonly => true,
153-
path => $facts['path'],
154-
subscribe => File["${path}/${name}"],
156+
157+
if $daemon_reload {
158+
Systemd::Daemon_reload[$name] ~> Service[$name]
159+
}
155160
}
156161
}
157162
}

‎spec/classes/init_spec.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
it { is_expected.not_to contain_class('systemd::coredump') }
2222
it { is_expected.not_to contain_class('systemd::oomd') }
2323
it { is_expected.not_to contain_exec('systemctl set-default multi-user.target') }
24+
it { is_expected.not_to contain_systemd__daemon_reload('global-lazy') }
2425

2526
context 'when enabling resolved and networkd' do
2627
let(:params) do
@@ -750,6 +751,14 @@
750751
it { is_expected.to contain_systemd__dropin_file('coredump_backtrace.conf').with_content(%r{^ExecStart=.*--backtrace$}) }
751752
end
752753
end
754+
755+
context 'with lazy daemon reloadin' do
756+
let :params do
757+
{ lazy_daemon_reload: true }
758+
end
759+
760+
it { is_expected.to contain_systemd__daemon_reload('global-lazy').with_lazy_reload(params[:lazy_daemon_reload]) }
761+
end
753762
end
754763
end
755764
end

‎spec/defines/daemon_reload.rb

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# frozen_string_literal: true
2+
3+
require 'spec_helper'
4+
5+
describe 'systemd::daemon_reload' do
6+
context 'supported operating systems' do
7+
on_supported_os.each do |os, facts|
8+
context "on #{os}" do
9+
let(:facts) { facts }
10+
let(:title) { 'irregardless' }
11+
12+
it { is_expected.to compile.with_all_deps }
13+
14+
context 'with defaults' do
15+
it do
16+
expect(subject).to contain_exec("systemd-#{title}-systemctl-daemon-reload").
17+
with_command('systemctl daemon-reload').
18+
with_refreshonly(true)
19+
20+
expect(subject).not_to contain_exec("systemd-#{title}-global-systemctl-daemon-check")
21+
end
22+
end
23+
24+
context 'when disabled' do
25+
let(:params) do
26+
{ 'enable' => false }
27+
end
28+
29+
it do
30+
expect(subject).not_to contain_exec("systemd-#{title}-systemctl-daemon-reload")
31+
end
32+
end
33+
34+
context 'with lazy reloading' do
35+
let(:pre_condition) { 'service { "test": }' }
36+
let(:params) do
37+
{ 'lazy_reload' => true }
38+
end
39+
40+
it do
41+
expect(subject).to contain_exec("systemd-#{title}-systemctl-daemon-reload").
42+
with_command('systemctl daemon-reload').
43+
with_refreshonly(true)
44+
45+
expect(subject).to contain_exec("systemd-#{title}-global-systemctl-daemon-check").
46+
with_command('systemctl daemon-reload').
47+
with_onlyif('systemctl show "*" --property=NeedDaemonReload | grep -q "=yes"').
48+
that_comes_before('Service[test]')
49+
end
50+
end
51+
end
52+
end
53+
end
54+
end

‎spec/defines/dropin_file_spec.rb

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,18 @@
2828
)
2929
}
3030

31+
it {
32+
expect(subject).to create_systemd__daemon_reload(title)
33+
}
34+
3135
it {
3236
expect(subject).to create_file("/etc/systemd/system/#{params[:unit]}.d/#{title}").with(
3337
ensure: 'file',
3438
content: %r{#{params[:content]}},
3539
mode: '0444',
3640
selinux_ignore_defaults: false
37-
)
41+
).
42+
that_notifies("Systemd::Daemon_reload[#{title}]")
3843
}
3944

4045
context 'notifies services' do
@@ -64,6 +69,7 @@
6469

6570
it { is_expected.to compile.with_all_deps }
6671
it { is_expected.to contain_service('myservice').that_subscribes_to("File[#{filename}]") }
72+
it { is_expected.to contain_systemd__daemon_reload(title).that_notifies('Service[myservice]') }
6773
end
6874
end
6975

@@ -143,6 +149,18 @@
143149
)
144150
}
145151
end
152+
153+
context 'with daemon_reload = false' do
154+
let(:params) do
155+
super().merge(daemon_reload: false)
156+
end
157+
158+
it { is_expected.to compile.with_all_deps }
159+
160+
it {
161+
expect(subject).not_to create_systemd__daemon_reload(title)
162+
}
163+
end
146164
end
147165
end
148166
end

‎spec/defines/timer_spec.rb

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,6 @@
8484

8585
it { is_expected.to contain_systemd__unit_file('foobar.timer').with_content("[Timer]\nOnCalendar=hourly") }
8686
it { is_expected.to contain_systemd__unit_file('foobar.service').with_content("[Service]\nExecStart=/bin/echo timer-fired").that_comes_before('Systemd::Unit_file[foobar.timer]') }
87-
88-
it {
89-
expect(subject).to create_exec('foobar.service-systemctl-daemon-reload').with(
90-
command: 'systemctl daemon-reload',
91-
refreshonly: true
92-
)
93-
}
9487
end
9588

9689
context 'with a bad timer name' do

0 commit comments

Comments
 (0)