Skip to content

Commit ca26866

Browse files
committed
Remove restart_service on service_limits define
Since 97dd16f this parameter is a bad idea. It doesn't do a daemon reload and it may restart at the wrong time. In fact, I'd argue it's always been a bad idea. The recommended alternative to this is to manage the service explicitly and let Puppet handle it. There is an automatic notify that takes care of it. Fixes voxpupuli#190
1 parent ab81336 commit ca26866

File tree

2 files changed

+17
-26
lines changed

2 files changed

+17
-26
lines changed

manifests/service_limits.pp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,12 @@
2626
#
2727
# * Mutually exclusive with ``$limits``
2828
#
29-
# @param restart_service
30-
# Restart the managed service after setting the limits
31-
#
3229
define systemd::service_limits (
3330
Enum['present', 'absent', 'file'] $ensure = 'present',
3431
Stdlib::Absolutepath $path = '/etc/systemd/system',
3532
Boolean $selinux_ignore_defaults = false,
3633
Optional[Systemd::ServiceLimits] $limits = undef,
3734
Optional[String] $source = undef,
38-
Boolean $restart_service = true
3935
) {
4036
include systemd
4137

@@ -67,14 +63,6 @@
6763
selinux_ignore_defaults => $selinux_ignore_defaults,
6864
content => $_content,
6965
source => $source,
70-
}
71-
72-
if $restart_service {
73-
exec { "restart ${name} because limits":
74-
command => "systemctl restart ${name}",
75-
path => $::path,
76-
refreshonly => true,
77-
subscribe => File["${path}/${name}.d/90-limits.conf"],
78-
}
66+
notify_service => true,
7967
}
8068
}

spec/defines/service_limits_spec.rb

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,21 @@
4545
with_content(%r{IODeviceWeight=/dev/weight2 20}).
4646
with_content(%r{IOReadBandwidthMax=/bw/max 10K})
4747
}
48-
it {
49-
is_expected.to create_exec("restart #{title} because limits").with(
50-
command: "systemctl restart #{title}",
51-
refreshonly: true
52-
)
53-
}
48+
49+
describe 'with service managed' do
50+
let(:pre_condition) do
51+
<<-PUPPET
52+
service { 'test':
53+
}
54+
PUPPET
55+
end
56+
57+
it { is_expected.to compile.with_all_deps }
58+
it do
59+
is_expected.to create_file("/etc/systemd/system/#{title}.d/90-limits.conf").
60+
that_notifies('Service[test]')
61+
end
62+
end
5463
end
5564

5665
describe 'ensured absent' do
@@ -59,13 +68,7 @@
5968
it { is_expected.to compile.with_all_deps }
6069
it do
6170
is_expected.to create_file("/etc/systemd/system/#{title}.d/90-limits.conf").
62-
with_ensure('absent').
63-
that_notifies("Exec[restart #{title} because limits]")
64-
end
65-
it do
66-
is_expected.to create_exec("restart #{title} because limits").
67-
with_command("systemctl restart #{title}").
68-
with_refreshonly(true)
71+
with_ensure('absent')
6972
end
7073
end
7174
end

0 commit comments

Comments
 (0)