Merge branch '10808-admin-cancel-job'

refs #10808

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 9556799aba9f74375d0419fae8e8d153bc97ffa1..2ae713928113af9817c0598ee9f25841d00cf4e9 100644 (file)
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -423,7 +423,7 @@ class Job < ArvadosModel
           output_changed? or
           log_changed? or
           tasks_summary_changed? or
-          state_changed? or
+          (state_changed? && state != Cancelled) or
           components_changed?
         logger.warn "User #{current_user.uuid if current_user} tried to change protected job attributes on locked #{self.class.to_s} #{uuid_was}"
         return false

diff --git a/services/api/config/application.rb b/services/api/config/application.rb
index e7002ddf1acdf639a1b6b62823ae643f49dc38ec..d3fee8ea1945ae13b77bb8b6168ab43cc8674dd5 100644 (file)
--- a/services/api/config/application.rb
+++ b/services/api/config/application.rb
@@ -36,10 +36,12 @@ module Server
 
     I18n.enforce_available_locales = false
 
+    # Before using the filesystem backend for Rails.cache, check
+    # whether we own the relevant directory. If we don't, using it is
+    # likely to either fail or (if we're root) pollute it and cause
+    # other processes to fail later.
     default_cache_path = Rails.root.join('tmp', 'cache')
     if not File.owned?(default_cache_path)
-      # If we don't own the cache dir, using it will either fail or
-      # (if we're root) pollute it so other processes fail later.
       STDERR.puts("Defaulting to memory cache, because #{default_cache_path} " \
                   "owner (uid=#{File::Stat.new(default_cache_path).uid}) " \
                   "is not me (uid=#{Process.euid})")

diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 1f80ea50f2742167446917aea2fb0b0e6d8f2052..761953e8eb4e441f3e30cacf86a1f45ddf816e6c 100644 (file)
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -307,6 +307,24 @@ class JobTest < ActiveSupport::TestCase
     assert_equal "Failed", job.state
   end
 
+  test "admin user can cancel a running job despite lock" do
+    set_user_from_auth :active_trustedclient
+    job = Job.create! job_attrs
+    job.lock current_user.uuid
+    assert_equal Job::Running, job.state
+
+    set_user_from_auth :spectator
+    assert_raises do
+      job.update_attributes!(state: Job::Cancelled)
+    end
+
+    set_user_from_auth :admin
+    job.reload
+    assert_equal Job::Running, job.state
+    job.update_attributes!(state: Job::Cancelled)
+    assert_equal Job::Cancelled, job.state
+  end
+
   test "verify job queue position" do
     job1 = Job.create! job_attrs
     assert_equal 'Queued', job1.state, "Incorrect job state for newly created job1"