X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/7696fe3db2dcf03fe8b4528080100bec2196da91..b5c15775caf865d8fed5d4839d4a082f6518bba4:/services/api/app/models/container_request.rb?ds=sidebyside

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index e54ebf9366..f5789f31f6 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -12,18 +12,22 @@ class ContainerRequest < ArvadosModel
   include CommonApiTemplate
   include WhitelistUpdate
 
-  belongs_to :container, foreign_key: :container_uuid, primary_key: :uuid
-  belongs_to :requesting_container, {
-               class_name: 'Container',
-               foreign_key: :requesting_container_uuid,
-               primary_key: :uuid,
-             }
+  belongs_to :container,
+             foreign_key: 'container_uuid',
+             primary_key: 'uuid',
+             optional: true
+  belongs_to :requesting_container,
+             class_name: 'Container',
+             foreign_key: 'requesting_container_uuid',
+             primary_key: 'uuid',
+             optional: true
 
   # 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: {}
   attribute :output_storage_classes, :jsonbArray, default: lambda { Rails.configuration.DefaultStorageClasses }
+  attribute :output_properties, :jsonbHash, default: {}
 
   serialize :environment, Hash
   serialize :mounts, Hash
@@ -32,10 +36,9 @@ class ContainerRequest < ArvadosModel
   serialize :scheduling_parameters, Hash
 
   after_find :fill_container_defaults_after_find
+  after_initialize { @state_was_when_initialized = self.state_was } # see finalize_if_needed
   before_validation :fill_field_defaults, :if => :new_record?
   before_validation :fill_container_defaults
-  before_validation :set_default_preemptible_scheduling_parameter
-  before_validation :set_container
   validates :command, :container_image, :output_path, :cwd, :presence => true
   validates :output_ttl, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
   validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 }
@@ -46,7 +49,9 @@ class ContainerRequest < ArvadosModel
   validate :check_update_whitelist
   validate :secret_mounts_key_conflict
   validate :validate_runtime_token
-  before_save :scrub_secrets
+  after_validation :scrub_secrets
+  after_validation :set_preemptible
+  after_validation :set_container
   before_create :set_requesting_container_uuid
   before_destroy :set_priority_zero
   after_save :update_priority
@@ -78,6 +83,8 @@ class ContainerRequest < ArvadosModel
     t.add :state
     t.add :use_existing
     t.add :output_storage_classes
+    t.add :output_properties
+    t.add :cumulative_cost
   end
 
   # Supported states for a container request
@@ -100,7 +107,13 @@ class ContainerRequest < ArvadosModel
   :output_path, :priority, :runtime_token,
   :runtime_constraints, :state, :container_uuid, :use_existing,
   :scheduling_parameters, :secret_mounts, :output_name, :output_ttl,
-  :output_storage_classes]
+  :output_storage_classes, :output_properties]
+
+  def self.any_preemptible_instances?
+    Rails.configuration.InstanceTypes.any? do |k, v|
+      v["Preemptible"]
+    end
+  end
 
   def self.limit_index_columns_read
     ["mounts"]
@@ -154,7 +167,7 @@ class ContainerRequest < ArvadosModel
         end
       elsif state == Committed
         # Behave as if the container is cancelled
-        update_attributes!(state: Final)
+        update!(state: Final)
       end
       return true
     end
@@ -165,8 +178,39 @@ class ContainerRequest < ArvadosModel
   def finalize!
     container = Container.find_by_uuid(container_uuid)
     if !container.nil?
-      update_collections(container: container)
+      # We don't want to add the container cost if the container was
+      # already finished when this CR was committed. But we are
+      # running in an after_save hook after a lock/reload, so
+      # state_was has already been updated to Committed regardless.
+      # Hence the need for @state_was_when_initialized.
+      if @state_was_when_initialized == Committed
+        # Add the final container cost to our cumulative cost (which
+        # may already be non-zero from previous attempts if
+        # container_count_max > 1).
+        self.cumulative_cost += container.cost + container.subrequests_cost
+      end
 
+      # Add our cumulative cost to the subrequests_cost of the
+      # requesting container, if any.
+      if self.requesting_container_uuid
+        Container.where(
+          uuid: self.requesting_container_uuid,
+          state: Container::Running,
+        ).each do |c|
+          c.subrequests_cost += self.cumulative_cost
+          c.save!
+        end
+      end
+
+      update_collections(container: container)
+      # update_collections makes a log collection that includes all of the logs
+      # for all of the containers associated with this request. For requests
+      # that are retried, this is the primary way users can get logs for
+      # failed containers.
+      # The code below makes a log collection that is a verbatim copy of the
+      # container's logs. This is required for container reuse: a container
+      # will not be reused if the owner cannot read a collection with its logs.
+      # See the "readable log" section of Container.find_reusable().
       if container.state == Container::Complete
         log_col = Collection.where(portable_data_hash: container.log).first
         if log_col
@@ -187,10 +231,17 @@ class ContainerRequest < ArvadosModel
         end
       end
     end
-    update_attributes!(state: Final)
+    update!(state: Final)
   end
 
   def update_collections(container:, collections: ['log', 'output'])
+
+    # Check if parent is frozen or trashed, in which case it isn't
+    # valid to create new collections in the project, so return
+    # without creating anything.
+    owner = Group.find_by_uuid(self.owner_uuid)
+    return if owner && !owner.admin_change_permitted
+
     collections.each do |out_type|
       pdh = container.send(out_type)
       next if pdh.nil?
@@ -216,11 +267,7 @@ class ContainerRequest < ArvadosModel
           owner_uuid: self.owner_uuid,
           name: coll_name,
           manifest_text: "",
-          storage_classes_desired: self.output_storage_classes,
-          properties: {
-            'type' => out_type,
-            'container_request' => uuid,
-          })
+          storage_classes_desired: self.output_storage_classes)
       end
 
       if out_type == "log"
@@ -232,11 +279,28 @@ class ContainerRequest < ArvadosModel
         manifest = dst.manifest_text
       end
 
+      merged_properties = {}
+      merged_properties['container_request'] = uuid
+
+      if out_type == 'output' and !requesting_container_uuid.nil?
+        # output of a child process, give it "intermediate" type by
+        # default.
+        merged_properties['type'] = 'intermediate'
+      else
+        merged_properties['type'] = out_type
+      end
+
+      if out_type == "output"
+        merged_properties.update(container.output_properties)
+        merged_properties.update(self.output_properties)
+      end
+
       coll.assign_attributes(
         portable_data_hash: Digest::MD5.hexdigest(manifest) + '+' + manifest.bytesize.to_s,
         manifest_text: manifest,
         trash_at: trash_at,
-        delete_at: trash_at)
+        delete_at: trash_at,
+        properties: merged_properties)
       coll.save_with_unique_name!
       self.send(out_type + '_uuid=', coll.uuid)
     end
@@ -246,6 +310,10 @@ class ContainerRequest < ArvadosModel
     super - ["mounts", "secret_mounts", "secret_mounts_md5", "runtime_token", "output_storage_classes"]
   end
 
+  def set_priority_zero
+    self.update!(priority: 0) if self.priority > 0 && self.state != Final
+  end
+
   protected
 
   def fill_field_defaults
@@ -268,7 +336,15 @@ class ContainerRequest < ArvadosModel
       errors.add :container_uuid, "can only be updated to nil."
       return false
     end
+    if self.container_count_changed?
+      errors.add :container_count, "cannot be updated directly."
+      return false
+    end
     if state_changed? and state == Committed and container_uuid.nil?
+      if self.command.length > 0 and self.command[0] == "arvados-cwl-runner"
+        # Special case, arvados-cwl-runner processes are always considered "supervisors"
+        self.scheduling_parameters['supervisor'] = true
+      end
       while true
         c = Container.resolve(self)
         c.lock!
@@ -283,18 +359,14 @@ class ContainerRequest < ArvadosModel
       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
-      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_container_uuid = self.container_uuid_was
+      old_container_log = Container.where(uuid: old_container_uuid).pluck(:log).first
+      return if old_container_log.nil?
 
-      old_logs = Collection.where(portable_data_hash: old_container.log).first
+      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
@@ -309,7 +381,7 @@ class ContainerRequest < ArvadosModel
       # 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)
+      dst.cp_r("./", "log for container #{old_container_uuid}", src)
       manifest = dst.manifest_text
 
       log_coll.assign_attributes(
@@ -320,8 +392,12 @@ class ContainerRequest < ArvadosModel
     end
   end
 
-  def set_default_preemptible_scheduling_parameter
-    if Rails.configuration.Containers.UsePreemptibleInstances && state == Committed && get_requesting_container()
+  def set_preemptible
+    if (new_record? || state_changed?) &&
+       state == Committed &&
+       Rails.configuration.Containers.AlwaysUsePreemptibleInstances &&
+       get_requesting_container_uuid() &&
+       self.class.any_preemptible_instances?
       self.scheduling_parameters['preemptible'] = true
     end
   end
@@ -394,14 +470,17 @@ class ContainerRequest < ArvadosModel
 
   def validate_scheduling_parameters
     if self.state == Committed
-      if scheduling_parameters.include? 'partitions' and
-         (!scheduling_parameters['partitions'].is_a?(Array) ||
+      if scheduling_parameters.include?('partitions') and
+        !scheduling_parameters['partitions'].nil? and
+        (!scheduling_parameters['partitions'].is_a?(Array) ||
           scheduling_parameters['partitions'].reject{|x| !x.is_a?(String)}.size !=
             scheduling_parameters['partitions'].size)
             errors.add :scheduling_parameters, "partitions must be an array of strings"
       end
-      if !Rails.configuration.Containers.UsePreemptibleInstances and scheduling_parameters['preemptible']
-        errors.add :scheduling_parameters, "preemptible instances are not allowed"
+      if scheduling_parameters['preemptible'] &&
+         (new_record? || state_changed?) &&
+         !self.class.any_preemptible_instances?
+        errors.add :scheduling_parameters, "preemptible instances are not configured in InstanceTypes"
       end
       if scheduling_parameters.include? 'max_run_time' and
         (!scheduling_parameters['max_run_time'].is_a?(Integer) ||
@@ -435,22 +514,15 @@ class ContainerRequest < ArvadosModel
 
     case self.state
     when Committed
-      permitted.push :priority, :container_count_max, :container_uuid
-
-      if self.container_uuid.nil?
-        self.errors.add :container_uuid, "has not been resolved to a container."
-      end
+      permitted.push :priority, :container_count_max, :container_uuid, :cumulative_cost
 
       if self.priority.nil?
         self.errors.add :priority, "cannot be nil"
       end
 
-      # Allow container count to increment by 1
-      if (self.container_uuid &&
-          self.container_uuid != self.container_uuid_was &&
-          self.container_count == 1 + (self.container_count_was || 0))
-        permitted.push :container_count
-      end
+      # Allow container count to increment (not by client, only by us
+      # -- see set_container)
+      permitted.push :container_count
 
       if current_user.andand.is_admin
         permitted.push :log_uuid
@@ -459,7 +531,7 @@ class ContainerRequest < ArvadosModel
     when Final
       if self.state_was == Committed
         # "Cancel" means setting priority=0, state=Committed
-        permitted.push :priority
+        permitted.push :priority, :cumulative_cost
 
         if current_user.andand.is_admin
           permitted.push :output_uuid, :log_uuid
@@ -501,29 +573,19 @@ class ContainerRequest < ArvadosModel
 
   def update_priority
     return unless saved_change_to_state? || saved_change_to_priority? || saved_change_to_container_uuid?
-    act_as_system_user do
-      Container.
-        where('uuid in (?)', [container_uuid_before_last_save, self.container_uuid].compact).
-        map(&:update_priority!)
-    end
-  end
-
-  def set_priority_zero
-    self.update_attributes!(priority: 0) if self.state != Final
+    update_priorities container_uuid_before_last_save if !container_uuid_before_last_save.nil? and container_uuid_before_last_save != self.container_uuid
+    update_priorities self.container_uuid if self.container_uuid
   end
 
   def set_requesting_container_uuid
-    c = get_requesting_container()
-    if !c.nil?
-      self.requesting_container_uuid = c.uuid
+    if (self.requesting_container_uuid = get_requesting_container_uuid())
       # Determine the priority of container request for the requesting
       # container.
       self.priority = ContainerRequest.where(container_uuid: self.requesting_container_uuid).maximum("priority") || 0
     end
   end
 
-  def get_requesting_container
-    return self.requesting_container_uuid if !self.requesting_container_uuid.nil?
-    Container.for_current_token
+  def get_requesting_container_uuid
+    return self.requesting_container_uuid || Container.for_current_token.andand.uuid
   end
 end