Inconsistent values for notify_service in replacement for systemd::service_limits

[johnwarburton]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 7.25.0
• Ruby: 2.7.8p225 (2023-03-30 revision 1f4d455848) [x86_64-linux]
• Distribution: Rocky Linux release 9.2 (Blue Onyx)
• Module version: 7.1.0

How to reproduce (e.g Puppet code you use)

The deprecation notice states:

The type systemd::service_limits is deprecated and systemd::manage_dropin or systemd::dropin_file should be used instead.

What are you seeing

Wanting to set LimitNOFILE with the puppet-systemd module, I read the deprecation notice and used systemd::dropin_file as we already have a working example. The limit was set correctly for the service, but on re-review of the module code, I felt using systemd::manage_dropin would be less error prone

When using service_entry in systemd::manage_dropin, the dropin file was correctly created but the service did not have its limit changed. I had to add notify_service: true for the service to be restarted and set the limit correctly

What behaviour did you expect instead

Given the deprecation notice and systemd::manage_dropin or systemd::dropin_file are offered as equal alternatives, I would expect both types to have the same defaults for notify_service, whereas we have different defaults

$ egrep -r 'Boolean.*notify_service' manifests/
manifests/manage_dropin.pp:  Boolean                          $notify_service          = false,
manifests/dropin_file.pp:  Boolean                                     $notify_service          = true,

Any additional information you'd like to impart

I am guessing some of this came from the work in #463
Also, I am well aware I could be told to RTFM, which I ended up doing, but in the spirit of the principle of least surprise and people who come after me, it would be nice to have a consistent default for notify_service (and IMHO, that would be true)

Thanks for such a great module, always appreciate your work

[smortex]

+1 for consistent behavior. IMHO, the default should be to restart what has to be restarted, and offer the possibility to opt-out from restarting services if needed be.