add jitter to periodic cert rotation and fix spurious writes

[fspmarshall]

Adds randomized interval helper, adds jitter to certificate rotation periodics, and fixes an issue where spurious write events were being generated due to unnecessary calls to UpsertCertAuthority.

[fspmarshall]

I'm not 100% satisfied with the changes to Upsert behavior, but I think its an overall improvement compared to what it was before. In the long run, I think this could be improved by having the auth server perform stringent checks under optimistic lock to ensure that the new CA state is one that should come after the current CA state. I'm not certain what the best plan of attack for this is yet (resource ID isn't meaningful across cluster boundaries, so the usual method doesn't work here).

[russjones]

What do these constants represent?

[fspmarshall]

In terms of what the math is doing, its randomizing up to one seventh of the supplied duration (i.e. creating a jitter on the range of [6n/7,n)).

In terms of why I chose that specific constant, its essentially arbitrary. We needed a smaller jitter than 1/2 (what the default jitter uses) for periodic operations since a jitter of 1/2 could worst-case double the amount of work. I went with a 1/7 jitter because most of our periodic operations happen on a 10 minute interval, and a 1/7 jitter will worst-case cause 1 additional operation per hour when dealing with 10-minute intervals. This is itself a pretty arbitrary goal, but it seemed reasonable. It means that if instances did sync up coincidentally, you'd expect them to be reasonably "desynced" within an hour. They won't be fully desynced, since repeated jitters tend to scatter on a normal distribution, but desynced enough to have a standard deviation of 1 minute.

[awly]

Can you push choosing these fractions up to the caller?
Something like func NewJitter(frac float64) Jitter // returns values between [(1-frac) * n : n), which can then be caller like NewJitter(0.5)

[fspmarshall]

Tried a few different iterations of having caller supply fraction manually. Everything I came up with felt a bit too brittle. Instead, I've renamed the DefaultJitter to HalfJitter and NewSmallJitter to NewSeventhJitter so that the caller now explicitly commits to the desired fraction by selecting the appropriate function. Ex:

interval.New(interval.Config{
    Duration:      defaults.HighResPollingPeriod,
    FirstDuration: utils.HalfJitter(defaults.HighResPollingPeriod),
    Jitter:        utils.NewSeventhJitter(),
})

[r0mant]

Not sure we want to panic in this case, why not just return an error as usual. Or at least rename function to MustNewInterval.

[fspmarshall]

This is deliberate. The Interval is meant to have the same contract as time.NewTicker, which has this condition:

if d <= 0 {
	panic(errors.New("non-positive interval for NewTicker"))
}

By keeping the panic conditions identical, we ensure that tickers and intervals can be swapped back and forth without changing validation requirements. If an invalid duration returned an error, then switching from an interval to a ticker would silently introduce a new panic condition.

[awly]

do we need exponential backoff here too?

[fspmarshall]

Code that needs backoff should use utils.Retry for that (we already use a combination of time.Ticker and utils.Retry in various parts of the codebase where we have periodics have a backoff component). I'd prefer to keep time.Ticker and interval.Interval interchangeable as much as possible so that its easy to swap back to time.Ticker if the extra features of the interval are no longer needed.

[awly]

Suggested change

	defer timer.Stop()
	defer timer.Stop()
	defer close(i.ch)

[fspmarshall]

Not closing the output channel is deliberate and consistent with the behavior of time.Ticker. From the docs of time.Ticker:

Stop does not close the channel, to prevent a concurrent goroutine reading from the channel from seeing an erroneous "tick".

[awly]

Gotcha, can you leave a comment about that in Interval.Stop too?

[awly]

Can you push choosing these fractions up to the caller?
Something like func NewJitter(frac float64) Jitter // returns values between [(1-frac) * n : n), which can then be caller like NewJitter(0.5)