-
Notifications
You must be signed in to change notification settings - Fork 23
Add support for multiple cron agent runs per hour #167
base: master
Are you sure you want to change the base?
Add support for multiple cron agent runs per hour #167
Conversation
Use map function from future parser to allow cron to run the agent more than once per hour.
@@ -51,6 +51,7 @@ | |||
$gentoo_keywords = $puppet::params::agent_keywords, | |||
$manage_package = true, | |||
$stringify_facts = $puppet::server::stringify_facts, | |||
$frequency = 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename this to $cron_frequency please? (only here, not in puppet::agent::cron)
I like it, thanks for working on it |
@@ -16,9 +17,13 @@ | |||
$cmd = "${puppet::params::puppet_cmd} agent --confdir ${puppet::params::puppet_confdir} --onetime --no-daemonize >/dev/null" | |||
} | |||
|
|||
$interval = 60 / $frequency | |||
$random_offset = fqdn_rand($interval) | |||
$cron_schedule = $frequency.map |$value| { ($value * $interval) + $random_offset} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhm, does this mean it's puppet 4 only code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@igalic correct, future-parser only. I'm no longer running current parser anywhere so I have no personal need for backwards compatibility, but I wanted to get feedback before going further because I realize that not everybody is ready to make the switch.
If requiring future parser isn't feasible for other puppet-puppet users, I think the second-best option would be to simply parameterize puppet::agent to accept a field passed directly to the minute
parameter on the cron resource, defaulting to fqdn_rand(60)
, and leaving a comment / documentation to suggest adding code like this future-parser-only material in the profile that wraps puppet-puppet for people who want to use it.
If future-parser-only code would cause problems for you, I'm totally fine solving this another way -- I just wanted to propose what I see as the most elegant fix first, in case others are fine with future parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gosh, i really want to switch.
i wonder how many of the modules i use would cough up blood the moment i do…
@thrnio how's that python tool coming you were working on, for testing catalog compiles? Seems like something @igalic could use to test for future parser support in his current infrastructure, if it's at a point where we could put it on github. |
@danieldreier I want to implement catalog comparison support, then I can take some time to pull the code out and clean it up for public consumption. I hope to spend some time later this week on it. |
Use map function from future parser to allow cron to run the agent more than once per hour.
This re-implements the functionality requested in PR #162, being the ability to run the agent multiple times per hour using cron.
I just hacked this together and wanted to get feedback, both from @tampakrap as to whether it meets his needs, and from other puppet-puppet users because this has a hard dependency on future parser and the compile will fail with current parser. Puppet Forge currently doesn't have a way to indicate a future parser dependency, so we'd need to update the readme with a prominent warning about the future parser dependency.
If this looks like a reasonable approach, I'll write some test coverage, update the readme, and update this PR.