From a542fcd019e1e3eb99dd9872aaa00ca99450800c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 30 Mar 2017 15:47:31 -0700 Subject: [PATCH] Revert "Add a global lock file to Certbot (#4369)" (#4445) This reverts commit 32122cfa21fe7ba43189658158949cfe16dc6717. --- certbot/cli.py | 2 - certbot/constants.py | 1 - certbot/interfaces.py | 3 -- certbot/main.py | 53 +------------------ certbot/tests/main_test.py | 51 +----------------- letsencrypt-auto-source/letsencrypt-auto | 6 --- .../pieces/letsencrypt-auto-requirements.txt | 6 --- setup.py | 1 - 8 files changed, 2 insertions(+), 121 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 0faf4a7c6..6217baa27 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1167,8 +1167,6 @@ def _paths_parser(helpful): help="Logs directory.") add("paths", "--server", default=flag_default("server"), help=config_help("server")) - add("paths", "--lock-path", default=flag_default("lock_path"), - help=config_help('lock_path')) def _plugins_parsing(helpful, plugins): diff --git a/certbot/constants.py b/certbot/constants.py index 24cf89199..382b0afb3 100644 --- a/certbot/constants.py +++ b/certbot/constants.py @@ -33,7 +33,6 @@ CLI_DEFAULTS = dict( auth_cert_path="./cert.pem", auth_chain_path="./chain.pem", strict_permissions=False, - lock_path="/tmp/.certbot.lock", debug_challenges=False, ) STAGING_URI = "https://acme-staging.api.letsencrypt.org/directory" diff --git a/certbot/interfaces.py b/certbot/interfaces.py index 33bde9430..213992993 100644 --- a/certbot/interfaces.py +++ b/certbot/interfaces.py @@ -222,9 +222,6 @@ class IConfig(zope.interface.Interface): key_dir = zope.interface.Attribute("Keys storage.") temp_checkpoint_dir = zope.interface.Attribute( "Temporary checkpoint directory.") - lock_path = zope.interface.Attribute( - "Path to the lock file used to prevent multiple instances of " - "Certbot from modifying your server's configuration at once.") no_verify_ssl = zope.interface.Attribute( "Disable verification of the ACME server's certificate.") diff --git a/certbot/main.py b/certbot/main.py index c6e61fbf9..ab2204428 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -8,7 +8,6 @@ import sys import time import traceback -import fasteners import zope.component from acme import jose @@ -867,56 +866,6 @@ def _post_logging_setup(config, plugins, cli_args): logger.debug("Discovered plugins: %r", plugins) -def acquire_file_lock(lock_path): - """Obtain a lock on the file at the specified path. - - :param str lock_path: path to the file to be locked - - :returns: lock file object representing the acquired lock - :rtype: fasteners.InterProcessLock - - :raises .Error: if the lock is held by another process - - """ - lock = fasteners.InterProcessLock(lock_path) - logger.debug("Attempting to acquire lock file %s", lock_path) - - try: - lock.acquire(blocking=False) - except IOError as err: - logger.debug(err) - logger.warning( - "Unable to access lock file %s. You should set --lock-file " - "to a writeable path to ensure multiple instances of " - "Certbot don't attempt modify your configuration " - "simultaneously.", lock_path) - else: - if not lock.acquired: - raise errors.Error( - "Another instance of Certbot is already running.") - - return lock - - -def _run_subcommand(config, plugins): - """Executes the Certbot subcommand specified in the configuration. - - :param .IConfig config: parsed configuration object - :param .PluginsRegistry plugins: available plugins - - :returns: return value from the specified subcommand - :rtype: str or int - - """ - lock = acquire_file_lock(config.lock_path) - - try: - return config.func(config, plugins) - finally: - if lock.acquired: - lock.release() - - def main(cli_args=sys.argv[1:]): """Command line argument parsing and main script execution.""" sys.excepthook = functools.partial(_handle_exception, config=None) @@ -944,7 +893,7 @@ def main(cli_args=sys.argv[1:]): zope.component.provideUtility(report) atexit.register(report.atexit_print_messages) - return _run_subcommand(config, plugins) + return config.func(config, plugins) if __name__ == "__main__": diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 02f241255..5d456bc3d 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -4,7 +4,6 @@ from __future__ import print_function import itertools import mock -import multiprocessing import os import shutil import traceback @@ -446,8 +445,7 @@ class MainTest(test_util.TempDirTestCase): # pylint: disable=too-many-public-me os.mkdir(self.logs_dir) self.standard_args = ['--config-dir', self.config_dir, '--work-dir', self.work_dir, - '--logs-dir', self.logs_dir, '--text', - '--lock-path', os.path.join(self.tempdir, 'certbot.lock')] + '--logs-dir', self.logs_dir, '--text'] def tearDown(self): # Reset globals in cli @@ -1305,52 +1303,5 @@ class TestHandleException(unittest.TestCase): traceback.format_exception_only(KeyboardInterrupt, interrupt))) -class TestAcquireFileLock(test_util.TempDirTestCase): - """Test main.acquire_file_lock.""" - - def setUp(self): - super(TestAcquireFileLock, self).setUp() - - self.lock_path = os.path.join(self.tempdir, 'certbot.lock') - - @mock.patch('certbot.main.logger') - def test_bad_path(self, mock_logger): - lock = main.acquire_file_lock(os.getcwd()) - self.assertTrue(mock_logger.warning.called) - self.assertFalse(lock.acquired) - - def test_held_lock(self): - # start child and wait for it to grab the lock - cv = multiprocessing.Condition() - cv.acquire() - child_args = (cv, self.lock_path,) - child = multiprocessing.Process(target=_hold_lock, args=child_args) - child.start() - cv.wait() - - # assert we can't grab lock and terminate the child - self.assertRaises(errors.Error, main.acquire_file_lock, self.lock_path) - cv.notify() - cv.release() - child.join() - self.assertEqual(child.exitcode, 0) - - -def _hold_lock(cv, lock_path): - """Acquire a file lock at lock_path and wait to release it. - - :param multiprocessing.Condition cv: condition for syncronization - :param str lock_path: path to the file lock - - """ - import fasteners - lock = fasteners.InterProcessLock(lock_path) - lock.acquire() - cv.acquire() - cv.notify() - cv.wait() - lock.release() - - if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 475d57fee..35c7a530c 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -727,9 +727,6 @@ cryptography==1.5.3 \ enum34==1.1.2 \ --hash=sha256:2475d7fcddf5951e92ff546972758802de5260bf409319a9f1934e6bbc8b1dc7 \ --hash=sha256:35907defb0f992b75ab7788f65fedc1cf20ffa22688e0e6f6f12afc06b3ea501 -fasteners==0.14.1 \ - --hash=sha256:564a115ff9698767df401efca29620cbb1a1c2146b7095ebd304b79cc5807a7c \ - --hash=sha256:427c76773fe036ddfa41e57d89086ea03111bbac57c55fc55f3006d027107e18 funcsigs==0.4 \ --hash=sha256:ff5ad9e2f8d9e5d1e8bbfbcf47722ab527cf0d51caeeed9da6d0f40799383fde \ --hash=sha256:d83ce6df0b0ea6618700fe1db353526391a8a3ada1b7aba52fed7a61da772033 @@ -742,9 +739,6 @@ ipaddress==1.0.16 \ linecache2==1.0.0 \ --hash=sha256:e78be9c0a0dfcbac712fe04fbf92b96cddae80b1b842f24248214c8496f006ef \ --hash=sha256:4b26ff4e7110db76eeb6f5a7b64a82623839d595c2038eeda662f2a2db78e97c -monotonic==1.3 \ - --hash=sha256:a8c7690953546c6bc8a4f05d347718db50de1225b29f4b9f346c0c6f19bdc286 \ - --hash=sha256:2b469e2d7dd403f7f7f79227fe5ad551ee1e76f8bb300ae935209884b93c7c1b ordereddict==1.1 \ --hash=sha256:1c35b4ac206cef2d24816c89f89cf289dd3d38cf7c449bb3fab7bf6d43f01b1f parsedatetime==2.1 \ diff --git a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt index fbf416d66..d70d24e2a 100644 --- a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt +++ b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt @@ -65,9 +65,6 @@ cryptography==1.5.3 \ enum34==1.1.2 \ --hash=sha256:2475d7fcddf5951e92ff546972758802de5260bf409319a9f1934e6bbc8b1dc7 \ --hash=sha256:35907defb0f992b75ab7788f65fedc1cf20ffa22688e0e6f6f12afc06b3ea501 -fasteners==0.14.1 \ - --hash=sha256:564a115ff9698767df401efca29620cbb1a1c2146b7095ebd304b79cc5807a7c \ - --hash=sha256:427c76773fe036ddfa41e57d89086ea03111bbac57c55fc55f3006d027107e18 funcsigs==0.4 \ --hash=sha256:ff5ad9e2f8d9e5d1e8bbfbcf47722ab527cf0d51caeeed9da6d0f40799383fde \ --hash=sha256:d83ce6df0b0ea6618700fe1db353526391a8a3ada1b7aba52fed7a61da772033 @@ -80,9 +77,6 @@ ipaddress==1.0.16 \ linecache2==1.0.0 \ --hash=sha256:e78be9c0a0dfcbac712fe04fbf92b96cddae80b1b842f24248214c8496f006ef \ --hash=sha256:4b26ff4e7110db76eeb6f5a7b64a82623839d595c2038eeda662f2a2db78e97c -monotonic==1.3 \ - --hash=sha256:a8c7690953546c6bc8a4f05d347718db50de1225b29f4b9f346c0c6f19bdc286 \ - --hash=sha256:2b469e2d7dd403f7f7f79227fe5ad551ee1e76f8bb300ae935209884b93c7c1b ordereddict==1.1 \ --hash=sha256:1c35b4ac206cef2d24816c89f89cf289dd3d38cf7c449bb3fab7bf6d43f01b1f parsedatetime==2.1 \ diff --git a/setup.py b/setup.py index 59f6dbd9e..0e8d19a22 100644 --- a/setup.py +++ b/setup.py @@ -43,7 +43,6 @@ install_requires = [ 'ConfigArgParse>=0.9.3', 'configobj', 'cryptography>=0.7', # load_pem_x509_certificate - 'fasteners', 'mock', 'parsedatetime>=1.3', # Calendar.parseDT 'PyOpenSSL',