Merge pull request #929 from jakedt/fixgcagain

Fix garbage collection when manifests may reference tags

data/model/tag.py

@@ -1,3 +1,5 @@
+import logging
+
 from uuid import uuid4
 
 from data.model import (image, db_transaction, DataModelException, _basequery,
@@ -6,6 +8,9 @@ from data.database import (RepositoryTag, Repository, Image, ImageStorage, Names
                            RepositoryNotification, get_epoch_timestamp, db_for_update)
 
 
+logger = logging.getLogger(__name__)
+
+
 def _tag_alive(query, now_ts=None):
   if now_ts is None:
     now_ts = get_epoch_timestamp()
@@ -19,11 +24,11 @@ def get_matching_tags(docker_image_id, storage_uuid, *args):
   image_query = image.get_repository_image_and_deriving(docker_image_id, storage_uuid)
 
   return _tag_alive(RepositoryTag
-                   .select(*args)
-                   .distinct()
-                   .join(Image)
-                   .join(ImageStorage)
-                   .where(Image.id << image_query, RepositoryTag.hidden == False))
+                    .select(*args)
+                    .distinct()
+                    .join(Image)
+                    .join(ImageStorage)
+                    .where(Image.id << image_query, RepositoryTag.hidden == False))
 
 
 def get_tags_for_image(image_id, *args):
@@ -135,12 +140,25 @@ def garbage_collect_tags(repo):
                                ~(RepositoryTag.lifetime_end_ts >> None),
                                (RepositoryTag.lifetime_end_ts <= expired_time))
                         .order_by(RepositoryTag.id))
-  if len(tags_to_delete) > 0:
-    (RepositoryTag
-     .delete()
-     .where(RepositoryTag.id << tags_to_delete)
-     .execute())
 
+  if len(tags_to_delete) > 0:
+    with db_transaction():
+      manifests_to_delete = (TagManifest
+                             .select(TagManifest.id)
+                             .join(RepositoryTag)
+                             .where(RepositoryTag.id << tags_to_delete))
+
+      num_deleted_manifests = (TagManifest
+                               .delete()
+                               .where(TagManifest.id << manifests_to_delete)
+                               .execute())
+
+      num_deleted_tags = (RepositoryTag
+                          .delete()
+                          .where(RepositoryTag.id << tags_to_delete)
+                          .execute())
+
+      logger.debug('Removed %s tags with %s manifests', num_deleted_tags, num_deleted_manifests)
 
 def get_tag_image(namespace_name, repository_name, tag_name):
   def limit_to_tag(query):

test/test_gc.py

@@ -4,6 +4,8 @@ import time
 from app import app, storage
 from initdb import setup_database_for_testing, finished_database_for_testing
 from data import model, database
+from endpoints.v2.manifest import _generate_and_store_manifest
+
 
 ADMIN_ACCESS_USER = 'devtable'
 PUBLIC_USER = 'public'
@@ -288,6 +290,15 @@ class TestGarbageCollection(unittest.TestCase):
     self.assertDeleted(repository, 'i2', 'i3')
     self.assertNotDeleted(repository, 'i1', 'f1')
 
+  def test_manifest_gc(self):
+    repository = self.createRepository(latest=['i1', 'i2', 'i3'], other=['i1', 'f1'])
+    _generate_and_store_manifest(ADMIN_ACCESS_USER, REPO, 'latest')
+
+    self._set_tag_expiration_policy(repository.namespace_user.username, 0)
+
+    self.deleteTag(repository, 'latest')
+    self.assertDeleted(repository, 'i2', 'i3')
+
 
 if __name__ == '__main__':
   unittest.main()