(PUP-2958) Protect the SSL state machine with an ssl lockfile#7530
(PUP-2958) Protect the SSL state machine with an ssl lockfile#7530kris-bosland merged 5 commits intopuppetlabs:masterfrom
Conversation
Pass cert and ssl providers to the state machine so that we can allow/expect on those instances rather than relying on any_instance, which is more brittle.
Use break instead of return to avoid non-local return.
Previously the Timeout::Error referred to a non-existent `@path` instance variable. Refer to the `path` argument instead and update the spec test to match.
Puppet will poll for the lockfile, up to the maximum `timeout` which defaults to 5 minutes. Add debugging messages so it's clear what's going on when running with `puppet agent -td`.
|
CLA signed by all contributors. |
|
jenkins please test this on windows2016-64a with servertests |
lib/puppet/ssl/state_machine.rb
Outdated
| # | ||
| # @private | ||
| class Puppet::SSL::StateMachine | ||
| LOCKFILE_TIMEOUT_SECS = 5 |
There was a problem hiding this comment.
Is this long enough? Can it be user configurable?
There was a problem hiding this comment.
Yeah I debated making it configurable. One thing is right now there isn't any feedback if the agent is stuck polling for the lock, unless you're running with debug. I imagine that could cause some confusion. Thinking about making the retry message a warning instead:
Warning: Failed to lock '/Users/josh/.puppetlabs/etc/puppet/ssl/ssl.lock' retrying in 1.94 milliseconds
Thoiughts? Then I could increase the default timeout and make it configurable.
There was a problem hiding this comment.
I like the retry message.
There was a problem hiding this comment.
I made it configurable. I didn't make the message a warning, because I'm concerned it could cause transient failures in CI, due to tests matching puppet agent output, especially in PE.
lib/puppet/ssl/state_machine.rb
Outdated
| # | ||
| # @private | ||
| class Puppet::SSL::StateMachine | ||
| LOCKFILE_TIMEOUT_SECS = 5 |
There was a problem hiding this comment.
I like the retry message.
joshcooper
force-pushed
the
rerevert-ssl-lock
branch
from
d8c09d9 to
48c64d3
Compare
|
At least one of the AppVeyor failures looks legit |
joshcooper
force-pushed
the
rerevert-ssl-lock
branch
from
48c64d3 to
cea456d
Compare
Adds a new `Puppet[:ssl_lockfile]` setting of type `:string`. We don't use `:file`, because we don't want puppet to create it when applying settings catalogs. Protect the ssl state machine with the ssl lockfile.
joshcooper
force-pushed
the
rerevert-ssl-lock
branch
from
cea456d to
be4abce
Compare
|
I reworked this to not use |
4 participants
Commit 926e0e6 locked the agent lockfile before running the SSL state machine, but had to be reverted in 3e82d0f due to
puppet infrastructurelocking the agent lock and runningpuppet ssl bootstrap.These commits introduce a new ssl lockfile so that the ssl and catalog application lock files have different scopes.