(PUP-1881) Correct Windows runinterval behavior #9397
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshcooper
reviewed
Jun 18, 2024
joshcooper
reviewed
Jun 18, 2024
ext/windows/service/daemon.rb
Outdated
| runinterval = 1800 | ||
| log_err("Failed to determine runinterval, defaulting to #{runinterval} seconds") | ||
| else | ||
| runinterval = runinterval.to_i |
There was a problem hiding this comment.
Wondering if we should be more defensive as String#to_i has some surprising behavior:
irb(main):002:0> "foo".to_i
=> 0
However, this seems more strict/correct:
irb(main):003:0> Integer("foo")
(irb):3:in `Integer': invalid value for Integer(): "foo" (ArgumentError)
from (irb):3:in `<main>'
In theory, this should never happen, because puppet config print runtinterval doesn't allow invalid durations to be set:
$ bundle exec puppet config set runinterval minutes --section main
Error: Invalid duration format '"minutes"' for parameter: runinterval
Error: Try 'puppet help config set' for usage
And if something else writes to puppet.conf, the puppet config print command will error and return empty string to stdout:
$ grep runinterval ~/.puppetlabs/etc/puppet/puppet.conf
runinterval=minutes
$ bundle exec puppet config print runinterval --section main
Error: Invalid duration format '"minutes"' for parameter: runinterval
Error: Try 'puppet help config print' for usage
That said, 0 means run all the time, so I think additional strictness would be good.
mhashizume
force-pushed
the
PUP-1881/main/windows-runinterval
branch
2 times, most recently
from
99bf548 to
525ebda
Compare
joshcooper
approved these changes
Jun 18, 2024
Previously when Puppet parsed a runinterval of 0 on Windows, it would set the runinterval to 1800 seconds, Puppet's default runinterval value. This was incorrect behavior, as on other platforms and in the documentation, a runinterval of 0 indicates that Puppet should run continuously. This commit updates the Windows daemon to set the runinterval to 1800 seconds when it receives an empty string and cast to integer in all other cases.
mhashizume
force-pushed
the
PUP-1881/main/windows-runinterval
branch
from
525ebda to
688bf6f
Compare
joshcooper
changed the title
|
@mhashizume does this need to get backported to 7.x? |
|
Backport failed for Please cherry-pick the changes locally. git fetch origin 7.x
git worktree add -d .worktree/backport-9397-to-7.x origin/7.x
cd .worktree/backport-9397-to-7.x
git checkout -b backport-9397-to-7.x
ancref=$(git merge-base 56d07e923c9c7cfd1a9c4e8553ae30fcbd82ee25 688bf6fb24a53f8a8ffefefcea972cabbe3c790f)
git cherry-pick -x $ancref..688bf6fb24a53f8a8ffefefcea972cabbe3c790f |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously when Puppet parsed a runinterval of 0 on Windows, it would set the runinterval to 1800 seconds, Puppet's default runinterval value. This was incorrect behavior, as on other platforms and in the documentation, a runinterval of 0 indicates that Puppet should run continuously.
This commit updates the Windows daemon to set the runinterval to 1800 seconds when it receives an empty string and cast to integer in all other cases.