Fix systemctl daemon-reload after file additions

manifests/daemon_reload.pp

@@ -0,0 +1,23 @@
+# @summary Run systemctl daemon-reload
+#
+# @api public
+#
+# @param name
+#   A globally unique name for the resource
+#
+# @param enable
+#   Enable the reload exec
+#
+#   * Added in case users want to disable the reload globally using a resource collector
+#
+define systemd::daemon_reload (
+  Boolean $enable = true
+) {
+  if $enable {
+    exec { "${module_name}-${name}-systemctl-daemon-reload":
+      command     => 'systemctl daemon-reload',
+      refreshonly => true,
+      path        => $facts['path'],
+    }
+  }
+}

manifests/dropin_file.pp

@@ -17,6 +17,7 @@
 # @param mode The mode to set on the dropin file
 # @param show_diff Whether to show the diff when updating dropin file
 # @param notify_service Notify a service for the unit, if it exists
+# @param daemon_reload Call systemd::daemon_reload
 #
 define systemd::dropin_file (
   Systemd::Unit                               $unit,
@@ -32,6 +33,7 @@
   String                                      $mode                    = '0444',
   Boolean                                     $show_diff               = true,
   Boolean                                     $notify_service          = false,
+  Boolean                                     $daemon_reload           = true,
 ) {
   include systemd
 
@@ -69,11 +71,26 @@
     show_diff               => $show_diff,
   }
 
+  if $daemon_reload {
+    ensure_resource('systemd::daemon_reload', $unit)
+
+    File[$full_filename] ~> Systemd::Daemon_reload[$unit]
+  }
+
   if $notify_service {
     File[$full_filename] ~> Service <| title == $unit or name == $unit |>
+
+    if $daemon_reload {
+      Systemd::Daemon_reload[$unit] ~> Service <| title == $unit or name == $unit |>
+    }
+
     if $unit =~ /\.service$/ {
       $short_service_name = regsubst($unit, /\.service$/, '')
       File[$full_filename] ~> Service <| title == $short_service_name or name == $short_service_name |>
+
+      if $daemon_reload {
+        Systemd::Daemon_reload[$unit] ~> Service <| title == $short_service_name or name == $short_service_name |>
+      }
     }
   }
 }

manifests/service_limits.pp

@@ -74,7 +74,8 @@
       command     => "systemctl restart ${name}",
       path        => $facts['path'],
       refreshonly => true,
-      subscribe   => File["${path}/${name}.d/90-limits.conf"],
     }
+
+    Systemd::Dropin_file["${name}-90-limits.conf"] ~> Exec["restart ${name} because limits"]
   }
 }

manifests/timer.pp

@@ -58,6 +58,9 @@
 # @param ensure
 #   Defines the desired state of the timer
 #
+# @param daemon_reload
+#   Call `systemd::daemon_reload`
+#
 define systemd::timer (
   Enum['present', 'absent', 'file']        $ensure = 'present',
   Stdlib::Absolutepath                     $path = '/etc/systemd/system',
@@ -72,6 +75,7 @@
   Boolean                                  $show_diff = true,
   Optional[Variant[Boolean, Enum['mask']]] $enable = undef,
   Optional[Boolean]                        $active = undef,
+  Boolean                                  $daemon_reload = true,
 ) {
   assert_type(Pattern['^.+\.timer$'],$name)
 
@@ -83,28 +87,30 @@
 
   if $service_content or $service_source {
     systemd::unit_file { $_service_unit:
-      ensure    => $ensure,
-      content   => $service_content,
-      source    => $service_source,
-      path      => $path,
-      owner     => $owner,
-      group     => $group,
-      mode      => $mode,
-      show_diff => $show_diff,
-      before    => Systemd::Unit_File[$name],
+      ensure        => $ensure,
+      content       => $service_content,
+      source        => $service_source,
+      path          => $path,
+      owner         => $owner,
+      group         => $group,
+      mode          => $mode,
+      show_diff     => $show_diff,
+      before        => Systemd::Unit_File[$name],
+      daemon_reload => $daemon_reload,
     }
   }
 
   systemd::unit_file { $name:
-    ensure    => $ensure,
-    content   => $timer_content,
-    source    => $timer_source,
-    path      => $path,
-    owner     => $owner,
-    group     => $group,
-    mode      => $mode,
-    show_diff => $show_diff,
-    enable    => $enable,
-    active    => $active,
+    ensure        => $ensure,
+    content       => $timer_content,
+    source        => $timer_source,
+    path          => $path,
+    owner         => $owner,
+    group         => $group,
+    mode          => $mode,
+    show_diff     => $show_diff,
+    enable        => $enable,
+    active        => $active,
+    daemon_reload => $daemon_reload,
   }
 }

manifests/unit_file.pp

@@ -61,6 +61,9 @@
 # @param service_parameters
 #   hash that will be passed with the splat operator to the service resource
 #
+# @param daemon_reload
+#   call `systemd::daemon-reload` to ensure that the modified unit file is loaded
+#
 # @example manage unit file + service
 #   systemd::unit_file { 'foo.service':
 #     content => file("${module_name}/foo.service"),
@@ -85,6 +88,7 @@
   Optional[Boolean]                        $hasstatus = undef,
   Boolean                                  $selinux_ignore_defaults = false,
   Hash[String[1], Any]                     $service_parameters = {},
+  Boolean                                  $daemon_reload = true
 ) {
   include systemd
 
@@ -125,6 +129,12 @@
     selinux_ignore_defaults => $selinux_ignore_defaults,
   }
 
+  if $daemon_reload {
+    ensure_resource('systemd::daemon_reload', $name)
+
+    File["${path}/${name}"] ~> Systemd::Daemon_reload[$name]
+  }
+
   if $enable != undef or $active != undef {
     service { $name:
       ensure     => $active,
@@ -143,15 +153,10 @@
       Service[$name] -> File["${path}/${name}"]
     } else {
       File["${path}/${name}"] ~> Service[$name]
-    }
-  } else {
-    # Work around https://tickets.puppetlabs.com/browse/PUP-9473
-    # and react to changes on static unit files (ie: .service triggered by .timer)
-    exec { "${name}-systemctl-daemon-reload":
-      command     => 'systemctl daemon-reload',
-      refreshonly => true,
-      path        => $facts['path'],
-      subscribe   => File["${path}/${name}"],
+
+      if $daemon_reload {
+        Systemd::Daemon_reload[$name] ~> Service[$name]
+      }
     }
   }
 }

spec/classes/init_spec.rb

@@ -21,6 +21,7 @@
         it { is_expected.not_to contain_class('systemd::coredump') }
         it { is_expected.not_to contain_class('systemd::oomd') }
         it { is_expected.not_to contain_exec('systemctl set-default multi-user.target') }
+        it { is_expected.not_to contain_systemd__daemon_reload('global-lazy') }
 
         context 'when enabling resolved and networkd' do
           let(:params) do

spec/defines/daemon_reload.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'systemd::daemon_reload' do
+  context 'supported operating systems' do
+    on_supported_os.each do |os, facts|
+      context "on #{os}" do
+        let(:facts) { facts }
+        let(:title) { 'irregardless' }
+
+        it { is_expected.to compile.with_all_deps }
+
+        context 'with defaults' do
+          it do
+            expect(subject).to contain_exec("systemd-#{title}-systemctl-daemon-reload").
+              with_command('systemctl daemon-reload').
+              with_refreshonly(true)
+
+            expect(subject).not_to contain_exec("systemd-#{title}-global-systemctl-daemon-check")
+          end
+        end
+
+        context 'when disabled' do
+          let(:params) do
+            { 'enable' => false }
+          end
+
+          it do
+            expect(subject).not_to contain_exec("systemd-#{title}-systemctl-daemon-reload")
+          end
+        end
+      end
+    end
+  end
+end

spec/defines/dropin_file_spec.rb

@@ -28,13 +28,18 @@
           )
         }
 
+        it {
+          expect(subject).to create_systemd__daemon_reload(params[:unit])
+        }
+
         it {
           expect(subject).to create_file("/etc/systemd/system/#{params[:unit]}.d/#{title}").with(
             ensure: 'file',
             content: %r{#{params[:content]}},
             mode: '0444',
             selinux_ignore_defaults: false
-          )
+          ).
+            that_notifies("Systemd::Daemon_reload[#{params[:unit]}]")
         }
 
         context 'notifies services' do
@@ -64,6 +69,7 @@
 
             it { is_expected.to compile.with_all_deps }
             it { is_expected.to contain_service('myservice').that_subscribes_to("File[#{filename}]") }
+            it { is_expected.to contain_systemd__daemon_reload(params[:unit]).that_notifies('Service[myservice]') }
           end
         end
 
@@ -143,6 +149,18 @@
             )
           }
         end
+
+        context 'with daemon_reload = false' do
+          let(:params) do
+            super().merge(daemon_reload: false)
+          end
+
+          it { is_expected.to compile.with_all_deps }
+
+          it {
+            expect(subject).not_to create_systemd__daemon_reload(params[:unit])
+          }
+        end
       end
     end
   end

spec/defines/timer_spec.rb

@@ -84,13 +84,6 @@
 
           it { is_expected.to contain_systemd__unit_file('foobar.timer').with_content("[Timer]\nOnCalendar=hourly") }
           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]') }
-
-          it {
-            expect(subject).to create_exec('foobar.service-systemctl-daemon-reload').with(
-              command: 'systemctl daemon-reload',
-              refreshonly: true
-            )
-          }
         end
 
         context 'with a bad timer name' do

spec/defines/unit_file_spec.rb

@@ -13,9 +13,14 @@
         it { is_expected.to compile.with_all_deps }
 
         context 'with defaults' do
+          it do
+            expect(subject).to create_systemd__daemon_reload(title)
+          end
+
           it do
             expect(subject).to contain_file("/etc/systemd/system/#{title}").
-              with_selinux_ignore_defaults(false)
+              with_selinux_ignore_defaults(false).
+              that_notifies("Systemd::Daemon_reload[#{title}]")
           end
         end
 
@@ -91,7 +96,8 @@
               without_hasrestart.
               without_flags.
               without_timeout.
-              that_subscribes_to("File[/etc/systemd/system/#{title}]")
+              that_subscribes_to("File[/etc/systemd/system/#{title}]").
+              that_subscribes_to("Systemd::Daemon_reload[#{title}]")
           end
         end
 
@@ -250,12 +256,15 @@
           end
         end
 
-        context 'when using default values for enable and active' do
+        context 'with daemon_reload = false' do
+          let(:params) do
+            super().merge(daemon_reload: false)
+          end
+
+          it { is_expected.to compile.with_all_deps }
+
           it {
-            expect(subject).to create_exec("#{title}-systemctl-daemon-reload").with(
-              command: 'systemctl daemon-reload',
-              refreshonly: true
-            )
+            expect(subject).not_to create_systemd__daemon_reload(title)
           }
         end
       end