From e1f0da5e2657adceb6bfec870f96ac7e604341a8 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 23 Sep 2016 10:26:10 -0400 Subject: [PATCH] 9953: PEP-8 --- .../dockercleaner/arvados_docker/cleaner.py | 25 +++++++++--- services/dockercleaner/tests/test_cleaner.py | 38 ++++++++++++++----- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/services/dockercleaner/arvados_docker/cleaner.py b/services/dockercleaner/arvados_docker/cleaner.py index 5531bf5ec4..9dd7b12a08 100755 --- a/services/dockercleaner/arvados_docker/cleaner.py +++ b/services/dockercleaner/arvados_docker/cleaner.py @@ -21,6 +21,7 @@ SUFFIX_SIZES = {suffix: 1024 ** exp for exp, suffix in enumerate('kmgt', 1)} logger = logging.getLogger('arvados_docker.cleaner') + def return_when_docker_not_found(result=None): # If the decorated function raises a 404 error from Docker, return # `result` instead. @@ -36,7 +37,9 @@ def return_when_docker_not_found(result=None): return docker_not_found_wrapper return docker_not_found_decorator + class DockerImage: + def __init__(self, image_hash): self.docker_id = image_hash['Id'] self.size = image_hash['VirtualSize'] @@ -47,6 +50,7 @@ class DockerImage: class DockerImages: + def __init__(self, target_size): self.target_size = target_size self.images = {} @@ -118,6 +122,7 @@ class DockerImages: class DockerEventHandlers: # This class maps Docker event types to the names of methods that should # receive those events. + def __init__(self): self.handler_names = collections.defaultdict(list) @@ -221,7 +226,8 @@ class DockerImageCleaner(DockerImageUseRecorder): try: self.docker_client.remove_image(image_id) except docker.errors.APIError as error: - logger.warning("Failed to remove image %s: %s", image_id, error) + logger.warning( + "Failed to remove image %s: %s", image_id, error) else: logger.info("Removed image %s", image_id) self.images.del_image(image_id) @@ -231,8 +237,9 @@ class DockerImageCleaner(DockerImageUseRecorder): unknown_ids = {image['Id'] for image in self.docker_client.images() if not self.images.has_image(image['Id'])} for image_id in (unknown_ids - self.logged_unknown): - logger.info("Image %s is loaded but unused, so it won't be cleaned", - image_id) + logger.info( + "Image %s is loaded but unused, so it won't be cleaned", + image_id) self.logged_unknown = unknown_ids @@ -245,6 +252,7 @@ def human_size(size_str): size_str = size_str[:-1] return int(size_str) * multiplier + def load_config(arguments): args = parse_arguments(arguments) @@ -261,6 +269,7 @@ def load_config(arguments): return config + def default_config(): return { 'Quota': '1G', @@ -268,6 +277,7 @@ def default_config(): 'Verbose': 0, } + def parse_arguments(arguments): class Formatter(argparse.ArgumentDefaultsHelpFormatter, argparse.RawDescriptionHelpFormatter): @@ -280,7 +290,7 @@ def parse_arguments(arguments): formatter_class=Formatter, ) parser.add_argument( - '--config', action='store', type=str, default='/etc/arvados/dockercleaner/config.json', + '--config', action='store', type=str, default='/etc/arvados/docker-cleaner/config.json', help="configuration file") deprecated = " (DEPRECATED -- use config file instead)" @@ -299,14 +309,16 @@ def parse_arguments(arguments): return parser.parse_args(arguments) + def setup_logging(config): log_handler = logging.StreamHandler() log_handler.setFormatter(logging.Formatter( - '%(asctime)s %(name)s[%(process)d] %(levelname)s: %(message)s', - '%Y-%m-%d %H:%M:%S')) + '%(asctime)s %(name)s[%(process)d] %(levelname)s: %(message)s', + '%Y-%m-%d %H:%M:%S')) logger.addHandler(log_handler) logger.setLevel(logging.ERROR - (10 * config['Verbose'])) + def run(config, docker_client): start_time = int(time.time()) logger.debug("Loading Docker activity through present") @@ -324,6 +336,7 @@ def run(config, docker_client): logger.info("Listening for docker events") cleaner.run() + def main(arguments=sys.argv[1:]): config = load_config(arguments) setup_logging(config) diff --git a/services/dockercleaner/tests/test_cleaner.py b/services/dockercleaner/tests/test_cleaner.py index cabafe975a..9fbd3e3014 100644 --- a/services/dockercleaner/tests/test_cleaner.py +++ b/services/dockercleaner/tests/test_cleaner.py @@ -15,13 +15,16 @@ from arvados_docker import cleaner MAX_DOCKER_ID = (16 ** 64) - 1 + def MockDockerId(): return '{:064x}'.format(random.randint(0, MAX_DOCKER_ID)) + def MockContainer(image_hash): return {'Id': MockDockerId(), 'Image': image_hash['Id']} + def MockImage(*, size=0, vsize=None, tags=[]): if vsize is None: vsize = random.randint(100, 2000000) @@ -31,6 +34,7 @@ def MockImage(*, size=0, vsize=None, tags=[]): 'Size': size, 'VirtualSize': vsize} + class MockEvent(dict): ENCODING = 'utf-8' event_seq = itertools.count(1) @@ -48,6 +52,7 @@ class MockEvent(dict): class MockException(docker.errors.APIError): + def __init__(self, status_code): response = mock.Mock(name='response') response.status_code = status_code @@ -55,6 +60,7 @@ class MockException(docker.errors.APIError): class DockerImageTestCase(unittest.TestCase): + def test_used_at_sets_last_used(self): image = cleaner.DockerImage(MockImage()) image.used_at(5) @@ -74,6 +80,7 @@ class DockerImageTestCase(unittest.TestCase): class DockerImagesTestCase(unittest.TestCase): + def setUp(self): self.mock_images = [] @@ -336,6 +343,7 @@ class DockerContainerCleanerTestCase(DockerImageUseRecorderTestCase): class HumanSizeTestCase(unittest.TestCase): + def check(self, human_str, count, exp): self.assertEqual(count * (1024 ** exp), cleaner.human_size(human_str)) @@ -362,6 +370,7 @@ class HumanSizeTestCase(unittest.TestCase): class RunTestCase(unittest.TestCase): + def setUp(self): self.config = cleaner.default_config() self.config['Quota'] = 1000000 @@ -384,6 +393,7 @@ class RunTestCase(unittest.TestCase): @mock.patch('docker.Client', name='docker_client') @mock.patch('arvados_docker.cleaner.run', name='cleaner_run') class MainTestCase(unittest.TestCase): + def test_client_api_version(self, run_mock, docker_client): with tempfile.NamedTemporaryFile(mode='wt') as cf: cf.write('{"Quota":"1000T"}') @@ -392,7 +402,8 @@ class MainTestCase(unittest.TestCase): self.assertEqual(1, docker_client.call_count) # 1.14 is the first version that's well defined, going back to # Docker 1.2, and still supported up to at least Docker 1.9. - # See . + # See + # . self.assertEqual('1.14', docker_client.call_args[1].get('version')) self.assertEqual(1, run_mock.call_count) @@ -400,18 +411,21 @@ class MainTestCase(unittest.TestCase): class ConfigTestCase(unittest.TestCase): + def test_load_config(self): with tempfile.NamedTemporaryFile(mode='wt') as cf: - cf.write('{"Quota":"1000T", "RemoveStoppedContainers":"always", "Verbose":2}') + cf.write( + '{"Quota":"1000T", "RemoveStoppedContainers":"always", "Verbose":2}') cf.flush() config = cleaner.load_config(['--config', cf.name]) - self.assertEqual(1000<<40, config['Quota']) + self.assertEqual(1000 << 40, config['Quota']) self.assertEqual("always", config['RemoveStoppedContainers']) self.assertEqual(2, config['Verbose']) def test_args_override_config(self): with tempfile.NamedTemporaryFile(mode='wt') as cf: - cf.write('{"Quota":"1000T", "RemoveStoppedContainers":"always", "Verbose":2}') + cf.write( + '{"Quota":"1000T", "RemoveStoppedContainers":"always", "Verbose":2}') cf.flush() config = cleaner.load_config([ '--config', cf.name, @@ -419,7 +433,7 @@ class ConfigTestCase(unittest.TestCase): '--remove-stopped-containers', 'never', '--verbose', ]) - self.assertEqual(1<<30, config['Quota']) + self.assertEqual(1 << 30, config['Quota']) self.assertEqual('never', config['RemoveStoppedContainers']) self.assertEqual(1, config['Verbose']) @@ -448,13 +462,16 @@ class ContainerRemovalTestCase(unittest.TestCase): def test_remove_onexit(self): self.config['RemoveStoppedContainers'] = 'onexit' cleaner.run(self.config, self.docker_client) - self.docker_client.remove_container.assert_called_once_with(self.newCID, v=True) + self.docker_client.remove_container.assert_called_once_with( + self.newCID, v=True) def test_remove_always(self): self.config['RemoveStoppedContainers'] = 'always' cleaner.run(self.config, self.docker_client) - self.docker_client.remove_container.assert_any_call(self.existingCID, v=True) - self.docker_client.remove_container.assert_any_call(self.newCID, v=True) + self.docker_client.remove_container.assert_any_call( + self.existingCID, v=True) + self.docker_client.remove_container.assert_any_call( + self.newCID, v=True) self.assertEqual(2, self.docker_client.remove_container.call_count) def test_remove_never(self): @@ -472,7 +489,8 @@ class ContainerRemovalTestCase(unittest.TestCase): # exited containers? self.docker_client.assert_has_calls([ mock.call.events(since=mock.ANY), - mock.call.containers(filters={'status':'exited'})]) + mock.call.containers(filters={'status': 'exited'})]) # Asked to delete the container twice? - self.docker_client.remove_container.assert_has_calls([mock.call(self.existingCID, v=True)] * 2) + self.docker_client.remove_container.assert_has_calls( + [mock.call(self.existingCID, v=True)] * 2) self.assertEqual(2, self.docker_client.remove_container.call_count) -- 2.30.2