From 4f41f79fa81e93ed2256de08d2af082514ddaea1 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 3 Nov 2015 18:15:23 -0500 Subject: [PATCH] Never load the full repo image list Always make smaller queries per tag to ensure we scale better Fixes #754 --- endpoints/api/tag.py | 31 ++++- static/directives/image-info-sidebar.html | 8 +- .../repo-view/repo-panel-changes.html | 71 ++++++----- .../directives/repo-view/repo-panel-tags.html | 2 +- static/directives/tag-operations-dialog.html | 8 +- .../directives/tag-specific-images-view.html | 6 +- .../repo-view/repo-panel-changes.js | 39 ++++--- .../directives/repo-view/repo-panel-tags.js | 2 +- static/js/directives/ui/image-info-sidebar.js | 5 + .../js/directives/ui/tag-operations-dialog.js | 110 +++++------------- .../directives/ui/tag-specific-image-view.js | 80 ++----------- static/js/graphing.js | 14 ++- static/js/pages/repo-view.js | 26 +---- static/js/services/image-loader-service.js | 110 ++++++++++++++++++ static/partials/repo-view.html | 10 +- 15 files changed, 268 insertions(+), 254 deletions(-) create mode 100644 static/js/services/image-loader-service.js diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index b657e5827..4233144ef 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -4,7 +4,7 @@ from flask import request, abort from endpoints.api import (resource, nickname, require_repo_read, require_repo_write, RepositoryParamResource, log_action, NotFound, validate_json_request, - path_param, parse_args, query_param) + path_param, parse_args, query_param, truthy_bool) from endpoints.api.image import image_view from data import model from auth.auth_context import get_authenticated_user @@ -135,7 +135,10 @@ class RepositoryTagImages(RepositoryParamResource): """ Resource for listing the images in a specific repository tag. """ @require_repo_read @nickname('listTagImages') - def get(self, namespace, repository, tag): + @parse_args + @query_param('owned', 'If specified, only images wholely owned by this tag are returned.', + type=truthy_bool, default=False) + def get(self, args, namespace, repository, tag): """ List the images for the specified repository tag. """ try: tag_image = model.tag.get_tag_image(namespace, repository, tag) @@ -144,15 +147,37 @@ class RepositoryTagImages(RepositoryParamResource): parent_images = model.image.get_parent_images(namespace, repository, tag_image) image_map = {} + + image_map[str(tag_image.id)] = tag_image + for image in parent_images: image_map[str(image.id)] = image + image_map_all = dict(image_map) + parents = list(parent_images) parents.reverse() all_images = [tag_image] + parents + # Filter the images returned to those not found in the ancestry of any of the other tags in + # the repository. + if args['owned']: + all_tags = model.tag.list_repository_tags(namespace, repository) + for current_tag in all_tags: + if current_tag.name == tag: + continue + + # Remove the tag's image ID. + tag_image_id = str(current_tag.image_id) + image_map.pop(tag_image_id, None) + + # Remove any ancestors: + for ancestor_id in current_tag.image.ancestors.split('/'): + image_map.pop(ancestor_id, None) + return { - 'images': [image_view(image, image_map) for image in all_images] + 'images': [image_view(image, image_map_all) for image in all_images + if not args['owned'] or (str(image.id) in image_map)] } diff --git a/static/directives/image-info-sidebar.html b/static/directives/image-info-sidebar.html index 657aac6db..a126c88ca 100644 --- a/static/directives/image-info-sidebar.html +++ b/static/directives/image-info-sidebar.html @@ -20,14 +20,14 @@
- {{ tag }} - (No Tags) + (No Tags)
+ ng-if="tracker.repository.can_write || getTags(imageData)"> @@ -35,7 +35,7 @@
    -
  • +
  • {{ tag }} diff --git a/static/directives/repo-view/repo-panel-changes.html b/static/directives/repo-view/repo-panel-changes.html index aed147d9d..a59c6a156 100644 --- a/static/directives/repo-view/repo-panel-changes.html +++ b/static/directives/repo-view/repo-panel-changes.html @@ -1,6 +1,6 @@
    -
    +
    +

    Visualize Tags:
    -
    + +
    +
    + +
    +
    +
    - -
    -
    - -
    -
    + +
    +
    + {{ currentTag }} +
    +
    + {{ currentImage.substr(0, 12) }}
    - -
    -
    - {{ currentTag }} -
    -
    - {{ currentImage.substr(0, 12) }} +
    + +
    -
    - -
    -
    - - -
    -
    + +
    -
    +
    -
    \ No newline at end of file diff --git a/static/directives/repo-view/repo-panel-tags.html b/static/directives/repo-view/repo-panel-tags.html index 0a3f7fae6..420f95f60 100644 --- a/static/directives/repo-view/repo-panel-tags.html +++ b/static/directives/repo-view/repo-panel-tags.html @@ -172,7 +172,7 @@
    \ No newline at end of file diff --git a/static/directives/tag-operations-dialog.html b/static/directives/tag-operations-dialog.html index 901a7792c..f71f92c6e 100644 --- a/static/directives/tag-operations-dialog.html +++ b/static/directives/tag-operations-dialog.html @@ -49,11 +49,11 @@
    + ng-show="isAnotherImageTag(toTagImage, tagToCreate)" + image-loader="imageLoader"> This will also delete any unattached images and delete the following images:
    @@ -100,7 +100,7 @@ {{ deleteTagInfo.tag }}?
    + image-loader="imageLoader" style="margin-top: 20px"> The following images and any other images not referenced by a tag will be deleted:
    diff --git a/static/directives/tag-specific-images-view.html b/static/directives/tag-specific-images-view.html index a1513384c..13f7275a1 100644 --- a/static/directives/tag-specific-images-view.html +++ b/static/directives/tag-specific-images-view.html @@ -1,4 +1,8 @@ -
    +
    +
    +
    + +
    image. + var images = imageLoader.images; var imageIDMap = {}; - this.images.map(function(image) { + + images.forEach(function(image) { imageIDMap[image.id] = image; }); @@ -91,12 +93,13 @@ angular.module('quay').directive('repoPanelChanges', function () { 'selectedTags': '=selectedTags', 'imagesResource': '=imagesResource', - 'images': '=images', + 'imageLoader': '=imageLoader', 'isEnabled': '=isEnabled' }, controller: function($scope, $element, $timeout, ApiService, UtilService, ImageMetadataService) { $scope.tagNames = []; + $scope.loading = true; $scope.$watch('selectedTags', function(selectedTags) { if (!selectedTags) { return; } @@ -110,18 +113,17 @@ angular.module('quay').directive('repoPanelChanges', function () { $scope.currentImage = null; $scope.currentTag = null; - if ($scope.tracker) { - refreshTree(); - } else { + $scope.loading = true; + $scope.imageLoader.loadImages($scope.selectedTagsSlice, function() { + $scope.loading = false; updateImages(); - } + }); }; var updateImages = function() { - if (!$scope.repository || !$scope.images || !$scope.isEnabled) { return; } - - $scope.tracker = new RepositoryImageTracker($scope.repository, $scope.images); + if (!$scope.repository || !$scope.imageLoader || !$scope.isEnabled) { return; } + $scope.tracker = new RepositoryImageTracker($scope.repository, $scope.imageLoader); if ($scope.selectedTagsSlice && $scope.selectedTagsSlice.length) { refreshTree(); } @@ -131,22 +133,25 @@ angular.module('quay').directive('repoPanelChanges', function () { $scope.$watch('repository', update); $scope.$watch('isEnabled', update); - $scope.$watch('images', updateImages); - $scope.updateState = function() { update(); }; var refreshTree = function() { - if (!$scope.repository || !$scope.images || !$scope.isEnabled) { return; } + if (!$scope.repository || !$scope.imageLoader || !$scope.isEnabled) { return; } if ($scope.selectedTagsSlice.length < 1) { return; } $('#image-history-container').empty(); + var getTagsForImage = function(image) { + return $scope.imageLoader.getTagsForImage(image); + }; + var tree = new ImageHistoryTree( $scope.repository.namespace, $scope.repository.name, - $scope.images, + $scope.imageLoader.images, + getTagsForImage, UtilService.getFirstMarkdownLineAsText, $scope.getTimeSince, ImageMetadataService.getEscapedFormattedCommand, @@ -194,8 +199,6 @@ angular.module('quay').directive('repoPanelChanges', function () { }; $scope.handleTagChanged = function(data) { - $scope.tracker = new RepositoryImageTracker($scope.repository, $scope.images); - data.removed.map(function(tag) { $scope.currentImage = null; $scope.currentTag = null; @@ -206,7 +209,7 @@ angular.module('quay').directive('repoPanelChanges', function () { $scope.currentTag = tag; }); - refreshTree(); + update(); }; } }; diff --git a/static/js/directives/repo-view/repo-panel-tags.js b/static/js/directives/repo-view/repo-panel-tags.js index 7c8689bc5..95e365a56 100644 --- a/static/js/directives/repo-view/repo-panel-tags.js +++ b/static/js/directives/repo-view/repo-panel-tags.js @@ -12,7 +12,7 @@ angular.module('quay').directive('repoPanelTags', function () { 'repository': '=repository', 'selectedTags': '=selectedTags', 'imagesResource': '=imagesResource', - 'images': '=images', + 'imageLoader': '=imageLoader', 'isEnabled': '=isEnabled', diff --git a/static/js/directives/ui/image-info-sidebar.js b/static/js/directives/ui/image-info-sidebar.js index 2c254a43e..2614a5f15 100644 --- a/static/js/directives/ui/image-info-sidebar.js +++ b/static/js/directives/ui/image-info-sidebar.js @@ -11,6 +11,7 @@ angular.module('quay').directive('imageInfoSidebar', function () { scope: { 'tracker': '=tracker', 'image': '=image', + 'imageLoader': '=imageLoader', 'tagSelected': '&tagSelected', 'addTagRequested': '&addTagRequested' @@ -25,6 +26,10 @@ angular.module('quay').directive('imageInfoSidebar', function () { return Date.parse(dateString); }; + $scope.getTags = function(imageData) { + return $scope.imageLoader.getTagsForImage(imageData); + }; + $scope.getFormattedCommand = ImageMetadataService.getFormattedCommand; } }; diff --git a/static/js/directives/ui/tag-operations-dialog.js b/static/js/directives/ui/tag-operations-dialog.js index 6c5dd2ce6..d2081c3e6 100644 --- a/static/js/directives/ui/tag-operations-dialog.js +++ b/static/js/directives/ui/tag-operations-dialog.js @@ -11,46 +11,31 @@ angular.module('quay').directive('tagOperationsDialog', function () { restrict: 'C', scope: { 'repository': '=repository', - 'images': '=images', 'actionHandler': '=actionHandler', - - 'getImages': '&getImages', + 'imageLoader': '=imageLoader', 'tagChanged': '&tagChanged' }, controller: function($scope, $element, $timeout, ApiService) { $scope.addingTag = false; - $scope.imagesInternal = []; - - $scope.$watch('images', function(images) { - if (!images) { return; } - $scope.imagesInternal = images; - }); var markChanged = function(added, removed) { - // Reload the repository and the images. + // Reload the repository. $scope.repository.get().then(function(resp) { $scope.repository = resp; + $scope.imageLoader.reset() - var params = { - 'repository': resp.namespace + '/' + resp.name - }; - - ApiService.listRepositoryImages(null, params).then(function(resp) { - $scope.images = resp.images; - - // Note: We need the timeout here so that Angular can $digest the images change - // on the parent scope before the tagChanged callback occurs. - $timeout(function() { - $scope.tagChanged({ - 'data': { 'added': added, 'removed': removed } - }); - }, 1); - }) + // Note: We need the timeout here so that Angular can $digest the images change + // on the parent scope before the tagChanged callback occurs. + $timeout(function() { + $scope.tagChanged({ + 'data': { 'added': added, 'removed': removed } + }); + }, 1); }); }; $scope.isAnotherImageTag = function(image, tag) { - if (!$scope.repository || !$scope.imagesInternal) { return; } + if (!$scope.repository) { return; } var found = $scope.repository.tags[tag]; if (found == null) { return false; } @@ -58,7 +43,7 @@ angular.module('quay').directive('tagOperationsDialog', function () { }; $scope.isOwnedTag = function(image, tag) { - if (!$scope.repository || !$scope.imagesInternal) { return; } + if (!$scope.repository) { return; } var found = $scope.repository.tags[tag]; if (found == null) { return false; } @@ -149,70 +134,39 @@ angular.module('quay').directive('tagOperationsDialog', function () { }, errorHandler); }; - var lazyLoadImages = function(callback) { - if ($scope.imagesInternal.length) { - callback(); - return; - } - - var isLoading = true; - $timeout(function() { - if (isLoading) { - $('#loadingImagesModal').modal({}); - } - }, 500); - - var cb = function(images) { - isLoading = false; - $('#loadingImagesModal').modal('hide'); - $scope.imagesInternal = images; - callback(); - }; - - $scope.getImages({'callback': cb}); - }; - $scope.actionHandler = { 'askDeleteTag': function(tag) { - lazyLoadImages(function() { - $scope.deleteTagInfo = { - 'tag': tag - }; - }); + $scope.deleteTagInfo = { + 'tag': tag + }; }, 'askDeleteMultipleTags': function(tags) { - lazyLoadImages(function() { - $scope.deleteMultipleTagsInfo = { - 'tags': tags - }; - }); + $scope.deleteMultipleTagsInfo = { + 'tags': tags + }; }, 'askAddTag': function(image) { - lazyLoadImages(function() { - $scope.tagToCreate = ''; - $scope.toTagImage = image; - $scope.addingTag = false; - $scope.addTagForm.$setPristine(); - $element.find('#createOrMoveTagModal').modal('show'); - }); + $scope.tagToCreate = ''; + $scope.toTagImage = image; + $scope.addingTag = false; + $scope.addTagForm.$setPristine(); + $element.find('#createOrMoveTagModal').modal('show'); }, 'askRevertTag': function(tag, image_id) { - lazyLoadImages(function() { - if (tag.image_id == image_id) { - bootbox.alert('This is the current image for the tag'); - return; - } + if (tag.image_id == image_id) { + bootbox.alert('This is the current image for the tag'); + return; + } - $scope.revertTagInfo = { - 'tag': tag, - 'image_id': image_id - }; + $scope.revertTagInfo = { + 'tag': tag, + 'image_id': image_id + }; - $element.find('#revertTagModal').modal('show'); - }); + $element.find('#revertTagModal').modal('show'); } }; } diff --git a/static/js/directives/ui/tag-specific-image-view.js b/static/js/directives/ui/tag-specific-image-view.js index 552d22d2d..755d7b7ae 100644 --- a/static/js/directives/ui/tag-specific-image-view.js +++ b/static/js/directives/ui/tag-specific-image-view.js @@ -12,13 +12,12 @@ angular.module('quay').directive('tagSpecificImagesView', function () { scope: { 'repository': '=repository', 'tag': '=tag', - 'images': '=images', + 'imageLoader': '=imageLoader', 'imageCutoff': '=imageCutoff' }, controller: function($scope, $element, UtilService) { $scope.getFirstTextLine = UtilService.getFirstMarkdownLineAsText; - - $scope.hasImages = false; + $scope.loading = false; $scope.tagSpecificImages = []; $scope.getImageListingClasses = function(image) { @@ -35,39 +34,8 @@ angular.module('quay').directive('tagSpecificImagesView', function () { return classes; }; - var forAllTagImages = function(tag, callback, opt_cutoff) { - if (!tag) { return; } - - if (!$scope.imageByDockerId) { - $scope.imageByDockerId = []; - for (var i = 0; i < $scope.images.length; ++i) { - var currentImage = $scope.images[i]; - $scope.imageByDockerId[currentImage.id] = currentImage; - } - } - - var tag_image = $scope.imageByDockerId[tag.image_id]; - if (!tag_image) { - return; - } - - callback(tag_image); - - var ancestors = tag_image.ancestors.split('/').reverse(); - for (var i = 0; i < ancestors.length; ++i) { - var image = $scope.imageByDockerId[ancestors[i]]; - if (image) { - if (image == opt_cutoff) { - return; - } - - callback(image); - } - } - }; - var refresh = function() { - if (!$scope.repository || !$scope.tag || !$scope.images) { + if (!$scope.repository || !$scope.tag || !$scope.imageLoader) { $scope.tagSpecificImages = []; return; } @@ -78,49 +46,15 @@ angular.module('quay').directive('tagSpecificImagesView', function () { return; } - var getIdsForTag = function(currentTag) { - var ids = {}; - forAllTagImages(currentTag, function(image) { - ids[image.id] = true; - }, $scope.imageCutoff); - return ids; - }; - - // Remove any IDs that match other tags. - var toDelete = getIdsForTag(tag); - for (var currentTagName in $scope.repository.tags) { - var currentTag = $scope.repository.tags[currentTagName]; - if (currentTag != tag) { - for (var id in getIdsForTag(currentTag)) { - delete toDelete[id]; - } - } - } - - // Return the matching list of images. - var images = []; - for (var i = 0; i < $scope.images.length; ++i) { - var image = $scope.images[i]; - if (toDelete[image.id]) { - images.push(image); - } - } - - images.sort(function(a, b) { - var result = new Date(b.created) - new Date(a.created); - if (result != 0) { - return result; - } - - return b.sort_index - a.sort_index; + $scope.loading = true; + $scope.imageLoader.getTagSpecificImages($scope.tag, function(images) { + $scope.loading = false; + $scope.tagSpecificImages = images; }); - - $scope.tagSpecificImages = images; }; $scope.$watch('repository', refresh); $scope.$watch('tag', refresh); - $scope.$watch('images', refresh); } }; return directiveDefinitionObject; diff --git a/static/js/graphing.js b/static/js/graphing.js index 1112261d6..67b5aa346 100644 --- a/static/js/graphing.js +++ b/static/js/graphing.js @@ -31,8 +31,8 @@ var DEPTH_WIDTH = 140; /** * Based off of http://mbostock.github.io/d3/talk/20111018/tree.html by Mike Bostock (@mbostock) */ -function ImageHistoryTree(namespace, name, images, formatComment, formatTime, formatCommand, - opt_tagFilter) { +function ImageHistoryTree(namespace, name, images, getTagsForImage, formatComment, formatTime, + formatCommand, opt_tagFilter) { /** * The namespace of the repo. @@ -49,6 +49,11 @@ function ImageHistoryTree(namespace, name, images, formatComment, formatTime, fo */ this.images_ = images; + /** + * Method to retrieve the tags for an image. + */ + this.getTagsForImage_ = getTagsForImage; + /** * Method to invoke to format a comment for an image. */ @@ -424,7 +429,7 @@ ImageHistoryTree.prototype.buildRoot_ = function() { "name": image.id.substr(0, 12), "children": [], "image": image, - "tags": image.tags, + "tags": this.getTagsForImage_(image), "level": null }; imageByDockerId[image.id] = imageNode; @@ -663,8 +668,9 @@ ImageHistoryTree.prototype.setTag_ = function(tagName) { this.currentImage_ = null; // Update the path. + var that = this; var tagImage = this.findImage_(function(image) { - return image.tags.indexOf(tagName || '(no tag specified)') >= 0; + return tagName && (that.getTagsForImage_(image).indexOf(tagName) >= 0); }); if (tagImage) { diff --git a/static/js/pages/repo-view.js b/static/js/pages/repo-view.js index 884dcae18..c5ae7f093 100644 --- a/static/js/pages/repo-view.js +++ b/static/js/pages/repo-view.js @@ -10,11 +10,11 @@ }) }]); - function RepoViewCtrl($scope, $routeParams, $location, $timeout, ApiService, UserService, AngularPollChannel) { + function RepoViewCtrl($scope, $routeParams, $location, $timeout, ApiService, UserService, AngularPollChannel, ImageLoaderService) { $scope.namespace = $routeParams.namespace; $scope.name = $routeParams.name; - $scope.imagesRequired = false; + var imageLoader = ImageLoaderService.getLoader($scope.namespace, $scope.name); // Tab-enabled counters. $scope.tagsShown = 0; @@ -25,8 +25,7 @@ $scope.viewScope = { 'selectedTags': [], 'repository': null, - 'images': null, - 'imagesResource': null, + 'imageLoader': imageLoader, 'builds': null, 'changesVisible': false }; @@ -72,17 +71,6 @@ }); }; - var loadImages = function(opt_callback) { - var params = { - 'repository': $scope.namespace + '/' + $scope.name - }; - - $scope.viewScope.imagesResource = ApiService.listRepositoryImagesAsResource(params).get(function(resp) { - $scope.viewScope.images = resp.images; - opt_callback && opt_callback(resp.images); - }); - }; - var loadRepositoryBuilds = function(callback) { var params = { 'repository': $scope.namespace + '/' + $scope.name, @@ -149,14 +137,6 @@ }, 10); }; - $scope.requireImages = function() { - // Lazily load the repo's images if this is the first call to a tab - // that needs the images. - if ($scope.viewScope.images == null) { - loadImages(); - } - }; - $scope.handleChangesState = function(value) { $scope.viewScope.changesVisible = value; }; diff --git a/static/js/services/image-loader-service.js b/static/js/services/image-loader-service.js new file mode 100644 index 000000000..2d9a1f843 --- /dev/null +++ b/static/js/services/image-loader-service.js @@ -0,0 +1,110 @@ +/** + * Helper service for tracking images needed by tags and caching them. + */ +angular.module('quay').factory('ImageLoaderService', ['ApiService', function(ApiService) { + var imageLoader = function(namespace, name) { + this.namespace = namespace; + this.name = name; + this.tagCache = {}; + this.images = []; + this.imageMap = {}; + this.imageTagMap = {}; + }; + + imageLoader.prototype.getTagSpecificImages = function(tag, callback) { + var errorDisplay = ApiService.errorDisplay('Could not load tag specific images', function() { + callback([]); + }); + + var params = { + 'repository': this.namespace + '/' + this.name, + 'tag': tag, + 'owned': true + }; + + ApiService.listTagImages(null, params).then(function(resp) { + callback(resp['images']); + }, errorDisplay); + }; + + imageLoader.prototype.getTagsForImage = function(image) { + return this.imageTagMap[image.id] || []; + }; + + imageLoader.prototype.registerTagImages_ = function(tag, images) { + this.tagCache[tag] = images; + + if (!images.length) { + return; + } + + var that = this; + images.forEach(function(image) { + if (!that.imageMap[image.id]) { + that.imageMap[image.id] = image; + that.images.push(image); + } + }); + + var rootImage = images[0]; + if (!this.imageTagMap[rootImage.id]) { + this.imageTagMap[rootImage.id] = []; + } + + this.imageTagMap[rootImage.id].push(tag); + } + + imageLoader.prototype.loadImages = function(tags, callback) { + var toLoad = []; + var that = this; + tags.forEach(function(tag) { + if (that.tagCache[tag]) { + return; + } + + toLoad.push(tag); + }); + + if (!toLoad.length) { + callback(); + return; + } + + var loadImages = function(index) { + if (index >= toLoad.length) { + callback(); + return; + } + + var tag = toLoad[index]; + var params = { + 'repository': that.namespace + '/' + that.name, + 'tag': tag, + }; + + ApiService.listTagImages(null, params).then(function(resp) { + that.registerTagImages_(tag, resp['images']); + loadImages(index + 1); + }, function() { + loadImages(index + 1); + }) + }; + + loadImages(0); + }; + + imageLoader.prototype.reset = function() { + this.tagCache = {}; + this.images = []; + this.imageMap = {}; + this.imageTagMap = {}; + }; + + var imageLoaderService = {}; + + imageLoaderService.getLoader = function(namespace, name) { + return new imageLoader(namespace, name); + }; + + return imageLoaderService +}]); \ No newline at end of file diff --git a/static/partials/repo-view.html b/static/partials/repo-view.html index 2ed0f1333..cd3cbca2b 100644 --- a/static/partials/repo-view.html +++ b/static/partials/repo-view.html @@ -34,8 +34,7 @@ + tab-hidden="handleChangesState(false)"> @@ -64,10 +63,8 @@
    @@ -83,8 +80,7 @@