Drop Puppet 4 and 5 support + daemon-reload code (#171)

* Drop Puppet 4 and 5 support This picks version 6.1.0 as a new lower bound since that contains code to automatically run daemon-reload if needed. Versions 4 and 5 are EOL. * Drop daemon-reload code Since Puppet 6.1.0 it's no longer needed to run daemon-reload manually when restarting a service. That means it's possible to drop this code. * Implement a workaround for PUP-9473. Prior to this commit, the follow code did not suffer from PUP-9473: systemd::unit_file { 'myservice.service': ensure => absent, active => true, } That's because the module considers that an invalid state and fails to compile. The follow code did trigger the bug: systemd::unit_file { 'myservice.service': ensure => absent, } service { 'myservice': ensure => running, require => Systemd::Unit_file['myservice.service'], } That's precisely what happens when a module switches from a module-provided unit file to a package-provided unit file.
voxpupuli · Feb 23, 2021 · 97dd16f · 97dd16f
1 parent 8f68b0d
commit 97dd16f
Show file tree

Hide file tree

Showing 13 changed files with 23 additions and 111 deletions.
diff --git a/README.md b/README.md
@@ -18,7 +18,7 @@ There are two ways to use this module.
 
 ### unit files
 
-Let this module handle file creation and systemd reloading.
+Let this module handle file creation.
 
 ```puppet
 systemd::unit_file { 'foo.service':
@@ -29,23 +29,18 @@ systemd::unit_file { 'foo.service':
 }
 ```
 
-Or handle file creation yourself and trigger systemd.
+This is equivalent to:
 
 ```puppet
-include systemd::systemctl::daemon_reload
-
 file { '/usr/lib/systemd/system/foo.service':
   ensure => file,
   owner  => 'root',
   group  => 'root',
   mode   => '0644',
   source => "puppet:///modules/${module_name}/foo.service",
 }
-~> Class['systemd::systemctl::daemon_reload']
-
-service {'foo':
-  ensure    => 'running',
-  subscribe => File['/usr/lib/systemd/system/foo.service'],
+~> service {'foo':
+  ensure => 'running',
 }
 ```
 
@@ -63,7 +58,7 @@ systemd::unit_file { 'foo.service':
 
 Drop-in files are used to add or alter settings of a unit without modifying the
 unit itself. As for the unit files, the module can handle the file and
-directory creation and systemd reloading:
+directory creation:
 
 ```puppet
 systemd::dropin_file { 'foo.conf':
@@ -75,11 +70,9 @@ systemd::dropin_file { 'foo.conf':
 }
 ```
 
-Or handle file and directory creation yourself and trigger systemd:
+This is equivalent to:
 
 ```puppet
-include systemd::systemctl::daemon_reload
-
 file { '/etc/systemd/system/foo.service.d':
   ensure => directory,
   owner  => 'root',
@@ -93,29 +86,18 @@ file { '/etc/systemd/system/foo.service.d/foo.conf':
   mode   => '0644',
   source => "puppet:///modules/${module_name}/foo.conf",
 }
-~> Class['systemd::systemctl::daemon_reload']
-
-service {'foo':
-  ensure    => 'running',
-  subscribe => File['/etc/systemd/system/foo.service.d/foo.conf'],
+~> service {'foo':
+  ensure => 'running',
 }
 ```
 
-Sometimes it's desirable to reload the systemctl daemon before a service is refreshed (for example:
-when overriding `ExecStart` or adding environment variables to the drop-in file).  In that case,
-use `daemon_reload => 'eager'` instead of the default `'lazy'`.  Be aware that the daemon could be
-reloaded multiple times if you have multiple `systemd::dropin_file` resources and any one of them
-is using `'eager'`.
-
 dropin-files can also be generated via hiera:
 
 ```yaml
-
 systemd::dropin_files:
   my-foo.conf:
     unit: foo.service
     source: puppet:///modules/${module_name}/foo.conf
-
 ```
 
 ### tmpfiles
@@ -148,7 +130,6 @@ Create a systemd timer unit and a systemd service unit to execute from
 that timer
 
 The following will create a timer unit and a service unit file.
-The execution of `systemctl daemon-reload` will occur.
 When `active` and `enable` are set to `true` the puppet service `runoften.timer` will be
 declared, started and enabled.
 
@@ -240,6 +221,12 @@ systemd::service_limits { 'foo.service':
 }
 ```
 
+### Daemon reloads
+
+Systemd caches unit files and their relations. This means it needs to reload, typically done via `systemctl daemon-reload`. Since Puppet 6.1.0 ([PUP-3483](https://tickets.puppetlabs.com/browse/PUP-3483)) takes care of this by calling `systemctl show $SERVICE -- --property=NeedDaemonReload` to determine if a reload is needed. Typically this works well and removes the need for `systemd::systemctl::daemon_reload` as provided prior to camptocamp/systemd 3.0.0. This avoids common circular dependencies.
+
+It does contain a workaround for [PUP-9473](https://tickets.puppetlabs.com/browse/PUP-9473) but there's no guarantee that this works in every case.
+
 ### network
 
 systemd-networkd is able to manage your network configuration. We provide a

diff --git a/REFERENCE.md b/REFERENCE.md
@@ -9,7 +9,6 @@
 #### Public Classes
 
 * [`systemd`](#systemd): This module allows triggering systemd commands once for all modules
-* [`systemd::systemctl::daemon_reload`](#systemdsystemctldaemon_reload): Reload the systemctl daemon
 * [`systemd::tmpfiles`](#systemdtmpfiles): Update the systemd temp files
 
 #### Private Classes
@@ -238,10 +237,6 @@ Data type: `Boolean`
 
 
 
-### `systemd::systemctl::daemon_reload`
-
-Reload the systemctl daemon
-
 ### `systemd::tmpfiles`
 
 Update the systemd temp files
@@ -371,14 +366,6 @@ Data type: `Boolean`
 
 Default value: ``true``
 
-##### `daemon_reload`
-
-Data type: `Enum['lazy', 'eager']`
-
-
-
-Default value: `'lazy'`
-
 ### `systemd::network`
 
 -- Define: systemd::network

diff --git a/manifests/dropin_file.pp b/manifests/dropin_file.pp
@@ -40,13 +40,6 @@
 # @attr show_diff
 #   Whether to show the diff when updating dropin file
 #
-# @attr daemon_reload
-#   Set to `lazy` to defer execution of a systemctl daemon reload.
-#   Minimizes the number of times the daemon is reloaded.
-#   Set to `eager` to immediately reload after the dropin file is updated.
-#   Useful if the daemon needs to be reloaded before a service is refreshed.
-#   May cause multiple daemon reloads.
-#
 define systemd::dropin_file (
   Systemd::Unit                               $unit,
   Systemd::Dropin                             $filename                = $name,
@@ -60,7 +53,6 @@
   String                                      $group                   = 'root',
   String                                      $mode                    = '0444',
   Boolean                                     $show_diff               = true,
-  Enum['lazy', 'eager']                       $daemon_reload           = 'lazy',
 ) {
   include systemd
 
@@ -95,16 +87,4 @@
     selinux_ignore_defaults => $selinux_ignore_defaults,
     show_diff               => $show_diff,
   }
-
-  if $daemon_reload == 'lazy' {
-    File["${path}/${unit}.d/${filename}"] ~> Class['systemd::systemctl::daemon_reload']
-  } else {
-    File["${path}/${unit}.d/${filename}"] ~> Exec["${unit}-systemctl-daemon-reload"]
-
-    exec { "${unit}-systemctl-daemon-reload":
-      command     => 'systemctl daemon-reload',
-      refreshonly => true,
-      path        => $facts['path'],
-    }
-  }
 }
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -150,8 +150,6 @@
   Hash                                                   $dropin_files = {},
   Hash                                                   $udev_rules = {},
 ) {
-  contain systemd::systemctl::daemon_reload
-
   create_resources('systemd::service_limits', $service_limits)
 
   if $manage_resolved and $facts['systemd_internal_services'] and $facts['systemd_internal_services']['systemd-resolved.service'] {

diff --git a/manifests/service_limits.pp b/manifests/service_limits.pp
@@ -75,7 +75,6 @@
       path        => $::path,
       refreshonly => true,
       subscribe   => File["${path}/${name}.d/90-limits.conf"],
-      require     => Class['systemd::systemctl::daemon_reload'],
     }
   }
 }
diff --git a/manifests/system.pp b/manifests/system.pp
@@ -12,7 +12,6 @@
       section => 'Manager',
       setting => $option,
       value   => $value,
-      notify  => Class['systemd::systemctl::daemon_reload'],
     }
   }
 }
diff --git a/manifests/systemctl/daemon_reload.pp b/manifests/systemctl/daemon_reload.pp
diff --git a/manifests/unit_file.pp b/manifests/unit_file.pp
@@ -85,7 +85,6 @@
     group     => $group,
     mode      => $mode,
     show_diff => $show_diff,
-    notify    => Class['systemd::systemctl::daemon_reload'],
   }
 
   if $enable != undef or $active != undef {
@@ -103,8 +102,15 @@
       }
       Service[$name] -> File["${path}/${name}"]
     } else {
-      Class['systemd::systemctl::daemon_reload'] -> Service[$name]
       File["${path}/${name}"] ~> Service[$name]
     }
+  } elsif $ensure == 'absent' {
+    # Work around https://tickets.puppetlabs.com/browse/PUP-9473
+    exec { "${name}-systemctl-daemon-reload":
+      command     => 'systemctl daemon-reload',
+      refreshonly => true,
+      path        => $facts['path'],
+      subscribe   => File["${path}/${name}"],
+    }
   }
 }
diff --git a/metadata.json b/metadata.json
@@ -81,7 +81,7 @@
   "requirements": [
     {
       "name": "puppet",
-      "version_requirement": ">= 4.10.10 < 7.0.0"
+      "version_requirement": ">= 6.1.0 < 7.0.0"
     }
   ],
   "pdk-version": "1.18.0",

diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb
@@ -8,7 +8,6 @@
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to create_class('systemd') }
-        it { is_expected.to create_class('systemd::systemctl::daemon_reload') }
         it { is_expected.to contain_class('systemd::journald') }
         it { is_expected.to create_service('systemd-journald') }
         it { is_expected.to have_ini_setting_resource_count(0) }

diff --git a/spec/classes/systemctl/daemon_reload_spec.rb b/spec/classes/systemctl/daemon_reload_spec.rb
diff --git a/spec/defines/dropin_file_spec.rb b/spec/defines/dropin_file_spec.rb
@@ -44,22 +44,6 @@
           it { is_expected.to create_file("/etc/systemd/system/#{params[:unit]}.d/#{title}").with_selinux_ignore_defaults(true) }
         end
 
-        context 'with daemon_reload => lazy (default)' do
-          it { is_expected.to create_file("/etc/systemd/system/#{params[:unit]}.d/#{title}").that_notifies('Class[systemd::systemctl::daemon_reload]') }
-
-          it { is_expected.not_to create_exec("#{params[:unit]}-systemctl-daemon-reload") }
-        end
-
-        context 'with daemon_reload => eager' do
-          let(:params) do
-            super().merge(daemon_reload: 'eager')
-          end
-
-          it { is_expected.to create_file("/etc/systemd/system/#{params[:unit]}.d/#{title}").that_notifies("Exec[#{params[:unit]}-systemctl-daemon-reload]") }
-
-          it { is_expected.to create_exec("#{params[:unit]}-systemctl-daemon-reload") }
-        end
-
         context 'with a bad unit type' do
           let(:title) { 'test.badtype' }
 

diff --git a/spec/defines/unit_file_spec.rb b/spec/defines/unit_file_spec.rb
@@ -15,7 +15,6 @@
             .with_ensure('file')
             .with_content(%r{#{params[:content]}})
             .with_mode('0444')
-            .that_notifies('Class[systemd::systemctl::daemon_reload]')
         end
 
         context 'with a bad unit type' do
@@ -45,7 +44,6 @@
               .with_enable(true)
               .with_provider('systemd')
               .that_subscribes_to("File[/etc/systemd/system/#{title}]")
-              .that_requires('Class[systemd::systemctl::daemon_reload]')
           end
         end