Document campotocamp/systemd soft dependency

[jarekgrzabel]

Description

This pull request is to merge changes in files listing module dependencies. Latest version (12.0.0) didn't mention camptocamp/systemd module requirement hence when running puppet module install theforeman-puppet puppet run failed with the dependency error.

Changes

README.md - added new dependency section listing all the dependencies
metadata.json - added new values to add dependency for camptocamp/systemd module.

[ekohl]

If this is added, a you can remove this line:

puppet-puppet/spec/spec_helper_acceptance.rb

Line 16 in 15a8ae7

install_module_from_forge('camptocamp-systemd', '>= 2.0.0 < 3.0.0')

However, I'm not sure we need to. It's only needed if you enable the one option and only if you use systemd. Note we also support platforms which are unlikely to ever support that (BSD). However, I can't find the document around optional dependency best practices now.

[jarekgrzabel]

If this is added, a you can remove this line:

puppet-puppet/spec/spec_helper_acceptance.rb

Line 16 in 15a8ae7

install_module_from_forge('camptocamp-systemd', '>= 2.0.0 < 3.0.0')
However, I'm not sure we need to. It's only needed if you enable the one option and only if you use systemd. Note we also support platforms which are unlikely to ever support that (BSD). However, I can't find the document around optional dependency best practices now.

Sorry I'm a bit confused now. Your systemd dependency inside the module is managed by code (

puppet-puppet/manifests/server/puppetserver.pp

Line 195 in 1cd2804

if $facts['service_provider'] == 'systemd' {

) ... if that dependency is not being met then you don't have any systemd conflict. So as long as your code doesn't inject that systemd depencency somewhere it won't be a problem. Downloading the systemd module itself won't cause any harm on BSD systems either unless the module is being called. Similar practice when you need to download the package and you need to support multiple providers (like apt on RHEL systems).

[ekohl]

https://puppet.com/docs/puppet/5.5/style_guide.html#dependencies states:

Soft dependencies should be called out in the README.md, and must not be enforced as a hard requirement in your metadata.json. A soft dependency is a dependency that is only required in a specific set of use cases. For an example, see the rabbitmq module.

This is a rather specific use case: if you enable file limits on the puppetserver when running on systemd. That's why I do feel that mentioning it in README.md should be sufficient.

[jarekgrzabel]

https://puppet.com/docs/puppet/5.5/style_guide.html#dependencies states:

Soft dependencies should be called out in the README.md, and must not be enforced as a hard requirement in your metadata.json. A soft dependency is a dependency that is only required in a specific set of use cases. For an example, see the rabbitmq module.

This is a rather specific use case: if you enable file limits on the puppetserver when running on systemd. That's why I do feel that mentioning it in README.md should be sufficient.

The problem is I don't really understand why you call this a soft dependency. Reason for that is you call part of this module in your code and even if you call it for some specific system type or distribution it is still not a suggested module to download. Without systemd module for that specific system type or distribution your puppet configuration module will not work at all and will drop big fat error.

How does this code work or BSD then if you don't include the systemd::dropin_file define anywhere?

[mmoll]

Not 100% sure, but I think this module would even work on Linux without the systemd modules, as long as the puppetserver features are not used...

The main problem is that in the default config, the dropin is set to absent, which requires the system modules...

[jarekgrzabel]

Are you sure that defaults don't call it ? I use your module to configure initial puppet master and I use only this part to get it configured intially:

/opt/puppetlabs/bin/puppet module install puppetlabs-puppetdb

cat > puppet_master_install.pp <<EOF
class { 'puppet': 
  server => true,
  server_foreman => false,
  dns_alt_names => [ "${puppetmaster_host_name}" ],
  server_external_nodes => '',
  autosign_entries => [ "*.puppet.aws" ],
  server_certname => "${puppetmaster_host_name}",
  client_certname => "${puppetmaster_host_name}",
  ca_server => "${puppetmaster_host_name}",
  server_ca_allow_sans => true,
  puppetmaster => "${puppetmaster_host_name}",
}
EOF
/opt/puppetlabs/bin/puppet apply puppet_master_install.pp

So I'm not calling any dropin stuff by default...

[ekohl]

Looks like you are right that we do manage it even if the limits are not set (ensure => absent) when running on systemd. However, if you set server => false (agent only) it's not included. The FreeBSD family also doesn't call it.

[jarekgrzabel]

I think I will have a problem with understanding what is the real soft and hard dependency in that case. I also disagree with Puppet RabbitMQ module dependency model but that's fine. I just think if you call any third party modules they should become an integral part of the module from 2 reasons:

1. You rely that your code will work on certain version of the other third party module and you probably writing your module you want to make sure that you test against these versions anyway.
2. If you don't control the version of third party module but rely on it in the code, if it gets updated and brings errors to your module code, people will start raising requests to fix the issue. Then you have a situation who's problem is that going to be to fix it.

Anyway I removed that hard dependency from metadata.json and changed README file to mention that it may require systemd module for compatibility with some systems.

[mmoll]

We'll fix the metadata.json nit on merge.

[mmoll]

merged, dzięki @dogjarek!