Fix clash with modules that manage the unit folder too

[hdeheer]

Other puppet modules in a puppet setup might manage the {unit}.d folder as well. For example puppetlabs/docker does (See https://github.com/puppetlabs/puppetlabs-docker/blob/main/manifests/service.pp). If one still wants to add limits overrides via this systemd module, puppet will complain about duplicate resource declaration.

[puppet-community-rangefinder[bot]]

systemd::dropin_file is a type

Breaking changes to this file WILL impact these 22 modules (exact match):

• MiamiOH-moab
• camptocamp-es_lite
• marionetoj-logrotate
• hfm-proxysql
• puppet-mlocate
• puppet-nftables
• mark0n-epics
• puppet-ferm
• saz-memcached
• puppet-proxysql
• puppet-snmp
• katello-qpid
• theforeman-dhcp
• simp-nfs
• simp-rsyslog
• theforeman-puppet
• simp-useradd
• osc-openondemand
• sensu-sensu
• puppet-check_mk
• theforeman-foreman
• treydock-slurm

Breaking changes to this file MAY impact these 4 modules (near match):

• aursu-grayloginstall
• simp-tpm2
• leoarnold-cups
• simp-selinux

This module is declared in 8 of 576 indexed public Puppetfiles.

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[hdeheer]

@ekohl You would think so but unfortunately in our case it doesn't. The detail is in the explanation of the function ensure_resource.

If the resource already exists but does not match the specified parameters, this function will attempt to recreate the resource leading to a duplicate resource definition error.

You can test yourself: ensure.pp

file { '/tmp/foo.d':
  ensure => directory
}

ensure_resource('file', '/tmp/foo.d', {
  ensure                  => directory,
  owner                   => 'root',
  group                   => 'root',
  recurse                 => false,
  purge                   => false,
  selinux_ignore_defaults => false,
})

Results in:

$ puppet apply ensure.pp
Error: Evaluation Error: Error while evaluating a Function Call, Duplicate declaration: File[/tmp/foo.d]
is already declared at (file: /tmp/ensure.pp, line: 1); cannot redeclare (file: /tmp/ensure.pp, line: 5)
(file: /tmp/ensure.pp, line: 5, column: 1) on node

[raphink]

Iirc, ensure_resource only works if the parameters are exactly the same in both declarations.

[hdeheer]

That's why we had to add the if !defined check. For standalone usage inside the systemd module, ensure_resource works like it should. But you will run into puppet failures when the {unit}.d folder is being managed by another puppet module. @raphink how can I get this change merged?

[ekohl]

@hdeheer do you know which parameters don't match?

[hdeheer]

@ekohl Yes. For example https://github.com/puppetlabs/puppetlabs-docker/blob/main/manifests/service.pp only uses:

• ensure

file { '/etc/systemd/system/docker.service.d':
  ensure => 'directory',
}

But the systemd module manifests/dropin_file.pp uses:

• ensure
• owner
• group
• recurse
• purge
• selinux_ignore_defaults

Thus resulting in ensure_resource throwing an error due to mismatch in amount of specified parameters.

Easy fix in our case is to use if !defined.

[ekohl]

I still think it's a bad idea. This module depends on certain behavior, such as purge. If that's not specified, you get unpredicatable behavior. Namely, depending on the parse order it could purge unmanaged files or not. That can be a nasty surprise.

[raphink]

I'm also not a fan of this at all. I'd rather contribute to the Docker module to depend on the systemd module.

[vox-pupuli-tasks[bot]]

Dear @hdeheer, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

[traylenator]

This should be closed.

Will close after the solstice.

[TheMeier]

Winter solstice 2023 @traylenator? :D