diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 0220f1039..630fc386b 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -111,6 +111,7 @@ More details about these changes can be found on our GitHub repo. * Fixed the apache component on openSUSE Tumbleweed which no longer provides an apache2ctl symlink and uses apachectl instead. * Fixed a typo in `certbot/crypto_util.py` causing an error upon attempting `secp521r1` key generation +* Installers (e.g. nginx, Apache) were being restarted unnecessarily after dry-run renewals. More details about these changes can be found on our GitHub repo. diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index ebfbfe1d0..627ab4bb6 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -1297,19 +1297,11 @@ def renew_cert(config, plugins, lineage): renewed_lineage = _get_and_save_cert(le_client, config, lineage=lineage) - notify = zope.component.getUtility(interfaces.IDisplay).notification - if installer is None: - notify("new certificate deployed without reload, fullchain is {0}".format( - lineage.fullchain), pause=False) - else: + if installer and not config.dry_run: # In case of a renewal, reload server to pick up new certificate. - # In principle we could have a configuration option to inhibit this - # from happening. - # Run deployer updater.run_renewal_deployer(config, renewed_lineage, installer) - installer.restart() - notify("new certificate deployed with reload of {0} server; fullchain is {1}".format( - config.installer, lineage.fullchain), pause=False) + display_util.notify(f"Reloading {config.installer} server after certificate renewal") + installer.restart() # type: ignore def certonly(config, plugins): diff --git a/certbot/certbot/_internal/plugins/selection.py b/certbot/certbot/_internal/plugins/selection.py index f0cce002f..cdf2c2355 100644 --- a/certbot/certbot/_internal/plugins/selection.py +++ b/certbot/certbot/_internal/plugins/selection.py @@ -2,10 +2,12 @@ import logging +from typing import Optional, Tuple import zope.component from certbot import errors from certbot import interfaces +from certbot._internal.plugins import disco from certbot.compat import os from certbot.display import util as display_util @@ -164,7 +166,9 @@ def record_chosen_plugins(config, plugins, auth, inst): config.authenticator, config.installer) -def choose_configurator_plugins(config, plugins, verb): +def choose_configurator_plugins(config: interfaces.IConfig, plugins: disco.PluginsRegistry, + verb: str) -> Tuple[Optional[interfaces.IInstaller], + Optional[interfaces.IAuthenticator]]: """ Figure out which configurator we're going to use, modifies config.authenticator and config.installer strings to reflect that choice if @@ -172,7 +176,7 @@ def choose_configurator_plugins(config, plugins, verb): :raises errors.PluginSelectionError if there was a problem - :returns: (an `IAuthenticator` or None, an `IInstaller` or None) + :returns: tuple of (`IInstaller` or None, `IAuthenticator` or None) :rtype: tuple """ diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index d86affca9..bcbfeb56e 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1496,6 +1496,24 @@ class MainTest(test_util.ConfigTestCase): # without installer self.assertIs(mock_run.called, False) + @mock.patch('certbot._internal.main.updater.run_renewal_deployer') + @mock.patch('certbot._internal.plugins.selection.choose_configurator_plugins') + @mock.patch('certbot._internal.main._init_le_client') + @mock.patch('certbot._internal.main._get_and_save_cert') + def test_renew_doesnt_restart_on_dryrun(self, mock_get_cert, mock_init, mock_choose, + mock_run_renewal_deployer): + """A dry-run renewal shouldn't try to restart the installer""" + self.config.dry_run = True + installer = mock.MagicMock() + mock_choose.return_value = (installer, mock.MagicMock()) + + main.renew_cert(self.config, None, None) + + self.assertEqual(mock_init.call_count, 1) + self.assertEqual(mock_get_cert.call_count, 1) + installer.restart.assert_not_called() + mock_run_renewal_deployer.assert_not_called() + class UnregisterTest(unittest.TestCase): def setUp(self):