Merge pull request #43 from scrapinghub/fix-image-in-use

Fix broken filtering for images used by containers

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 filter_images_in_use(images, image_tags_in_use):
     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
 

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

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',