Skip to content

Commit

Permalink
Remove restart_service on service_limits define
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ekohl committed Jan 26, 2023
1 parent 40a0f77 commit 6254e04
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 28 deletions.
15 changes: 1 addition & 14 deletions manifests/service_limits.pp
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,12 @@
#
# * Mutually exclusive with ``$limits``
#
# @param restart_service
# Restart the managed service after setting the limits
#
define systemd::service_limits (
Enum['present', 'absent', 'file'] $ensure = 'present',
Stdlib::Absolutepath $path = '/etc/systemd/system',
Boolean $selinux_ignore_defaults = false,
Optional[Systemd::ServiceLimits] $limits = undef,
Optional[String] $source = undef,
Boolean $restart_service = true
) {
include systemd

Expand Down Expand Up @@ -67,15 +63,6 @@
selinux_ignore_defaults => $selinux_ignore_defaults,
content => $_content,
source => $source,
}

if $restart_service {
exec { "restart ${name} because limits":
command => "systemctl restart ${name}",
path => $facts['path'],
refreshonly => true,
}

Systemd::Dropin_file["${name}-90-limits.conf"] ~> Exec["restart ${name} because limits"]
notify_service => true,
}
}
29 changes: 15 additions & 14 deletions spec/defines/service_limits_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,20 @@
with_content(%r{IOReadBandwidthMax=/bw/max 10K})
}

it {
expect(subject).to create_exec("restart #{title} because limits").with(
command: "systemctl restart #{title}",
refreshonly: true
)
}
describe 'with service managed' do
let(:pre_condition) do
<<-PUPPET
service { 'test':
}
PUPPET
end

it { is_expected.to compile.with_all_deps }
it do
is_expected.to create_file("/etc/systemd/system/#{title}.d/90-limits.conf").
that_notifies('Service[test]')
end
end
end

describe 'ensured absent' do
Expand All @@ -66,14 +74,7 @@

it do
expect(subject).to create_file("/etc/systemd/system/#{title}.d/90-limits.conf").
with_ensure('absent').
that_notifies("Exec[restart #{title} because limits]")
end

it do
expect(subject).to create_exec("restart #{title} because limits").
with_command("systemctl restart #{title}").
with_refreshonly(true)
with_ensure('absent')
end
end
end
Expand Down

0 comments on commit 6254e04

Please sign in to comment.