Dependency cycles when using with theforeman/puppet-dhcp

[fraenki]

Hi,

I get the following dependency cycles when using this in conjunction with theforeman/puppet-dhcp:

Error: Found 1 dependency cycle:
(Exec[pkg update] => Package[isc-dhcp44-server] => File[/usr/local/etc] => File[/usr/local/etc/pkg.conf] => Exec[pkg update])

The only thing I've noticed is that theforeman/puppet-dhcp manages the /usr/local/etc directory and has a package referenced:

  file { $dhcp_dir:
    mode    => '0755',
    require => Package[$packagename],
  }

(https://github.com/theforeman/puppet-dhcp/blob/53e8e4abe0f8da9d85aec883098f2e43e09ca859/manifests/init.pp#L68-L71)

I've read the relevant parts in theforeman/puppet-dhcp and it looks good to me, not an uncommon pattern.

It looks like it's an unfortunate choice of resource relationships causes this issue. Do you see any chance to adjust the related resource relationships in the pkng module to avoid this kind of dependency cycle?

[zachfi]

Hi, thank you for raising the issue. I've looked this over a bit and I don't see a simple solution without a change to either this module or the dhcp module. This is tricky, but a couple things come to mind.

I do wonder what situation requires the file resource you point to. I'd expect that in the case of redhat and debian both, that the /etc/dhcp directory would be created with the correct permissions without modification. The concat resources already require the package, so maybe it lines up without that resource.

I see also that the dhcp module is using a somewhat older pattern of passing parameters based on hiera data. Converting to this newer pattern could make it much simpler to include a new boolean that says something like manage_dhcp: false for freebsd, and returns true for everything else. Wrapping a condition around the file resource for the value of that variable could skip the resource on freebsd.

The ordering of this module is largely managed by these lines, which are the source of the resource conflict.

  Exec['pkg update']
  -> Package <| provider == 'pkgng' or provider == undef |>

Without the ordering here, changes to pkg.conf or repo files may require more puppet runs to be accurate, and could result in package installation from undesirable sources the first run, and then reinstall those packages from another repo on the second puppet run.

Updating the dhcp module as mentioned above I think would be my preference, but I'm also not seeing a clear path forward without that change. It would also get dhcp following a more modern hiera pattern of module if that were included in the final solution.