From 48aff82709769b098321c738f3444b9bdaa694c6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 21 Oct 2020 07:08:36 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-5-stable-ee --- app/models/ci/bridge.rb | 30 +++++++- app/models/ci/build.rb | 45 +++++++++--- app/models/ci/build_pending_state.rb | 6 ++ app/models/ci/build_trace_chunk.rb | 104 +++++++++++++++++++++++---- app/models/ci/build_trace_chunks/database.rb | 2 + app/models/ci/deleted_object.rb | 37 ++++++++++ app/models/ci/job_artifact.rb | 32 ++++++--- app/models/ci/pipeline.rb | 46 ++++++++---- 8 files changed, 253 insertions(+), 49 deletions(-) create mode 100644 app/models/ci/deleted_object.rb (limited to 'app/models/ci') diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index 1697067f633..2e725e0baff 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -27,7 +27,7 @@ module Ci # rubocop:enable Cop/ActiveRecordSerialize state_machine :status do - after_transition created: :pending do |bridge| + after_transition [:created, :manual] => :pending do |bridge| next unless bridge.downstream_project bridge.run_after_commit do @@ -46,6 +46,10 @@ module Ci event :scheduled do transition all => :scheduled end + + event :actionize do + transition created: :manual + end end def self.retry(bridge, current_user) @@ -126,9 +130,27 @@ module Ci false end + def playable? + return false unless ::Gitlab::Ci::Features.manual_bridges_enabled?(project) + + action? && !archived? && manual? + end + def action? - false + return false unless ::Gitlab::Ci::Features.manual_bridges_enabled?(project) + + %w[manual].include?(self.when) + end + + # rubocop: disable CodeReuse/ServiceClass + # We don't need it but we are taking `job_variables_attributes` parameter + # to make it consistent with `Ci::Build#play` method. + def play(current_user, job_variables_attributes = nil) + Ci::PlayBridgeService + .new(project, current_user) + .execute(self) end + # rubocop: enable CodeReuse/ServiceClass def artifacts? false @@ -185,6 +207,10 @@ module Ci [] end + def target_revision_ref + downstream_pipeline_params.dig(:target_revision, :ref) + end + private def cross_project_params diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 99580a52e96..9ff70ece947 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -327,6 +327,8 @@ module Ci after_transition any => [:success, :failed, :canceled] do |build| build.run_after_commit do + build.run_status_commit_hooks! + BuildFinishedWorker.perform_async(id) end end @@ -524,7 +526,6 @@ module Ci .concat(job_jwt_variables) .concat(scoped_variables) .concat(job_variables) - .concat(environment_changed_page_variables) .concat(persisted_environment_variables) .to_runner_variables end @@ -561,15 +562,6 @@ module Ci end end - def environment_changed_page_variables - Gitlab::Ci::Variables::Collection.new.tap do |variables| - break variables unless environment_status && Feature.enabled?(:modifed_path_ci_variables, project) - - variables.append(key: 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS', value: environment_status.changed_paths.join(',')) - variables.append(key: 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS', value: environment_status.changed_urls.join(',')) - end - end - def deploy_token_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| break variables unless gitlab_deploy_token @@ -780,6 +772,11 @@ module Ci end end + def has_expired_locked_archive_artifacts? + locked_artifacts? && + artifacts_expire_at.present? && artifacts_expire_at < Time.current + end + def has_expiring_archive_artifacts? has_expiring_artifacts? && job_artifacts_archive.present? end @@ -901,7 +898,11 @@ module Ci def collect_test_reports!(test_reports) test_reports.get_suite(group_name).tap do |test_suite| each_report(Ci::JobArtifact::TEST_REPORT_FILE_TYPES) do |file_type, blob| - Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite, job: self) + Gitlab::Ci::Parsers.fabricate!(file_type).parse!( + blob, + test_suite, + job: self + ) end end end @@ -963,8 +964,30 @@ module Ci pending_state.try(:delete) end + def run_on_status_commit(&block) + status_commit_hooks.push(block) + end + + def max_test_cases_per_report + # NOTE: This is temporary and will be replaced later by a value + # that would come from an actual application limit. + ::Gitlab.com? ? 500_000 : 0 + end + + protected + + def run_status_commit_hooks! + status_commit_hooks.reverse_each do |hook| + instance_eval(&hook) + end + end + private + def status_commit_hooks + @status_commit_hooks ||= [] + end + def auto_retry strong_memoize(:auto_retry) do Gitlab::Ci::Build::AutoRetry.new(self) diff --git a/app/models/ci/build_pending_state.rb b/app/models/ci/build_pending_state.rb index 45f323adec2..299c67f441d 100644 --- a/app/models/ci/build_pending_state.rb +++ b/app/models/ci/build_pending_state.rb @@ -9,4 +9,10 @@ class Ci::BuildPendingState < ApplicationRecord enum failure_reason: CommitStatus.failure_reasons validates :build, presence: true + + def crc32 + trace_checksum.try do |checksum| + checksum.to_s.split('crc32:').last.to_i(16) + end + end end diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb index 444742062d9..6926ccd9438 100644 --- a/app/models/ci/build_trace_chunk.rb +++ b/app/models/ci/build_trace_chunk.rb @@ -3,9 +3,11 @@ module Ci class BuildTraceChunk < ApplicationRecord extend ::Gitlab::Ci::Model + include ::Comparable include ::FastDestroyAll include ::Checksummable include ::Gitlab::ExclusiveLeaseHelpers + include ::Gitlab::OptimisticLocking belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id @@ -29,6 +31,7 @@ module Ci } scope :live, -> { redis } + scope :persisted, -> { not_redis.order(:chunk_index) } class << self def all_stores @@ -63,12 +66,24 @@ module Ci get_store_class(store).delete_keys(value) end end + + ## + # Sometimes we do not want to read raw data. This method makes it easier + # to find attributes that are just metadata excluding raw data. + # + def metadata_attributes + attribute_names - %w[raw_data] + end end def data @data ||= get_data.to_s end + def crc32 + checksum.to_i + end + def truncate(offset = 0) raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0 return if offset == size # Skip the following process as it doesn't affect anything @@ -102,22 +117,47 @@ module Ci (start_offset...end_offset) end - def persist_data! - in_lock(*lock_params) { unsafe_persist_data! } - end - def schedule_to_persist! - return if persisted? + return if flushed? Ci::BuildTraceChunkFlushWorker.perform_async(id) end - def persisted? - !redis? - end + ## + # It is possible that we run into two concurrent migrations. It might + # happen that a chunk gets migrated after being loaded by another worker + # but before the worker acquires a lock to perform the migration. + # + # We are using Redis locking to ensure that we perform this operation + # inside an exclusive lock, but this does not prevent us from running into + # race conditions related to updating a model representation in the + # database. Optimistic locking is another mechanism that help here. + # + # We are using optimistic locking combined with Redis locking to ensure + # that a chunk gets migrated properly. + # + # We are catching an exception related to an exclusive lock not being + # acquired because it is creating a lot of noise, and is a result of + # duplicated workers running in parallel for the same build trace chunk. + # + def persist_data! + in_lock(*lock_params) do # exclusive Redis lock is acquired first + raise FailedToPersistDataError, 'Modifed build trace chunk detected' if has_changes_to_save? - def live? - redis? + self.reset.then do |chunk| # we ensure having latest lock_version + chunk.unsafe_persist_data! # we migrate the data and update data store + end + end + rescue FailedToObtainLockError + metrics.increment_trace_operation(operation: :stalled) + rescue ActiveRecord::StaleObjectError + raise FailedToPersistDataError, <<~MSG + Data migration race condition detected + + store: #{data_store} + build: #{build.id} + index: #{chunk_index} + MSG end ## @@ -126,11 +166,28 @@ module Ci # no chunk with higher index in the database. # def final? - build.pending_state.present? && - build.trace_chunks.maximum(:chunk_index).to_i == chunk_index + build.pending_state.present? && chunks_max_index == chunk_index end - private + def flushed? + !redis? + end + + def migrated? + flushed? + end + + def live? + redis? + end + + def <=>(other) + return unless self.build_id == other.build_id + + self.chunk_index <=> other.chunk_index + end + + protected def get_data # Redis / database return UTF-8 encoded string by default @@ -145,12 +202,19 @@ module Ci current_size = current_data&.bytesize.to_i unless current_size == CHUNK_SIZE || final? - raise FailedToPersistDataError, 'Data is not fulfilled in a bucket' + raise FailedToPersistDataError, <<~MSG + data is not fulfilled in a bucket + + size: #{current_size} + state: #{pending_state?} + max: #{chunks_max_index} + index: #{chunk_index} + MSG end self.raw_data = nil self.data_store = new_store - self.checksum = crc32(current_data) + self.checksum = self.class.crc32(current_data) ## # We need to so persist data then save a new store identifier before we @@ -199,10 +263,20 @@ module Ci size == CHUNK_SIZE end + private + + def pending_state? + build.pending_state.present? + end + def current_store self.class.get_store_class(data_store) end + def chunks_max_index + build.trace_chunks.maximum(:chunk_index).to_i + end + def lock_params ["trace_write:#{build_id}:chunks:#{chunk_index}", { ttl: WRITE_LOCK_TTL, diff --git a/app/models/ci/build_trace_chunks/database.rb b/app/models/ci/build_trace_chunks/database.rb index ea8072099c6..7448afba4c2 100644 --- a/app/models/ci/build_trace_chunks/database.rb +++ b/app/models/ci/build_trace_chunks/database.rb @@ -17,6 +17,8 @@ module Ci def data(model) model.raw_data + rescue ActiveModel::MissingAttributeError + model.reset.raw_data end def set_data(model, new_data) diff --git a/app/models/ci/deleted_object.rb b/app/models/ci/deleted_object.rb new file mode 100644 index 00000000000..e74946eda16 --- /dev/null +++ b/app/models/ci/deleted_object.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Ci + class DeletedObject < ApplicationRecord + extend Gitlab::Ci::Model + + mount_uploader :file, DeletedObjectUploader + + scope :ready_for_destruction, ->(limit) do + where('pick_up_at < ?', Time.current).limit(limit) + end + + scope :lock_for_destruction, ->(limit) do + ready_for_destruction(limit) + .select(:id) + .order(:pick_up_at) + .lock('FOR UPDATE SKIP LOCKED') + end + + def self.bulk_import(artifacts) + attributes = artifacts.each.with_object([]) do |artifact, accumulator| + record = artifact.to_deleted_object_attrs + accumulator << record if record[:store_dir] && record[:file] + end + + self.insert_all(attributes) if attributes.any? + end + + def delete_file_from_storage + file.remove! + true + rescue => exception + Gitlab::ErrorTracking.track_exception(exception) + false + end + end +end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 8bbb92e319f..02e17afdab0 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -46,7 +46,8 @@ module Ci terraform: 'tfplan.json', cluster_applications: 'gl-cluster-applications.json', requirements: 'requirements.json', - coverage_fuzzing: 'gl-coverage-fuzzing.json' + coverage_fuzzing: 'gl-coverage-fuzzing.json', + api_fuzzing: 'gl-api-fuzzing-report.json' }.freeze INTERNAL_TYPES = { @@ -65,11 +66,8 @@ module Ci cluster_applications: :gzip, lsif: :zip, - # All these file formats use `raw` as we need to store them uncompressed - # for Frontend to fetch the files and do analysis - # When they will be only used by backend, they can be `gzipped`. - accessibility: :raw, - codequality: :raw, + # Security reports and license scanning reports are raw artifacts + # because they used to be fetched by the frontend, but this is not the case anymore. sast: :raw, secret_detection: :raw, dependency_scanning: :raw, @@ -77,16 +75,24 @@ module Ci dast: :raw, license_management: :raw, license_scanning: :raw, + + # All these file formats use `raw` as we need to store them uncompressed + # for Frontend to fetch the files and do analysis + # When they will be only used by backend, they can be `gzipped`. + accessibility: :raw, + codequality: :raw, performance: :raw, browser_performance: :raw, load_performance: :raw, terraform: :raw, requirements: :raw, - coverage_fuzzing: :raw + coverage_fuzzing: :raw, + api_fuzzing: :raw }.freeze DOWNLOADABLE_TYPES = %w[ accessibility + api_fuzzing archive cobertura codequality @@ -194,7 +200,8 @@ module Ci requirements: 22, ## EE-specific coverage_fuzzing: 23, ## EE-specific browser_performance: 24, ## EE-specific - load_performance: 25 ## EE-specific + load_performance: 25, ## EE-specific + api_fuzzing: 26 ## EE-specific } # `file_location` indicates where actual files are stored. @@ -283,6 +290,15 @@ module Ci max_size&.megabytes.to_i end + def to_deleted_object_attrs + { + file_store: file_store, + store_dir: file.store_dir.to_s, + file: file_identifier, + pick_up_at: expire_at || Time.current + } + end + private def set_size diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 47eba685afe..684b6387ab1 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -27,6 +27,13 @@ module Ci sha_attribute :source_sha sha_attribute :target_sha + # Ci::CreatePipelineService returns Ci::Pipeline so this is the only place + # where we can pass additional information from the service. This accessor + # is used for storing the processed CI YAML contents for linting purposes. + # There is an open issue to address this: + # https://gitlab.com/gitlab-org/gitlab/-/issues/259010 + attr_accessor :merged_yaml + belongs_to :project, inverse_of: :all_pipelines belongs_to :user belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' @@ -42,6 +49,7 @@ module Ci has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :latest_statuses_ordered_by_stage, -> { latest.order(:stage_idx, :stage) }, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline + has_many :latest_statuses, -> { latest }, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :processables, class_name: 'Ci::Processable', foreign_key: :commit_id, inverse_of: :pipeline has_many :bridges, class_name: 'Ci::Bridge', foreign_key: :commit_id, inverse_of: :pipeline has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline @@ -577,11 +585,11 @@ module Ci end def retried - @retried ||= (statuses.order(id: :desc) - statuses.latest) + @retried ||= (statuses.order(id: :desc) - latest_statuses) end def coverage - coverage_array = statuses.latest.map(&:coverage).compact + coverage_array = latest_statuses.map(&:coverage).compact if coverage_array.size >= 1 '%.2f' % (coverage_array.reduce(:+) / coverage_array.size) end @@ -821,16 +829,28 @@ module Ci end def same_family_pipeline_ids - if ::Gitlab::Ci::Features.child_of_child_pipeline_enabled?(project) - ::Gitlab::Ci::PipelineObjectHierarchy.new( - base_and_ancestors(same_project: true), options: { same_project: true } - ).base_and_descendants.select(:id) - else - # If pipeline is a child of another pipeline, include the parent - # and the siblings, otherwise return only itself and children. - parent = parent_pipeline || self - [parent.id] + parent.child_pipelines.pluck(:id) - end + ::Gitlab::Ci::PipelineObjectHierarchy.new( + base_and_ancestors(same_project: true), options: { same_project: true } + ).base_and_descendants.select(:id) + end + + def build_with_artifacts_in_self_and_descendants(name) + builds_in_self_and_descendants + .ordered_by_pipeline # find job in hierarchical order + .with_downloadable_artifacts + .find_by_name(name) + end + + def builds_in_self_and_descendants + Ci::Build.latest.where(pipeline: self_and_descendants) + end + + # Without using `unscoped`, caller scope is also included into the query. + # Using `unscoped` here will be redundant after Rails 6.1 + def self_and_descendants + ::Gitlab::Ci::PipelineObjectHierarchy + .new(self.class.unscoped.where(id: id), options: { same_project: true }) + .base_and_descendants end def bridge_triggered? @@ -875,7 +895,7 @@ module Ci end def builds_with_coverage - builds.with_coverage + builds.latest.with_coverage end def has_reports?(reports_scope) -- cgit v1.2.1