X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/72513393310d7f22688afa93f51b05e1d42bae08..7407f41105f8000bb3908d41a31daaf3a30d9440:/services/api/app/models/container.rb diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index ba5c1c28ca..912a801a6f 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -17,16 +17,22 @@ class Container < ArvadosModel extend DbCurrentTime extend LogReuseInfo + # Posgresql JSONB columns should NOT be declared as serialized, Rails 5 + # already know how to properly treat them. + attribute :secret_mounts, :jsonbHash, default: {} + attribute :runtime_status, :jsonbHash, default: {} + attribute :runtime_auth_scopes, :jsonbHash, default: {} + serialize :environment, Hash serialize :mounts, Hash serialize :runtime_constraints, Hash 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 + before_validation :check_lock + before_validation :check_unlock 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 @@ -37,7 +43,7 @@ class Container < ArvadosModel after_validation :assign_auth before_save :sort_serialized_attrs before_save :update_secret_mounts_md5 - before_save :scrub_secret_mounts + before_save :scrub_secrets before_save :clear_runtime_status_when_queued after_save :update_cr_logs after_save :handle_completed @@ -67,6 +73,9 @@ class Container < ArvadosModel t.add :state t.add :auth_uuid t.add :scheduling_parameters + t.add :runtime_user_uuid + t.add :runtime_auth_scopes + t.add :lock_count end # Supported states for a container @@ -83,7 +92,8 @@ class Container < ArvadosModel nil => [Queued], Queued => [Locked, Cancelled], Locked => [Queued, Running, Cancelled], - Running => [Complete, Cancelled] + Running => [Complete, Cancelled], + Complete => [Cancelled] } def self.limit_index_columns_read @@ -91,15 +101,15 @@ class Container < ArvadosModel end def self.full_text_searchable_columns - super - ["secret_mounts", "secret_mounts_md5"] + super - ["secret_mounts", "secret_mounts_md5", "runtime_token"] end def self.searchable_columns *args - super - ["secret_mounts_md5"] + super - ["secret_mounts_md5", "runtime_token"] end def logged_attributes - super.except('secret_mounts') + super.except('secret_mounts', 'runtime_token') end def state_transitions @@ -146,17 +156,37 @@ class Container < ArvadosModel # Create a new container (or find an existing one) to satisfy the # given container request. def self.resolve(req) - c_attrs = { - command: req.command, - cwd: req.cwd, - environment: req.environment, - output_path: req.output_path, - container_image: resolve_container_image(req.container_image), - mounts: resolve_mounts(req.mounts), - runtime_constraints: resolve_runtime_constraints(req.runtime_constraints), - scheduling_parameters: req.scheduling_parameters, - secret_mounts: req.secret_mounts, - } + if req.runtime_token.nil? + runtime_user = if req.modified_by_user_uuid.nil? + current_user + else + User.find_by_uuid(req.modified_by_user_uuid) + end + runtime_auth_scopes = ["all"] + else + auth = ApiClientAuthorization.validate(token: req.runtime_token) + if auth.nil? + raise ArgumentError.new "Invalid runtime token" + end + runtime_user = User.find_by_id(auth.user_id) + runtime_auth_scopes = auth.scopes + end + c_attrs = act_as_user runtime_user do + { + command: req.command, + cwd: req.cwd, + environment: req.environment, + output_path: req.output_path, + container_image: resolve_container_image(req.container_image), + mounts: resolve_mounts(req.mounts), + runtime_constraints: resolve_runtime_constraints(req.runtime_constraints), + scheduling_parameters: req.scheduling_parameters, + secret_mounts: req.secret_mounts, + runtime_token: req.runtime_token, + runtime_user_uuid: runtime_user.uuid, + runtime_auth_scopes: runtime_auth_scopes + } + end act_as_system_user do if req.use_existing && (reusable = find_reusable(c_attrs)) reusable @@ -179,7 +209,7 @@ class Container < ArvadosModel rc = {} defaults = { 'keep_cache_ram' => - Rails.configuration.container_default_keep_cache_ram, + Rails.configuration.Containers.DefaultKeepCacheRAM, } defaults.merge(runtime_constraints).each do |k, v| if v.is_a? Array @@ -201,7 +231,11 @@ class Container < ArvadosModel if mount['kind'] != 'collection' next end - if (uuid = mount.delete 'uuid') + + uuid = mount.delete 'uuid' + + if mount['portable_data_hash'].nil? and !uuid.nil? + # PDH not supplied, try by UUID c = Collection. readable_by(current_user). where(uuid: uuid). @@ -210,13 +244,7 @@ class Container < ArvadosModel 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 + mount['portable_data_hash'] = c.portable_data_hash end end return c_mounts @@ -310,41 +338,42 @@ class Container < ArvadosModel nil end - def check_lock_fail - if self.state != Queued - raise LockFailedError.new("cannot lock when #{self.state}") - elsif self.priority <= 0 - raise LockFailedError.new("cannot lock when priority<=0") + def lock + self.with_lock do + if self.state != Queued + raise LockFailedError.new("cannot lock when #{self.state}") + end + self.update_attributes!(state: Locked) end end - def lock - # Check invalid state transitions once before getting the lock - # (because it's cheaper that way) and once after getting the lock - # (because state might have changed while acquiring the lock). - check_lock_fail - transaction do - reload - check_lock_fail - update_attributes!(state: Locked) + def check_lock + if state_was == Queued and state == Locked + if self.priority <= 0 + raise LockFailedError.new("cannot lock when priority<=0") + end + self.lock_count = self.lock_count+1 end end - def check_unlock_fail - if self.state != Locked - raise InvalidStateTransitionError.new("cannot unlock when #{self.state}") - elsif self.locked_by_uuid != current_api_client_authorization.uuid - raise InvalidStateTransitionError.new("locked by a different token") + def unlock + self.with_lock do + if self.state != Locked + raise InvalidStateTransitionError.new("cannot unlock when #{self.state}") + end + self.update_attributes!(state: Queued) end end - def unlock - # Check invalid state transitions twice (see lock) - check_unlock_fail - transaction do - reload(lock: 'FOR UPDATE') - check_unlock_fail - update_attributes!(state: Queued) + def check_unlock + if state_was == Locked and state == Queued + if self.locked_by_uuid != current_api_client_authorization.uuid + raise ArvadosModel::PermissionDeniedError.new("locked by a different token") + end + if self.lock_count >= Rails.configuration.Containers.MaxDispatchAttempts + self.state = Cancelled + self.runtime_status = {error: "Failed to start container. Cancelled after exceeding 'Containers.MaxDispatchAttempts' (lock_count=#{self.lock_count})"} + end end end @@ -355,6 +384,9 @@ class Container < ArvadosModel else kwargs = {} end + if users_list.select { |u| u.is_admin }.any? + return super + end Container.where(ContainerRequest.readable_by(*users_list).where("containers.uuid = container_requests.container_uuid").exists) end @@ -362,6 +394,19 @@ class Container < ArvadosModel [Complete, Cancelled].include?(self.state) end + def self.for_current_token + return if !current_api_client_authorization + _, _, _, container_uuid = Thread.current[:token].split('/') + if container_uuid.nil? + Container.where(auth_uuid: current_api_client_authorization.uuid).first + else + Container.where('auth_uuid=? or (uuid=? and runtime_token=?)', + current_api_client_authorization.uuid, + container_uuid, + current_api_client_authorization.token).first + end + end + protected def fill_field_defaults @@ -378,6 +423,10 @@ class Container < ArvadosModel current_user.andand.is_admin end + def permission_to_destroy + current_user.andand.is_admin + end + def ensure_owner_uuid_is_permitted # validate_change ensures owner_uuid can't be changed at all -- # except during create, which requires admin privileges. Checking @@ -415,12 +464,13 @@ class Container < ArvadosModel permitted.push(:owner_uuid, :command, :container_image, :cwd, :environment, :mounts, :output_path, :priority, :runtime_constraints, :scheduling_parameters, - :secret_mounts) + :secret_mounts, :runtime_token, + :runtime_user_uuid, :runtime_auth_scopes) end case self.state when Locked - permitted.push :priority, :runtime_status, :log + permitted.push :priority, :runtime_status, :log, :lock_count when Queued permitted.push :priority @@ -441,7 +491,7 @@ class Container < ArvadosModel when Running permitted.push :finished_at, *progress_attrs when Queued, Locked - permitted.push :finished_at, :log + permitted.push :finished_at, :log, :runtime_status end else @@ -449,10 +499,16 @@ 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]) + if self.state_was == Running && + !current_api_client_authorization.nil? && + (current_api_client_authorization.uuid == self.auth_uuid || + current_api_client_authorization.token == self.runtime_token) + # The contained process itself can write final attrs but can't + # change priority or log. + permitted.push *final_attrs + permitted = permitted - [:log, :priority] + elsif !current_user.andand.is_admin + raise PermissionDeniedError 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. @@ -511,39 +567,44 @@ class Container < ArvadosModel def assign_auth if self.auth_uuid_changed? - return errors.add :auth_uuid, 'is readonly' + return errors.add :auth_uuid, 'is readonly' end if not [Locked, Running].include? self.state - # don't need one - self.auth.andand.update_attributes(expires_at: db_current_time) + # Don't need one. If auth already exists, expire it. + # + # We use db_transaction_time here (not db_current_time) to + # ensure the token doesn't validate later in the same + # transaction (e.g., in a test case) by satisfying expires_at > + # transaction timestamp. + self.auth.andand.update_attributes(expires_at: db_transaction_time) self.auth = nil return elsif self.auth # already have one return end - cr = ContainerRequest. - where('container_uuid=? and priority>0', self.uuid). - order('priority desc'). - first - if !cr - return errors.add :auth_uuid, "cannot be assigned because priority <= 0" - end - if cr.runtime_token.nil? + if self.runtime_token.nil? + if self.runtime_user_uuid.nil? + # legacy behavior, we don't have a runtime_user_uuid so get + # the user from the highest priority container request, needed + # when performing an upgrade and there are queued containers, + # and some tests. + cr = ContainerRequest. + where('container_uuid=? and priority>0', self.uuid). + order('priority desc'). + first + if !cr + return errors.add :auth_uuid, "cannot be assigned because priority <= 0" + end + self.runtime_user_uuid = cr.modified_by_user_uuid + self.runtime_auth_scopes = ["all"] + end + # generate a new token self.auth = ApiClientAuthorization. - create!(user_id: User.find_by_uuid(cr.modified_by_user_uuid).id, - api_client_id: 0) - self.runtime_user_uuid = cr.modified_by_user_uuid - self.runtime_auth_scopes = self.auth.scopes - else - # using cr.runtime_token - self.auth = ApiClientAuthorization.validate(token: cr.runtime_token) - if self.auth.nil? - raise ArgumentError.new "Invalid runtime token" - end - self.runtime_user_uuid = User.find_by_id(self.auth.user_id).uuid - self.runtime_auth_scopes = self.auth.scopes + create!(user_id: User.find_by_uuid(self.runtime_user_uuid).id, + api_client_id: 0, + scopes: self.runtime_auth_scopes) end end @@ -560,6 +621,9 @@ class Container < ArvadosModel if self.scheduling_parameters_changed? self.scheduling_parameters = self.class.deep_sort_hash(self.scheduling_parameters) end + if self.runtime_auth_scopes_changed? + self.runtime_auth_scopes = self.runtime_auth_scopes.sort + end end def update_secret_mounts_md5 @@ -569,12 +633,13 @@ class Container < ArvadosModel end end - def scrub_secret_mounts + def scrub_secrets # 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 = {} + self.runtime_token = nil end end @@ -589,60 +654,76 @@ class Container < ArvadosModel # This container is finished so finalize any associated container requests # that are associated with this container. if self.state_changed? and self.final? - act_as_system_user do - - if self.state == Cancelled - retryable_requests = ContainerRequest.where("container_uuid = ? and priority > 0 and state = 'Committed' and container_count < container_count_max", uuid) - else - retryable_requests = [] - end + # These get wiped out by with_lock (which reloads the record), + # so record them now in case we need to schedule a retry. + prev_secret_mounts = self.secret_mounts_was + prev_runtime_token = self.runtime_token_was + + # Need to take a lock on the container to ensure that any + # concurrent container requests that might try to reuse this + # container will block until the container completion + # transaction finishes. This ensure that concurrent container + # requests that try to reuse this container are finalized (on + # Complete) or don't reuse it (on Cancelled). + self.with_lock do + act_as_system_user do + if self.state == Cancelled + retryable_requests = ContainerRequest.where("container_uuid = ? and priority > 0 and state = 'Committed' and container_count < container_count_max", uuid) + else + retryable_requests = [] + end - if retryable_requests.any? - c_attrs = { - command: self.command, - cwd: self.cwd, - environment: self.environment, - output_path: self.output_path, - container_image: self.container_image, - mounts: self.mounts, - runtime_constraints: self.runtime_constraints, - scheduling_parameters: self.scheduling_parameters - } - c = Container.create! c_attrs - retryable_requests.each do |cr| - cr.with_lock do - leave_modified_by_user_alone do - # Use row locking because this increments container_count - cr.container_uuid = c.uuid - cr.save! + if retryable_requests.any? + c_attrs = { + command: self.command, + cwd: self.cwd, + environment: self.environment, + output_path: self.output_path, + container_image: self.container_image, + mounts: self.mounts, + runtime_constraints: self.runtime_constraints, + scheduling_parameters: self.scheduling_parameters, + secret_mounts: prev_secret_mounts, + runtime_token: prev_runtime_token, + runtime_user_uuid: self.runtime_user_uuid, + runtime_auth_scopes: self.runtime_auth_scopes + } + c = Container.create! c_attrs + retryable_requests.each do |cr| + cr.with_lock do + leave_modified_by_user_alone do + # Use row locking because this increments container_count + cr.container_uuid = c.uuid + cr.save! + end end end end - end - # Notify container requests associated with this container - ContainerRequest.where(container_uuid: uuid, - state: ContainerRequest::Committed).each do |cr| - leave_modified_by_user_alone do - cr.finalize! + # Notify container requests associated with this container + ContainerRequest.where(container_uuid: uuid, + state: ContainerRequest::Committed).each do |cr| + leave_modified_by_user_alone do + cr.finalize! + end end - end - # Cancel outstanding container requests made by this container. - ContainerRequest. - includes(:container). - where(requesting_container_uuid: uuid, - state: ContainerRequest::Committed).each do |cr| - leave_modified_by_user_alone do - 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) + # Cancel outstanding container requests made by this container. + ContainerRequest. + includes(:container). + where(requesting_container_uuid: uuid, + state: ContainerRequest::Committed).each do |cr| + leave_modified_by_user_alone do + 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