Break on failure to deploy cert (#4003)

* Break on failure to deploy cert

* Add error message for unable to install cert

* Add unit test

3 files changed, 16 insertions, 5 deletions

diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py
index f3acb5560..f19a07910 100644
--- a/certbot-nginx/certbot_nginx/configurator.py
+++ b/certbot-nginx/certbot_nginx/configurator.py
@@ -191,11 +191,9 @@ class NginxConfigurator(common.Plugin):
                         vhost.filep, vhost.names)
         except errors.MisconfigurationError as error:
             logger.debug(error)
-            logger.warning(
-                "Cannot find a cert or key directive in %s for %s. "
-                "VirtualHost was not modified.", vhost.filep, vhost.names)
             # Presumably break here so that the virtualhost is not modified
-            return False
+            raise errors.PluginError("Cannot find a cert or key directive in {0} for {1}. "
+                "VirtualHost was not modified.".format(vhost.filep, vhost.names))
 
         self.save_notes += ("Changed vhost at %s with addresses of %s\n" %
                             (vhost.filep,
diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py
index 6886127d7..cc36aa0de 100644
--- a/certbot-nginx/certbot_nginx/tests/configurator_test.py
+++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py
@@ -158,6 +158,18 @@ class NginxConfiguratorTest(util.NginxTest):
             "example/chain.pem",
             None)
 
+    @mock.patch('certbot_nginx.parser.NginxParser.add_server_directives')
+    def test_deploy_cert_raise_on_add_error(self, mock_add_server_directives):
+        mock_add_server_directives.side_effect = errors.MisconfigurationError()
+        self.assertRaises(
+            errors.PluginError,
+            self.config.deploy_cert,
+            "migration.com",
+            "example/cert.pem",
+            "example/key.pem",
+            "example/chain.pem",
+            "example/fullchain.pem")
+
     def test_deploy_cert(self):
         server_conf = self.config.parser.abs_path('server.conf')
         nginx_conf = self.config.parser.abs_path('nginx.conf')
diff --git a/certbot/client.py b/certbot/client.py
index cfd2b8487..b40a169e6 100644
--- a/certbot/client.py
+++ b/certbot/client.py
@@ -376,7 +376,8 @@ class Client(object):
 
         chain_path = None if chain_path is None else os.path.abspath(chain_path)
 
-        with error_handler.ErrorHandler(self.installer.recovery_routine):
+        msg = ("Unable to install the certificate")
+        with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg):
             for dom in domains:
                 self.installer.deploy_cert(
                     domain=dom, cert_path=os.path.abspath(cert_path),