6429: Complete/Cancelled containers finalize associated container requests;

container requests update priority for associated containers.

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 15bbbe017d28e78cf959d39d83d5da07044f907e..79c98681897ef449665b3b296ca440c6dfcd6687 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -17,7 +17,6 @@ class Container < ArvadosModel
   validate :validate_state_change
   validate :validate_change
   after_save :request_finalize
-  after_save :process_tree_priority
 
   has_many :container_requests, :foreign_key => :container_uuid, :class_name => 'ContainerRequest', :primary_key => :uuid
 
@@ -151,22 +150,18 @@ class Container < ArvadosModel
     # that are associated with this container.
     if self.state_changed? and [Complete, Cancelled].include? self.state
       act_as_system_user do
-        # Note using update_all skips model validation and callbacks.
-        ContainerRequest.update_all({:state => ContainerRequest::Final}, ['container_uuid=?', uuid])
-      end
-    end
-  end
+        # Try to close container requests associated with this container
+        ContainerRequest.where(container_uuid: uuid,
+                               :state => ContainerRequest::Committed).each do |cr|
+          cr.state = ContainerRequest::Final
+          cr.save
+        end
 
-  def process_tree_priority
-    # This container is cancelled so cancel any container requests made by this
-    # container.
-    if self.priority_changed? and self.priority == 0
-      # This could propagate any parent priority to the children (not just
-      # priority 0)
-      act_as_system_user do
-        ContainerRequest.where(requesting_container_uuid: uuid).each do |cr|
-          cr.priority = self.priority
-          cr.save!
+        # Try to close any outstanding container requests made by this container.
+        ContainerRequest.where(requesting_container_uuid: uuid,
+                               :state => ContainerRequest::Committed).each do |cr|
+          cr.state = ContainerRequest::Final
+          cr.save
         end
       end
     end

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 1011f6fd755d6beb1354244a127b3aad994ac836..4e76a0e08aa00ecb974026caa279248d2fd68ed5 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -145,12 +145,16 @@ class ContainerRequest < ArvadosModel
   end
 
   def update_priority
-    if self.state == Committed and (self.state_changed? or
-                                    self.priority_changed? or
-                                    self.container_uuid_changed?)
-      c = Container.find_by_uuid self.container_uuid
-      act_as_system_user do
-        c.update_priority!
+    if [Committed, Final].include? self.state and (self.state_changed? or
+                                                   self.priority_changed? or
+                                                   self.container_uuid_changed?)
+      [self.container_uuid_was, self.container_uuid].each do |cuuid|
+        unless cuuid.nil?
+          c = Container.find_by_uuid cuuid
+          act_as_system_user do
+            c.update_priority!
+          end
+        end
       end
     end
   end

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 6d1a57699b5d0a10f5b89c8ded887a7a9207686f..6470b785e271a0ae475b67b3aebbb14a1a69ce2d 100644 (file)
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -199,7 +199,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
   end
 
-  test "Container cancel process tree" do
+  test "Container request finalize" do
     set_user_from_auth :active_trustedclient
     c = ContainerRequest.new
     c.state = "Committed"
@@ -210,31 +210,15 @@ class ContainerRequestTest < ActiveSupport::TestCase
     c.priority = 5
     c.save!
 
-    c2 = ContainerRequest.new
-    c2.state = "Committed"
-    c2.container_image = "img"
-    c2.command = ["foo", "bar"]
-    c2.output_path = "/tmp"
-    c2.cwd = "/tmp"
-    c2.priority = 10
-    c2.requesting_container_uuid = c.container_uuid
-    c2.save!
-
     t = Container.find_by_uuid c.container_uuid
     assert_equal 5, t.priority
 
-    t2 = Container.find_by_uuid c2.container_uuid
-    assert_equal 10, t2.priority
-
-    c.priority = 0
+    c.state = "Final"
     c.save!
 
     t.reload
     assert_equal 0, t.priority
 
-    t2.reload
-    assert_equal 0, t2.priority
-
   end