From 31790ab145be58fadcd07f53e56537a84a118c20 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Tue, 27 Apr 2021 11:33:16 +0200
Subject: [PATCH] Remove restart_service on service_limits define

Since 97dd16fa32886b5b0f77a6f38a4953d4c1511c0e 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 #190
---
 manifests/service_limits.pp         | 14 +-------------
 spec/defines/service_limits_spec.rb | 23 +++++++++++++----------
 2 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/manifests/service_limits.pp b/manifests/service_limits.pp
index d0048029..9eb90792 100644
--- a/manifests/service_limits.pp
+++ b/manifests/service_limits.pp
@@ -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',
   Optional[Boolean]                 $selinux_ignore_defaults = false,
   Optional[Systemd::ServiceLimits]  $limits                  = undef,
   Optional[String]                  $source                  = undef,
-  Boolean                           $restart_service         = true
 ) {
   include systemd
 
@@ -67,14 +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        => $::path,
-      refreshonly => true,
-      subscribe   => File["${path}/${name}.d/90-limits.conf"],
-    }
+    notify_service          => true,
   }
 }
diff --git a/spec/defines/service_limits_spec.rb b/spec/defines/service_limits_spec.rb
index a8270087..86c76906 100644
--- a/spec/defines/service_limits_spec.rb
+++ b/spec/defines/service_limits_spec.rb
@@ -45,12 +45,6 @@
               .with_content(%r{IODeviceWeight=/dev/weight2 20})
               .with_content(%r{IOReadBandwidthMax=/bw/max 10K})
           }
-          it {
-            is_expected.to create_exec("restart #{title} because limits").with(
-              command: "systemctl restart #{title}",
-              refreshonly: true,
-            )
-          }
         end
 
         describe 'ensured absent' do
@@ -60,12 +54,21 @@
           it do
             is_expected.to create_file("/etc/systemd/system/#{title}.d/90-limits.conf")
               .with_ensure('absent')
-              .that_notifies("Exec[restart #{title} because limits]")
           end
+        end
+
+        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_exec("restart #{title} because limits")
-              .with_command("systemctl restart #{title}")
-              .with_refreshonly(true)
+            is_expected.to create_file("/etc/systemd/system/#{title}.d/90-limits.conf")
+              .that_notifies('Service[test]')
           end
         end
       end