Merge pull request #1512 from coreos-inc/optimize-queries

Optimize various queries

data/model/image.py

@@ -34,26 +34,41 @@ def get_repository_image_and_deriving(docker_image_id, storage_uuid):
                               (Image.id == image_found.id))
 
 
+def get_parent_images_with_placements(namespace_name, repository_name, image_obj):
+  """ Returns a list of parent Image objects starting with the most recent parent
+      and ending with the base layer. The images in this query will include the storage and
+      placements.
+  """
+  return _get_parent_images(namespace_name, repository_name, image_obj, include_placements=True)
+
+
 def get_parent_images(namespace_name, repository_name, image_obj):
   """ Returns a list of parent Image objects starting with the most recent parent
-      and ending with the base layer.
+      and ending with the base layer. The images in this query will include the storage but
+      not the placements.
   """
+  return _get_parent_images(namespace_name, repository_name, image_obj, include_placements=False)
+
+
+def _get_parent_images(namespace_name, repository_name, image_obj, include_placements=False):
   parents = image_obj.ancestors
 
   # Ancestors are in the format /<root>/<intermediate>/.../<parent>/, with each path section
   # containing the database Id of the image row.
   parent_db_ids = parents.strip('/').split('/')
-
   if parent_db_ids == ['']:
     return []
 
   def filter_to_parents(query):
     return query.where(Image.id << parent_db_ids)
 
-  parents = get_repository_images_base(namespace_name, repository_name, filter_to_parents)
+  if include_placements:
+    parents = get_repository_images_base(namespace_name, repository_name, filter_to_parents)
+  else:
+    parents = _get_repository_images_and_storages(namespace_name, repository_name,
+                                                  filter_to_parents)
 
   id_to_image = {unicode(image.id): image for image in parents}
-
   return [id_to_image[parent_id] for parent_id in reversed(parent_db_ids)]
 
 
@@ -79,6 +94,19 @@ def get_repo_image_extended(namespace_name, repository_name, docker_image_id):
   return images[0]
 
 
+def _get_repository_images_and_storages(namespace_name, repository_name, query_modifier):
+  query = (Image
+           .select(Image, ImageStorage)
+           .join(ImageStorage)
+           .switch(Image)
+           .join(Repository)
+           .join(Namespace, on=(Repository.namespace_user == Namespace.id))
+           .where(Repository.name == repository_name, Namespace.username == namespace_name))
+
+  query = query_modifier(query)
+  return query
+
+
 def _get_repository_images(namespace_name, repository_name, query_modifier):
   query = (Image
            .select()
@@ -134,7 +162,7 @@ def invert_placement_query_results(placement_query):
 def lookup_repository_images(repo, docker_image_ids):
   return (Image
           .select(Image, ImageStorage)
-          .join(ImageStorage) # TODO(jschorr): Remove once no longer needed in v2/manifest.py.
+          .join(ImageStorage)
           .where(Image.repository == repo, Image.docker_image_id << docker_image_ids))
 
 

data/model/tag.py

@@ -162,20 +162,40 @@ def garbage_collect_tags(repo):
 
       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):
-    return _tag_alive(query
-                      .switch(Image)
-                      .join(RepositoryTag)
-                      .where(RepositoryTag.name == tag_name))
 
-  images = image.get_repository_images_base(namespace_name, repository_name, limit_to_tag)
+def _get_repo_tag_image(tag_name, include_storage, modifier):
+  query = Image.select().join(RepositoryTag)
+
+  if include_storage:
+    query = (Image.select(Image, ImageStorage)
+                  .join(ImageStorage)
+                  .switch(Image)
+                  .join(RepositoryTag))
+
+  images = _tag_alive(modifier(query.where(RepositoryTag.name == tag_name)))
   if not images:
     raise DataModelException('Unable to find image for tag.')
   else:
     return images[0]
 
 
+def get_repo_tag_image(repo, tag_name, include_storage=False):
+  def modifier(query):
+    return query.where(RepositoryTag.repository == repo)
+
+  return _get_repo_tag_image(tag_name, include_storage, modifier)
+
+
+def get_tag_image(namespace_name, repository_name, tag_name, include_storage=False):
+  def modifier(query):
+    return (query.switch(RepositoryTag)
+                 .join(Repository)
+                 .join(Namespace)
+                 .where(Namespace.username == namespace_name, Repository.name == repository_name))
+
+  return _get_repo_tag_image(tag_name, include_storage, modifier)
+
+
 def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None):
   query = (RepositoryTag
            .select(RepositoryTag, Image)
@@ -226,7 +246,6 @@ def store_tag_manifest(namespace, repo_name, tag_name, docker_image_id, manifest
 def get_active_tag(namespace, repo_name, tag_name):
   return _tag_alive(RepositoryTag
                     .select()
-                    .join(Image)
                     .join(Repository)
                     .join(Namespace, on=(Repository.namespace_user == Namespace.id))
                     .where(RepositoryTag.name == tag_name, Repository.name == repo_name,
@@ -263,7 +282,6 @@ def _load_repo_manifests(namespace, repo_name):
     return _tag_alive(TagManifest
                       .select(TagManifest, RepositoryTag)
                       .join(RepositoryTag)
-                      .join(Image)
                       .join(Repository)
                       .join(Namespace, on=(Namespace.id == Repository.namespace_user))
                       .where(Repository.name == repo_name, Namespace.username == namespace))

endpoints/api/image.py

@@ -7,7 +7,6 @@ from endpoints.api import (resource, nickname, require_repo_read, RepositoryPara
                            format_date, path_param)
 from endpoints.exception import NotFound
 from data import model
-from util.cache import cache_control_flask_restful
 
 
 def image_view(image, image_map, include_ancestors=True):

endpoints/api/tag.py

@@ -98,7 +98,7 @@ class RepositoryTag(RepositoryParamResource):
 
     original_image_id = None
     try:
-      original_tag_image = model.tag.get_tag_image(namespace, repository, tag)
+      original_tag_image = model.tag.get_repo_tag_image(image.repository, tag)
       if original_tag_image:
         original_image_id = original_tag_image.docker_image_id
     except model.DataModelException:

endpoints/v2/manifest.py

@@ -514,7 +514,7 @@ def delete_manifest_by_digest(namespace_name, repo_name, manifest_ref):
 
 def _generate_and_store_manifest(namespace_name, repo_name, tag_name):
   # First look up the tag object and its ancestors
-  image = model.tag.get_tag_image(namespace_name, repo_name, tag_name)
+  image = model.tag.get_tag_image(namespace_name, repo_name, tag_name, include_storage=True)
   parents = model.image.get_parent_images(namespace_name, repo_name, image)
 
   # If the manifest is being generated under the library namespace, then we make its namespace

endpoints/verbs.py

@@ -37,7 +37,8 @@ def _open_stream(formatter, namespace, repository, tag, synthetic_image_id, imag
   # For performance reasons, we load the full image list here, cache it, then disconnect from
   # the database.
   with database.UseThenDisconnect(app.config):
-    image_list = list(model.image.get_parent_images(namespace, repository, repo_image))
+    image_list = list(model.image.get_parent_images_with_placements(namespace, repository,
+                                                                    repo_image))
     image_list.insert(0, repo_image)
 
   def get_image_json(image):

test/test_secscan.py

@@ -147,7 +147,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_get_layer_success(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     with HTTMock(get_layer_success_mock, response_content):
       result = self.api.get_layer_data(layer, include_vulnerabilities=True)
       self.assertIsNotNone(result)
@@ -155,7 +155,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_get_layer_failure(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     with HTTMock(get_layer_failure_mock, response_content):
       result = self.api.get_layer_data(layer, include_vulnerabilities=True)
       self.assertIsNone(result)
@@ -171,7 +171,7 @@ class TestSecurityScanner(unittest.TestCase):
       # Already registered.
       pass
 
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     self.assertFalse(layer.security_indexed)
     self.assertEquals(-1, layer.security_indexed_engine)
 
@@ -199,7 +199,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_analyze_layer_success(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     self.assertFalse(layer.security_indexed)
     self.assertEquals(-1, layer.security_indexed_engine)
 
@@ -212,7 +212,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_analyze_layer_failure(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     self.assertFalse(layer.security_indexed)
     self.assertEquals(-1, layer.security_indexed_engine)
 
@@ -225,7 +225,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_analyze_layer_internal_error(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     self.assertFalse(layer.security_indexed)
     self.assertEquals(-1, layer.security_indexed_engine)
 
@@ -238,7 +238,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_analyze_layer_bad_request(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     self.assertFalse(layer.security_indexed)
     self.assertEquals(-1, layer.security_indexed_engine)
 
@@ -253,7 +253,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_analyze_layer_missing_storage(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     self.assertFalse(layer.security_indexed)
     self.assertEquals(-1, layer.security_indexed_engine)
 
@@ -273,7 +273,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_analyze_layer_success_events(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     self.assertFalse(layer.security_indexed)
     self.assertEquals(-1, layer.security_indexed_engine)
 
@@ -356,7 +356,7 @@ class TestSecurityScanner(unittest.TestCase):
     }
 
   def test_notification_new_layers_not_vulnerable(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
 
     # Add a repo event for the layer.
@@ -394,7 +394,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_notification_delete(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
 
     # Add a repo event for the layer.
@@ -413,7 +413,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_notification_new_layers(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
 
     # Add a repo event for the layer.
@@ -464,7 +464,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_notification_no_new_layers(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
 
     # Add a repo event for the layer.
@@ -484,7 +484,7 @@ class TestSecurityScanner(unittest.TestCase):
 
 
   def test_notification_no_new_layers_increased_severity(self):
-    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
     layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
 
     # Add a repo event for the layer.
@@ -558,7 +558,7 @@ class TestSecurityScanner(unittest.TestCase):
     def get_notification(url, request):
       if url.query.find('page=nextpage') >= 0:
         pages_called.append('GET-2')
-        layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, COMPLEX_REPO, 'prod')
+        layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, COMPLEX_REPO, 'prod', include_storage=True)
         layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
 
         data = {
@@ -568,7 +568,7 @@ class TestSecurityScanner(unittest.TestCase):
         return json.dumps(data)
       else:
         pages_called.append('GET-1')
-        layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+        layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
         layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
 
         notification_data = self._get_notification_data([layer_id], [layer_id])