X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/18239026d35e377e01ad852be8e67e5328f069e5..d89876219e668a3a97a6c61f92320bad0c0527c8:/services/api/app/models/container_request.rb diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb index be4d3cc8e6..b30b8cc1d9 100644 --- a/services/api/app/models/container_request.rb +++ b/services/api/app/models/container_request.rb @@ -19,13 +19,16 @@ class ContainerRequest < ArvadosModel primary_key: :uuid, } - serialize :properties, Hash + # Posgresql JSONB columns should NOT be declared as serialized, Rails 5 + # already know how to properly treat them. + attribute :properties, :jsonbHash, default: {} + attribute :secret_mounts, :jsonbHash, default: {} + serialize :environment, Hash serialize :mounts, Hash serialize :runtime_constraints, Hash serialize :command, Array serialize :scheduling_parameters, Hash - serialize :secret_mounts, Hash before_validation :fill_field_defaults, :if => :new_record? before_validation :validate_runtime_constraints @@ -116,20 +119,67 @@ class ContainerRequest < ArvadosModel end def finalize_if_needed - if state == Committed && Container.find_by_uuid(container_uuid).final? - reload - act_as_system_user do - leave_modified_by_user_alone do - finalize! + return if state != Committed + while true + # get container lock first, then lock current container request + # (same order as Container#handle_completed). Locking always + # reloads the Container and ContainerRequest records. + c = Container.find_by_uuid(container_uuid) + c.lock! if !c.nil? + self.lock! + + if !c.nil? && container_uuid != c.uuid + # After locking, we've noticed a race, the container_uuid is + # different than the container record we just loaded. This + # can happen if Container#handle_completed scheduled a new + # container for retry and set container_uuid while we were + # waiting on the container lock. Restart the loop and get the + # new container. + redo + end + + if !c.nil? + if state == Committed && c.final? + # The current container is + act_as_system_user do + leave_modified_by_user_alone do + finalize! + end + end end + elsif state == Committed + # Behave as if the container is cancelled + update_attributes!(state: Final) end + return true end end # Finalize the container request after the container has # finished/cancelled. def finalize! - update_collections(container: Container.find_by_uuid(container_uuid)) + container = Container.find_by_uuid(container_uuid) + if !container.nil? + update_collections(container: container) + + if container.state == Container::Complete + log_col = Collection.where(portable_data_hash: container.log).first + if log_col + # Need to save collection + completed_coll = Collection.new( + owner_uuid: self.owner_uuid, + name: "Container log for container #{container_uuid}", + properties: { + 'type' => 'log', + 'container_request' => self.uuid, + 'container_uuid' => container_uuid, + }, + portable_data_hash: log_col.portable_data_hash, + manifest_text: log_col.manifest_text) + completed_coll.save_with_unique_name! + end + end + end update_attributes!(state: Final) end @@ -137,6 +187,10 @@ class ContainerRequest < ArvadosModel collections.each do |out_type| pdh = container.send(out_type) next if pdh.nil? + c = Collection.where(portable_data_hash: pdh).first + next if c.nil? + manifest = c.manifest_text + coll_name = "Container #{out_type} for request #{uuid}" trash_at = nil if out_type == 'output' @@ -147,7 +201,6 @@ class ContainerRequest < ArvadosModel trash_at = db_current_time + self.output_ttl end end - manifest = Collection.where(portable_data_hash: pdh).first.manifest_text coll_uuid = self.send(out_type + '_uuid') coll = coll_uuid.nil? ? nil : Collection.where(uuid: coll_uuid).first @@ -163,10 +216,11 @@ class ContainerRequest < ArvadosModel end if out_type == "log" + # Copy the log into a merged collection src = Arv::Collection.new(manifest) dst = Arv::Collection.new(coll.manifest_text) dst.cp_r("./", ".", src) - dst.cp_r("./", "container #{container.uuid}", src) + dst.cp_r("./", "log for container #{container.uuid}", src) manifest = dst.manifest_text end @@ -191,8 +245,9 @@ class ContainerRequest < ArvadosModel self.environment ||= {} self.runtime_constraints ||= {} self.mounts ||= {} + self.secret_mounts ||= {} self.cwd ||= "." - self.container_count_max ||= Rails.configuration.container_count_max + self.container_count_max ||= Rails.configuration.Containers.MaxRetryAttempts self.scheduling_parameters ||= {} self.output_ttl ||= 0 self.priority ||= 0 @@ -206,40 +261,53 @@ class ContainerRequest < ArvadosModel return false end if state_changed? and state == Committed and container_uuid.nil? - self.container_uuid = Container.resolve(self).uuid + while true + c = Container.resolve(self) + c.lock! + if c.state == Container::Cancelled + # Lost a race, we have a lock on the container but the + # container was cancelled in a different request, restart + # the loop and resolve request to a new container. + redo + end + self.container_uuid = c.uuid + break + end end if self.container_uuid != self.container_uuid_was if self.container_count_changed? errors.add :container_count, "cannot be updated directly." return false - else - self.container_count += 1 - if self.container_uuid_was - old_container = Container.find_by_uuid(self.container_uuid_was) - old_logs = Collection.where(portable_data_hash: old_container.log).first - if old_logs - log_coll = coll_uuid.nil? ? nil : Collection.where(uuid: coll_uuid).first - if self.log_uuid.nil? - log_coll = Collection.new( - owner_uuid: self.owner_uuid, - name: coll_name = "Container log for request #{uuid}", - manifest_text: "") - end + end - # copy logs from old container into CR's log collection - src = Arv::Collection.new(old_logs.manifest_text) - dst = Arv::Collection.new(log_coll.manifest_text) - dst.cp_r("./", "container #{container.uuid}", src) - manifest = dst.manifest_text - - log_coll.assign_attributes( - portable_data_hash: Digest::MD5.hexdigest(manifest) + '+' + manifest.bytesize.to_s, - manifest_text: manifest) - log_coll.save_with_unique_name! - self.log_uuid = log_coll.uuid - end - end + self.container_count += 1 + return if self.container_uuid_was.nil? + + old_container = Container.find_by_uuid(self.container_uuid_was) + return if old_container.nil? + + old_logs = Collection.where(portable_data_hash: old_container.log).first + return if old_logs.nil? + + log_coll = self.log_uuid.nil? ? nil : Collection.where(uuid: self.log_uuid).first + if self.log_uuid.nil? + log_coll = Collection.new( + owner_uuid: self.owner_uuid, + name: coll_name = "Container log for request #{uuid}", + manifest_text: "") end + + # copy logs from old container into CR's log collection + src = Arv::Collection.new(old_logs.manifest_text) + dst = Arv::Collection.new(log_coll.manifest_text) + dst.cp_r("./", "log for container #{old_container.uuid}", src) + manifest = dst.manifest_text + + log_coll.assign_attributes( + portable_data_hash: Digest::MD5.hexdigest(manifest) + '+' + manifest.bytesize.to_s, + manifest_text: manifest) + log_coll.save_with_unique_name! + self.log_uuid = log_coll.uuid end end @@ -248,7 +316,7 @@ class ContainerRequest < ArvadosModel if self.state == Committed # If preemptible instances (eg: AWS Spot Instances) are allowed, # ask them on child containers by default. - if Rails.configuration.preemptible_instances and !c.nil? and + if Rails.configuration.Containers.UsePreemptibleInstances and !c.nil? and self.scheduling_parameters['preemptible'].nil? self.scheduling_parameters['preemptible'] = true end @@ -318,7 +386,7 @@ class ContainerRequest < ArvadosModel scheduling_parameters['partitions'].size) errors.add :scheduling_parameters, "partitions must be an array of strings" end - if !Rails.configuration.preemptible_instances and scheduling_parameters['preemptible'] + if !Rails.configuration.Containers.UsePreemptibleInstances and scheduling_parameters['preemptible'] errors.add :scheduling_parameters, "preemptible instances are not allowed" end if scheduling_parameters.include? 'max_run_time' and