Merge branch '13138-request-error' refs #13138
[arvados.git] / services / api / app / models / container.rb
index 60911bd55c74593838e7cce2932e06ce3c13581c..3765266405d86f41a1a3a10372b79d1c17aea14f 100644 (file)
@@ -1,3 +1,7 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
 require 'log_reuse_info'
 require 'whitelist_update'
 require 'safe_json'
@@ -16,17 +20,22 @@ class Container < ArvadosModel
   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 :set_timestamps
-  validates :command, :container_image, :output_path, :cwd, :priority, :presence => true
+  validates :command, :container_image, :output_path, :cwd, :priority, { presence: true }
+  validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
   validate :validate_state_change
   validate :validate_change
   validate :validate_lock
   validate :validate_output
   after_validation :assign_auth
   before_save :sort_serialized_attrs
+  before_save :update_secret_mounts_md5
+  before_save :scrub_secret_mounts
   after_save :handle_completed
+  after_save :propagate_priority
 
   has_many :container_requests, :foreign_key => :container_uuid, :class_name => 'ContainerRequest', :primary_key => :uuid
   belongs_to :auth, :class_name => 'ApiClientAuthorization', :foreign_key => :auth_uuid, :primary_key => :uuid
@@ -73,19 +82,56 @@ class Container < ArvadosModel
     ["mounts"]
   end
 
+  def self.full_text_searchable_columns
+    super - ["secret_mounts", "secret_mounts_md5"]
+  end
+
+  def self.searchable_columns *args
+    super - ["secret_mounts_md5"]
+  end
+
+  def logged_attributes
+    super.except('secret_mounts')
+  end
+
   def state_transitions
     State_transitions
   end
 
+  # Container priority is the highest "computed priority" of any
+  # matching request. The computed priority of a container-submitted
+  # request is the priority of the submitting container. The computed
+  # priority of a user-submitted request is a function of
+  # user-assigned priority and request creation time.
   def update_priority!
-    if [Queued, Locked, Running].include? self.state
-      # Update the priority of this container to the maximum priority of any of
-      # its committed container requests and save the record.
-      self.priority = ContainerRequest.
-        where(container_uuid: uuid,
+    return if ![Queued, Locked, Running].include?(state)
+    p = ContainerRequest.
+        where('container_uuid=? and priority>0', uuid).
+        includes(:requesting_container).
+        lock(true).
+        map do |cr|
+      if cr.requesting_container
+        cr.requesting_container.priority
+      else
+        (cr.priority << 50) - (cr.created_at.to_time.to_f * 1000).to_i
+      end
+    end.max || 0
+    update_attributes!(priority: p)
+  end
+
+  def propagate_priority
+    return true unless priority_changed?
+    act_as_system_user do
+      # Update the priority of child container requests to match new
+      # priority of the parent container (ignoring requests with no
+      # container assigned, because their priority doesn't matter).
+      ContainerRequest.
+        where(requesting_container_uuid: self.uuid,
               state: ContainerRequest::Committed).
-        maximum('priority')
-      self.save!
+        where('container_uuid is not null').
+        includes(:container).
+        map(&:container).
+        map(&:update_priority!)
     end
   end
 
@@ -101,6 +147,7 @@ class Container < ArvadosModel
       mounts: resolve_mounts(req.mounts),
       runtime_constraints: resolve_runtime_constraints(req.runtime_constraints),
       scheduling_parameters: req.scheduling_parameters,
+      secret_mounts: req.secret_mounts,
     }
     act_as_system_user do
       if req.use_existing && (reusable = find_reusable(c_attrs))
@@ -179,50 +226,54 @@ class Container < ArvadosModel
   def self.find_reusable(attrs)
     log_reuse_info { "starting with #{Container.all.count} container records in database" }
     candidates = Container.where_serialized(:command, attrs[:command])
-    log_reuse_info { "have #{candidates.count} candidates after filtering on command #{attrs[:command].inspect}" }
+    log_reuse_info(candidates) { "after filtering on command #{attrs[:command].inspect}" }
 
     candidates = candidates.where('cwd = ?', attrs[:cwd])
-    log_reuse_info { "have #{candidates.count} candidates after filtering on cwd #{attrs[:cwd].inspect}" }
+    log_reuse_info(candidates) { "after filtering on cwd #{attrs[:cwd].inspect}" }
 
     candidates = candidates.where_serialized(:environment, attrs[:environment])
-    log_reuse_info { "have #{candidates.count} candidates after filtering on environment #{attrs[:environment].inspect}" }
+    log_reuse_info(candidates) { "after filtering on environment #{attrs[:environment].inspect}" }
 
     candidates = candidates.where('output_path = ?', attrs[:output_path])
-    log_reuse_info { "have #{candidates.count} candidates after filtering on output_path #{attrs[:output_path].inspect}" }
+    log_reuse_info(candidates) { "after filtering on output_path #{attrs[:output_path].inspect}" }
 
     image = resolve_container_image(attrs[:container_image])
     candidates = candidates.where('container_image = ?', image)
-    log_reuse_info { "have #{candidates.count} candidates after filtering on container_image #{image.inspect} (resolved from #{attrs[:container_image].inspect})" }
+    log_reuse_info(candidates) { "after filtering on container_image #{image.inspect} (resolved from #{attrs[:container_image].inspect})" }
 
     candidates = candidates.where_serialized(:mounts, resolve_mounts(attrs[:mounts]))
-    log_reuse_info { "have #{candidates.count} candidates after filtering on mounts #{attrs[:mounts].inspect}" }
+    log_reuse_info(candidates) { "after filtering on mounts #{attrs[:mounts].inspect}" }
+
+    candidates = candidates.where('secret_mounts_md5 = ?', Digest::MD5.hexdigest(SafeJSON.dump(self.deep_sort_hash(attrs[:secret_mounts]))))
+    log_reuse_info(candidates) { "after filtering on mounts #{attrs[:mounts].inspect}" }
 
     candidates = candidates.where_serialized(:runtime_constraints, resolve_runtime_constraints(attrs[:runtime_constraints]))
-    log_reuse_info { "have #{candidates.count} candidates after filtering on runtime_constraints #{attrs[:runtime_constraints].inspect}" }
+    log_reuse_info(candidates) { "after filtering on runtime_constraints #{attrs[:runtime_constraints].inspect}" }
+
+    log_reuse_info { "checking for state=Complete with readable output and log..." }
 
-    # Check for Completed candidates whose output and log are both readable.
     select_readable_pdh = Collection.
       readable_by(current_user).
       select(:portable_data_hash).
       to_sql
 
     usable = candidates.where(state: Complete, exit_code: 0)
-    log_reuse_info { "have #{usable.count} with state=Complete, exit_code=0" }
+    log_reuse_info(usable) { "with state=Complete, exit_code=0" }
 
     usable = usable.where("log IN (#{select_readable_pdh})")
-    log_reuse_info { "have #{usable.count} with log readable by current user #{current_user.uuid}" }
+    log_reuse_info(usable) { "with readable log" }
 
     usable = usable.where("output IN (#{select_readable_pdh})")
-    log_reuse_info { "have #{usable.count} with output readable by current user #{current_user.uuid}" }
+    log_reuse_info(usable) { "with readable output" }
 
-    usable = usable.order('finished_at ASC').
-      limit(1).first
+    usable = usable.order('finished_at ASC').limit(1).first
     if usable
-      log_reuse_info { "done, reusing completed container #{usable.uuid}" }
+      log_reuse_info { "done, reusing container #{usable.uuid} with state=Complete" }
       return usable
     end
 
     # Check for Running candidates and return the most likely to finish sooner.
+    log_reuse_info { "checking for state=Running..." }
     running = candidates.where(state: Running).
               order('progress desc, started_at asc').
               limit(1).first
@@ -292,17 +343,13 @@ class Container < ArvadosModel
   end
 
   def self.readable_by(*users_list)
-    if users_list.select { |u| u.is_admin }.any?
-      return self
+    # Load optional keyword arguments, if they exist.
+    if users_list.last.is_a? Hash
+      kwargs = users_list.pop
+    else
+      kwargs = {}
     end
-    user_uuids = users_list.map { |u| u.uuid }
-    uuid_list = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
-    uuid_list.uniq!
-    permitted = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:uuids))"
-    joins(:container_requests).
-      where("container_requests.uuid IN #{permitted} OR "+
-            "container_requests.owner_uuid IN (:uuids)",
-            uuids: uuid_list)
+    Container.where(ContainerRequest.readable_by(*users_list).where("containers.uuid = container_requests.container_uuid").exists)
   end
 
   def final?
@@ -317,7 +364,7 @@ class Container < ArvadosModel
     self.runtime_constraints ||= {}
     self.mounts ||= {}
     self.cwd ||= "."
-    self.priority ||= 1
+    self.priority ||= 0
     self.scheduling_parameters ||= {}
   end
 
@@ -361,7 +408,8 @@ class Container < ArvadosModel
     if self.new_record?
       permitted.push(:owner_uuid, :command, :container_image, :cwd,
                      :environment, :mounts, :output_path, :priority,
-                     :runtime_constraints, :scheduling_parameters)
+                     :runtime_constraints, :scheduling_parameters,
+                     :secret_mounts)
     end
 
     case self.state
@@ -384,7 +432,7 @@ class Container < ArvadosModel
       when Running
         permitted.push :finished_at, :output, :log
       when Queued, Locked
-        permitted.push :finished_at
+        permitted.push :finished_at, :log
       end
 
     else
@@ -420,12 +468,10 @@ class Container < ArvadosModel
     # that a container cannot "claim" a collection that it doesn't otherwise
     # have access to just by setting the output field to the collection PDH.
     if output_changed?
-      c = Collection.unscoped do
-        Collection.
-            readable_by(current_user).
+      c = Collection.
+            readable_by(current_user, {include_trash: true}).
             where(portable_data_hash: self.output).
             first
-      end
       if !c
         errors.add :output, "collection must exist and be readable by current user."
       end
@@ -472,6 +518,22 @@ class Container < ArvadosModel
     end
   end
 
+  def update_secret_mounts_md5
+    if self.secret_mounts_changed?
+      self.secret_mounts_md5 = Digest::MD5.hexdigest(
+        SafeJSON.dump(self.class.deep_sort_hash(self.secret_mounts)))
+    end
+  end
+
+  def scrub_secret_mounts
+    # this runs after update_secret_mounts_md5, so the
+    # secret_mounts_md5 will still reflect the secrets that are being
+    # scrubbed here.
+    if self.state_changed? && self.final?
+      self.secret_mounts = {}
+    end
+  end
+
   def handle_completed
     # This container is finished so finalize any associated container requests
     # that are associated with this container.
@@ -500,7 +562,7 @@ class Container < ArvadosModel
             cr.with_lock do
               # Use row locking because this increments container_count
               cr.container_uuid = c.uuid
-              cr.save
+              cr.save!
             end
           end
         end
@@ -511,11 +573,21 @@ class Container < ArvadosModel
           cr.finalize!
         end
 
-        # Try to cancel any outstanding container requests made by this container.
-        ContainerRequest.where(requesting_container_uuid: uuid,
-                               state: ContainerRequest::Committed).each do |cr|
-          cr.priority = 0
-          cr.save
+        # Cancel outstanding container requests made by this container.
+        ContainerRequest.
+          includes(:container).
+          where(requesting_container_uuid: uuid,
+                state: ContainerRequest::Committed).each do |cr|
+          cr.update_attributes!(priority: 0)
+          cr.container.reload
+          if cr.container.state == Container::Queued || cr.container.state == Container::Locked
+            # If the child container hasn't started yet, finalize the
+            # child CR now instead of leaving it "on hold", i.e.,
+            # Queued with priority 0.  (OTOH, if the child is already
+            # running, leave it alone so it can get cancelled the
+            # usual way, get a copy of the log collection, etc.)
+            cr.update_attributes!(state: ContainerRequest::Final)
+          end
         end
       end
     end