Puppet Community

Sorry for the bother but this is literally a roadblock for us right now. I'm hoping that without that one change, things will be fine

Is there anyone else that might be able to review?

tvaughan: btw, you may find <https://github.com/theforeman/puppet-puppet/issues/832> also interesting - I only saw this on Ubuntu Focal and I should probably file a bug against systemd

tl;dr: on Ubuntu 20.04 systemctl show --property NeedDaemonReload is unreliable when the $unit.d directory is created

anyone have any thoughts on this? and if Tim is busy, maybe we can drop Arch support, or at least tests? <https://github.com/voxpupuli/puppet-rabbitmq/pull/905>

Thanks for the review <@U11L48K6J> An approval of <https://github.com/voxpupuli/puppet-yum/pull/253> and release with this fix would be handy for EL8+

Honestly, just switching it to `include` is sufficient to prevent circular deps and issues with class resource declarations

<@U01AZE7JFPE> What were you trying to solve with that? :point_up:

I was trying to prevent circular dep loops

Yeah, just switching over to `include` will do that

I don't think it would be a correct change. plugin class makes sure that versionlock package is installed , why wouldn't it be required ?

The `include` ensures the RPM is loaded, but a puppet `require` makes a dep loop if you want to use a param to generate a bunch of locks inline

<https://github.com/voxpupuli/puppet-yum/pull/253/files#diff-26ceebd537058327adf8aa88179f114e1497893397deddcc737995677e6cb460R47>

Yeah, I see where that could cause an issue pretty quickly.

Yep, that test works as expected. I flipped it back and did, indeed, get dependency cycles.

:facepalm: Oh...I see the issue. The `yum::plugin::versionlock` class should not have calls to `yum::versionlock` in it if it's going to use `require`. The `require` is generally correct in terms of wanting to make sure that the package is installed first.

So, either that package loop needs to go into a class or the `yum::plugin` call needs to go into a class.

(Or you leave it as-is because it's probably fine)

But it's not 100% guaranteed to be correct

right, in this case it just so happens that existing directory is used to store configuration file, but in general case there would be a failure if package creates a directory structure.

<@U01AZE7JFPE> Yeah, the `require` needs to go back but the `plugin::versionlock::packages` parameter needs to either be removed or farmed out to another class to do the resource creation.