From: Tom Clegg Date: Tue, 25 Sep 2018 18:50:44 +0000 (-0400) Subject: 10181: Keep CR logs synchronized with container logs while running. X-Git-Tag: 1.3.0~97^2~7 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/a1e2cc595d148ea36f0f65c6750650a5eb034405 10181: Keep CR logs synchronized with container logs while running. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index e8a70499a9..3bb9cbf8e8 100644 --- 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 f430ae9185..bbec421084 100644 --- 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 0c5e2e7ad7..03e7850a5a 100644 --- 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