X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/f672f727fe79bf6642a2daab641a1ef5c84648df..e6f2c5ddcd8b8faba1f27cd9890fe8f24e6b72c8:/services/api/app/models/container.rb

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 376be55ffb..16f8cd798f 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -138,7 +138,7 @@ class Container < ArvadosModel
   end
 
   def propagate_priority
-    return true unless priority_changed?
+    return true unless saved_change_to_priority?
     act_as_system_user do
       # Update the priority of child container requests to match new
       # priority of the parent container (ignoring requests with no
@@ -339,7 +339,7 @@ class Container < ArvadosModel
   end
 
   def lock
-    self.with_lock do
+    self.reload.with_lock do
       if self.state != Queued
         raise LockFailedError.new("cannot lock when #{self.state}")
       end
@@ -357,7 +357,7 @@ class Container < ArvadosModel
   end
 
   def unlock
-    self.with_lock do
+    self.reload.with_lock do
       if self.state != Locked
         raise InvalidStateTransitionError.new("cannot unlock when #{self.state}")
       end
@@ -556,7 +556,7 @@ class Container < ArvadosModel
     # If self.final?, this update is superfluous: the final log/output
     # update will be done when handle_completed calls finalize! on
     # each requesting CR.
-    return if self.final? || !self.log_changed?
+    return if self.final? || !saved_change_to_log?
     leave_modified_by_user_alone do
       ContainerRequest.where(container_uuid: self.uuid).each do |cr|
         cr.update_collections(container: self, collections: ['log'])
@@ -570,8 +570,13 @@ class Container < ArvadosModel
          return errors.add :auth_uuid, 'is readonly'
     end
     if not [Locked, Running].include? self.state
-      # don't need one
-      self.auth.andand.update_attributes(expires_at: db_current_time)
+      # Don't need one. If auth already exists, expire it.
+      #
+      # We use db_transaction_time here (not db_current_time) to
+      # ensure the token doesn't validate later in the same
+      # transaction (e.g., in a test case) by satisfying expires_at >
+      # transaction timestamp.
+      self.auth.andand.update_attributes(expires_at: db_transaction_time)
       self.auth = nil
       return
     elsif self.auth
@@ -648,11 +653,11 @@ class Container < ArvadosModel
   def handle_completed
     # This container is finished so finalize any associated container requests
     # that are associated with this container.
-    if self.state_changed? and self.final?
-      # These get wiped out by with_lock (which reloads the record),
+    if saved_change_to_state? and self.final?
+      # These get wiped out (with_lock requires to explicitly reload the record),
       # so record them now in case we need to schedule a retry.
-      prev_secret_mounts = self.secret_mounts_was
-      prev_runtime_token = self.runtime_token_was
+      prev_secret_mounts = secret_mounts_before_last_save
+      prev_runtime_token = runtime_token_before_last_save
 
       # Need to take a lock on the container to ensure that any
       # concurrent container requests that might try to reuse this
@@ -660,7 +665,7 @@ class Container < ArvadosModel
       # transaction finishes.  This ensure that concurrent container
       # requests that try to reuse this container are finalized (on
       # Complete) or don't reuse it (on Cancelled).
-      self.with_lock do
+      self.reload.with_lock do
         act_as_system_user do
           if self.state == Cancelled
             retryable_requests = ContainerRequest.where("container_uuid = ? and priority > 0 and state = 'Committed' and container_count < container_count_max", uuid)
@@ -685,7 +690,7 @@ class Container < ArvadosModel
             }
             c = Container.create! c_attrs
             retryable_requests.each do |cr|
-              cr.with_lock do
+              cr.reload.with_lock do
                 leave_modified_by_user_alone do
                   # Use row locking because this increments container_count
                   cr.container_uuid = c.uuid