Add a random sleep for noninteractive renewals

[schoen]

Fixes #6391.

[schoen]

The testing for this is a little tricky because the tox tests cause isatty() to be False all the time, meaning that every renewal test ends up sleeping and we end up getting over the maximum allowed test time. It looks like we need to mock it in the other direction, to persuade most of the tests that the input is interactive.

[ohemorange]

Is this ready for a review?

[schoen]

Hi @ohemorange, I'm working on this today—I think the functionality is right but the tests are a little tricky because currently all renewal tests are doing the random sleep which makes Travis take way too long to complete them. Hopefully the tests will be fixed up later today and then you could review it!

[ohemorange]

I'm guessing the title change means it's ready for a review now?

[schoen]

Yes please!

[ohemorange]

The meat of it looks good!

[ohemorange]

It's probably safer to put this down in main the function, so we catch any subcommand that people might have put in their crontab.

[schoen]

Per our chat discussion, maybe we can find out in Sumologic whether there are any load spikes that can be attributed to uses of certonly (I suspect that they're all due to uses renew). In our chat I pointed out that there might be other unattended uses for initial issuance (if people wrap Certbot with their own scripts) and so we might not want to introduce this delay across-the-board in order to avoid slowing down random scripted uses of Certbot that aren't related to renewals.

I mean, an extreme example could be if someone had set up some kind of unattended script that calls certbot certificates in order to feed some sort of status monitor—it might be annoying to have this command take several minutes to execute! And since it doesn't interact with the CA at all, there's no resource-related reason to make it delay before running.

[mnordhoff]

I mean, an extreme example could be if someone had set up some kind of unattended script that calls certbot certificates in order to feed some sort of status monitor—it might be annoying to have this command take several minutes to execute! And since it doesn't interact with the CA at all, there's no resource-related reason to make it delay before running.

It does hit OCSP. (And, for certbot-auto, PyPI.)

[ohemorange]

After checking the logs, looks like most of the spikiness comes from renews (not all! but a lot of it) so I'm cool keeping it here.

[filipkis]

This is causing a problem in my renew script which doesn't use hooks, but instead turns of webserver runs certbot and then turns webserver back on. Meaning, sometimes the server is down for several minutes due to this change.

Could you provide CLI argument to disable this. Or even better, might be good that it's not enabled by default.

[ohemorange]

@filipkis, can your renew script be refactored to use hooks?

If it currently looks like:

some code before; maybe another line of code
certbot renew
some code after; maybe a second line of after code

Then, you should be able to refactor that to:

certbot renew --pre-hook some code before; maybe another line of code --post-hook some code after; maybe a second line of after code

[filipkis]

Yes, it can and that's what I've done (after digging into the docs).

Thank you for suggesting though. Might be useful to document this change somewhere else in case others stumble on the same issues.