From e8e3534335d09288ee84e93c0a3e9c044387c593 Mon Sep 17 00:00:00 2001
From: schoen <schoen@loyalty.org>
Date: Tue, 20 Nov 2018 20:55:51 -0800
Subject: [PATCH] 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 6bde65a7384131052a5c90abaca5028615fc5186.

* Revert "Somewhat ugly synthetic PTY trick"

This reverts commit 89c704a4be5722dc0babaf3358be1cdcfea240e3.

* Log specific duration of random sleep

* Test coverage for random sleep() logic in main.py
---
 CHANGELOG.md               |  4 +++-
 certbot/main.py            | 12 ++++++++++++
 certbot/tests/main_test.py | 22 ++++++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a25890929..2bb08d4e7 100644
--- a/CHANGELOG.md
+++ b/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
 
diff --git a/certbot/main.py b/certbot/main.py
index 5d5251dd2..692dafeed 100644
--- a/certbot/main.py
+++ b/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:
diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index 6c35f1fdb..2e53606a2 100644
--- a/certbot/tests/main_test.py
+++ b/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