camptocamp / puppet-varnish

Apache License 2.0
36 stars 25 forks source link

varnish_param should require File['/etc/systemd/system/varnish.service'] #28

Open cjeanneret opened 9 years ago

cjeanneret commented 9 years ago

The following puppet receipt will fail in most cases because there's a missing requirement in varnish_param type:

      Service<| title == 'varnish' |> {
        provider => 'systemd',
      }
      class {'::varnish':
        listen_port     => '80',
        group           => 'varnish',
        multi_instances => false,
        secret_file     => '/etc/varnish/secret',
        user            => 'varnish',
      }

      varnish_param {'shm_reclen':
        value  => 4084,
        notify => [
          Exec['systemctl-daemon-reload'],
          Service['varnish'],
        ],
      }

Here's an acceptance test in order to show this behavior.

If we put a dependence like this, it fails due to a dependency cycle:

      Service<| title == 'varnish' |> {
        provider => 'systemd',
      }
      class {'::varnish':
        listen_port     => '80',
        group           => 'varnish',
        multi_instances => false,
        secret_file     => '/etc/varnish/secret',
        user            => 'varnish',
      }->
      varnish_param {'shm_reclen':
        value  => 4084,
        notify => [
          Exec['systemctl-daemon-reload'],
          Service['varnish'],
        ],
      }
Error: Could not apply complete catalog: Found 1 dependency cycle:
(Service[varnish] => Class[Varnish::Service] => Class[Varnish] => Class[Varnish] => Varnish_param[shm_reclen] => Service[varnish])
cjeanneret commented 9 years ago

Note: using systemd capability to override only small parts of the unit file might correct this issue for good:

file {'/etc/systemd/system/varnish.service.d':
  ensure => directory,
  group  => root,
  mode  => '0755',
  owner  => root,
}

and modify varnish_param so that it writes service.conf in this directory, like already done here.

This might be the best way.