[ARVADOS] updated: a72205728f94f5261b657766e01f5767dc15d4b5
Git user
git at public.curoverse.com
Mon Mar 13 16:40:32 EDT 2017
Summary of changes:
services/api/app/models/collection.rb | 100 +++++++++++++++------------
services/api/app/models/container_request.rb | 2 +-
services/api/app/models/job.rb | 11 +--
3 files changed, 58 insertions(+), 55 deletions(-)
via a72205728f94f5261b657766e01f5767dc15d4b5 (commit)
from e1e0bec5d9828fff1b9269322d415789675e6fab (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit a72205728f94f5261b657766e01f5767dc15d4b5
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Mon Mar 13 16:40:26 2017 -0400
8567: Refactor code that queries migration links into get_compatible_images.
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 03d0f7b..29e5725 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -316,30 +316,53 @@ class Collection < ArvadosModel
[hash_part, size_part].compact.join '+'
end
- def self.get_compatible_image(readers, pattern, coll_match)
- if coll_match.nil?
- return nil
+ def self.get_compatible_images(readers, pattern, collections)
+ if collections.empty?
+ return []
end
- manifest = Keep::Manifest.new(coll_match.manifest_text)
- if manifest.exact_file_count?(1)
- if manifest.files[0][1] =~ pattern
- # Looks like a compatible image
- return coll_match
- else
- # Doesn't match the expected pattern, see if there is a migration link.
- migrate_pdh = self.docker_migration_pdh(readers, coll_match.portable_data_hash)
- if migrate_pdh != coll_match.portable_data_hash
- # See if the migrated image is compatible.
- coll_match = readable_by(*readers).where(portable_data_hash: migrate_pdh).limit(1).first
- return get_compatible_image(readers, pattern, coll_match)
+
+ migrations = Hash[
+ Link.where('tail_uuid in (?) AND link_class=? AND links.owner_uuid=?',
+ collections.map(&:portable_data_hash),
+ 'docker_image_migration',
+ system_user_uuid).
+ order('links.created_at asc').
+ map { |l|
+ [l.tail_uuid, l.head_uuid]
+ }]
+
+ migrated_collections = Hash[
+ Collection.readable_by(*readers).
+ where('portable_data_hash in (?)', migrations.values).
+ map { |c|
+ [c.portable_data_hash, c]
+ }]
+
+ collections.map { |c|
+ # Check if the listed image is compatible first, if not, then try the
+ # migration link.
+ manifest = Keep::Manifest.new(c.manifest_text)
+ if manifest.exact_file_count?(1) and manifest.files[0][1] =~ pattern
+ c
+ elsif m = migrated_collections[migrations[c.portable_data_hash]]
+ manifest = Keep::Manifest.new(m.manifest_text)
+ if manifest.exact_file_count?(1) and manifest.files[0][1] =~ pattern
+ m
end
end
- end
- return nil
- end
-
- # Return array of Collection objects
- def self.find_all_for_docker_image(search_term, search_tag=nil, readers=nil)
+ }.compact
+ end
+
+ # Resolve a Docker repo+tag, hash, or collection PDH to an array of
+ # Collection objects, sorted by timestamp starting with the most recent
+ # match.
+ #
+ # If filter_compatible_format is true (the default), only return image
+ # collections which are support by the installation as indicated by
+ # Rails.configuration.docker_image_formats. Will follow
+ # 'docker_image_migration' links if search_term resolves to an incompatible
+ # image, but an equivalent compatible image is available.
+ def self.find_all_for_docker_image(search_term, search_tag=nil, readers=nil, filter_compatible_format=true)
readers ||= [Thread.current[:user]]
base_search = Link.
readable_by(*readers).
@@ -347,7 +370,8 @@ class Collection < ArvadosModel
joins("JOIN collections ON links.head_uuid = collections.uuid").
order("links.created_at DESC")
- if Rails.configuration.docker_image_formats.include? 'v1' and Rails.configuration.docker_image_formats.include? 'v2'
+ if (Rails.configuration.docker_image_formats.include? 'v1' and
+ Rails.configuration.docker_image_formats.include? 'v2') or filter_compatible_format == false
pattern = /^(sha256:)?[0-9A-Fa-f]{64}\.tar$/
elsif Rails.configuration.docker_image_formats.include? 'v2'
pattern = /^(sha256:)[0-9A-Fa-f]{64}\.tar$/
@@ -361,9 +385,9 @@ class Collection < ArvadosModel
# that looks like a Docker image, return it.
if loc = Keep::Locator.parse(search_term)
loc.strip_hints!
- coll_match = readable_by(*readers).where(portable_data_hash: loc.to_s).limit(1).first
- if compatible_img = get_compatible_image(readers, pattern, coll_match)
- return [compatible_img]
+ coll_match = readable_by(*readers).where(portable_data_hash: loc.to_s).limit(1)
+ if compatible_img = get_compatible_images(readers, pattern, coll_match)
+ return compatible_img
end
end
@@ -393,34 +417,20 @@ class Collection < ArvadosModel
-link.created_at.to_i]
end
- Collection.where('uuid in (?)', uuid_timestamps.keys).map { |c|
- get_compatible_image(readers, pattern, c)
- }.compact.sort_by { |c|
+ sorted = Collection.where('uuid in (?)', uuid_timestamps.keys).sort_by { |c|
uuid_timestamps[c.uuid]
}
+ compatible = get_compatible_images(readers, pattern, sorted)
+ if sorted.length > 0 and compatible.empty?
+ raise ArvadosModel::UnresolvableContainerError.new "Matching Docker image is incompatible with 'docker_image_formats' configuration."
+ end
+ compatible
end
def self.for_latest_docker_image(search_term, search_tag=nil, readers=nil)
find_all_for_docker_image(search_term, search_tag, readers).first
end
- # If the given pdh is an old-format docker image, old-format images
- # aren't supported by the compute nodes according to site config,
- # and a migrated new-format image is available, return the migrated
- # image's pdh. Otherwise, just return pdh.
- def self.docker_migration_pdh(read_users, pdh)
- if Rails.configuration.docker_image_formats.include?('v1')
- return pdh
- end
- Collection.readable_by(*read_users).
- joins('INNER JOIN links ON head_uuid=portable_data_hash').
- where('tail_uuid=? AND link_class=? AND links.owner_uuid=?',
- pdh, 'docker_image_migration', system_user_uuid).
- order('links.created_at desc').
- select('portable_data_hash').
- first.andand.portable_data_hash || pdh
- end
-
def self.searchable_columns operator
super - ["manifest_text"]
end
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 6bb8fb0..87c3ebe 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -235,7 +235,7 @@ class ContainerRequest < ArvadosModel
if !coll
raise ArvadosModel::UnresolvableContainerError.new "docker image #{container_image.inspect} not found"
end
- return Collection.docker_migration_pdh([current_user], coll.portable_data_hash)
+ coll.portable_data_hash
end
def set_container
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 80a03e2..2fdfbb1 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -149,7 +149,7 @@ class Job < ArvadosModel
image_hashes = Array.wrap(operand).flat_map do |search_term|
image_search, image_tag = search_term.split(':', 2)
Collection.
- find_all_for_docker_image(image_search, image_tag, read_users).
+ find_all_for_docker_image(image_search, image_tag, read_users, filter_compatible_format: false).
map(&:portable_data_hash)
end
filters << [attr, operator.sub(/ docker$/, ""), image_hashes]
@@ -217,8 +217,7 @@ class Job < ArvadosModel
else
image_locator = nil
end
- filters << ["docker_image_locator", "=",
- Collection.docker_migration_pdh(read_users, image_locator)]
+ filters << ["docker_image_locator", "=", image_locator]
if sdk_version = attrs[:runtime_constraints].andand["arvados_sdk_version"]
filters += default_git_filters("arvados_sdk_version", "arvados", sdk_version)
end
@@ -440,12 +439,6 @@ class Job < ArvadosModel
[false, "not found for #{image_search}"]
end
end
- Rails.logger.info("docker_image_locator is #{docker_image_locator}")
- if docker_image_locator && docker_image_locator_changed?
- self.docker_image_locator =
- Collection.docker_migration_pdh([current_user], docker_image_locator)
- end
- Rails.logger.info("docker_image_locator is #{docker_image_locator}")
end
def permission_to_update
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list