X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/72873affa7f249faa16d5d21200e935d27aea911..43ad590772de48fbc3a6a45654445bab79a0bdc1:/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 b8cadb271b..0c2ad09655 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -1,28 +1,49 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
 require 'whitelist_update'
 
 class ContainerRequest < ArvadosModel
+  include ArvadosModelUpdates
   include HasUuid
   include KindAndEtag
   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,
+             }
+
   serialize :properties, Hash
   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
-  before_validation :validate_scheduling_parameters
+  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 }
+  validate :validate_datatypes
+  validate :validate_scheduling_parameters
   validate :validate_state_change
-  validate :validate_change
+  validate :check_update_whitelist
+  validate :secret_mounts_key_conflict
+  validate :validate_runtime_token
+  before_save :scrub_secrets
+  before_create :set_requesting_container_uuid
+  before_destroy :set_priority_zero
   after_save :update_priority
   after_save :finalize_if_needed
-  before_create :set_requesting_container_uuid
 
   api_accessible :user, extend: :common do |t|
     t.add :command
@@ -41,6 +62,7 @@ class ContainerRequest < ArvadosModel
     t.add :output_name
     t.add :output_path
     t.add :output_uuid
+    t.add :output_ttl
     t.add :priority
     t.add :properties
     t.add :requesting_container_uuid
@@ -64,20 +86,41 @@ class ContainerRequest < ArvadosModel
     Committed => [Final]
   }
 
+  AttrsPermittedAlways = [:owner_uuid, :state, :name, :description, :properties]
+  AttrsPermittedBeforeCommit = [:command, :container_count_max,
+  :container_image, :cwd, :environment, :filters, :mounts,
+  :output_path, :priority, :runtime_token,
+  :runtime_constraints, :state, :container_uuid, :use_existing,
+  :scheduling_parameters, :secret_mounts, :output_name, :output_ttl]
+
+  def self.limit_index_columns_read
+    ["mounts"]
+  end
+
+  def logged_attributes
+    super.except('secret_mounts', 'runtime_token')
+  end
+
   def state_transitions
     State_transitions
   end
 
   def skip_uuid_read_permission_check
-    # XXX temporary until permissions are sorted out.
-    %w(modified_by_client_uuid container_uuid requesting_container_uuid)
+    # The uuid_read_permission_check prevents users from making
+    # references to objects they can't view.  However, in this case we
+    # don't want to do that check since there's a circular dependency
+    # where user can't view the container until the user has
+    # constructed the container request that references the container.
+    %w(container_uuid)
   end
 
   def finalize_if_needed
     if state == Committed && Container.find_by_uuid(container_uuid).final?
       reload
       act_as_system_user do
-        finalize!
+        leave_modified_by_user_alone do
+          finalize!
+        end
       end
     end
   end
@@ -85,52 +128,49 @@ 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?
-      if self.output_name and out_type == 'output'
-        coll_name = self.output_name
-      else
-        coll_name = "Container #{out_type} for request #{uuid}"
-      end
-      manifest = Collection.unscoped.where(portable_data_hash: pdh).first.manifest_text
-      begin
-        coll = Collection.create!(owner_uuid: owner_uuid,
-                                  manifest_text: manifest,
-                                  portable_data_hash: pdh,
-                                  name: coll_name,
-                                  properties: {
-                                    'type' => out_type,
-                                    'container_request' => uuid,
-                                  })
-      rescue ActiveRecord::RecordNotUnique => rn
-        # In case this is executed as part of a transaction: When a Postgres exception happens,
-        # the following statements on the same transaction become invalid, so a rollback is
-        # needed. One example are Unit Tests, every test is enclosed inside a transaction so
-        # that the database can be reverted before every new test starts.
-        # See: http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Exception+handling+and+rolling+back
-        ActiveRecord::Base.connection.execute 'ROLLBACK'
-        raise unless out_type == 'output' and self.output_name
-        # Postgres specific unique name check. See ApplicationController#create for
-        # a detailed explanation.
-        raise unless rn.original_exception.is_a? PG::UniqueViolation
-        err = rn.original_exception
-        detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL)
-        raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail
-        # Output collection name collision detected: append a timestamp.
-        coll_name = "#{self.output_name} #{Time.now.getgm.strftime('%FT%TZ')}"
-        retry
-      end
+      coll_name = "Container #{out_type} for request #{uuid}"
+      trash_at = nil
       if out_type == 'output'
-        out_coll = coll.uuid
-      else
-        log_coll = coll.uuid
+        if self.output_name
+          coll_name = self.output_name
+        end
+        if self.output_ttl > 0
+          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
+      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
+    super - ["mounts", "secret_mounts", "secret_mounts_md5", "runtime_token"]
   end
 
   protected
@@ -143,93 +183,8 @@ class ContainerRequest < ArvadosModel
     self.cwd ||= "."
     self.container_count_max ||= Rails.configuration.container_count_max
     self.scheduling_parameters ||= {}
-  end
-
-  # Create a new container (or find an existing one) to satisfy this
-  # request.
-  def resolve
-    c_mounts = mounts_for_container
-    c_runtime_constraints = runtime_constraints_for_container
-    c_container_image = container_image_for_container
-    c = act_as_system_user do
-      c_attrs = {command: self.command,
-                 cwd: self.cwd,
-                 environment: self.environment,
-                 output_path: self.output_path,
-                 container_image: c_container_image,
-                 mounts: c_mounts,
-                 runtime_constraints: c_runtime_constraints}
-
-      reusable = self.use_existing ? Container.find_reusable(c_attrs) : nil
-      if not reusable.nil?
-        reusable
-      else
-        c_attrs[:scheduling_parameters] = self.scheduling_parameters
-        Container.create!(c_attrs)
-      end
-    end
-    self.container_uuid = c.uuid
-  end
-
-  # Return a runtime_constraints hash that complies with
-  # self.runtime_constraints but is suitable for saving in a container
-  # record, i.e., has specific values instead of ranges.
-  #
-  # Doing this as a step separate from other resolutions, like "git
-  # revision range to commit hash", makes sense only when there is no
-  # opportunity to reuse an existing container (e.g., container reuse
-  # is not implemented yet, or we have already found that no existing
-  # containers are suitable).
-  def runtime_constraints_for_container
-    rc = {}
-    runtime_constraints.each do |k, v|
-      if v.is_a? Array
-        rc[k] = v[0]
-      else
-        rc[k] = v
-      end
-    end
-    rc
-  end
-
-  # Return a mounts hash suitable for a Container, i.e., with every
-  # readonly collection UUID resolved to a PDH.
-  def mounts_for_container
-    c_mounts = {}
-    mounts.each do |k, mount|
-      mount = mount.dup
-      c_mounts[k] = mount
-      if mount['kind'] != 'collection'
-        next
-      end
-      if (uuid = mount.delete 'uuid')
-        c = Collection.
-          readable_by(current_user).
-          where(uuid: uuid).
-          select(:portable_data_hash).
-          first
-        if !c
-          raise ArvadosModel::UnresolvableContainerError.new "cannot mount collection #{uuid.inspect}: not found"
-        end
-        if mount['portable_data_hash'].nil?
-          # PDH not supplied by client
-          mount['portable_data_hash'] = c.portable_data_hash
-        elsif mount['portable_data_hash'] != c.portable_data_hash
-          # UUID and PDH supplied by client, but they don't agree
-          raise ArgumentError.new "cannot mount collection #{uuid.inspect}: current portable_data_hash #{c.portable_data_hash.inspect} does not match #{c['portable_data_hash'].inspect} in request"
-        end
-      end
-    end
-    return c_mounts
-  end
-
-  # Return a container_image PDH suitable for a Container.
-  def container_image_for_container
-    coll = Collection.for_latest_docker_image(container_image)
-    if !coll
-      raise ArvadosModel::UnresolvableContainerError.new "docker image #{container_image.inspect} not found"
-    end
-    return Collection.docker_migration_pdh([current_user], coll.portable_data_hash)
+    self.output_ttl ||= 0
+    self.priority ||= 0
   end
 
   def set_container
@@ -240,7 +195,7 @@ class ContainerRequest < ArvadosModel
       return false
     end
     if state_changed? and state == Committed and container_uuid.nil?
-      resolve
+      self.container_uuid = Container.resolve(self).uuid
     end
     if self.container_uuid != self.container_uuid_was
       if self.container_count_changed?
@@ -252,23 +207,69 @@ class ContainerRequest < ArvadosModel
     end
   end
 
+  def set_default_preemptible_scheduling_parameter
+    c = get_requesting_container()
+    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
+        self.scheduling_parameters['preemptible'].nil?
+          self.scheduling_parameters['preemptible'] = true
+      end
+    end
+  end
+
   def validate_runtime_constraints
     case self.state
     when Committed
-      ['vcpus', 'ram'].each do |k|
-        if not (runtime_constraints.include? k and
-                runtime_constraints[k].is_a? Integer and
-                runtime_constraints[k] > 0)
-          errors.add :runtime_constraints, "#{k} must be a positive integer"
+      [['vcpus', true],
+       ['ram', true],
+       ['keep_cache_ram', false]].each do |k, required|
+        if !required && !runtime_constraints.include?(k)
+          next
+        end
+        v = runtime_constraints[k]
+        unless (v.is_a?(Integer) && v > 0)
+          errors.add(:runtime_constraints,
+                     "[#{k}]=#{v.inspect} must be a positive integer")
         end
       end
+    end
+  end
 
-      if runtime_constraints.include? 'keep_cache_ram' and
-         (!runtime_constraints['keep_cache_ram'].is_a?(Integer) or
-          runtime_constraints['keep_cache_ram'] <= 0)
-            errors.add :runtime_constraints, "keep_cache_ram must be a positive integer"
-      elsif !runtime_constraints.include? 'keep_cache_ram'
-        runtime_constraints['keep_cache_ram'] = Rails.configuration.container_default_keep_cache_ram
+  def validate_datatypes
+    command.each do |c|
+      if !c.is_a? String
+        errors.add(:command, "must be an array of strings but has entry #{c.class}")
+      end
+    end
+    environment.each do |k,v|
+      if !k.is_a?(String) || !v.is_a?(String)
+        errors.add(:environment, "must be an map of String to String but has entry #{k.class} to #{v.class}")
+      end
+    end
+    [:mounts, :secret_mounts].each do |m|
+      self[m].each do |k, v|
+        if !k.is_a?(String) || !v.is_a?(Hash)
+          errors.add(m, "must be an map of String to Hash but is has entry #{k.class} to #{v.class}")
+        end
+        if v["kind"].nil?
+          errors.add(m, "each item must have a 'kind' field")
+        end
+        [[String, ["kind", "portable_data_hash", "uuid", "device_type",
+                   "path", "commit", "repository_name", "git_url"]],
+         [Integer, ["capacity"]]].each do |t, fields|
+          fields.each do |f|
+            if !v[f].nil? && !v[f].is_a?(t)
+              errors.add(m, "#{k}: #{f} must be a #{t} but is #{v[f].class}")
+            end
+          end
+        end
+        ["writable", "exclude_from_output"].each do |f|
+          if !v[f].nil? && !v[f].is_a?(TrueClass) && !v[f].is_a?(FalseClass)
+            errors.add(m, "#{k}: #{f} must be a #{t} but is #{v[f].class}")
+          end
+        end
       end
     end
   end
@@ -281,81 +282,116 @@ 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']
+        errors.add :scheduling_parameters, "preemptible instances are not allowed"
+      end
+      if scheduling_parameters.include? 'max_run_time' and
+        (!scheduling_parameters['max_run_time'].is_a?(Integer) ||
+          scheduling_parameters['max_run_time'] < 0)
+          errors.add :scheduling_parameters, "max_run_time must be positive integer"
+      end
     end
   end
 
-  def validate_change
-    permitted = [:owner_uuid]
+  def check_update_whitelist
+    permitted = AttrsPermittedAlways.dup
 
-    case self.state
-    when Uncommitted
-      # Permit updating most fields
-      permitted.push :command, :container_count_max,
-                     :container_image, :cwd, :description, :environment,
-                     :filters, :mounts, :name, :output_path, :priority,
-                     :properties, :requesting_container_uuid, :runtime_constraints,
-                     :state, :container_uuid, :use_existing, :scheduling_parameters,
-                     :output_name
+    if self.new_record? || self.state_was == Uncommitted
+      # Allow create-and-commit in a single operation.
+      permitted.push(*AttrsPermittedBeforeCommit)
+    end
 
+    case self.state
     when Committed
-      if container_uuid.nil?
-        errors.add :container_uuid, "has not been resolved to a container."
+      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
+
+      if self.priority.nil?
+        self.errors.add :priority, "cannot be nil"
       end
 
-      if priority.nil?
-        errors.add :priority, "cannot be nil"
+      # 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
 
-      # Can update priority, container count, name and description
-      permitted.push :priority, :container_count, :container_count_max, :container_uuid,
-                     :name, :description
-
-      if self.state_changed?
-        # Allow create-and-commit in a single operation.
-        permitted.push :command, :container_image, :cwd, :description, :environment,
-                       :filters, :mounts, :name, :output_path, :properties,
-                       :requesting_container_uuid, :runtime_constraints,
-                       :state, :container_uuid, :use_existing, :scheduling_parameters,
-                       :output_name
+      if current_user.andand.is_admin
+        permitted.push :log_uuid
       end
 
     when Final
-      if not current_user.andand.is_admin and not (self.name_changed? || self.description_changed?)
-        errors.add :state, "of container request can only be set to Final by system."
+      if self.state_was == Committed
+        # "Cancel" means setting priority=0, state=Committed
+        permitted.push :priority
+
+        if current_user.andand.is_admin
+          permitted.push :output_uuid, :log_uuid
+        end
       end
 
-      if self.state_changed? || self.name_changed? || self.description_changed? || self.output_uuid_changed? || self.log_uuid_changed?
-          permitted.push :state, :name, :description, :output_uuid, :log_uuid
-      else
-        errors.add :state, "does not allow updates"
+    end
+
+    super(permitted)
+  end
+
+  def secret_mounts_key_conflict
+    secret_mounts.each do |k, v|
+      if mounts.has_key?(k)
+        errors.add(:secret_mounts, 'conflict with non-secret mounts')
+        return false
       end
+    end
+  end
 
-    else
-      errors.add :state, "invalid value"
+  def validate_runtime_token
+    if !self.runtime_token.nil? && self.runtime_token_changed?
+      if !runtime_token[0..2] == "v2/"
+        errors.add :runtime_token, "not a v2 token"
+        return
+      end
+      if ApiClientAuthorization.validate(token: runtime_token).nil?
+        errors.add :runtime_token, "failed validation"
+      end
     end
+  end
 
-    check_update_whitelist permitted
+  def scrub_secrets
+    if self.state == Final
+      self.secret_mounts = {}
+      self.runtime_token = nil
+    end
   end
 
   def update_priority
-    if self.state_changed? or
-        self.priority_changed? or
-        self.container_uuid_changed?
-      act_as_system_user do
-        Container.
-          where('uuid in (?)',
-                [self.container_uuid_was, self.container_uuid].compact).
-          map(&:update_priority!)
-      end
+    return unless state_changed? || priority_changed? || container_uuid_changed?
+    act_as_system_user do
+      Container.
+        where('uuid in (?)', [self.container_uuid_was, self.container_uuid].compact).
+        map(&:update_priority!)
     end
   end
 
+  def set_priority_zero
+    self.update_attributes!(priority: 0) if self.state != Final
+  end
+
   def set_requesting_container_uuid
-    return !new_record? if self.requesting_container_uuid   # already set
+    c = get_requesting_container()
+    if !c.nil?
+      self.requesting_container_uuid = c.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
 
-    token_uuid = current_api_client_authorization.andand.uuid
-    container = Container.where('auth_uuid=?', token_uuid).order('created_at desc').first
-    self.requesting_container_uuid = container.uuid if container
-    true
+  def get_requesting_container
+    return self.requesting_container_uuid if !self.requesting_container_uuid.nil?
+    Container.for_current_token
   end
 end