[ARVADOS] created: 46c5fde8d33d7816c0c7cb0409e35e88f4eb2889

[git at public.curoverse.com]

        at  46c5fde8d33d7816c0c7cb0409e35e88f4eb2889 (commit)


commit 46c5fde8d33d7816c0c7cb0409e35e88f4eb2889
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/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..2492fc0 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 "can't specify SDK version with Docker image" 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
--