diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 344dde3d046242462cc60d3c4f70c62df95c72c1..67dcf47436b4eacbbbac0c6df6ff834c018dc37b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -13,6 +13,7 @@ class Build < Ci::Processable include Ci::TrackEnvironmentUsage include EachBatch include Ci::Taggable + include ChronicDurationAttribute extend ::Gitlab::Utils::Override @@ -140,11 +141,19 @@ class Build < Ci::Processable delegate :google_play_integration, to: :project delegate :diffblue_cover_integration, to: :project delegate :ensure_persistent_ref, to: :pipeline - delegate :enable_debug_trace!, to: :metadata serialize :options # rubocop:disable Cop/ActiveRecordSerialize serialize :yaml_variables, coder: Gitlab::Serializer::Ci::Variables # rubocop:disable Cop/ActiveRecordSerialize + chronic_duration_attr_reader :timeout_human_readable, :timeout + + enum :timeout_source, { + unknown_timeout_source: 1, + project_timeout_source: 2, + runner_timeout_source: 3, + job_timeout_source: 4 + } + delegate :name, to: :project, prefix: true validates :coverage, numericality: true, allow_blank: true @@ -169,11 +178,6 @@ class Build < Ci::Processable ) end - scope :with_exposed_artifacts, -> do - joins(:metadata).merge(Ci::BuildMetadata.with_exposed_artifacts) - .includes(:metadata, :job_artifacts_metadata) - end - scope :with_artifacts_not_expired, -> { with_downloadable_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.current) } scope :with_pipeline_locked_artifacts, -> { joins(:pipeline).where('pipeline.locked': Ci::Pipeline.lockeds[:artifacts_locked]) } scope :last_month, -> { where('created_at > ?', Date.today - 1.month) } @@ -246,6 +250,10 @@ def with_preloads preload(:job_artifacts_archive, :job_artifacts, :tags, project: [:namespace]) end + # TODO: need to propagate attributes for prototype_id + # TODO: do the same for Ci::Bridge. + # TODO: the setter methods from metadatable will fail here! Need to explicitly + # propagate the metadata or prototype/info records. def clone_accessors %i[pipeline project ref tag options name allow_failure stage_idx @@ -345,8 +353,8 @@ def supported_keyset_orderings # rubocop:enable CodeReuse/ServiceClass # - after_transition pending: :running do |build| - build.ensure_metadata.update_timeout_state + after_transition pending: :running do |build| # rubocop:disable Sytle/SymbolProc -- reason: old code + build.update_timeout_state end after_transition pending: :running do |build| @@ -447,12 +455,6 @@ def auto_retry_allowed? auto_retry.allowed? end - def exit_code=(value) - return unless value - - ensure_metadata.exit_code = value.to_i.clamp(0, Gitlab::Database::MAX_SMALLINT_VALUE) - end - def auto_retry_expected? failed? && auto_retry_allowed? end @@ -1097,7 +1099,7 @@ def max_test_cases_per_report def debug_mode? # perform the check on both sides in case the runner version is old - metadata&.debug_trace_enabled? || + debug_trace_enabled? || Gitlab::Utils.to_boolean(variables['CI_DEBUG_SERVICES']&.value, default: false) || Gitlab::Utils.to_boolean(variables['CI_DEBUG_TRACE']&.value, default: false) end diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb index 2b15920d0f91829ea993b4533342999ac3a34e69..b487aa9af4d3f17d071fa74fce5f6bfa58aff40a 100644 --- a/app/models/ci/build_metadata.rb +++ b/app/models/ci/build_metadata.rb @@ -43,22 +43,8 @@ class BuildMetadata < Ci::ApplicationRecord end scope :with_interruptible, -> { where(interruptible: true) } - scope :with_exposed_artifacts, -> { where(has_exposed_artifacts: true) } - enum :timeout_source, { - unknown_timeout_source: 1, - project_timeout_source: 2, - runner_timeout_source: 3, - job_timeout_source: 4 - } - - def update_timeout_state - timeout = timeout_with_highest_precedence - - return unless timeout - - update(timeout: timeout.value, timeout_source: timeout.source) - end + enum :timeout_source, ::Ci::Build.timeout_sources def enable_debug_trace! self.debug_trace_enabled = true @@ -71,37 +57,5 @@ def enable_debug_trace! def set_build_project self.project_id ||= build.project_id end - - def timeout_with_highest_precedence - [(job_timeout || project_timeout), runner_timeout].compact.min_by(&:value) - end - - def project_timeout - strong_memoize(:project_timeout) do - BuildTimeout.new(project&.build_timeout, :project_timeout_source) - end - end - - def job_timeout - return unless build.options - - strong_memoize(:job_timeout) do - if timeout_from_options = build.options[:job_timeout] - BuildTimeout.new(timeout_from_options, :job_timeout_source) - end - end - end - - def runner_timeout - return unless runner_timeout_set? - - strong_memoize(:runner_timeout) do - BuildTimeout.new(build.runner.maximum_timeout, :runner_timeout_source) - end - end - - def runner_timeout_set? - build.runner&.maximum_timeout.to_i > 0 - end end end diff --git a/app/models/ci/job_prototype.rb b/app/models/ci/job_prototype.rb new file mode 100644 index 0000000000000000000000000000000000000000..ba3a5d0973533dba3e01caab472804b50fa8ed2c --- /dev/null +++ b/app/models/ci/job_prototype.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +module Ci + class JobPrototype < Ci::ApplicationRecord + include Ci::Partitionable + + self.table_name = :p_ci_job_prototypes + self.primary_key = :id + + # IMPORTANT: append new attributes at the end of this list. Do not change the order! + # Order is important for the checksum calculation. + # TODO: consider including a lot more data initially and scale back later if needed. + # E.g. `when`, `allow_failure`, `coverage_regex`, and other attributes outside `options` + # could be unique across many jobs. Later we could consider dropping columns from `p_ci_builds` + # while retaining the unique data. + CONFIG_ATTRIBUTES = [ + :options, + :yaml_variables, + :id_tokens, + :secrets, + :interruptible + ].freeze + + partitionable scope: ->(_) { Ci::Pipeline.current_partition_value }, partitioned: true + + # TODO: add schema that includes `options` and `id_tokens`, `secrets`, etc. + # validates :config, json_schema: { filename: 'job_prototype_config' } + + # TODO: add other validations + + attribute :config, ::Gitlab::Database::Type::SymbolizedJsonb.new + + scope :with_interruptible, -> { where(interruptible: true) } + + scope :scoped_job, -> do + where(arel_table[:job_id].eq(Ci::Build.arel_table[:id])) + .where(arel_table[:partition_id].eq(Ci::Build.arel_table[:partition_id])) + end + + def self.find_or_initialize_for_seed(partition_id:, project_id:, config:) + config, checksum = sanitize_and_checksum(config) + + finder_attributes = { + partition_id: partition_id, + project_id: project_id, + checksum: checksum + } + + prototype = find_by(**finder_attributes) + prototype || new(finder_attributes.merge(config: config, interruptible: config[:interruptible])) + end + + def self.sanitize_and_checksum(config) + sanitized_config = config.symbolize_keys.slice(*CONFIG_ATTRIBUTES) + + checksum = saitized_config + .then { |data| Gitlab::Json.dump(data) } + .then { |data| Digest::SHA256.hexdigest(data) } + + [sanitized_config, checksum] + end + + # POC NOTES: commenting these to see if we can fix specs without having these methods. + # The prototype must be immutable so these should not be used. + # Consider maybe adding a spec helper that implements `update_config` + # + # def options=(value) + # update_config(options: value) + # end + + # def yaml_variables=(value) + # update_config(yaml_variables: value) + # end + + private + + # POC NOTES: this should only be used in specs, if necessary. + def update_config(attrs) + new_attrs = (config || {}).merge(attrs) + new_config, new_checksum = sanitize_and_checksum(new_attrs) + + self.config = new_config + self.checksum = new_checksum + end + end +end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index b4e698f95b73f2366b71f14eef6d2cf0790f0b5b..51efb09667bc1d6b14c33ca2408ff0f0ca33000f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -1256,7 +1256,7 @@ def has_archive_artifacts? end def has_exposed_artifacts? - complete? && builds.latest.with_exposed_artifacts.exists? + complete? && builds.latest.select_with_exposed_artifacts.any? end def has_erasable_artifacts? diff --git a/app/models/ci/processable.rb b/app/models/ci/processable.rb index 304fb01f6ecb4ee2f75e2bfb27d84bb224d105bd..4f07f80e098a8e99bb390bb7c60ce32272318e85 100644 --- a/app/models/ci/processable.rb +++ b/app/models/ci/processable.rb @@ -17,6 +17,7 @@ class Processable < ::CommitStatus has_one :sourced_pipeline, class_name: 'Ci::Sources::Pipeline', foreign_key: :source_job_id, inverse_of: :source_job has_one :trigger, through: :pipeline + belongs_to :prototype, class_name: 'Ci::JobPrototype', autosave: true belongs_to :resource_group, class_name: 'Ci::ResourceGroup', inverse_of: :processables accepts_nested_attributes_for :needs @@ -36,14 +37,24 @@ class Processable < ::CommitStatus where('NOT EXISTS (?)', needs) end + # TODO: check if query performance remain acceptable scope :interruptible, -> do - joins(:metadata).merge(Ci::BuildMetadata.with_interruptible) + joins(:metadata, :prototype) + .merge(Ci::BuildMetadata.with_interruptible) + .or(Ci::JobPrototype.with_interruptible) end + # TODO: check if query performance remain acceptable scope :not_interruptible, -> do - joins(:metadata).where.not( - Ci::BuildMetadata.table_name => { id: Ci::BuildMetadata.scoped_build.with_interruptible.select(:id) } - ) + joins(:metadata, :prototype) + .where.not( + Ci::BuildMetadata.table_name => { + id: Ci::BuildMetadata.scoped_build.with_interruptible.select(Ci::BuildMetadata.arel_table[:id]) + }) + .where.not( + Ci::JobPrototype.table_name => { + job_id: Ci::JobPrototype.scoped_job.with_interruptible.select(Ci::JobPrototype.arel_table[:job_id]) + }) end state_machine :status do @@ -264,8 +275,10 @@ def manual_job? self.when == 'manual' end + # POC NOTES: the `playble?` condition embeds the `!archived?` check. + # This will make `manual_confirmation` a processing data. def manual_confirmation_message - options[:manual_confirmation] if manual_job? + options[:manual_confirmation] if manual_job? && playable? end private diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index 5497d22afdb51e375f3f61a75dbd8191ff2a2c38..e0004ffaba8702d5fafa7a6dfd016264957c1d50 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -7,6 +7,9 @@ module Ci # module Metadatable extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize + + ForbiddenWriteError = Class.new(StandardError) included do has_one :metadata, @@ -19,25 +22,43 @@ module Metadatable accepts_nested_attributes_for :metadata - delegate :timeout, to: :metadata, prefix: true, allow_nil: true - delegate :interruptible, to: :metadata, prefix: false, allow_nil: true delegate :environment_auto_stop_in, to: :metadata, prefix: false, allow_nil: true - delegate :id_tokens, to: :metadata, allow_nil: true - delegate :exit_code, to: :metadata, allow_nil: true before_validation :ensure_metadata, on: :create scope :with_project_and_metadata, -> do joins(:metadata).includes(:metadata).preload(:project) end + + def self.select_with_exposed_artifacts + includes(:metadata, :prototype).select(&:has_exposed_artifacts?) + end end def has_exposed_artifacts? - !!metadata&.has_exposed_artifacts? + options.dig(:artifacts, :expose_as).present? end def ensure_metadata - metadata || build_metadata(project: project) + metadata || + (can_write_metadata? && build_metadata(project: project)) + end + + def timeout_human_readable + (use_prototype? && super) || metadata&.timeout_human_readable + end + + def timeout_value + (use_prototype? && self.timeout) || metadata&.timeout + end + + def update_timeout_state + timeout = timeout_with_highest_precedence + + return unless timeout + + self.update(timeout: timeout.value, timeout_source: timeout.source) + ensure_metadata.update(timeout: timeout.value, timeout_source: timeout.source) if can_write_metadata? end def degenerated? @@ -47,22 +68,36 @@ def degenerated? def degenerate! self.class.transaction do self.update!(options: nil, yaml_variables: nil) - self.needs.all.delete_all - self.metadata&.destroy + self.prototype&.destroy! + self.metadata&.destroy! yield if block_given? end end + def exit_code + (use_prototype? && self[:exit_code]) || metadata&.exit_code + end + + def exit_code=(value) + return unless value + + safe_value = value.to_i.clamp(0, Gitlab::Database::MAX_SMALLINT_VALUE) + + self[:exit_code] = safe_value + ensure_metadata.exit_code = safe_value if can_write_metadata? + end + def options - read_metadata_attribute(:options, :config_options, {}) + read_metadata_attribute(:options, :config_options, :options, {}) end def yaml_variables - read_metadata_attribute(:yaml_variables, :config_variables, []) + read_metadata_attribute(:yaml_variables, :config_variables, :yaml_variables, []) end def options=(value) write_metadata_attribute(:options, :config_options, value) + return unless can_write_metadata? ensure_metadata.tap do |metadata| # Store presence of exposed artifacts in build metadata to make it easier to query @@ -76,41 +111,98 @@ def yaml_variables=(value) end def interruptible - metadata&.interruptible + (use_prototype? && prototype&.interruptible) || metadata&.interruptible end def interruptible=(value) - ensure_metadata.interruptible = value + ensure_metadata.interruptible = value if can_write_metadata? + end + + def id_tokens + read_metadata_attribute(nil, :id_tokens, :id_tokens, {}) end def id_tokens? - metadata&.id_tokens.present? + id_tokens.present? end def id_tokens=(value) - ensure_metadata.id_tokens = value + ensure_metadata.id_tokens = value if can_write_metadata? + end + + def enable_debug_trace! + update!(debug_trace_enabled: true) + ensure_metadata.enable_debug_trace! if can_write_metadata? + end + + def debug_trace_enabled? + (use_prototype? && self[:debug_trace_enabled]) || metadata&.debug_trace_enabled? end def enqueue_immediately? - !!options[:enqueue_immediately] + # We moved this to Redis in a different MR. end def set_enqueue_immediately! - # ensures that even if `config_options: nil` in the database we set the - # new value correctly. - self.options = options.merge(enqueue_immediately: true) + # We moved this to Redis in a different MR. end private - def read_metadata_attribute(legacy_key, metadata_key, default_value = nil) - read_attribute(legacy_key) || metadata&.read_attribute(metadata_key) || default_value + def read_metadata_attribute(legacy_key, metadata_key, prototype_key, default_value = nil) + (legacy_key && read_attribute(legacy_key)) || + (use_prototype? && prototype&.config && prototype.config[prototype_key]) || + metadata&.read_attribute(metadata_key) || + default_value end def write_metadata_attribute(legacy_key, metadata_key, value) + return unless can_write_metadata? + ensure_metadata.write_attribute(metadata_key, value) write_attribute(legacy_key, nil) end + + # ----- TIMEOUT METHODS ----- + def timeout_with_highest_precedence + [job_timeout || project_timeout, runner_timeout].compact.min_by(&:value) + end + + def project_timeout + Ci::BuildMetadata::BuildTimeout.new(project&.build_timeout, :project_timeout_source) + end + strong_memoize_attr :project_timeout + + def job_timeout + return unless options + + timeout_from_options = options[:job_timeout] + return unless timeout_from_options + + Ci::BuildMetadata::BuildTimeout.new(timeout_from_options, :job_timeout_source) if timeout_from_options + end + strong_memoize_attr :job_timeout + + def runner_timeout + return unless runner_timeout_set? + + Ci::BuildMetadata::BuildTimeout.new(runner.maximum_timeout, :runner_timeout_source) + end + strong_memoize_attr :runner_timeout + + def runner_timeout_set? + runner&.maximum_timeout.to_i > 0 + end + + def can_write_metadata? + Feature.disabled?(:stop_writing_ci_builds_metadata, project) + end + strong_memoize_attr :can_write_metadata? + + def use_prototype? + prototype.present? && Feature.enabled?(:read_from_ci_job_prototypes, project) + end + strong_memoize_attr :use_prototype? end end diff --git a/app/models/concerns/ci/partitionable/testing.rb b/app/models/concerns/ci/partitionable/testing.rb index 284cf32d2519532503d741d6b15f22339b06b804..1cca8f31a6c086c0043ea6f800df9b1c64b5f9c7 100644 --- a/app/models/concerns/ci/partitionable/testing.rb +++ b/app/models/concerns/ci/partitionable/testing.rb @@ -22,6 +22,7 @@ module Testing Ci::JobAnnotation Ci::JobArtifact Ci::JobArtifactReport + Ci::JobPrototype Ci::JobVariable Ci::Pipeline Ci::PendingBuild diff --git a/app/presenters/ci/build_metadata_presenter.rb b/app/presenters/ci/build_metadata_presenter.rb index 7465e6daab045bd9a8442f7656b0f572818c46c5..c3248dcde9c80ee623bbc6808aba7d1f13daaa13 100644 --- a/app/presenters/ci/build_metadata_presenter.rb +++ b/app/presenters/ci/build_metadata_presenter.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Ci + # TODO: this needs to present a job object and leverage the methods in Metadatable + # so we can automatically switch the data source (prototypes/builds). class BuildMetadataPresenter < Gitlab::View::Presenter::Delegated TIMEOUT_SOURCES = { unknown_timeout_source: nil, @@ -9,14 +11,14 @@ class BuildMetadataPresenter < Gitlab::View::Presenter::Delegated job_timeout_source: 'job' }.freeze - presents ::Ci::BuildMetadata, as: :metadata + presents ::Ci::Build, as: :build delegator_override :timeout_source def timeout_source - return unless metadata.timeout_source? + return unless build.timeout_source? - TIMEOUT_SOURCES[metadata.timeout_source.to_sym] || - metadata.timeout_source + TIMEOUT_SOURCES[build.timeout_source.to_sym] || + build.timeout_source end end end diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index df1d249ac894c94bc9dbed4fb2afa963a6579529..b4168120c865ecb92e5203b99f8b3c11c146b96d 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -8,7 +8,7 @@ class BuildDetailsEntity < Ci::JobEntity expose :stuck?, as: :stuck expose :user, using: UserEntity expose :runner, using: RunnerEntity - expose :metadata, using: BuildMetadataEntity + expose :metadata, using: BuildMetadataEntity # TODO: add Ci::JobInfoPresenter as abstraction expose :pipeline, using: Ci::PipelineEntity expose :deployment_status, if: ->(*) { build.deployment_job? } do @@ -113,6 +113,7 @@ class BuildDetailsEntity < Ci::JobEntity private alias_method :build, :object + alias_method :metadata, :object def build_failed_issue_options { title: "Job Failed ##{build.id}", diff --git a/app/serializers/build_metadata_entity.rb b/app/serializers/build_metadata_entity.rb index 6242ee8957d5c7b121b0595c8c9cea981f4d9b25..e81d8390ec168c155700b26f96f100ba236f54fa 100644 --- a/app/serializers/build_metadata_entity.rb +++ b/app/serializers/build_metadata_entity.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true class BuildMetadataEntity < Grape::Entity + # POC NOTES: we are leveraging Metadatable methods here to change the source + # of the data when we switch between builds and prototypes. expose :timeout_human_readable expose :timeout_source do |metadata| - metadata.present.timeout_source + # TODO: `metadata` here refers to the job actually. We should improve the readability. + metadata.present.timeout_source # calls Ci::BuildMetadataPresenter#timeout_source end end diff --git a/app/services/ci/find_exposed_artifacts_service.rb b/app/services/ci/find_exposed_artifacts_service.rb index abbeb101be2db37057156037bee3d352393bf56c..eb26bfe7722265b8022633675d576af999d24122 100644 --- a/app/services/ci/find_exposed_artifacts_service.rb +++ b/app/services/ci/find_exposed_artifacts_service.rb @@ -15,7 +15,7 @@ class FindExposedArtifactsService < ::BaseService def for_pipeline(pipeline, limit: MAX_EXPOSED_ARTIFACTS) results = [] - pipeline.builds.latest.with_exposed_artifacts.find_each do |job| + pipeline.builds.latest.select_with_exposed_artifacts.each do |job| if job_exposed_artifacts = for_job(job) results << job_exposed_artifacts end diff --git a/app/services/ci/pipelines/add_job_service.rb b/app/services/ci/pipelines/add_job_service.rb index dfbb37cf0dc17502386b6b4bc2dbf0951654fc82..2da6e8a8cbdcba2d3ceec7285fc92d94d75e447d 100644 --- a/app/services/ci/pipelines/add_job_service.rb +++ b/app/services/ci/pipelines/add_job_service.rb @@ -43,6 +43,7 @@ def assign_pipeline_attributes(job) # update metadata since it might have been lazily initialised before this call # metadata is present on `Ci::Processable` + # POC NOTES: remove this when removing `stop_writing_ci_builds_metadata` feature flag if job.respond_to?(:metadata) && job.metadata job.metadata.project = pipeline.project job.metadata.partition_id = pipeline.partition_id diff --git a/config/feature_flags/wip/read_from_ci_job_prototypes.yml b/config/feature_flags/wip/read_from_ci_job_prototypes.yml new file mode 100644 index 0000000000000000000000000000000000000000..cdac92d42c7d3dfe9ed432be3c5b95e575c923ba --- /dev/null +++ b/config/feature_flags/wip/read_from_ci_job_prototypes.yml @@ -0,0 +1,10 @@ +--- +name: read_from_ci_job_prototypes +description: +feature_issue_url: +introduced_by_url: +rollout_issue_url: +milestone: '18.1' +group: group::ci platform +type: wip +default_enabled: false diff --git a/config/feature_flags/wip/stop_writing_ci_builds_metadata.yml b/config/feature_flags/wip/stop_writing_ci_builds_metadata.yml new file mode 100644 index 0000000000000000000000000000000000000000..5cb05013c69f232697881400eb1e4128c77669a1 --- /dev/null +++ b/config/feature_flags/wip/stop_writing_ci_builds_metadata.yml @@ -0,0 +1,10 @@ +--- +name: stop_writing_ci_builds_metadata +description: +feature_issue_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193948 +rollout_issue_url: +milestone: '18.1' +group: group::ci platform +type: wip +default_enabled: false diff --git a/config/feature_flags/wip/write_to_ci_job_prototypes.yml b/config/feature_flags/wip/write_to_ci_job_prototypes.yml new file mode 100644 index 0000000000000000000000000000000000000000..63c5bcb6af746c0e08232727cefc41e8bea0332a --- /dev/null +++ b/config/feature_flags/wip/write_to_ci_job_prototypes.yml @@ -0,0 +1,10 @@ +--- +name: write_to_ci_job_prototypes +description: +feature_issue_url: +introduced_by_url: +rollout_issue_url: +milestone: '18.1' +group: group::ci platform +type: wip +default_enabled: false diff --git a/config/initializers/postgres_partitioning.rb b/config/initializers/postgres_partitioning.rb index a26ca2cf2472b250ef082ab96c5a92b35db249de..cc2b53bd4db5a2dd564887458caa553add9fba3e 100644 --- a/config/initializers/postgres_partitioning.rb +++ b/config/initializers/postgres_partitioning.rb @@ -25,6 +25,7 @@ Ci::JobAnnotation, Ci::JobArtifact, Ci::JobArtifactReport, + Ci::JobPrototype, Ci::Pipeline, Ci::PipelineConfig, Ci::PipelineVariable, diff --git a/db/docs/p_ci_job_prototypes.yml b/db/docs/p_ci_job_prototypes.yml new file mode 100644 index 0000000000000000000000000000000000000000..f4765949ab6bea0a6da8f4ae69fc871570bc8f27 --- /dev/null +++ b/db/docs/p_ci_job_prototypes.yml @@ -0,0 +1,14 @@ +--- +table_name: p_ci_job_prototypes +classes: +- Ci::JobPrototype +feature_categories: +- continuous_integration +description: Unique job configurations across pipelines +introduced_by_url: +milestone: '18.1' +gitlab_schema: gitlab_ci +sharding_key: + project_id: projects +table_size: small +schema_inconsistencies: \ No newline at end of file diff --git a/db/migrate/20250609131551_create_p_ci_job_prototypes.rb b/db/migrate/20250609131551_create_p_ci_job_prototypes.rb new file mode 100644 index 0000000000000000000000000000000000000000..24f82a705c1b68157f70ef49f8a13978a4a5737f --- /dev/null +++ b/db/migrate/20250609131551_create_p_ci_job_prototypes.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreatePCiJobPrototypes < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + disable_ddl_transaction! + milestone '18.1' + + TABLE_NAME = :p_ci_job_prototypes + UNIQUE_INDEX = :index_ci_job_prototypes_on_partition_id_and_checksum + + def up + create_table TABLE_NAME, primary_key: [:id, :partition_id], + options: 'PARTITION BY LIST (partition_id)', if_not_exists: true do |t| + t.bigserial :id, null: false + t.bigint :partition_id, null: false + t.bigint :project_id, null: false + t.boolean :interruptible, default: false, null: false, index: true + t.jsonb :config, default: {}, null: false + # TODO: checksum should be bytea: https://docs.gitlab.com/development/database/sha1_as_binary/ + t.text :checksum, limit: 255, null: false + t.timestamps_with_timezone null: false + end + + add_concurrent_partitioned_index( + TABLE_NAME, [:partition_id, :project_id, :checksum], unique: true, name: UNIQUE_INDEX) + + # TODO: need to do the proper partitioning steps + with_lock_retries do + connection.execute(<<~SQL) + LOCK TABLE ONLY #{TABLE_NAME} IN ACCESS EXCLUSIVE MODE; + SQL + + connection.execute(<<~SQL) + CREATE TABLE IF NOT EXISTS gitlab_partitions_dynamic.ci_job_prototypes_100 + PARTITION OF #{TABLE_NAME} + FOR VALUES IN (100); + SQL + end + end + + def down + remove_concurrent_partitioned_index_by_name(TABLE_NAME, UNIQUE_INDEX) + + drop_table TABLE_NAME + end +end diff --git a/db/migrate/20250609151252_add_prototype_id_to_ci_builds.rb b/db/migrate/20250609151252_add_prototype_id_to_ci_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..aee2753c907cf502fb762e8bc51d603162294ef5 --- /dev/null +++ b/db/migrate/20250609151252_add_prototype_id_to_ci_builds.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddPrototypeIdToCiBuilds < Gitlab::Database::Migration[2.3] + milestone '18.1' + + def up + # TODO: create association table + add_column :p_ci_builds, :prototype_id, :bigint # rubocop:disable Migration/PreventAddingColumns -- PoC: we will create association table + end + + def down + remove_column :p_ci_builds, :prototype_id + end +end diff --git a/db/migrate/20250620121504_add_intrinsic_data_to_p_ci_builds.rb b/db/migrate/20250620121504_add_intrinsic_data_to_p_ci_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..50f9a55fadacb3dbecdd3bf2d7a7245d1d95f463 --- /dev/null +++ b/db/migrate/20250620121504_add_intrinsic_data_to_p_ci_builds.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddIntrinsicDataToPCiBuilds < Gitlab::Database::Migration[2.3] + # disable_ddl_transaction! + milestone '18.1' + + def up + # rubocop:disable Migration/PreventAddingColumns -- PoC: we will create association table + add_column :p_ci_builds, :debug_trace_enabled, :boolean + add_column :p_ci_builds, :timeout, :integer + add_column :p_ci_builds, :timeout_source, :integer, limit: 2 + add_column :p_ci_builds, :exit_code, :integer, limit: 2 + # TODO: we need to handle limit by truncating new data and existing migrated data. + add_column :p_ci_builds, :downstream_errors, :text, limit: 255 + # rubocop:enable Migration/PreventAddingColumns end + end + + def down + remove_column :p_ci_builds, :debug_trace_enabled + remove_column :p_ci_builds, :timeout + remove_column :p_ci_builds, :timeout_source + remove_column :p_ci_builds, :exit_code + remove_column :p_ci_builds, :downstream_errors + end +end diff --git a/db/schema_migrations/20250609131551 b/db/schema_migrations/20250609131551 new file mode 100644 index 0000000000000000000000000000000000000000..7d6abfaa8c193d4ae82fcf7f8e6501f562b987f7 --- /dev/null +++ b/db/schema_migrations/20250609131551 @@ -0,0 +1 @@ +b38064da0f3e9378d1479040b859fefe1b3ceac3be757fe9531ecf9568a3217a \ No newline at end of file diff --git a/db/schema_migrations/20250609151252 b/db/schema_migrations/20250609151252 new file mode 100644 index 0000000000000000000000000000000000000000..8aba27618791227653f40af77ef5c63762f8673e --- /dev/null +++ b/db/schema_migrations/20250609151252 @@ -0,0 +1 @@ +674d381e0dc94c2a8d791c2ee5ed4bd333dfc3b68795251449ac19308152dc88 \ No newline at end of file diff --git a/db/schema_migrations/20250620121504 b/db/schema_migrations/20250620121504 new file mode 100644 index 0000000000000000000000000000000000000000..a2c36746df4a2c52bd333e030cbfd6c0d2097fd3 --- /dev/null +++ b/db/schema_migrations/20250620121504 @@ -0,0 +1 @@ +14bbacc5f209a0739098c6267828bf8fb409d5789823a54ddf3dac12230db8c1 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1daf7885ab574454002b181d465a028bf9af4cdd..9bc9c3ace5ebab8e2c59f83fde0fe759e04d4069 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4648,6 +4648,12 @@ CREATE TABLE p_ci_builds ( user_id bigint, execution_config_id bigint, upstream_pipeline_partition_id bigint, + prototype_id bigint, + debug_trace_enabled boolean, + timeout integer, + timeout_source smallint, + exit_code smallint, + downstream_errors character varying, CONSTRAINT check_1e2fbd1b39 CHECK ((lock_version IS NOT NULL)), CONSTRAINT check_9aa9432137 CHECK ((project_id IS NOT NULL)) ) @@ -4728,6 +4734,19 @@ CREATE TABLE p_ci_job_artifacts ( ) PARTITION BY LIST (partition_id); +CREATE TABLE p_ci_job_prototypes ( + id bigint NOT NULL, + partition_id bigint NOT NULL, + project_id bigint NOT NULL, + interruptible boolean DEFAULT false NOT NULL, + config jsonb DEFAULT '{}'::jsonb NOT NULL, + checksum text NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + CONSTRAINT check_c43b283d76 CHECK ((char_length(checksum) <= 255)) +) +PARTITION BY LIST (partition_id); + CREATE TABLE p_ci_pipeline_variables ( key character varying NOT NULL, value text, @@ -18919,6 +18938,15 @@ CREATE SEQUENCE p_ci_job_annotations_id_seq ALTER SEQUENCE p_ci_job_annotations_id_seq OWNED BY p_ci_job_annotations.id; +CREATE SEQUENCE p_ci_job_prototypes_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE p_ci_job_prototypes_id_seq OWNED BY p_ci_job_prototypes.id; + CREATE SEQUENCE p_ci_workloads_id_seq START WITH 1 INCREMENT BY 1 @@ -27802,6 +27830,8 @@ ALTER TABLE ONLY p_catalog_resource_sync_events ALTER COLUMN id SET DEFAULT next ALTER TABLE ONLY p_ci_builds_metadata ALTER COLUMN id SET DEFAULT nextval('ci_builds_metadata_id_seq'::regclass); +ALTER TABLE ONLY p_ci_job_prototypes ALTER COLUMN id SET DEFAULT nextval('p_ci_job_prototypes_id_seq'::regclass); + ALTER TABLE ONLY p_ci_workloads ALTER COLUMN id SET DEFAULT nextval('p_ci_workloads_id_seq'::regclass); ALTER TABLE ONLY p_knowledge_graph_enabled_namespaces ALTER COLUMN id SET DEFAULT nextval('p_knowledge_graph_enabled_namespaces_id_seq'::regclass); @@ -30557,6 +30587,9 @@ ALTER TABLE ONLY p_ci_job_artifact_reports ALTER TABLE ONLY p_ci_job_artifacts ADD CONSTRAINT p_ci_job_artifacts_pkey PRIMARY KEY (id, partition_id); +ALTER TABLE ONLY p_ci_job_prototypes + ADD CONSTRAINT p_ci_job_prototypes_pkey PRIMARY KEY (id, partition_id); + ALTER TABLE ONLY p_ci_pipeline_variables ADD CONSTRAINT p_ci_pipeline_variables_pkey PRIMARY KEY (id, partition_id); @@ -34470,6 +34503,8 @@ CREATE INDEX index_ci_instance_runner_monthly_usages_on_project_and_month ON ci_ CREATE UNIQUE INDEX index_ci_instance_variables_on_key ON ci_instance_variables USING btree (key); +CREATE UNIQUE INDEX index_ci_job_prototypes_on_partition_id_and_checksum ON ONLY p_ci_job_prototypes USING btree (partition_id, project_id, checksum); + CREATE INDEX index_ci_job_token_authorizations_on_origin_project_id ON ci_job_token_authorizations USING btree (origin_project_id); CREATE INDEX index_ci_job_token_group_scope_links_on_added_by_id ON ci_job_token_group_scope_links USING btree (added_by_id); @@ -36474,6 +36509,8 @@ CREATE INDEX index_p_ci_job_annotations_on_project_id ON ONLY p_ci_job_annotatio CREATE INDEX index_p_ci_job_artifact_reports_on_project_id ON ONLY p_ci_job_artifact_reports USING btree (project_id); +CREATE INDEX index_p_ci_job_prototypes_on_interruptible ON ONLY p_ci_job_prototypes USING btree (interruptible); + CREATE INDEX index_p_ci_pipeline_variables_on_project_id ON ONLY p_ci_pipeline_variables USING btree (project_id); CREATE INDEX index_p_ci_pipelines_config_on_project_id ON ONLY p_ci_pipelines_config USING btree (project_id); diff --git a/ee/app/models/concerns/ee/ci/metadatable.rb b/ee/app/models/concerns/ee/ci/metadatable.rb index 015f2c72d10bc355cd92e2a6f20c65c6fe67a1f5..11c82935ca78596207e7d6f7cc655cc5e1b8de0c 100644 --- a/ee/app/models/concerns/ee/ci/metadatable.rb +++ b/ee/app/models/concerns/ee/ci/metadatable.rb @@ -5,16 +5,16 @@ module Ci module Metadatable extend ActiveSupport::Concern - prepended do - delegate :secrets, to: :metadata, allow_nil: true + def secrets + (use_prototype? && prototype&.config&.dig(:secrets)) || metadata&.secrets end def secrets? - !!metadata&.secrets? + secrets.present? end def secrets=(value) - ensure_metadata.secrets = value + ensure_metadata.secrets = value if can_write_metadata? end end end diff --git a/ee/spec/services/ci/register_job_service_spec.rb b/ee/spec/services/ci/register_job_service_spec.rb index c96af99035b0101f5e1b5a656583e3c7c1cfaf6b..564a0156c9a145f0eca374947c28ab6392f040be 100644 --- a/ee/spec/services/ci/register_job_service_spec.rb +++ b/ee/spec/services/ci/register_job_service_spec.rb @@ -235,9 +235,7 @@ rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - pending_build.metadata.update!( - id_tokens: { 'TEST_ID_TOKEN' => { aud: 'https://client.test' } } - ) + allow(pending_build).to receive(:id_tokens).and_return('TEST_ID_TOKEN' => { aud: 'https://client.test' }) create(:ci_variable, project: project, key: 'VAULT_SERVER_URL', value: 'https://vault.example.com') end @@ -259,7 +257,7 @@ allow_next_found_instance_of(Ci::Build) do |pending_build| expect(pending_build).to receive(:run!).ordered.and_call_original expect(pending_build).to receive(:ensure_metadata).ordered.and_return(stubbed_build_metadata) - expect(stubbed_build_metadata).to receive(:update_timeout_state).ordered + expect(pending_build).to receive(:update_timeout_state).ordered expect(pending_build).to receive(:job_jwt_variables).ordered.and_call_original end diff --git a/lib/api/entities/ci/job_request/runner_info.rb b/lib/api/entities/ci/job_request/runner_info.rb index 96336a1080eb1a54dcd008325a3dbf2d2a96b3a5..501873f0a9cfbfe0e7eb83c095360383a38ba33c 100644 --- a/lib/api/entities/ci/job_request/runner_info.rb +++ b/lib/api/entities/ci/job_request/runner_info.rb @@ -5,7 +5,7 @@ module Entities module Ci module JobRequest class RunnerInfo < Grape::Entity - expose :metadata_timeout, as: :timeout + expose :timeout_value, as: :timeout expose :runner_session_url end end diff --git a/lib/ci/job_token/jwt.rb b/lib/ci/job_token/jwt.rb index 09a7c11e89b984a997161972fd86d483e54782b9..9bbd1aa89e7ce1790effd9a7df1f558938e01705 100644 --- a/lib/ci/job_token/jwt.rb +++ b/lib/ci/job_token/jwt.rb @@ -73,7 +73,7 @@ def subject_type end def expire_time(job) - ttl = [::JSONWebToken::Token::DEFAULT_EXPIRE_TIME, job.metadata_timeout.to_i].max + ttl = [::JSONWebToken::Token::DEFAULT_EXPIRE_TIME, job.timeout_value.to_i].max Time.current + ttl + LEEWAY end diff --git a/lib/gitlab/ci/build/step.rb b/lib/gitlab/ci/build/step.rb index 0fb87943a6f5a55db17bf753336a8c4bd7bebdf5..c4ddc8b34c3b375228ce0f26d100f688b2fb6581 100644 --- a/lib/gitlab/ci/build/step.rb +++ b/lib/gitlab/ci/build/step.rb @@ -15,7 +15,7 @@ class << self def from_commands(job) self.new(:script).tap do |step| step.script = job.options[:before_script].to_a + job.options[:script].to_a - step.timeout = job.metadata_timeout + step.timeout = job.timeout_value step.when = WHEN_ON_SUCCESS end end @@ -25,7 +25,7 @@ def from_release(job) self.new(:release).tap do |step| step.script = Gitlab::Ci::Build::Releaser.new(job: job).script - step.timeout = job.metadata_timeout + step.timeout = job.timeout_value step.when = WHEN_ON_SUCCESS end end @@ -36,7 +36,7 @@ def from_after_script(job) self.new(:after_script).tap do |step| step.script = after_script - step.timeout = job.metadata_timeout + step.timeout = job.timeout_value step.when = WHEN_ALWAYS step.allow_failure = true end diff --git a/lib/gitlab/ci/jwt.rb b/lib/gitlab/ci/jwt.rb index 67c34f744fd123d13f06a7af9e80229b86a54a35..d46f8126cf86ced54a4b291e0a95804eb55f6da3 100644 --- a/lib/gitlab/ci/jwt.rb +++ b/lib/gitlab/ci/jwt.rb @@ -7,7 +7,7 @@ class Jwt < JwtBase DEFAULT_EXPIRE_TIME = 60 * 5 def self.for_build(build) - self.new(build, ttl: build.metadata_timeout).encoded + self.new(build, ttl: build.timeout_value).encoded end def initialize(build, ttl:) diff --git a/lib/gitlab/ci/jwt_v2.rb b/lib/gitlab/ci/jwt_v2.rb index 0f601155e01e0dff7fa5f9a42f6b16e2d12e168a..c40150508b467a957db2b12e5b602264436bad1b 100644 --- a/lib/gitlab/ci/jwt_v2.rb +++ b/lib/gitlab/ci/jwt_v2.rb @@ -12,7 +12,7 @@ class JwtV2 < Jwt def self.for_build( build, aud:, sub_components: [:project_path, :ref_type, :ref], target_audience: nil) - new(build, ttl: build.metadata_timeout, aud: aud, sub_components: sub_components, + new(build, ttl: build.timeout_value, aud: aud, sub_components: sub_components, target_audience: target_audience).encoded end diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index c47fbebca5fbed4c69c98543e3644f1d81c96b83..df5b8ad90872d4a9eb2928288dd39c8243916335 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -65,7 +65,7 @@ def errors strong_memoize_attr :errors def attributes - @seed_attributes + attrs = @seed_attributes .deep_merge(pipeline_attributes) .deep_merge(rules_attributes) .deep_merge(allow_failure_criteria_attributes) @@ -73,7 +73,9 @@ def attributes .deep_merge(runner_tags) .deep_merge(build_execution_config_attribute) .deep_merge(scoped_user_id_attribute) - .except(:stage) + + attrs = add_prototype_association(attrs) + attrs.except(:stage) end def bridge? @@ -229,6 +231,32 @@ def calculate_yaml_variables! from: @context.root_variables, to: @job_variables, inheritance: @root_variables_inheritance ) end + + # TODO: this currently fires N+1 queries when checking for existing prototypes. + # We should eventually look into querying prototypes in bulk. + def add_prototype_association(attrs) + return attrs if Feature.disabled?(:write_to_ci_job_prototypes, @pipeline.project) + + prototype_attributes = ::Ci::JobPrototype::CONFIG_ATTRIBUTES + config = if Feature.enabled?(:stop_writing_ci_builds_metadata, @pipeline.project) + # To stop writing to metadata we need to delete the attributes being moved to prototypes + # Once stop_writing_ci_builds_metadata is deleted we shoul encapsulate CONFIG_ATTRIBUTES in + # a method inside Ci::JobPrototype#extract_attributes! + prototype_attributes.each_with_object({}) do |attribute, hash| + hash[attribute] = attrs.delete(attribute) + end + else + # copy attributes from metadata without deleting original ones + attrs.slice(*prototype_attributes) + end + + prototype = @context.find_or_build_prototype( + partition_id: attrs.fetch(:partition_id), + project_id: attrs.fetch(:project).id, + config: config) + + attrs.merge(prototype: prototype) + end end end end diff --git a/lib/gitlab/ci/pipeline/seed/context.rb b/lib/gitlab/ci/pipeline/seed/context.rb index 0c2a312e87051c59d1132a75efbdf6cb87570ff2..cf6f137ce18d3d0c1ef5769ad41a3696414a9a6a 100644 --- a/lib/gitlab/ci/pipeline/seed/context.rb +++ b/lib/gitlab/ci/pipeline/seed/context.rb @@ -12,6 +12,7 @@ def initialize(pipeline, root_variables: [], logger: nil) @root_variables = root_variables @logger = logger || build_logger @execution_configs = {} + @prototypes = {} end def find_or_build_execution_config(attributes) @@ -23,6 +24,13 @@ def find_or_build_execution_config(attributes) ) end + def find_or_build_prototype(partition_id:, project_id:, config:) + @prototypes[config] ||= ::Ci::JobPrototype.find_or_initialize_for_seed( + partition_id: partition_id, + project_id: project_id, + config: config) + end + private def build_logger diff --git a/spec/factories/ci/processable.rb b/spec/factories/ci/processable.rb index d5a149b670cee06112638acdd5fb94794082f4fd..e1f5df51fc2ed1c1e67e0f55874aea3f145b9906 100644 --- a/spec/factories/ci/processable.rb +++ b/spec/factories/ci/processable.rb @@ -11,6 +11,15 @@ scheduling_type { 'stage' } partition_id { pipeline.partition_id } + interruptible { false } + + prototype do + ::Ci::JobPrototype.find_or_initialize_for_seed( + partition_id: partition_id, + project_id: project.id, + config: { options: options, yaml_variables: yaml_variables, interruptible: interruptible }) + end + options do {} end diff --git a/spec/lib/ci/job_token/jwt_spec.rb b/spec/lib/ci/job_token/jwt_spec.rb index da82a9496595c366bda509111bda15d37cb303e4..1fb4d749daf8b29247994ec535a7e2aefd543387 100644 --- a/spec/lib/ci/job_token/jwt_spec.rb +++ b/spec/lib/ci/job_token/jwt_spec.rb @@ -128,16 +128,16 @@ it 'returns expiration time with leeway' do freeze_time do - allow(job).to receive(:metadata_timeout).and_return(2.hours) + allow(job).to receive(:timeout_value).and_return(2.hours) expected_time = Time.current + 2.hours + described_class::LEEWAY expect(expire_time).to eq(expected_time) end end - it 'uses default expire time when metadata_timeout is smaller' do + it 'uses default expire time when timeout_value is smaller' do freeze_time do - allow(job).to receive(:metadata_timeout).and_return(1.minute) + allow(job).to receive(:timeout_value).and_return(1.minute) expected_time = Time.current + ::JSONWebToken::Token::DEFAULT_EXPIRE_TIME + diff --git a/spec/lib/gitlab/ci/jwt_spec.rb b/spec/lib/gitlab/ci/jwt_spec.rb index 20eac17c6bfb188f847aa7403e2c50d47990cee5..74a19d4152bddadffdcc65a2ddcc46744e85a4f5 100644 --- a/spec/lib/gitlab/ci/jwt_spec.rb +++ b/spec/lib/gitlab/ci/jwt_spec.rb @@ -244,7 +244,7 @@ end it 'generates JWT for the given job with ttl equal to build timeout' do - expect(build).to receive(:metadata_timeout).and_return(3_600) + expect(build).to receive(:timeout_value).and_return(3_600) payload, _headers = JWT.decode(jwt, rsa_key.public_key, true, { algorithm: 'RS256' }) ttl = payload["exp"] - payload["iat"] @@ -253,7 +253,7 @@ end it 'generates JWT for the given job with default ttl if build timeout is not set' do - expect(build).to receive(:metadata_timeout).and_return(nil) + expect(build).to receive(:timeout_value).and_return(nil) payload, _headers = JWT.decode(jwt, rsa_key.public_key, true, { algorithm: 'RS256' }) ttl = payload["exp"] - payload["iat"] diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 85fa6af6fad14e6ae7ce49342ee22c223a403e59..ece1f83e4cc9b86f36e6a17f40d06d95882d62bb 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -26,6 +26,7 @@ it { is_expected.to belong_to(:build) } it { is_expected.to belong_to(:project) } + # TODO: move to metadatable concern specs describe '#update_timeout_state' do subject { metadata } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 04dab9db0b9a3cd01a24a7d53559d65c817660eb..9afe3305ab4150265f1c1291b3ad835580c0fab2 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -5333,23 +5333,19 @@ def run_job_without_exception context 'when metadata has debug_trace_enabled true' do before do - build.metadata.update!(debug_trace_enabled: true) + build.enable_debug_trace! end it { is_expected.to eq true } end context 'when metadata has debug_trace_enabled false' do - before do - build.metadata.update!(debug_trace_enabled: false) - end - it { is_expected.to eq false } end context 'when metadata does not exist' do before do - build.metadata.destroy! + build.metadata&.destroy! end it { is_expected.to eq false } @@ -5360,10 +5356,7 @@ def run_job_without_exception let(:exit_code) { 1 } let(:options) { {} } - before do - build.options.merge!(options) - build.save! - end + let!(:build) { create(:ci_build, options: options) } subject(:drop_with_exit_code) do build.drop_with_exit_code!(:unknown_failure, exit_code) @@ -5371,7 +5364,7 @@ def run_job_without_exception it 'correctly sets the exit code' do expect { drop_with_exit_code } - .to change { build.reload.metadata&.exit_code }.from(nil).to(1) + .to change { build.reload.exit_code }.from(nil).to(1) end shared_examples 'drops the build without changing allow_failure' do @@ -5467,7 +5460,7 @@ def run_job_without_exception it 'wraps around to max size of a signed smallint' do expect { drop_with_exit_code } - .to change { build.reload.metadata&.exit_code }.from(nil).to(32767) + .to change { build.reload.exit_code }.from(nil).to(32767) end end end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 9f2895afc9eaa6f38e475908ab724bd77e6bfe3b..c2c8de89e39d18486447a93bef4f42188943aa0e 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -217,12 +217,12 @@ let(:expected_steps) do [{ 'name' => 'script', 'script' => %w[echo], - 'timeout' => job.metadata_timeout, + 'timeout' => job.timeout_value, 'when' => 'on_success', 'allow_failure' => false }, { 'name' => 'after_script', 'script' => %w[ls date], - 'timeout' => job.metadata_timeout, + 'timeout' => job.timeout_value, 'when' => 'always', 'allow_failure' => true }] end