From 89da31b4ecee39014725a8d02d05a4b9f1661802 Mon Sep 17 00:00:00 2001 From: Alexander Chekunkov Date: Fri, 21 Apr 2017 18:12:42 +0100 Subject: [PATCH] Fix broken filtering for images used by containers Error calling remove_image image=xxx:latest 409 Client Error: Conflict ("conflict: unable to remove repository reference "xxx:latest" (must force) - container 91ab644a7b31 is using its referenced image 7caea3c78386") --- docker_custodian/docker_gc.py | 19 +++++++++++++--- tests/conftest.py | 4 +++- tests/docker_gc_test.py | 43 +++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/docker_custodian/docker_gc.py b/docker_custodian/docker_gc.py index 9049c92..cfcfa83 100644 --- a/docker_custodian/docker_gc.py +++ b/docker_custodian/docker_gc.py @@ -83,10 +83,16 @@ def get_dangling_volumes(client): def cleanup_images(client, max_image_age, dry_run, exclude_set): # re-fetch container list so that we don't include removed containers - image_tags_in_use = set( - container['Image'] for container in get_all_containers(client)) - images = filter_images_in_use(get_all_images(client), image_tags_in_use) + containers = get_all_containers(client) + images = get_all_images(client) + if docker.utils.compare_version('1.21', client._version) < 0: + image_tags_in_use = {container['Image'] for container in containers} + images = filter_images_in_use(images, image_tags_in_use) + else: + # ImageID field was added in 1.21 + image_ids_in_use = {container['ImageID'] for container in containers} + images = filter_images_in_use_by_id(images, image_ids_in_use) images = filter_excluded_images(images, exclude_set) for image_summary in reversed(list(images)): @@ -120,6 +126,13 @@ def image_not_in_use(image_summary): return filter(image_not_in_use, images) +def filter_images_in_use_by_id(images, image_ids_in_use): + def image_not_in_use(image_summary): + return image_summary['Id'] not in image_ids_in_use + + return filter(image_not_in_use, images) + + def is_image_old(image, min_date): return dateutil.parser.parse(image['Created']) < min_date diff --git a/tests/conftest.py b/tests/conftest.py index 075d13c..2fbc83c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,4 +48,6 @@ def later_time(): @pytest.fixture def mock_client(): - return mock.create_autospec(docker.Client) + client = mock.create_autospec(docker.Client) + client._version = '1.17' + return client diff --git a/tests/docker_gc_test.py b/tests/docker_gc_test.py index a7dbfd9..b980181 100644 --- a/tests/docker_gc_test.py +++ b/tests/docker_gc_test.py @@ -164,6 +164,49 @@ def test_filter_images_in_use(): assert list(actual) == expected +def test_filter_images_in_use_by_id(mock_client, now): + mock_client._version = '1.21' + mock_client.containers.return_value = [ + {'Id': 'abcd', 'ImageID': '1'}, + {'Id': 'abbb', 'ImageID': '2'}, + ] + mock_containers = [ + { + 'Id': 'abcd', + 'Name': 'one', + 'State': { + 'Running': False, + 'FinishedAt': '2014-01-01T01:01:01Z' + } + }, + { + 'Id': 'abbb', + 'Name': 'two', + 'State': { + 'Running': True, + 'FinishedAt': '2014-01-01T01:01:01Z' + } + } + ] + mock_client.inspect_container.side_effect = iter(mock_containers) + mock_client.images.return_value = [ + {'Id': '1', 'Created': '2014-01-01T01:01:01Z'}, + {'Id': '2', 'Created': '2014-01-01T01:01:01Z'}, + {'Id': '3', 'Created': '2014-01-01T01:01:01Z'}, + {'Id': '4', 'Created': '2014-01-01T01:01:01Z'}, + {'Id': '5', 'Created': '2014-01-01T01:01:01Z'}, + {'Id': '6', 'Created': '2014-01-01T01:01:01Z'}, + ] + mock_client.inspect_image.side_effect = lambda image: { + 'Id': image, + 'Created': '2014-01-01T01:01:01Z' + } + docker_gc.cleanup_images(mock_client, now, False, set()) + assert mock_client.remove_image.mock_calls == [ + mock.call(image=id_) for id_ in ['6', '5', '4', '3'] + ] + + def test_filter_excluded_images(): exclude_set = set([ 'user/one:latest',