This module is not indempotent

[roidelapluie]

Due to systemd design, this module is not indempotent.

The problem

include ::systemd
file { '/usr/lib/systemd/system/foo.service':
  ensure => file,
  owner  => 'root',
  group  => 'root',
  mode   => '0644',
  source => "puppet:///modules/${module_name}/foo.service",
} ~>
Exec['systemctl-daemon-reload']

service{
  'foo':
    ensure => started,
    subscribe => File['/usr/lib/systemd/system/foo.service']
    require => Exec['systemctl-daemon-reload']
  }
}

file { '/usr/lib/systemd/system/foo2.service':
  ensure => file,
  owner  => 'root',
  group  => 'xyz',
  mode   => '0644',
} ~>
Exec['systemctl-daemon-reload']

First run:

• File[foo.service] is changed
• File[foo2.service] file fails during catalog execution (imagine that group xyz does not exist)
• Exec['systemctl-daemon-reload'] is not called because File[foo2] failed
• Service[foo] is not restarted because Exec['systemctl-daemon-reload'] is skipped

Fix File[foo2.service].

Second run:

• File[foo.service] is not changed
• File[foo2.service] file is changed
• Exec['systemctl-daemon-reload'] istriggered
• Service[foo] is not restarted because File[foo.service]has not been changed uring this run.

Proposal

Use Defines instead of classee

file { '/usr/lib/systemd/system/foo.service':
  ensure => file,
  owner  => 'root',
  group  => 'root',
  mode   => '0644',
  source => "puppet:///modules/${module_name}/foo.service",
} ~>
systemd::daemonreload{
  'foo.service':
}

service{
  'foo':
    ensure => started,
    subscribe => File['/usr/lib/systemd/system/foo.service']
    require => Systemd::Daemonreload['foo']
  }
}

file { '/usr/lib/systemd/system/foo2.service':
  ensure => file,
  owner  => 'root',
  group  => 'root',
  mode   => '0644',
  source => "NONEXISTANT",
} ~>
systemd::daemonreload{
  'foo2.service':
}

Upides of proposal

It makes the module indempotent if used as described here.

Downsides

calling systemd daemon-reload is blocking and may cause problems in very large systems.

Mitigation: Use only one reload.

Backwards compatibility: Offer a class which contains systemd::daemonreload {'generic':}

TODO

• [ ] Collect some comments
• [ ] Implement the define
• [ ] Implement backward compatibility
• [ ] Document that in README

[faxm0dem]

as discussed on freenode/#puppet with @roidelapluie the proposed change is to replace the Exec[daemon-reload] with a defined type. Now #2 is an open discussion on wether to replace the Exec with a Class.

So I guess this boils down to: should we use a Class or a Define? I'll update the discussion in #2

[roidelapluie]

https://gist.github.com/roidelapluie/069223753a576983de39

[roidelapluie]

@raphink @mcanevet @mfournier can we have your input?

[rnelson0]

Is this and #2 still being worked on? There was some interest from puppet/r10k in using this module but this issue came up as a potential issue.

[bastelfreak]

ping @raphink @mcanevet. Do you have any objections on this? Let us know if there isn't any active work on this module. If so, we could transfer it to Vox Pupuli?

[roidelapluie]

No work ATM on my side

[cedef]

I would like to know if this module is still maintained or if it should be migrated to Vox Pupuli as suggested by @bastelfreak

[raphink]

We actually only use the main class of this module, which is why we're not very reactive on adding features to it. Transfering to Vox Pupuli is probably a good option.

[bastelfreak]

Hi @raphink. Let me know if you want to transfer it. You currently have the github permissions to do it.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[trevor-vaughan]

Pretty sure this is resolved. Is anyone still having issues with this?

[jhoblitt]

I believe this is a non-issue, File[foo.service] should still notify Exec['systemctl-daemon-reload'] even if other resources, which also notify the Exec, fail. If that isn't the case, than it is a bug in the puppet core.