Add a random sleep for noninteractive renewals (#6393)

* WIP on adding a random sleep for noninteractive renewal

* Update changelog

* Log the fact that we're randomly sleeping

* stdin may better define interactivity than stdout

* Try mocking time.sleep for all tests

* Move mocked sleep elsewhere

* mock the right object

* Somewhat ugly synthetic PTY trick

* Move set -u down below self-exec

* Revert "Move set -u down below self-exec"

This reverts commit 6bde65a738.

* Revert "Somewhat ugly synthetic PTY trick"

This reverts commit 89c704a4be.

* Log specific duration of random sleep

* Test coverage for random sleep() logic in main.py

CHANGELOG.md

@@ -6,7 +6,9 @@ Certbot adheres to [Semantic Versioning](http://semver.org/).
 
 ### Added
 
-*
+* Noninteractive renewals with `certbot renew` (those not started from a
+  terminal) now randomly sleep 1-480 seconds before beginning work in
+  order to spread out load spikes on the server side.
 
 ### Changed
 

certbot/main.py

@@ -4,7 +4,9 @@ from __future__ import print_function
 import functools
 import logging.handlers
 import os
+import random
 import sys
+import time
 
 import configobj
 import josepy as jose
@@ -1243,6 +1245,16 @@ def renew(config, unused_plugins):
     :rtype: None
 
     """
+    if not sys.stdin.isatty():
+        # Noninteractive renewals include a random delay in order to spread
+        # out the load on the certificate authority servers, even if many
+        # users all pick the same time for renewals.  This delay precedes
+        # running any hooks, so that side effects of the hooks (such as
+        # shutting down a web service) aren't prolonged unnecessarily.
+        sleep_time = random.randint(1, 60*8)
+        logger.info("Non-interactive renewal: random delay of %s seconds", sleep_time)
+        time.sleep(sleep_time)
+
     try:
         renewal.handle_renewal_request(config)
     finally:

certbot/tests/main_test.py

@@ -520,6 +520,8 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
                               '--work-dir', self.config.work_dir,
                               '--logs-dir', self.config.logs_dir, '--text']
 
+        self.mock_sleep = mock.patch('time.sleep').start()
+
     def tearDown(self):
         # Reset globals in cli
         reload_module(cli)
@@ -1093,6 +1095,26 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
         args = ["renew", "--reuse-key"]
         self._test_renewal_common(True, [], args=args, should_renew=True, reuse_key=True)
 
+    @mock.patch('sys.stdin')
+    def test_noninteractive_renewal_delay(self, stdin):
+        stdin.isatty.return_value = False
+        test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf')
+        args = ["renew", "--dry-run", "-tvv"]
+        self._test_renewal_common(True, [], args=args, should_renew=True)
+        self.assertEqual(self.mock_sleep.call_count, 1)
+        # in main.py:
+        #     sleep_time = random.randint(1, 60*8)
+        sleep_call_arg = self.mock_sleep.call_args[0][0]
+        self.assertTrue(1 <= sleep_call_arg <= 60*8)
+
+    @mock.patch('sys.stdin')
+    def test_interactive_no_renewal_delay(self, stdin):
+        stdin.isatty.return_value = True
+        test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf')
+        args = ["renew", "--dry-run", "-tvv"]
+        self._test_renewal_common(True, [], args=args, should_renew=True)
+        self.assertEqual(self.mock_sleep.call_count, 0)
+
     @mock.patch('certbot.renewal.should_renew')
     def test_renew_skips_recent_certs(self, should_renew):
         should_renew.return_value = False