Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
Feb 23, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12f609c
Drop Puppet 4 and 5 support
ekohl Jan 13, 2021
a76a4b6
Drop daemon-reload code
ekohl Jan 13, 2021
deaf035
Implement a workaround for PUP-9473.
ekohl Feb 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 14 additions & 27 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand All @@ -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',
}
```

Expand All @@ -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':
Expand All @@ -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',
Expand All @@ -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
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down
13 changes: 0 additions & 13 deletions REFERENCE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -238,10 +237,6 @@ Data type: `Boolean`



### `systemd::systemctl::daemon_reload`

Reload the systemctl daemon

### `systemd::tmpfiles`

Update the systemd temp files
Expand Down Expand Up @@ -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
Expand Down
20 changes: 0 additions & 20 deletions manifests/dropin_file.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -60,7 +53,6 @@
String $group = 'root',
String $mode = '0444',
Boolean $show_diff = true,
Enum['lazy', 'eager'] $daemon_reload = 'lazy',
) {
include systemd

Expand Down Expand Up @@ -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'],
}
}
}
2 changes: 0 additions & 2 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'] {
Expand Down
1 change: 0 additions & 1 deletion manifests/service_limits.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
path => $::path,
refreshonly => true,
subscribe => File["${path}/${name}.d/90-limits.conf"],
require => Class['systemd::systemctl::daemon_reload'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit unsure about this (which is also why this is a draft). This isn't a service type so it doesn't perform the reload if needed. Not sure how to deal with this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raphink any thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks likes it should stay then...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if the class to require is gone, should there be a local daemon reload?

}
}
}
1 change: 0 additions & 1 deletion manifests/system.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
section => 'Manager',
setting => $option,
value => $value,
notify => Class['systemd::systemctl::daemon_reload'],
}
}
}
10 changes: 0 additions & 10 deletions manifests/systemctl/daemon_reload.pp
View file
Open in desktop

This file was deleted.

10 changes: 8 additions & 2 deletions manifests/unit_file.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
group => $group,
mode => $mode,
show_diff => $show_diff,
notify => Class['systemd::systemctl::daemon_reload'],
}

if $enable != undef or $active != undef {
Expand All @@ -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}"],
}
}
}
2 changes: 1 addition & 1 deletion metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion spec/classes/init_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down
15 changes: 0 additions & 15 deletions spec/classes/systemctl/daemon_reload_spec.rb
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions spec/defines/dropin_file_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' }

Expand Down
2 changes: 0 additions & 2 deletions spec/defines/unit_file_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down