X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/10c1e7359286edd6562c52304e9706449a9ee53f..2130de30acf0a3b89e06494f957aacb350c15067:/services/api/app/models/container.rb diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index 7176bda926..079ac4c299 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -23,11 +23,13 @@ class Container < ArvadosModel serialize :command, Array serialize :scheduling_parameters, Hash serialize :secret_mounts, Hash + serialize :runtime_status, Hash before_validation :fill_field_defaults, :if => :new_record? before_validation :set_timestamps validates :command, :container_image, :output_path, :cwd, :priority, { presence: true } validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validate :validate_runtime_status validate :validate_state_change validate :validate_change validate :validate_lock @@ -36,6 +38,8 @@ class Container < ArvadosModel before_save :sort_serialized_attrs 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 } @@ -58,6 +62,7 @@ class Container < ArvadosModel t.add :priority t.add :progress t.add :runtime_constraints + t.add :runtime_status t.add :started_at t.add :state t.add :auth_uuid @@ -276,9 +281,10 @@ class Container < ArvadosModel return usable end - # Check for Running candidates and return the most likely to finish sooner. + # Check for non-failing Running candidates and return the most likely to finish sooner. log_reuse_info { "checking for state=Running..." } running = candidates.where(state: Running). + where("(runtime_status->'error') is null"). order('progress desc, started_at asc'). limit(1).first if running @@ -372,24 +378,11 @@ class Container < ArvadosModel current_user.andand.is_admin end - def permission_to_update - # Override base permission check to allow auth_uuid to set progress and - # output (only). Whether it is legal to set progress and output in the current - # state has already been checked in validate_change. - current_user.andand.is_admin || - (!current_api_client_authorization.nil? and - [self.auth_uuid, self.locked_by_uuid].include? current_api_client_authorization.uuid) - end - def ensure_owner_uuid_is_permitted - # Override base permission check to allow auth_uuid to set progress and - # output (only). Whether it is legal to set progress and output in the current - # state has already been checked in validate_change. - if !current_api_client_authorization.nil? and self.auth_uuid == current_api_client_authorization.uuid - check_update_whitelist [:progress, :output] - else - super - end + # validate_change ensures owner_uuid can't be changed at all -- + # except during create, which requires admin privileges. Checking + # permission here would be superfluous. + true end def set_timestamps @@ -402,8 +395,21 @@ class Container < ArvadosModel end end + # Check that well-known runtime status keys have desired data types + def validate_runtime_status + [ + 'error', 'errorDetail', 'warning', 'warningDetail', 'activity' + ].each do |k| + if self.runtime_status.andand.include?(k) && !self.runtime_status[k].is_a?(String) + errors.add(:runtime_status, "'#{k}' value must be a string") + end + end + end + def validate_change permitted = [:state] + progress_attrs = [:progress, :runtime_status, :log, :output] + final_attrs = [:exit_code, :finished_at] if self.new_record? permitted.push(:owner_uuid, :command, :container_image, :cwd, @@ -413,24 +419,27 @@ class Container < ArvadosModel end case self.state - when Queued, Locked + when Locked + permitted.push :priority, :runtime_status, :log + + when Queued permitted.push :priority when Running - permitted.push :priority, :progress, :output + permitted.push :priority, *progress_attrs if self.state_changed? permitted.push :started_at end when Complete if self.state_was == Running - permitted.push :finished_at, :output, :log, :exit_code + permitted.push *final_attrs, *progress_attrs end when Cancelled case self.state_was when Running - permitted.push :finished_at, :output, :log + permitted.push :finished_at, *progress_attrs when Queued, Locked permitted.push :finished_at, :log end @@ -440,6 +449,15 @@ class Container < ArvadosModel return false end + if current_api_client_authorization.andand.uuid.andand == self.auth_uuid + # The contained process itself can update progress indicators, + # but can't change priority etc. + permitted = permitted & (progress_attrs + final_attrs + [:state] - [:log]) + elsif self.locked_by_uuid && self.locked_by_uuid != current_api_client_authorization.andand.uuid + # When locked, progress fields cannot be updated by the wrong + # dispatcher, even though it has admin privileges. + permitted = permitted - progress_attrs + end check_update_whitelist permitted end @@ -478,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' @@ -534,6 +565,13 @@ class Container < ArvadosModel end end + def clear_runtime_status_when_queued + # Avoid leaking status messages between different dispatch attempts + if self.state_was == Locked && self.state == Queued + self.runtime_status = {} + end + end + def handle_completed # This container is finished so finalize any associated container requests # that are associated with this container.