version 4 doesn't obey manage_cron_daily parameter

[vchepkov]

In version 4 when manage_cron_daily is set to false module removes /etc/cron.daily/logrotate
That's managing the file, isn't it? I think this behavior is improper, resource should be left alone for user to handle

[baurmatt]

Same for us. We explicitly chose to set manage_cron_daily => false and manage our own cron. The introduced change leads to a duplicate declaration of the Logrotate::Cron['daily'] resource. I would expect that a parameter prefixed with manage_ include or excludes a resource from the catalog. For the actual state management I would expect a parameter prefixed with ensure_.

This change was introduced in #137

I tend to revert this change.

@cliff-wakefield as author of the PR and @bastelfreak and @dhollinger as reviewer, could you please let us know your opinion on this? :)

[Rathios]

This is unexpected behavior we noticed as well. We wanted the Logrotate module to leave the file alone but the module decided instead to remove it. The expected behavior of manage_ parameters is whether or not to care, not whether or not to set present or absent.

[cliff-wakefield]

While I don't fully agree I can see why you may want to split ensure_ and manage_ intentions, but it does create confusion.

The intent of the original PR was to allow cron entries for daily and hourly to be managed outside of the module. Such that you could override the OS defaults for the exact time daily and hourly entries executed.

If we split the manage_ and ensure_ we need to know the desired behaviour.

manage_cron_	ensure_cron_	Result
false	absent	Module will ensure the cron entry does not exist and thus not manage it
false	present	Ambiguous, module is unable to not manage the content but ensure it exists
true	present	Module will ensure the cron entry exists and manage its content
true	absent	Ambiguous, module is unable to manage the content but ensure it doesn't exist

So with that logic, it doesn't work.

I think ensure_cron_ would need to have three allowed values present, absent and ignore resulting in

manage_cron_	ensure_cron_	Result
false	absent	Module will ensure the cron entry does not exist
false	present	Alert & Fail with an error as the module can't manage the existence of the file without managing it
false	ignore	Module will not manage it and will not remove cron entry if it exists
true	absent	Module will ensure the cron entry does not exist, despite being told to manage it
true	present	Module will ensure the cron entry exists and manage it
true	ignore	Alert & Fail with an error as the module can't manage the content but ignore the file

Thoughts @bastelfreak , @dhollinger ?

[legooolas]

I've also hit this and would prefer manage_cron_daily to toggle managing the resource or not, not whether it is present or absent.