[ARVADOS] created: db8f54721e4c101a7491b5c412e0bc51672dc7de

git at public.curoverse.com git at public.curoverse.com
Tue Nov 11 15:33:41 EST 2014


        at  db8f54721e4c101a7491b5c412e0bc51672dc7de (commit)


commit db8f54721e4c101a7491b5c412e0bc51672dc7de
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Nov 11 15:33:40 2014 -0500

    4027: Add arvados_sdk_version runtime constraint to API server.
    
    Crunch will use this information to provide a specific SDK version in
    the runtime environment.

diff --git a/doc/api/schema/Job.html.textile.liquid b/doc/api/schema/Job.html.textile.liquid
index f94f093..e2363ac 100644
--- a/doc/api/schema/Job.html.textile.liquid
+++ b/doc/api/schema/Job.html.textile.liquid
@@ -23,9 +23,9 @@ table(table table-bordered table-condensed).
 |_. Attribute|_. Type|_. Description|_. Notes|
 |script|string|The filename of the job script.|This program will be invoked by Crunch for each job task. It is given as a path to an executable file, relative to the @/crunch_scripts@ directory in the Git tree specified by the _repository_ and _script_version_ attributes.|
 |script_parameters|hash|The input parameters for the job.|Conventionally, one of the parameters is called @"input"@. Typically, some parameter values are collection UUIDs. Ultimately, though, the significance of parameters is left entirely up to the script itself.|
-|repository|string|Git repository|Given as the name of a locally hosted git repository.|
+|repository|string|Git repository|Given as the name of a locally hosted Git repository.|
 |script_version|string|Git commit|During a **create** transaction, this is the Git branch, tag, or hash supplied by the client. Before the job starts, Arvados updates it to the full 40-character SHA-1 hash of the commit used by the job.
-See "Script versions":#script_version below for more detail about acceptable ways to specify a commit.|
+See "Specifying Git versions":#script_version below for more detail about acceptable ways to specify a commit.|
 |cancelled_by_client_uuid|string|API client ID|Is null if job has not been cancelled|
 |cancelled_by_user_uuid|string|Authenticated user ID|Is null if job has not been cancelled|
 |cancelled_at|datetime|When job was cancelled|Is null if job has not been cancelled|
@@ -41,17 +41,20 @@ See "Script versions":#script_version below for more detail about acceptable way
 |nondeterministic|boolean|The job is expected to produce different results if run more than once.|If true, this job will not be considered as a candidate for automatic re-use when submitting subsequent identical jobs.|
 |submit_id|string|Unique ID provided by client when job was submitted|Optional. This can be used by a client to make the "jobs.create":{{site.baseurl}}/api/methods/jobs.html#create method idempotent.|
 |priority|string|||
+|arvados_sdk_version|string|Git commit hash that specifies the SDK version to use from the Arvados repository|This is set by searching the Arvados repository for a match for the arvados_sdk_version runtime constraint.|
+|docker_image_locator|string|Portable data hash of the collection that contains the Docker image to use|This is set by searching readable collections for a match for the docker_image runtime constraint.|
 |runtime_constraints|hash|Constraints that must be satisfied by the job/task scheduler in order to run the job.|See below.|
 
-h3(#script_version). Script versions
+h3(#script_version). Specifying Git versions
 
-The @script_version@ attribute is typically given as a branch, tag, or commit hash, but there are many more ways to specify a Git commit. The "specifying revisions" section of the "gitrevisions manual page":http://git-scm.com/docs/gitrevisions.html has a definitive list. Arvados accepts @script_version@ in any format listed there that names a single commit (not a tree, a blob, or a range of commits). However, some kinds of names can be expected to resolve differently in Arvados than they do in your local repository. For example, <code>HEAD@{1}</code> refers to the local reflog, and @origin/master@ typically refers to a remote branch: neither is likely to work as desired if given as a @script_version at .
+The script_version attribute and arvados_sdk_version runtime constraint are typically given as a branch, tag, or commit hash, but there are many more ways to specify a Git commit. The "specifying revisions" section of the "gitrevisions manual page":http://git-scm.com/docs/gitrevisions.html has a definitive list. Arvados accepts Git versions in any format listed there that names a single commit (not a tree, a blob, or a range of commits). However, some kinds of names can be expected to resolve differently in Arvados than they do in your local repository. For example, <code>HEAD@{1}</code> refers to the local reflog, and @origin/master@ typically refers to a remote branch: neither is likely to work as desired if given as a Git version.
 
 h3. Runtime constraints
 
 table(table table-bordered table-condensed).
 |_. Key|_. Type|_. Description|_. Implemented|
-|docker_image|string|The Docker image that this Job needs to run.  If specified, Crunch will create a Docker container from this image, and run the Job's script inside that.  The Keep mount and work directories will be available as volumes inside this container.  The image must be uploaded to Arvados using @arv keep docker at .  You may specify the image in any format that Docker accepts, such as @arvados/jobs@, @debian:latest@, or the Docker image id.  Alternatively, you may specify the UUID of the image Collection, returned by @arv keep docker at .|✓|
+|arvados_sdk_version|string|The Git version of the SDKs to use from the Arvados git repository.  See "Specifying Git versions":#script_version for more detail about acceptable ways to specify a commit.||
+|docker_image|string|The Docker image that this Job needs to run.  If specified, Crunch will create a Docker container from this image, and run the Job's script inside that.  The Keep mount and work directories will be available as volumes inside this container.  The image must be uploaded to Arvados using @arv keep docker at .  You may specify the image in any format that Docker accepts, such as @arvados/jobs@, @debian:latest@, or the Docker image id.  Alternatively, you may specify the UUID or portable data hash of the image Collection, returned by @arv keep docker at .|✓|
 |min_nodes|integer||✓|
 |max_nodes|integer|||
 |min_cores_per_node|integer|Require that each node assigned to this Job have the specified number of CPU cores|✓|
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 864e608..6e01de9 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -2,7 +2,7 @@ class Job < ArvadosModel
   include HasUuid
   include KindAndEtag
   include CommonApiTemplate
-  attr_protected :docker_image_locator
+  attr_protected :arvados_sdk_version, :docker_image_locator
   serialize :script_parameters, Hash
   serialize :runtime_constraints, Hash
   serialize :tasks_summary, Hash
@@ -11,6 +11,7 @@ class Job < ArvadosModel
   before_validation :set_priority
   before_validation :update_state_from_old_state_attrs
   validate :ensure_script_version_is_commit
+  validate :find_arvados_sdk_version
   validate :find_docker_image_locator
   validate :validate_status
   validate :validate_state_change
@@ -45,6 +46,7 @@ class Job < ArvadosModel
     t.add :nondeterministic
     t.add :repository
     t.add :supplied_script_version
+    t.add :arvados_sdk_version
     t.add :docker_image_locator
     t.add :queue_position
     t.add :node_uuids
@@ -149,28 +151,43 @@ 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.
-    unless runtime_constraints.is_a? Hash
-      # We're still in validation stage, so we can't assume
-      # runtime_constraints isn't something horrible like an array or
-      # a string. Treat those cases as "no docker image supplied";
-      # other validations will fail anyway.
-      self.docker_image_locator = nil
-      return true
+  def resolve_runtime_constraint(key, attr_sym)
+    if ((runtime_constraints.is_a? Hash) and
+        (search = runtime_constraints[key]))
+      ok, result = yield search
+    else
+      ok, result = true, nil
     end
-    image_search = runtime_constraints['docker_image']
-    image_tag = runtime_constraints['docker_image_tag']
-    if image_search.nil?
-      self.docker_image_locator = nil
-      true
-    elsif coll = Collection.for_latest_docker_image(image_search, image_tag)
-      self.docker_image_locator = coll.portable_data_hash
-      true
+    if ok
+      send("#{attr_sym}=".to_sym, result)
     else
-      errors.add(:docker_image_locator, "not found for #{image_search}")
-      false
+      errors.add(attr_sym, result)
+    end
+    ok
+  end
+
+  def find_arvados_sdk_version
+    resolve_runtime_constraint("arvados_sdk_version",
+                               :arvados_sdk_version) do |git_search|
+      commits = Commit.find_commit_range(current_user, "arvados",
+                                         nil, git_search, nil)
+      if commits.andand.any?
+        [true, commits.first]
+      else
+        [false, "#{git_search} does not resolve to a commit"]
+      end
+    end
+  end
+
+  def find_docker_image_locator
+    resolve_runtime_constraint("docker_image",
+                               :docker_image_locator) do |image_search|
+      image_tag = runtime_constraints['docker_image_tag']
+      if coll = Collection.for_latest_docker_image(image_search, image_tag)
+        [true, coll.portable_data_hash]
+      else
+        [false, "not found for #{image_search}"]
+      end
     end
   end
 
diff --git a/services/api/db/migrate/20141111133038_add_arvados_sdk_version_to_jobs.rb b/services/api/db/migrate/20141111133038_add_arvados_sdk_version_to_jobs.rb
new file mode 100644
index 0000000..214919d
--- /dev/null
+++ b/services/api/db/migrate/20141111133038_add_arvados_sdk_version_to_jobs.rb
@@ -0,0 +1,13 @@
+class AddArvadosSdkVersionToJobs < ActiveRecord::Migration
+  def up
+    change_table :jobs do |t|
+      t.column :arvados_sdk_version, :string
+    end
+  end
+
+  def down
+    change_table :jobs do |t|
+      t.remove :arvados_sdk_version
+    end
+  end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 48bc57a..2eca2ca 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -432,7 +432,8 @@ CREATE TABLE jobs (
     docker_image_locator character varying(255),
     priority integer DEFAULT 0 NOT NULL,
     description text,
-    state character varying(255)
+    state character varying(255),
+    arvados_sdk_version character varying(255)
 );
 
 
@@ -2029,4 +2030,6 @@ INSERT INTO schema_migrations (version) VALUES ('20140918153541');
 
 INSERT INTO schema_migrations (version) VALUES ('20140918153705');
 
-INSERT INTO schema_migrations (version) VALUES ('20140924091559');
\ No newline at end of file
+INSERT INTO schema_migrations (version) VALUES ('20140924091559');
+
+INSERT INTO schema_migrations (version) VALUES ('20141111133038');
\ No newline at end of file
diff --git a/services/api/test/fixtures/repositories.yml b/services/api/test/fixtures/repositories.yml
index e173bf5..38632dd 100644
--- a/services/api/test/fixtures/repositories.yml
+++ b/services/api/test/fixtures/repositories.yml
@@ -3,6 +3,11 @@ crunch_dispatch_test:
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz # active user
   name: crunch_dispatch_test
 
+arvados:
+  uuid: zzzzz-s0uqq-arvadosrepo0123
+  owner_uuid: zzzzz-j7d0g-fffffffffffffff # all users
+  name: arvados
+
 foo:
   uuid: zzzzz-s0uqq-382brsig8rp3666
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz # active user
diff --git a/services/api/test/functional/arvados/v1/commits_controller_test.rb b/services/api/test/functional/arvados/v1/commits_controller_test.rb
index f7f99d1..ceaebff 100644
--- a/services/api/test/functional/arvados/v1/commits_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/commits_controller_test.rb
@@ -25,7 +25,8 @@ class Arvados::V1::CommitsControllerTest < ActionController::TestCase
   #test "test_branch1" do
     # complains "fatal: bad object 077ba2ad3ea24a929091a9e6ce545c93199b8e57"
     a = Commit.find_commit_range(users(:active), nil, nil, 'master', nil)
-    assert_equal ['f35f99b7d32bac257f5989df02b9f12ee1a9b0d6', '077ba2ad3ea24a929091a9e6ce545c93199b8e57'], a
+    assert_includes(a, 'f35f99b7d32bac257f5989df02b9f12ee1a9b0d6')
+    assert_includes(a, '077ba2ad3ea24a929091a9e6ce545c93199b8e57')
 
   #test "test_branch2" do
     a = Commit.find_commit_range(users(:active), 'foo', nil, 'b1', nil)
diff --git a/services/api/test/helpers/git_test_helper.rb b/services/api/test/helpers/git_test_helper.rb
index 39e506f..67e99c1 100644
--- a/services/api/test/helpers/git_test_helper.rb
+++ b/services/api/test/helpers/git_test_helper.rb
@@ -15,15 +15,13 @@ module GitTestHelper
   def self.included base
     base.setup do
       @tmpdir = Dir.mktmpdir()
-      `cp test/test.git.tar #{@tmpdir} && cd #{@tmpdir} && tar xf test.git.tar`
-      @orig_git_repositories_dir = Rails.configuration.git_repositories_dir
+      system("tar", "-xC", @tmpdir, "-f", "test/test.git.tar")
       Rails.configuration.git_repositories_dir = "#{@tmpdir}/test"
       Commit.refresh_repositories
     end
 
     base.teardown do
       FileUtils.remove_entry @tmpdir, true
-      Rails.configuration.git_repositories_dir = @orig_git_repositories_dir
       Commit.refresh_repositories
     end
   end
diff --git a/services/api/test/test.git.tar b/services/api/test/test.git.tar
index 6845160..ae46601 100644
Binary files a/services/api/test/test.git.tar and b/services/api/test/test.git.tar differ
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index cf1e26c..32f9e3e 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -110,24 +110,35 @@ class JobTest < ActiveSupport::TestCase
     assert_equal(image_uuid, job.docker_image_locator)
   end
 
-  test "can't create Job with Docker image locator" do
+  def check_attrs_unset(job, attrs)
+    assert_empty(attrs.each_key.map { |key| job.send(key) }.compact,
+                 "job has values for #{attrs.keys}")
+  end
+
+  def check_creation_prohibited(attrs)
     begin
-      job = Job.new job_attrs(docker_image_locator: BAD_COLLECTION)
+      job = Job.new(job_attrs(attrs))
     rescue ActiveModel::MassAssignmentSecurity::Error
       # Test passes - expected attribute protection
     else
-      assert_nil job.docker_image_locator
+      check_attrs_unset(job, attrs)
     end
   end
 
-  test "can't assign Docker image locator to Job" do
-    job = Job.new job_attrs
-    begin
-      Job.docker_image_locator = BAD_COLLECTION
-    rescue NoMethodError
-      # Test passes - expected attribute protection
+  def check_modification_prohibited(attrs)
+    job = Job.new(job_attrs)
+    attrs.each_pair do |key, value|
+      assert_raises(NoMethodError) { job.send("{key}=".to_sym, value) }
     end
-    assert_nil job.docker_image_locator
+    check_attrs_unset(job, attrs)
+  end
+
+  test "can't create Job with Docker image locator" do
+    check_creation_prohibited(docker_image_locator: BAD_COLLECTION)
+  end
+
+  test "can't assign Docker image locator to Job" do
+    check_modification_prohibited(docker_image_locator: BAD_COLLECTION)
   end
 
   [
@@ -281,4 +292,57 @@ class JobTest < ActiveSupport::TestCase
     assert_not_equal job1.queue_position, job2.queue_position
   end
 
+  SDK_MASTER = "ca68b24e51992e790f29df5cc4bc54ce1da4a1c2"
+  SDK_TAGGED = "00634b2b8a492d6f121e3cf1d6587b821136a9a7"
+
+  def sdk_constraint(version)
+    {runtime_constraints: {"arvados_sdk_version" => version}}
+  end
+
+  def check_job_sdk_version(expected)
+    job = yield
+    if expected.nil?
+      refute(job.valid?, "job valid with bad Arvados SDK version")
+    else
+      assert(job.valid?, "job not valid with good Arvados SDK version")
+      assert_equal(expected, job.arvados_sdk_version)
+    end
+  end
+
+  { "master" => SDK_MASTER,
+    "commit2" => SDK_TAGGED,
+    SDK_TAGGED[0, 8] => SDK_TAGGED,
+    "__nonexistent__" => nil,
+  }.each_pair do |search, commit_hash|
+    test "creating job with SDK version '#{search}'" do
+      check_job_sdk_version(commit_hash) do
+        Job.new(job_attrs(sdk_constraint(search)))
+      end
+    end
+
+    test "updating job with SDK version '#{search}'" do
+      job = Job.create!(job_attrs)
+      assert_nil job.arvados_sdk_version
+      check_job_sdk_version(commit_hash) do
+        job.runtime_constraints = sdk_constraint(search)[:runtime_constraints]
+        job
+      end
+    end
+  end
+
+  test "clear the SDK version" do
+    job = Job.create!(job_attrs(sdk_constraint("master")))
+    assert_equal(SDK_MASTER, job.arvados_sdk_version)
+    job.runtime_constraints = {}
+    assert(job.valid?, "job invalid after clearing SDK version")
+    assert_nil(job.arvados_sdk_version)
+  end
+
+  test "can't create job with SDK version assigned directly" do
+    check_creation_prohibited(arvados_sdk_version: SDK_MASTER)
+  end
+
+  test "can't modify job to assign SDK version directly" do
+    check_modification_prohibited(arvados_sdk_version: SDK_MASTER)
+  end
 end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list