10181: Keep CR logs synchronized with container logs while running.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index e8a70499a929ed0192015044e6abd0fe6eb46bcd..3bb9cbf8e842ea4adaaa03bc78e653d11abe3aa0 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -39,6 +39,7 @@ class Container < ArvadosModel
   before_save :update_secret_mounts_md5
   before_save :scrub_secret_mounts
   before_save :clear_runtime_status_when_queued
+  after_save :update_cr_logs
   after_save :handle_completed
   after_save :propagate_priority
   after_commit { UpdatePriority.run_update_thread }
@@ -495,6 +496,19 @@ class Container < ArvadosModel
     end
   end
 
+  def update_cr_logs
+    # 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?
+    leave_modified_by_user_alone do
+      ContainerRequest.where(container_uuid: self.uuid).each do |cr|
+        cr.update_collections(container: self, collections: ['log'])
+        cr.save!
+      end
+    end
+  end
+
   def assign_auth
     if self.auth_uuid_changed?
       return errors.add :auth_uuid, 'is readonly'

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index f430ae918546ed53fe7eb1ef482fefea5d46a9c7..bbec4210846ca95e918611fd23189e03f7433893 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -123,11 +123,13 @@ class ContainerRequest < ArvadosModel
   # Finalize the container request after the container has
   # finished/cancelled.
   def finalize!
-    out_coll = nil
-    log_coll = nil
-    c = Container.find_by_uuid(container_uuid)
-    ['output', 'log'].each do |out_type|
-      pdh = c.send(out_type)
+    update_collections(container: Container.find_by_uuid(container_uuid))
+    update_attributes!(state: Final)
+  end
+
+  def update_collections(container:, collections: ['log', 'output'])
+    collections.each do |out_type|
+      pdh = container.send(out_type)
       next if pdh.nil?
       coll_name = "Container #{out_type} for request #{uuid}"
       trash_at = nil
@@ -141,24 +143,25 @@ class ContainerRequest < ArvadosModel
       end
       manifest = Collection.where(portable_data_hash: pdh).first.manifest_text
 
-      coll = Collection.new(owner_uuid: owner_uuid,
-                            manifest_text: manifest,
-                            portable_data_hash: pdh,
-                            name: coll_name,
-                            trash_at: trash_at,
-                            delete_at: trash_at,
-                            properties: {
-                              'type' => out_type,
-                              'container_request' => uuid,
-                            })
-      coll.save_with_unique_name!
-      if out_type == 'output'
-        out_coll = coll.uuid
-      else
-        log_coll = coll.uuid
+      coll_uuid = self.send(out_type + '_uuid')
+      coll = coll_uuid.nil? ? nil : Collection.where(uuid: coll_uuid).first
+      if !coll
+        coll = Collection.new(
+          owner_uuid: self.owner_uuid,
+          name: coll_name,
+          properties: {
+            'type' => out_type,
+            'container_request' => uuid,
+          })
       end
+      coll.assign_attributes(
+        portable_data_hash: pdh,
+        manifest_text: manifest,
+        trash_at: trash_at,
+        delete_at: trash_at)
+      coll.save_with_unique_name!
+      self.send(out_type + '_uuid=', coll.uuid)
     end
-    update_attributes!(state: Final, output_uuid: out_coll, log_uuid: log_coll)
   end
 
   def self.full_text_searchable_columns
@@ -312,6 +315,10 @@ class ContainerRequest < ArvadosModel
         permitted.push :container_count
       end
 
+      if current_user.andand.is_admin
+        permitted.push :log_uuid
+      end
+
     when Final
       if self.state_was == Committed
         # "Cancel" means setting priority=0, state=Committed

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 0c5e2e7ad7a8e25ad3c1127ed7d9c20599de1cfe..03e7850a5ab2f4cf31641633916495fac2e54f11 100644 (file)
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -653,22 +653,48 @@ class ContainerTest < ActiveSupport::TestCase
   end
 
   test "locked_by_uuid can update log when locked/running, and output when running" do
-    c, _ = minimal_new
+    logcoll = collections(:real_log_collection)
+    c, cr1 = minimal_new
+    cr2 = ContainerRequest.new(DEFAULT_ATTRS)
+    cr2.state = ContainerRequest::Committed
+    act_as_user users(:active) do
+      cr2.save!
+    end
+    assert_equal cr1.container_uuid, cr2.container_uuid
+
+    logpdh_time1 = logcoll.portable_data_hash
+
     set_user_from_auth :dispatch1
     c.lock
     assert_equal c.locked_by_uuid, Thread.current[:api_client_authorization].uuid
-    assert c.update_attributes(log: collections(:real_log_collection).portable_data_hash)
-    assert c.update_attributes(state: Container::Running)
+    c.update_attributes!(log: logpdh_time1)
+    c.update_attributes!(state: Container::Running)
+    cr1.reload
+    cr2.reload
+    cr1log_uuid = cr1.log_uuid
+    cr2log_uuid = cr2.log_uuid
+    assert_not_nil cr1log_uuid
+    assert_not_nil cr2log_uuid
+    assert_not_equal logcoll.uuid, cr1log_uuid
+    assert_not_equal logcoll.uuid, cr2log_uuid
+    assert_not_equal cr1log_uuid, cr2log_uuid
+
+    logcoll.update_attributes!(manifest_text: logcoll.manifest_text + ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt\n")
+    logpdh_time2 = logcoll.portable_data_hash
 
     assert c.update_attributes(output: collections(:collection_owned_by_active).portable_data_hash)
-    assert c.update_attributes(log: nil)
-    assert c.update_attributes(log: collections(:real_log_collection).portable_data_hash)
-    assert c.update_attributes(state: Container::Complete, log: collections(:real_log_collection).portable_data_hash)
+    assert c.update_attributes(log: logpdh_time2)
+    assert c.update_attributes(state: Container::Complete, log: logcoll.portable_data_hash)
     c.reload
-    assert_equal c.output, collections(:collection_owned_by_active).portable_data_hash
-    assert_equal c.log, collections(:real_log_collection).portable_data_hash
+    assert_equal collections(:collection_owned_by_active).portable_data_hash, c.output
+    assert_equal logpdh_time2, c.log
     refute c.update_attributes(output: nil)
     refute c.update_attributes(log: nil)
+    cr1.reload
+    cr2.reload
+    assert_equal cr1log_uuid, cr1.log_uuid
+    assert_equal cr2log_uuid, cr2.log_uuid
+    assert_equal [logpdh_time2], Collection.where(uuid: [cr1log_uuid, cr2log_uuid]).to_a.collect(&:portable_data_hash).uniq
   end
 
   test "auth_uuid can set output, progress on running container -- but not state, log" do