[ARVADOS] updated: 70912031ac176870d3830b5cb2550bbb5f24c409

git at public.curoverse.com git at public.curoverse.com
Tue Jun 17 13:13:23 EDT 2014


Summary of changes:
 .../controllers/pipeline_instances_controller.rb   | 111 +++++++++++----------
 .../app/views/application/_content.html.erb        |   4 +-
 sdk/cli/bin/crunch-job                             |  43 ++++++--
 sdk/python/arvados/commands/keepdocker.py          |   3 +-
 services/api/app/models/arvados_model.rb           |  24 +++--
 services/api/app/models/collection.rb              |  45 +++++++++
 services/api/app/models/job.rb                     |  18 ++++
 .../20140611173003_add_docker_locator_to_jobs.rb   |   5 +
 services/api/db/schema.rb                          |   4 +-
 services/api/test/fixtures/collections.yml         |  11 ++
 services/api/test/fixtures/links.yml               |  95 +++++++++++++++++-
 services/api/test/integration/select_test.rb       |  14 ++-
 .../test/integration/serialized_encoding_test.rb   |   2 +-
 services/api/test/unit/job_test.rb                 | 108 +++++++++++++++++++-
 14 files changed, 406 insertions(+), 81 deletions(-)
 create mode 100644 services/api/db/migrate/20140611173003_add_docker_locator_to_jobs.rb

       via  70912031ac176870d3830b5cb2550bbb5f24c409 (commit)
       via  d7db321d0f4363b70ae6b4e214d14abab5016bfe (commit)
       via  f443fdcac93c38a39949e0dd441fdabc5761d6f6 (commit)
       via  a4fd1b6edb38288c173f6b7db9413764010069ae (commit)
       via  99b93ceaa2b9af988f2c3261d4cfb49dc8113f98 (commit)
       via  3d9568727cc0dd4f5f778791ce387370b5f11070 (commit)
      from  0143d26db80dabb66825246cd15b0bd06a12eec8 (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 70912031ac176870d3830b5cb2550bbb5f24c409
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Tue Jun 17 09:43:48 2014 -0400

    Ajax load of tab panels now includes any query parameters the page was
    originally loaded with.  Also fixed related problem where the 'compare' route
    of pipeline_instances controller wasn't aware of tab partials.  closes #3013

diff --git a/apps/workbench/app/controllers/pipeline_instances_controller.rb b/apps/workbench/app/controllers/pipeline_instances_controller.rb
index 97c5e17..e84d0e4 100644
--- a/apps/workbench/app/controllers/pipeline_instances_controller.rb
+++ b/apps/workbench/app/controllers/pipeline_instances_controller.rb
@@ -115,73 +115,80 @@ class PipelineInstancesController < ApplicationController
 
     @rows = []          # each is {name: S, components: [...]}
 
-    # Build a table: x=pipeline y=component
-    @objects.each_with_index do |pi, pi_index|
-      pipeline_jobs(pi).each do |component|
-        # Find a cell with the same name as this component but no
-        # entry for this pipeline
-        target_row = nil
-        @rows.each_with_index do |row, row_index|
-          if row[:name] == component[:name] and !row[:components][pi_index]
-            target_row = row
+    if params['tab_pane'] == "Compare" or params['tab_pane'].nil?
+      # Build a table: x=pipeline y=component
+      @objects.each_with_index do |pi, pi_index|
+        pipeline_jobs(pi).each do |component|
+          # Find a cell with the same name as this component but no
+          # entry for this pipeline
+          target_row = nil
+          @rows.each_with_index do |row, row_index|
+            if row[:name] == component[:name] and !row[:components][pi_index]
+              target_row = row
+            end
           end
+          if !target_row
+            target_row = {name: component[:name], components: []}
+            @rows << target_row
+          end
+          target_row[:components][pi_index] = component
         end
-        if !target_row
-          target_row = {name: component[:name], components: []}
-          @rows << target_row
-        end
-        target_row[:components][pi_index] = component
       end
-    end
 
-    @rows.each do |row|
-      # Build a "normal" pseudo-component for this row by picking the
-      # most common value for each attribute. If all values are
-      # equally common, there is no "normal".
-      normal = {}              # attr => most common value
-      highscore = {}           # attr => how common "normal" is
-      score = {}               # attr => { value => how common }
-      row[:components].each do |pj|
-        next if pj.nil?
-        pj.each do |k,v|
-          vstr = for_comparison v
-          score[k] ||= {}
-          score[k][vstr] = (score[k][vstr] || 0) + 1
-          highscore[k] ||= 0
-          if score[k][vstr] == highscore[k]
-            # tie for first place = no "normal"
-            normal.delete k
-          elsif score[k][vstr] == highscore[k] + 1
-            # more pipelines have v than anything else
-            highscore[k] = score[k][vstr]
-            normal[k] = vstr
+      @rows.each do |row|
+        # Build a "normal" pseudo-component for this row by picking the
+        # most common value for each attribute. If all values are
+        # equally common, there is no "normal".
+        normal = {}              # attr => most common value
+        highscore = {}           # attr => how common "normal" is
+        score = {}               # attr => { value => how common }
+        row[:components].each do |pj|
+          next if pj.nil?
+          pj.each do |k,v|
+            vstr = for_comparison v
+            score[k] ||= {}
+            score[k][vstr] = (score[k][vstr] || 0) + 1
+            highscore[k] ||= 0
+            if score[k][vstr] == highscore[k]
+              # tie for first place = no "normal"
+              normal.delete k
+            elsif score[k][vstr] == highscore[k] + 1
+              # more pipelines have v than anything else
+              highscore[k] = score[k][vstr]
+              normal[k] = vstr
+            end
           end
         end
-      end
 
-      # Add a hash in component[:is_normal]: { attr => is_the_value_normal? }
-      row[:components].each do |pj|
-        next if pj.nil?
-        pj[:is_normal] = {}
-        pj.each do |k,v|
-          pj[:is_normal][k] = (normal.has_key?(k) && normal[k] == for_comparison(v))
+        # Add a hash in component[:is_normal]: { attr => is_the_value_normal? }
+        row[:components].each do |pj|
+          next if pj.nil?
+          pj[:is_normal] = {}
+          pj.each do |k,v|
+            pj[:is_normal][k] = (normal.has_key?(k) && normal[k] == for_comparison(v))
+          end
         end
       end
     end
 
-    provenance, pips = graph(@objects)
+    if params['tab_pane'] == "Graph"
+      provenance, pips = graph(@objects)
 
-    @pipelines = @objects
+      @pipelines = @objects
 
-    if provenance
-      @prov_svg = ProvenanceHelper::create_provenance_graph provenance, "provenance_svg", {
-        :request => request,
-        :all_script_parameters => true,
-        :combine_jobs => :script_and_version,
-        :script_version_nodes => true,
-        :pips => pips }
+      if provenance
+        @prov_svg = ProvenanceHelper::create_provenance_graph provenance, "provenance_svg", {
+          :request => request,
+          :all_script_parameters => true,
+          :combine_jobs => :script_and_version,
+          :script_version_nodes => true,
+          :pips => pips }
+      end
     end
+
     @object = @objects.first
+
+    show
   end
 
   def show_pane_list
diff --git a/apps/workbench/app/views/application/_content.html.erb b/apps/workbench/app/views/application/_content.html.erb
index 33ec16f..418923c 100644
--- a/apps/workbench/app/views/application/_content.html.erb
+++ b/apps/workbench/app/views/application/_content.html.erb
@@ -26,7 +26,7 @@
     if (!tab_pane_valid_state[pane]) {
       tab_pane_valid_state[pane] = true;
       $(document).trigger('ajax:send');
-      $.ajax('<%=j url_for() %>?tab_pane='+pane, {dataType: 'html', type: 'GET'}).
+      $.ajax('<%=j url_for() %>?<%= raw(controller.request.query_string) %>&tab_pane='+pane, {dataType: 'html', type: 'GET'}).
         done(function(data, status, jqxhr) {
           $('#' + pane + ' > div > div').html(data);
           $(document).trigger('ajax:complete');
@@ -68,7 +68,7 @@
 <% else %>
        data-object-uuid="<%= @object.uuid %>"
 <% end %>
->
+  >
 
 <% content_for :js do %>
   <% if i == 0 %>

commit d7db321d0f4363b70ae6b4e214d14abab5016bfe
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jun 12 15:52:21 2014 -0400

    2879: crunch-job uses Job Docker information provided by API server.

diff --git a/sdk/cli/bin/crunch-job b/sdk/cli/bin/crunch-job
index 96f7354..2b8a823 100755
--- a/sdk/cli/bin/crunch-job
+++ b/sdk/cli/bin/crunch-job
@@ -501,13 +501,23 @@ if (!$have_slurm)
 
 # If this job requires a Docker image, install that.
 my $docker_bin = "/usr/bin/docker.io";
-my $docker_image = $Job->{runtime_constraints}->{docker_image} || "";
-if ($docker_image) {
+my ($docker_locator, $docker_hash);
+if ($docker_locator = $Job->{docker_image_locator}) {
+  $docker_hash = find_docker_hash($docker_locator);
+  if (!$docker_hash)
+  {
+    croak("No Docker image hash found from locator $docker_locator");
+  }
+  my $docker_install_script = qq{
+if ! $docker_bin images -q --no-trunc | grep -qxF \Q$docker_hash\E; then
+    arv-get \Q$docker_locator/$docker_hash.tar\E | $docker_bin load
+fi
+};
   my $docker_pid = fork();
   if ($docker_pid == 0)
   {
     srun (["srun", "--nodelist=" . join(',', @node)],
-          [$docker_bin, 'pull', $docker_image]);
+          ["/bin/sh", "-ec", $docker_install_script]);
     exit ($?);
   }
   while (1)
@@ -516,11 +526,9 @@ if ($docker_image) {
     freeze_if_want_freeze ($docker_pid);
     select (undef, undef, undef, 0.1);
   }
-  # If the Docker image was specified as a hash, pull will fail.
-  # Ignore that error.  We'll see what happens when we try to run later.
-  if (($? != 0) && ($docker_image !~ /^[0-9a-fA-F]{5,64}$/))
+  if ($? != 0)
   {
-    croak("Installing Docker image $docker_image returned exit code $?");
+    croak("Installing Docker image from $docker_locator returned exit code $?");
   }
 }
 
@@ -639,7 +647,7 @@ for (my $todo_ptr = 0; $todo_ptr <= $#jobstep_todo; $todo_ptr ++)
 	  "&& perl -";
     }
     $command .= "&& exec arv-mount --allow-other $ENV{TASK_KEEPMOUNT} --exec ";
-    if ($docker_image)
+    if ($docker_hash)
     {
       $command .= "crunchstat -cgroup-root=/sys/fs/cgroup -cgroup-parent=docker -cgroup-cid=$ENV{TASK_WORK}/docker.cid -poll=10000 ";
       $command .= "$docker_bin run -i -a stdin -a stdout -a stderr --cidfile=$ENV{TASK_WORK}/docker.cid ";
@@ -659,7 +667,7 @@ for (my $todo_ptr = 0; $todo_ptr <= $#jobstep_todo; $todo_ptr ++)
           $command .= "-e \Q$env_key=$env_val\E ";
         }
       }
-      $command .= "\Q$docker_image\E ";
+      $command .= "\Q$docker_hash\E ";
     } else {
       $command .= "crunchstat -cgroup-root=/sys/fs/cgroup -poll=10000 "
     }
@@ -1437,6 +1445,21 @@ sub must_lock_now
   }
 }
 
+sub find_docker_hash {
+  # Given a Keep locator, search for a matching link to find the Docker hash
+  # of the stored image.
+  my $locator = shift;
+  my $links_result = $arv->{links}->{list}->execute(
+    filters => [["head_uuid", "=", $locator],
+                ["link_class", "=", "docker_image_hash"]],
+    limit => 1);
+  my $docker_hash;
+  foreach my $link (@{$links_result->{items}}) {
+    $docker_hash = lc($link->{name});
+  }
+  return $docker_hash;
+}
+
 __DATA__
 #!/usr/bin/perl
 

commit f443fdcac93c38a39949e0dd441fdabc5761d6f6
Author: Brett Smith <brett at curoverse.com>
Date:   Wed Jun 11 17:43:27 2014 -0400

    2879: API server maps Job Docker image constraints to Collections.
    
    Doing this translation in the API server helps ensure that one piece
    of software makes one consistent decision about which Docker image
    should be used to run the Job.

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 745f0bf..64a6bb0 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -147,4 +147,49 @@ class Collection < ArvadosModel
     raise "uuid #{uuid} has no hash part" if !hash_part
     [hash_part, size_part].compact.join '+'
   end
+
+  def self.for_latest_docker_image(search_term, search_tag=nil, readers=nil)
+    readers ||= [Thread.current[:user]]
+    base_search = Link.
+      readable_by(*readers).
+      readable_by(*readers, table_name: "collections").
+      joins("JOIN collections ON links.head_uuid = collections.uuid").
+      order("links.created_at DESC")
+
+    # If the search term is a Collection locator with an associated
+    # Docker image hash link, return that Collection.
+    coll_matches = base_search.
+      where(link_class: "docker_image_hash", collections: {uuid: search_term})
+    if match = coll_matches.first
+      return find_by_uuid(match.head_uuid)
+    end
+
+    # Find Collections with matching Docker image repository+tag pairs.
+    matches = base_search.
+      where(link_class: "docker_image_repo+tag",
+            name: "#{search_term}:#{search_tag || 'latest'}")
+
+    # If that didn't work, find Collections with matching Docker image hashes.
+    if matches.empty?
+      matches = base_search.
+        where("link_class = ? and name LIKE ?",
+              "docker_image_hash", "#{search_term}%")
+    end
+
+    # Select the image that was created most recently.  Note that the
+    # SQL search order and fallback timestamp values are chosen so
+    # that if image timestamps are missing, we use the image with the
+    # newest link.
+    latest_image_link = nil
+    latest_image_timestamp = "1900-01-01T00:00:00Z"
+    matches.find_each do |link|
+      link_timestamp = link.properties.fetch("image_timestamp",
+                                             "1900-01-01T00:00:01Z")
+      if link_timestamp > latest_image_timestamp
+        latest_image_link = link
+        latest_image_timestamp = link_timestamp
+      end
+    end
+    latest_image_link.nil? ? nil : find_by_uuid(latest_image_link.head_uuid)
+  end
 end
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 51fb7c2..db0734f 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -2,9 +2,11 @@ class Job < ArvadosModel
   include HasUuid
   include KindAndEtag
   include CommonApiTemplate
+  attr_protected :docker_image_locator
   serialize :script_parameters, Hash
   serialize :runtime_constraints, Hash
   serialize :tasks_summary, Hash
+  before_validation :find_docker_image_locator
   before_create :ensure_unique_submit_id
   before_create :ensure_script_version_is_commit
   before_update :ensure_script_version_is_commit
@@ -38,6 +40,7 @@ class Job < ArvadosModel
     t.add :nondeterministic
     t.add :repository
     t.add :supplied_script_version
+    t.add :docker_image_locator
   end
 
   def assert_finished
@@ -98,6 +101,21 @@ class Job < ArvadosModel
     true
   end
 
+  def find_docker_image_locator
+    # Find the Collection that holds the Docker image specified in the
+    # runtime constraints, and store its locator in docker_image_locator.
+    image_search = runtime_constraints['docker_image']
+    image_tag = runtime_constraints['docker_image_tag']
+    if image_search.nil?
+      self.docker_image_locator = nil
+    elsif coll = Collection.for_latest_docker_image(image_search, image_tag)
+      self.docker_image_locator = coll.uuid
+    else
+      errors.add(:docker_image_locator, "Docker image not found")
+      false
+    end
+  end
+
   def dependencies
     deps = {}
     queue = self.script_parameters.values
diff --git a/services/api/db/migrate/20140611173003_add_docker_locator_to_jobs.rb b/services/api/db/migrate/20140611173003_add_docker_locator_to_jobs.rb
new file mode 100644
index 0000000..3186e56
--- /dev/null
+++ b/services/api/db/migrate/20140611173003_add_docker_locator_to_jobs.rb
@@ -0,0 +1,5 @@
+class AddDockerLocatorToJobs < ActiveRecord::Migration
+  def change
+    add_column :jobs, :docker_image_locator, :string
+  end
+end
diff --git a/services/api/db/schema.rb b/services/api/db/schema.rb
index 1ef80ab..1f9b501 100644
--- a/services/api/db/schema.rb
+++ b/services/api/db/schema.rb
@@ -11,8 +11,9 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
+ActiveRecord::Schema.define(:version => 20140611173003) do
+
 
-ActiveRecord::Schema.define(:version => 20140602143352) do
 
   create_table "api_client_authorizations", :force => true do |t|
     t.string   "api_token",                                           :null => false
@@ -195,6 +196,7 @@ ActiveRecord::Schema.define(:version => 20140602143352) do
     t.string   "repository"
     t.boolean  "output_is_persistent",     :default => false, :null => false
     t.string   "supplied_script_version"
+    t.string   "docker_image_locator"
   end
 
   add_index "jobs", ["created_at"], :name => "index_jobs_on_created_at"
diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 201299b..bce7df1 100644
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -58,3 +58,14 @@ multilevel_collection_2:
   modified_at: 2014-02-03T17:22:54Z
   updated_at: 2014-02-03T17:22:54Z
   manifest_text: "./dir1/sub1 0:0:a 0:0:b\n./dir2/sub2 0:0:c 0:0:d\n"
+
+docker_image:
+  # This Collection has links with Docker image metadata.
+  uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+  owner_uuid: qr1hi-tpzed-000000000000000
+  created_at: 2014-06-11T17:22:54Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  modified_at: 2014-06-11T17:22:54Z
+  updated_at: 2014-06-11T17:22:54Z
+  manifest_text: ". d21353cfe035e3e384563ee55eadbb2f+67108864 5c77a43e329b9838cbec18ff42790e57+55605760 0:122714624:d8309758b8fe2c81034ffc8a10c36460b77db7bc5e7b448c4e5b684f9d95a678.tar\n"
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 1729bd2..ae2511b 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -408,7 +408,7 @@ has_symbol_keys_in_database_somehow:
   uuid: zzzzz-o0j2j-enl1wg58310loc6
   owner_uuid: zzzzz-tpzed-000000000000000
   created_at: 2014-05-28 16:24:02.314722162 Z
-  modified_by_client_uuid: 
+  modified_by_client_uuid:
   modified_by_user_uuid: zzzzz-tpzed-000000000000000
   modified_at: 2014-05-28 16:24:02.314484982 Z
   tail_uuid: ~
@@ -440,3 +440,96 @@ bug2931_link_with_null_head_uuid:
   tail_uuid: ~
   head_uuid: ~
   properties: {}
+
+active_user_permission_to_docker_image_collection:
+  uuid: zzzzz-o0j2j-dp1d8395ldqw33s
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2014-01-24 20:42:26 -0800
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-01-24 20:42:26 -0800
+  updated_at: 2014-01-24 20:42:26 -0800
+  tail_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  link_class: permission
+  name: can_read
+  head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+  properties: {}
+
+docker_image_collection_hash:
+  uuid: zzzzz-o0j2j-dockercollhasha
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-06-11 14:30:00.184389725 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-11 14:30:00.184019565 Z
+  updated_at: 2014-06-11 14:30:00.183829316 Z
+  link_class: docker_image_hash
+  name: d8309758b8fe2c81034ffc8a10c36460b77db7bc5e7b448c4e5b684f9d95a678
+  tail_uuid: ~
+  head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+  properties:
+    image_timestamp: 2014-06-10T14:30:00.184019565Z
+
+docker_image_collection_repository:
+  uuid: zzzzz-o0j2j-dockercollrepos
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-06-11 14:30:00.184389725 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-11 14:30:00.184019565 Z
+  updated_at: 2014-06-11 14:30:00.183829316 Z
+  link_class: docker_image_repository
+  name: arvados/apitestfixture
+  tail_uuid: ~
+  head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+  properties:
+    image_timestamp: 2014-06-10T14:30:00.184019565Z
+
+docker_image_collection_tag:
+  uuid: zzzzz-o0j2j-dockercolltagbb
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-06-11 14:30:00.184389725 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-11 14:30:00.184019565 Z
+  updated_at: 2014-06-11 14:30:00.183829316 Z
+  link_class: docker_image_repo+tag
+  name: arvados/apitestfixture:latest
+  tail_uuid: ~
+  head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+  properties:
+    image_timestamp: 2014-06-10T14:30:00.184019565Z
+
+docker_image_collection_tag2:
+  uuid: zzzzz-o0j2j-dockercolltagbc
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-06-11 14:30:00.184389725 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-11 14:30:00.184019565 Z
+  updated_at: 2014-06-11 14:30:00.183829316 Z
+  link_class: docker_image_repo+tag
+  name: arvados/apitestfixture:june10
+  tail_uuid: ~
+  head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+  properties:
+    image_timestamp: 2014-06-10T14:30:00.184019565Z
+
+ancient_docker_image_collection_hash:
+  # This image helps test that searches for Docker images find
+  # the latest available image: the hash is the same as
+  # docker_image_collection_hash, but it points to a different
+  # Collection and has an older image timestamp.
+  uuid: zzzzz-o0j2j-dockercollhashz
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-06-12 14:30:00.184389725 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-12 14:30:00.184019565 Z
+  updated_at: 2014-06-12 14:30:00.183829316 Z
+  link_class: docker_image_hash
+  name: d8309758b8fe2c81034ffc8a10c36460b77db7bc5e7b448c4e5b684f9d95a678
+  tail_uuid: ~
+  head_uuid: b519d9cb706a29fc7ea24dbea2f05851+249025
+  properties:
+    image_timestamp: 2010-06-10T14:30:00.184019565Z
diff --git a/services/api/test/integration/select_test.rb b/services/api/test/integration/select_test.rb
index b5f09df..2057565 100644
--- a/services/api/test/integration/select_test.rb
+++ b/services/api/test/integration/select_test.rb
@@ -35,6 +35,16 @@ class SelectTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def assert_link_classes_ascend(current_class, prev_class)
+    # Databases and Ruby don't always agree about string ordering with
+    # punctuation.  If the strings aren't ascending normally, check
+    # that they're equal up to punctuation.
+    if current_class < prev_class
+      class_prefix = current_class.split(/\W/).first
+      assert prev_class.start_with?(class_prefix)
+    end
+  end
+
   test "select two columns with order" do
     get "/arvados/v1/links", {:format => :json, :select => ['link_class', 'uuid'], :order => ['link_class asc', "uuid desc"]}, auth(:active)
     assert_response :success
@@ -49,7 +59,7 @@ class SelectTest < ActionDispatch::IntegrationTest
         prev_uuid = "zzzzz-zzzzz-zzzzzzzzzzzzzzz"
       end
 
-      assert i['link_class'] >= prev_link_class
+      assert_link_classes_ascend(i['link_class'], prev_link_class)
       assert i['uuid'] < prev_uuid
 
       prev_link_class = i['link_class']
@@ -71,7 +81,7 @@ class SelectTest < ActionDispatch::IntegrationTest
         prev_uuid = "zzzzz-zzzzz-zzzzzzzzzzzzzzz"
       end
 
-      assert i['link_class'] >= prev_link_class
+      assert_link_classes_ascend(i['link_class'], prev_link_class)
       assert i['uuid'] < prev_uuid
 
       prev_link_class = i['link_class']
diff --git a/services/api/test/integration/serialized_encoding_test.rb b/services/api/test/integration/serialized_encoding_test.rb
index ed30fdb..269018d 100644
--- a/services/api/test/integration/serialized_encoding_test.rb
+++ b/services/api/test/integration/serialized_encoding_test.rb
@@ -13,7 +13,7 @@ class SerializedEncodingTest < ActionDispatch::IntegrationTest
 
     job: {
       repository: 'foo',
-      runtime_constraints: {docker_image: 'arvados/jobs'},
+      runtime_constraints: {docker_image: 'arvados/apitestfixture'},
       script: 'hash',
       script_version: 'master',
       script_parameters: {pattern: 'foobar'},
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 5079316..5f53b2a 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -1,7 +1,109 @@
 require 'test_helper'
 
 class JobTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  BAD_COLLECTION = "#{'f' * 32}+0"
+
+  setup do
+    set_user_from_auth :active
+  end
+
+  test "Job without Docker image doesn't get locator" do
+    job = Job.new
+    assert job.valid?
+    assert_nil job.docker_image_locator
+  end
+
+  { 'name' => [:links, :docker_image_collection_repository, :name],
+    'hash' => [:links, :docker_image_collection_hash, :name],
+    'locator' => [:collections, :docker_image, :uuid],
+  }.each_pair do |spec_type, (fixture_type, fixture_name, fixture_attr)|
+    test "Job initialized with Docker image #{spec_type} gets locator" do
+      image_spec = send(fixture_type, fixture_name).send(fixture_attr)
+      job = Job.new(runtime_constraints: {'docker_image' => image_spec})
+      assert(job.valid?, "Docker image #{spec_type} was invalid")
+      assert_equal(collections(:docker_image).uuid, job.docker_image_locator)
+    end
+
+    test "Job modified with Docker image #{spec_type} gets locator" do
+      job = Job.new
+      assert job.valid?
+      assert_nil job.docker_image_locator
+      image_spec = send(fixture_type, fixture_name).send(fixture_attr)
+      job.runtime_constraints['docker_image'] = image_spec
+      assert(job.valid?, "modified Docker image #{spec_type} was invalid")
+      assert_equal(collections(:docker_image).uuid, job.docker_image_locator)
+    end
+  end
+
+  test "removing a Docker runtime constraint removes the locator" do
+    image_locator = collections(:docker_image).uuid
+    job = Job.new(runtime_constraints: {'docker_image' => image_locator})
+    assert job.valid?
+    assert_equal(image_locator, job.docker_image_locator)
+    job.runtime_constraints = {}
+    assert(job.valid?, "clearing runtime constraints made the Job invalid")
+    assert_nil job.docker_image_locator
+  end
+
+  test "locate a Docker image with a repository + tag" do
+    image_repo, image_tag =
+      links(:docker_image_collection_tag2).name.split(':', 2)
+    job = Job.new(runtime_constraints:
+                  {'docker_image' => image_repo,
+                    'docker_image_tag' => image_tag})
+    assert(job.valid?, "Job with Docker tag search invalid")
+    assert_equal(collections(:docker_image).uuid, job.docker_image_locator)
+  end
+
+  test "can't locate a Docker image with a nonexistent tag" do
+    image_repo = links(:docker_image_collection_repository).name
+    image_tag = '__nonexistent tag__'
+    job = Job.new(runtime_constraints:
+                  {'docker_image' => image_repo,
+                    'docker_image_tag' => image_tag})
+    assert(job.invalid?, "Job with bad Docker tag valid")
+  end
+
+  test "locate a Docker image with a partial hash" do
+    image_hash = links(:docker_image_collection_hash).name[0..24]
+    job = Job.new(runtime_constraints: {'docker_image' => image_hash})
+    assert(job.valid?, "Job with partial Docker image hash failed")
+    assert_equal(collections(:docker_image).uuid, job.docker_image_locator)
+  end
+
+  { 'name' => 'arvados_test_nonexistent',
+    'hash' => 'f' * 64,
+    'locator' => BAD_COLLECTION,
+  }.each_pair do |spec_type, image_spec|
+    test "Job validation fails with nonexistent Docker image #{spec_type}" do
+      job = Job.new(runtime_constraints: {'docker_image' => image_spec})
+      assert(job.invalid?, "nonexistent Docker image #{spec_type} was valid")
+    end
+  end
+
+  test "Job validation fails with non-Docker Collection constraint" do
+    job = Job.new(runtime_constraints:
+                  {'docker_image' => collections(:foo_file).uuid})
+    assert(job.invalid?, "non-Docker Collection constraint was valid")
+  end
+
+  test "can't create Job with Docker image locator" do
+    begin
+      job = Job.new(docker_image_locator: BAD_COLLECTION)
+    rescue ActiveModel::MassAssignmentSecurity::Error
+      # Test passes - expected attribute protection
+    else
+      assert_nil job.docker_image_locator
+    end
+  end
+
+  test "can't assign Docker image locator to Job" do
+    job = Job.new
+    begin
+      Job.docker_image_locator = BAD_COLLECTION
+    rescue NoMethodError
+      # Test passes - expected attribute protection
+    end
+    assert_nil job.docker_image_locator
+  end
 end

commit a4fd1b6edb38288c173f6b7db9413764010069ae
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Jun 16 10:27:38 2014 -0400

    2879: Let ArvadosModel.readable_by callers specify table name.
    
    This makes it possible to use readable_by in queries with arbitrary
    joins.

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 95fd055..2df6686 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -91,6 +91,13 @@ class ArvadosModel < ActiveRecord::Base
     # Get rid of troublesome nils
     users_list.compact!
 
+    # Load optional keyword arguments, if they exist.
+    if users_list.last.is_a? Hash
+      kwargs = users_list.pop
+    else
+      kwargs = {}
+    end
+
     # Check if any of the users are admin.  If so, we're done.
     if users_list.select { |u| u.is_admin }.empty?
 
@@ -101,6 +108,7 @@ class ArvadosModel < ActiveRecord::Base
         collect { |uuid| sanitize(uuid) }.join(', ')
       sql_conds = []
       sql_params = []
+      sql_table = kwargs.fetch(:table_name, table_name)
       or_object_uuid = ''
 
       # This row is owned by a member of users_list, or owned by a group
@@ -113,25 +121,25 @@ class ArvadosModel < ActiveRecord::Base
       # to this row, or to the owner of this row (see join() below).
       permitted_uuids = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (#{sanitized_uuid_list}))"
 
-      sql_conds += ["#{table_name}.owner_uuid in (?)",
-                    "#{table_name}.uuid in (?)",
-                    "#{table_name}.uuid IN #{permitted_uuids}"]
+      sql_conds += ["#{sql_table}.owner_uuid in (?)",
+                    "#{sql_table}.uuid in (?)",
+                    "#{sql_table}.uuid IN #{permitted_uuids}"]
       sql_params += [uuid_list, user_uuids]
 
-      if self == Link and users_list.any?
+      if sql_table == "links" and users_list.any?
         # This row is a 'permission' or 'resources' link class
         # The uuid for a member of users_list is referenced in either the head
         # or tail of the link
-        sql_conds += ["(#{table_name}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) AND (#{table_name}.head_uuid IN (?) OR #{table_name}.tail_uuid IN (?)))"]
+        sql_conds += ["(#{sql_table}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) AND (#{sql_table}.head_uuid IN (?) OR #{sql_table}.tail_uuid IN (?)))"]
         sql_params += [user_uuids, user_uuids]
       end
 
-      if self == Log and users_list.any?
+      if sql_table == "logs" and users_list.any?
         # Link head points to the object described by this row
-        sql_conds += ["#{table_name}.object_uuid IN #{permitted_uuids}"]
+        sql_conds += ["#{sql_table}.object_uuid IN #{permitted_uuids}"]
 
         # This object described by this row is owned by this user, or owned by a group readable by this user
-        sql_conds += ["#{table_name}.object_owner_uuid in (?)"]
+        sql_conds += ["#{sql_table}.object_owner_uuid in (?)"]
         sql_params += [uuid_list]
       end
 

commit 99b93ceaa2b9af988f2c3261d4cfb49dc8113f98
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jun 12 10:10:24 2014 -0400

    2879: Fix SLURM node separator when installing Docker images.

diff --git a/sdk/cli/bin/crunch-job b/sdk/cli/bin/crunch-job
index 8b47177..96f7354 100755
--- a/sdk/cli/bin/crunch-job
+++ b/sdk/cli/bin/crunch-job
@@ -506,7 +506,7 @@ if ($docker_image) {
   my $docker_pid = fork();
   if ($docker_pid == 0)
   {
-    srun (["srun", "--nodelist=" . join(' ', @node)],
+    srun (["srun", "--nodelist=" . join(',', @node)],
           [$docker_bin, 'pull', $docker_image]);
     exit ($?);
   }

commit 3d9568727cc0dd4f5f778791ce387370b5f11070
Author: Brett Smith <brett at curoverse.com>
Date:   Fri Jun 13 13:40:45 2014 -0400

    2879: Store Docker image repo+tag together in one tag.
    
    It doesn't make sense to query the tag independently of the
    repository, and this change will make the common case of querying both
    together easier.

diff --git a/sdk/python/arvados/commands/keepdocker.py b/sdk/python/arvados/commands/keepdocker.py
index abf60f2..236d9fc 100644
--- a/sdk/python/arvados/commands/keepdocker.py
+++ b/sdk/python/arvados/commands/keepdocker.py
@@ -204,7 +204,8 @@ def main(arguments=None):
     make_link('docker_image_hash', image_hash, **link_base)
     if not image_hash.startswith(args.image.lower()):
         make_link('docker_image_repository', args.image, **link_base)
-        make_link('docker_image_tag', args.tag, **link_base)
+        make_link('docker_image_repo+tag', '{}:{}'.format(args.image, args.tag),
+                  **link_base)
 
     # Clean up.
     image_file.close()

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list