[ARVADOS] updated: 1a2e18ce382675b73fd6f8d54eea29bf47f26bb3

git at public.curoverse.com git at public.curoverse.com
Thu Nov 20 11:26:09 EST 2014


Summary of changes:
 doc/api/schema/Job.html.textile.liquid             |  2 +-
 services/api/app/models/job.rb                     |  8 ++++---
 .../arvados/v1/job_reuse_controller_test.rb        |  5 ++++-
 services/api/test/unit/job_test.rb                 | 25 ++++++++++++++++++----
 4 files changed, 31 insertions(+), 9 deletions(-)

       via  1a2e18ce382675b73fd6f8d54eea29bf47f26bb3 (commit)
       via  d6db40a49670b9d5612a6012c0e33639c1fabd4c (commit)
      from  48ccafc46692ae60300039937a15c0ec6f106697 (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 1a2e18ce382675b73fd6f8d54eea29bf47f26bb3
Merge: 48ccafc d6db40a
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Nov 20 11:25:21 2014 -0500

    Merge branch '4027-api-sdk-requires-docker-wip'
    
    Refs #4027.  Closes #4626, #4629.


commit d6db40a49670b9d5612a6012c0e33639c1fabd4c
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Nov 20 09:52:53 2014 -0500

    4027: API Jobs that specify SDK version must also use Docker.
    
    Because compute nodes will be upgraded regularly, we don't want to
    guarantee that we'll have the necessary tools to run older SDK
    versions.  The primary motivation for this feature is to install SDKs
    into Docker images, so we require that to be specified.
    
    This commit removes a few `job.valid?` assertions from the job unit
    tests.  These are redundant when they come after `Job.create!` because
    that will raise an exception if the job isn't valid.

diff --git a/doc/api/schema/Job.html.textile.liquid b/doc/api/schema/Job.html.textile.liquid
index e2363ac..e583041 100644
--- a/doc/api/schema/Job.html.textile.liquid
+++ b/doc/api/schema/Job.html.textile.liquid
@@ -53,7 +53,7 @@ h3. Runtime constraints
 
 table(table table-bordered table-condensed).
 |_. Key|_. Type|_. Description|_. Implemented|
-|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.||
+|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.  If you use this, you must also specify a @docker_image@ constraint (see below).||
 |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|||
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 6e01de9..4a464f5 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -171,10 +171,12 @@ class Job < ArvadosModel
                                :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
+      if commits.nil? or commits.empty?
         [false, "#{git_search} does not resolve to a commit"]
+      elsif not runtime_constraints["docker_image"]
+        [false, "cannot be specified without a Docker image constraint"]
+      else
+        [true, commits.first]
       end
     end
   end
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index a3eebf3..f9ecbf5 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -671,7 +671,10 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
   test "can't reuse job with older Arvados SDK version" do
     params = {
       script_version: "31ce37fe365b3dc204300a3e4c396ad333ed0556",
-      runtime_constraints: {"arvados_sdk_version" => "master"},
+      runtime_constraints: {
+        "arvados_sdk_version" => "master",
+        "docker_image" => links(:docker_image_collection_tag).name,
+      },
     }
     check_new_job_created_from(job: params)
   end
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 65ca935..e885b69 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -189,7 +189,6 @@ class JobTest < ActiveSupport::TestCase
   ].each do |parameters|
     test "verify job status #{parameters}" do
       job = Job.create! job_attrs
-      assert job.valid?, job.errors.full_messages.to_s
       assert_equal 'Queued', job.state, "job.state"
 
       parameters.each do |parameter|
@@ -280,11 +279,9 @@ class JobTest < ActiveSupport::TestCase
 
   test "verify job queue position" do
     job1 = Job.create! job_attrs
-    assert job1.valid?, job1.errors.full_messages.to_s
     assert_equal 'Queued', job1.state, "Incorrect job state for newly created job1"
 
     job2 = Job.create! job_attrs
-    assert job2.valid?, job2.errors.full_messages.to_s
     assert_equal 'Queued', job2.state, "Incorrect job state for newly created job2"
 
     assert_not_nil job1.queue_position, "Expected non-nil queue position for job1"
@@ -296,7 +293,10 @@ class JobTest < ActiveSupport::TestCase
   SDK_TAGGED = "00634b2b8a492d6f121e3cf1d6587b821136a9a7"
 
   def sdk_constraint(version)
-    {runtime_constraints: {"arvados_sdk_version" => version}}
+    {runtime_constraints: {
+        "arvados_sdk_version" => version,
+        "docker_image" => links(:docker_image_collection_tag).name,
+      }}
   end
 
   def check_job_sdk_version(expected)
@@ -347,6 +347,23 @@ class JobTest < ActiveSupport::TestCase
     assert_nil(job.arvados_sdk_version)
   end
 
+  test "job with SDK constraint, without Docker image is invalid" do
+    sdk_attrs = sdk_constraint("master")
+    sdk_attrs[:runtime_constraints].delete("docker_image")
+    job = Job.create(job_attrs(sdk_attrs))
+    refute(job.valid?, "Job valid with SDK version, without Docker image")
+    sdk_errors = job.errors.messages[:arvados_sdk_version] || []
+    refute_empty(sdk_errors.grep(/\bDocker\b/),
+                 "no Job SDK errors mention that Docker is required")
+  end
+
+  test "invalid to clear Docker image constraint when SDK constraint exists" do
+    job = Job.create!(job_attrs(sdk_constraint("master")))
+    job.runtime_constraints.delete("docker_image")
+    refute(job.valid?,
+           "Job with SDK constraint valid after clearing Docker image")
+  end
+
   test "can't create job with SDK version assigned directly" do
     check_creation_prohibited(arvados_sdk_version: SDK_MASTER)
   end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list