[ARVADOS] created: 88dc02bbde0cb0d6b916a98e4bb789412420d9ad

git at public.curoverse.com git at public.curoverse.com
Thu Jun 12 15:52:30 EDT 2014


        at  88dc02bbde0cb0d6b916a98e4bb789412420d9ad (commit)


commit 88dc02bbde0cb0d6b916a98e4bb789412420d9ad
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 a1e74ed07479ff3781aac4e90a80e62a7d0a11df
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 a69d65a7ae8fa6ea6550fbd44ca46546faa3dd5e
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/job.rb b/services/api/app/models/job.rb
index 51fb7c2..bf8119d 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,48 @@ 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_name = runtime_constraints['docker_image']
+    if image_name.nil?
+      self.docker_image_locator = nil
+      return
+    elsif not (new_record? or runtime_constraints_changed?)
+      return
+    # Accept images specified as a locator, if there's an associated
+    # docker_image_hash link.
+    elsif coll = Collection.joins(:links_via_head).
+        where(uuid: image_name,
+              links: {link_class: 'docker_image_hash'}).first
+      self.docker_image_locator = (coll.links_via_head.any?) ? coll.uuid : nil
+    else  # Accept images specified by their Docker metadata.
+      link_search = {name: image_name}
+      if image_name =~ /^[0-9A-Fa-f]{64}$/
+        link_search[:link_class] = 'docker_image_hash'
+      else
+        # Search for a match on image repository + tag.
+        link_search[:link_class] = 'docker_image_repository'
+        tag_name = runtime_constraints['docker_image_tag'] || 'latest'
+        link_search[:head_uuid] = Link.select(:head_uuid).
+          where(link_class: 'docker_image_tag', name: tag_name).
+          map(&:head_uuid)
+      end
+      links = Link.where(link_search)
+      # Select the image that was created most recently.
+      latest = links.first
+      links.find_each do |link|
+        latest = link if (link.properties['image_timestamp'] >
+                          latest.properties['image_timestamp'])
+      end
+      self.docker_image_locator = latest.andand.head_uuid
+    end
+    if docker_image_locator.nil?
+      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 1fb1608..cca2b26 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,82 @@ bug2931_link_with_null_head_uuid:
   tail_uuid: ~
   head_uuid: ~
   properties: {}
+
+docker_image_collection_hash:
+  uuid: zzzzz-o0j2j-dockercollhasha
+  owner_uuid: zzzzz-tpzed-000000000000000
+  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-000000000000000
+  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-000000000000000
+  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_tag
+  name: 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-000000000000000
+  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_tag
+  name: 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-000000000000000
+  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/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..3e9488c 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -1,7 +1,91 @@
 require 'test_helper'
 
 class JobTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  BAD_COLLECTION = "#{'f' * 32}+0"
+
+  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 = links(:docker_image_collection_repository).name
+    image_tag = links(:docker_image_collection_tag2).name
+    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
+
+  { '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
+    assert_raises(ActiveModel::MassAssignmentSecurity::Error) do
+      Job.new(docker_image_locator: BAD_COLLECTION)
+    end
+  end
+
+  test "can't assign Docker image locator to Job" do
+    job = Job.new
+    assert_raises(NoMethodError) do
+      Job.docker_image_locator = BAD_COLLECTION
+    end
+  end
 end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list