systemd::dropin_file doesn't cause a systemd daemon-reload

[kenyon]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 6.23.0
• Ruby: 2.5.9
• Distribution: Ubuntu 18.04
• Module version: 3.5.1

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

systemd::dropin_file { 'runtimedirectory.conf':
  unit    => 'nginx.service',
  content => @(EOT),
    # Managed by Puppet
    [Service]
    RuntimeDirectory=nginx
    | EOT
}

What are you seeing

systemd not reloaded.

What behaviour did you expect instead

systemctl daemon-reload should be done.

[ekohl]

Can you check what the output of systemctl show -p NeedDaemonReload nginx.service is when you do think a reload is needed? Also the debug output of a puppet run should show it running that command.

Or is nginx.service completely unmanaged here?

[simondeziel]

I am also affected by the problem at hand. Here's a quick reproducer with zfs-zed.service that is not managed itself, only its dropin file is:

  systemd::dropin_file { 'test.conf':
    unit    => 'zfs-zed.service',
    content => @(EOT),
      # Managed by Puppet
      [Service]
      Restart=always
      | EOT
  }

Then running the agent:

# puppet agent -t
Info: Using environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
Info: Caching catalog for sdeziel-lemur.sdeziel.info
Info: Applying configuration version '1636377736'
Notice: /Stage[main]/Main/Node[sdeziel-lemur.sdeziel.info]/Systemd::Dropin_file[test.conf]/File[/etc/systemd/system/zfs-zed.service.d/test.conf]/ensure: defined content as '{md5}ae5b6c908a22d6b183053ce0a6789025' (corrective)
Notice: Applied catalog in 2.23 seconds

# systemctl show -p NeedDaemonReload zfs-zed.service
NeedDaemonReload=yes

Then with PR #237:

# puppet agent -t
Info: Using environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
Info: Caching catalog for sdeziel-lemur.sdeziel.info
Info: Applying configuration version '1636377835'
Notice: /Stage[main]/Main/Node[sdeziel-lemur.sdeziel.info]/Systemd::Dropin_file[test.conf]/File[/etc/systemd/system/zfs-zed.service.d/test.conf]/ensure: defined content as '{md5}ae5b6c908a22d6b183053ce0a6789025' (corrective)
Info: /Stage[main]/Main/Node[sdeziel-lemur.sdeziel.info]/Systemd::Dropin_file[test.conf]/File[/etc/systemd/system/zfs-zed.service.d/test.conf]: Scheduling refresh of Exec[test.conf-dropin-systemctl-daemon-reload]
Notice: /Stage[main]/Main/Node[sdeziel-lemur.sdeziel.info]/Systemd::Dropin_file[test.conf]/Exec[test.conf-dropin-systemctl-daemon-reload]: Triggered 'refresh' from 1 event
Notice: Applied catalog in 2.65 seconds

# systemctl show -p NeedDaemonReload zfs-zed.service
NeedDaemonReload=no

@ekohl, the key is the base unit_file is not managed by Puppet.

[kenyon]

Yep, same as what @simondeziel wrote. In my case I'm using our Vox Pupuli nginx module to manage nginx, but needed to add a dropin file as a workaround for voxpupuli/puppet-nginx/issues/1372, and noticed this problem with lack of daemon-reload.

[ekohl]

Right, that makes sense. I do wonder how to solve it. Perhaps it should be behind a boolean to either manage it or not? That way users who do manage the service don't get duplicate daemon reloads. I wouldn't want to depend on defined() since it's compile order dependent.

[traylenator]

This crops up with a .timer and .service where the service is never managed by puppet.

Where's the duplicate reload? If we call reload explicity in the puppet run then the one that puppet does automagicly in service restart won't happen. .. Or is there another one?

[ekohl]

That's a good point. I suppose the only duplicate reload would be if there are multiple drop in files.

[traylenator]

There's a good case for adding back the systemctl::daemon_reload class or whatever it was. Multiple notifications to that or the exec would of course de-duplicate things.

Whatever its better to do it > 1 rather than not at all.

[ekohl]

The single class was a recipe for disaster. I've run into problems because of it multiple times. In that case I'd prefer a define that creates one for a service (ideally using onlyif/unless for idempotency rather than refreshonly). Then use ensure_resource in the drop in file.

[simondeziel]

Some systems probably have hundreds or thousands of systemd units managed through puppet so the onlyif/unless overhead added to each run could quickly add up. The systemctl::daemon_reload class had a nice singleton effect, too bad it was a disaster, cause it would have been an easy fix.

[duritong]

This also crops up if content for .service via a systemd::timeris changed and systemd continues to invoke the old cronjob.

Somehow the internal logic of puppet when to execute a daemon-reload is not stable nor trustworthy and we end up adding manual reloads all over the place :-(

[duritong]

This also crops up if content for .service via a systemd::timeris changed and systemd continues to invoke the old cronjob.

I guess this issue got fixed with: #199

[unixsurfer]

I have this problem as well when I use dropin _file for puppet-run.serice, which is installed by systemd::timer, to add OnFailure systemd setting for the aforementioned systemd service. The

class profile::puppet::agent::common {

  $systemd_unit_content = @("SYSTEMD_UNIT")
   # Managed by Puppet
   #
    [Unit]
    Description=Systemd Service for reporting status of Puppet Agent run
    After=network.target

    [Service]
    Type=oneshot
    ExecStart=/usr/bin/logger -t puppet-agent -p local0.err "puppet agent run failed"
    User=root
    Group=root

    [Install]
    WantedBy=puppet-run.service

    | SYSTEMD_UNIT
  systemd::unit_file { 'puppet-report.service':
    content => $systemd_unit_content,
    enable => true,
  }

  $onfailure_content = @("ONFAILURE_CONTENT")
    # Managed by Puppet
    #
    [Unit]
    OnFailure=puppet-report.service

    | ONFAILURE_CONTENT
  systemd::dropin_file { 'onfailure.conf':
    unit           => 'puppet-run.service',
    content        => $onfailure_content,
    require        => Systemd::Unit_file['puppet-report.service'],
    notify_service => true,
  }
}

and systemctl show -p NeedDaemonReload for puppet-run.service shows:

# systemctl show -p NeedDaemonReload puppet-run.service
NeedDaemonReload=yes

I run puppet in debug and verbose mode and I didn't see systemctl show -p running.
I am using puppet 7.12.0 version and 3.5.1 version of puppet-systemd module

[antaflos]

Just stumbled across this issue as well. I guess we need to re-implement systemd::daemon_reload ourselves in order for drop-in files to work correctly, possibly as part of a profile::systemd wrapper.

[antaflos]

We implemented this by means of a profile::systemd class that is included on every systemd-capable node. It contains the following:

  Systemd::Dropin_file <||>
  ~> exec { 'systemctl-daemon-reload':
    command     => 'systemctl daemon-reload',
    path        => $facts['path'],
    refreshonly => true,
  }

Is there a reason this couldn't be part of the systemd module itself? It collects all changes to dropin files and runs systemctl daemon-reload exactly once.

[ekohl]

It used to have that, but it was very common to run into dependency cycles. Suppose you need one service fully running before you configure another, which is quite common.

[trevor-vaughan]

@ekohl If you need to have a service running before configuring another, then your systemd configuration files should reflect that using triggers.

I'm now also running into this issue with systemd::dropin_file. I thought that the original workaound might have fixed things but it has not.

[ekohl]

My main point is that I think a single daemon-reload (like via a class) is likely to cause dependency cycles, which is why I initially pushed for removal. That's why I think we should seriously consider a define. Probably something that we instantiate once per service and then use ensure_resource in every dropin file.

[trevor-vaughan]

Running some tests now and will PR soon.