summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2018-09-02 16:35:15 +0200
committerKamil Trzciński <ayufan@ayufan.eu>2019-01-04 16:38:17 +0100
commit0103d5be960e620342c67436ddd64ca9e729d7a8 (patch)
treeb4f2cdd4a5ef8f6c906d71c674cc5f13f791c889
parentb647ad96f6e7cd1e6ca078635bb1ea49ee7d589f (diff)
downloadgitlab-ce-0103d5be960e620342c67436ddd64ca9e729d7a8.tar.gz
Add config_options|variables to BuildMetadatakamil-refactor-ci-builds-v5
These are data columns that store runtime configuration of build needed to execute it on runner and within pipeline. The definition of this data is that once used, and when no longer needed (due to retry capability) they can be freely removed. They use `jsonb` on PostgreSQL, and `text` on MySQL (due to lacking support for json datatype on old enough version).
-rw-r--r--app/models/ci/build.rb52
-rw-r--r--app/models/ci/build_metadata.rb12
-rw-r--r--app/services/ci/retry_build_service.rb2
-rw-r--r--app/views/projects/ci/lints/_create.html.haml9
-rw-r--r--config/initializers/ar_mysql_jsonb_support.rb31
-rw-r--r--db/fixtures/development/14_pipelines.rb5
-rw-r--r--db/migrate/20181219145521_add_options_to_build_metadata.rb15
-rw-r--r--db/schema.rb2
-rw-r--r--doc/development/migration_style_guide.md25
-rw-r--r--lib/gitlab/ci/build/step.rb1
-rw-r--r--lib/gitlab/ci/config/entry/job.rb7
-rw-r--r--lib/gitlab/ci/yaml_processor.rb1
-rw-r--r--lib/gitlab/import_export/import_export.yml1
-rw-r--r--lib/gitlab/import_export/relation_factory.rb1
-rw-r--r--lib/gitlab/utils.rb10
-rw-r--r--lib/serializers/json.rb34
-rw-r--r--spec/factories/ci/builds.rb56
-rw-r--r--spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb2
-rw-r--r--spec/features/projects/environments/environments_spec.rb6
-rw-r--r--spec/features/projects/pipelines/pipeline_spec.rb2
-rw-r--r--spec/features/projects/pipelines/pipelines_spec.rb15
-rw-r--r--spec/javascripts/fixtures/jobs.rb3
-rw-r--r--spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb5
-rw-r--r--spec/lib/gitlab/ci/build/step_spec.rb9
-rw-r--r--spec/lib/gitlab/ci/config/entry/global_spec.rb2
-rw-r--r--spec/lib/gitlab/ci/config/entry/job_spec.rb11
-rw-r--r--spec/lib/gitlab/ci/config/entry/jobs_spec.rb2
-rw-r--r--spec/lib/gitlab/ci/pipeline/seed/build_spec.rb5
-rw-r--r--spec/lib/gitlab/ci/yaml_processor_spec.rb22
-rw-r--r--spec/lib/gitlab/utils_spec.rb16
-rw-r--r--spec/lib/serializers/json_spec.rb102
-rw-r--r--spec/migrations/delete_inconsistent_internal_id_records_spec.rb7
-rw-r--r--spec/models/ci/build_metadata_spec.rb4
-rw-r--r--spec/models/ci/build_spec.rb333
-rw-r--r--spec/requests/api/runner_spec.rb8
-rw-r--r--spec/services/ci/process_pipeline_service_spec.rb6
-rw-r--r--spec/services/ci/register_job_service_spec.rb7
37 files changed, 597 insertions, 234 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index aeb35538d67..dc6f8ae1a7f 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -8,10 +8,15 @@ module Ci
include ObjectStorage::BackgroundMove
include Presentable
include Importable
+ include IgnorableColumn
include Gitlab::Utils::StrongMemoize
include Deployable
include HasRef
+ BuildArchivedError = Class.new(StandardError)
+
+ ignore_column :commands
+
belongs_to :project, inverse_of: :builds
belongs_to :runner
belongs_to :trigger_request
@@ -31,7 +36,7 @@ module Ci
has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
end
- has_one :metadata, class_name: 'Ci::BuildMetadata'
+ has_one :metadata, class_name: 'Ci::BuildMetadata', autosave: true
has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build
accepts_nested_attributes_for :runner_session
@@ -273,11 +278,14 @@ module Ci
# degenerated build is one that cannot be run by Runner
def degenerated?
- self.options.nil?
+ self.options.blank?
end
def degenerate!
- self.update!(options: nil, yaml_variables: nil, commands: nil)
+ Build.transaction do
+ self.update!(options: nil, yaml_variables: nil)
+ self.metadata&.destroy
+ end
end
def archived?
@@ -624,11 +632,23 @@ module Ci
end
def when
- read_attribute(:when) || build_attributes_from_config[:when] || 'on_success'
+ read_attribute(:when) || 'on_success'
+ end
+
+ def options
+ read_metadata_attribute(:options, :config_options, {})
end
def yaml_variables
- read_attribute(:yaml_variables) || build_attributes_from_config[:yaml_variables] || []
+ read_metadata_attribute(:yaml_variables, :config_variables, [])
+ end
+
+ def options=(value)
+ write_metadata_attribute(:options, :config_options, value)
+ end
+
+ def yaml_variables=(value)
+ write_metadata_attribute(:yaml_variables, :config_variables, value)
end
def user_variables
@@ -904,8 +924,11 @@ module Ci
# have the old integer only format. This method returns the retry option
# normalized as a hash in 11.5+ format.
def normalized_retry
- value = options&.dig(:retry)
- value.is_a?(Integer) ? { max: value } : value.to_h
+ strong_memoize(:normalized_retry) do
+ value = options&.dig(:retry)
+ value = value.is_a?(Integer) ? { max: value } : value.to_h
+ value.with_indifferent_access
+ end
end
def build_attributes_from_config
@@ -929,5 +952,20 @@ module Ci
def project_destroyed?
project.pending_delete?
end
+
+ def read_metadata_attribute(legacy_key, metadata_key, default_value = nil)
+ read_attribute(legacy_key) || metadata&.read_attribute(metadata_key) || default_value
+ end
+
+ def write_metadata_attribute(legacy_key, metadata_key, value)
+ # save to metadata or this model depending on the state of feature flag
+ if Feature.enabled?(:ci_build_metadata_config)
+ ensure_metadata.write_attribute(metadata_key, value)
+ write_attribute(legacy_key, nil)
+ else
+ write_attribute(legacy_key, value)
+ metadata&.write_attribute(metadata_key, nil)
+ end
+ end
end
end
diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb
index 9d588b862bd..38390f49217 100644
--- a/app/models/ci/build_metadata.rb
+++ b/app/models/ci/build_metadata.rb
@@ -13,8 +13,12 @@ module Ci
belongs_to :build, class_name: 'Ci::Build'
belongs_to :project
+ before_create :set_build_project
+
validates :build, presence: true
- validates :project, presence: true
+
+ serialize :config_options, Serializers::JSON # rubocop:disable Cop/ActiveRecordSerialize
+ serialize :config_variables, Serializers::JSON # rubocop:disable Cop/ActiveRecordSerialize
chronic_duration_attr_reader :timeout_human_readable, :timeout
@@ -33,5 +37,11 @@ module Ci
update(timeout: timeout, timeout_source: timeout_source)
end
+
+ private
+
+ def set_build_project
+ self.project_id ||= self.build.project_id
+ end
end
end
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index 218f1e63d08..fab8a179843 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -2,7 +2,7 @@
module Ci
class RetryBuildService < ::BaseService
- CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
+ CLONE_ACCESSORS = %i[pipeline project ref tag options name
allow_failure stage stage_id stage_idx trigger_request
yaml_variables when environment coverage_regex
description tag_list protected].freeze
diff --git a/app/views/projects/ci/lints/_create.html.haml b/app/views/projects/ci/lints/_create.html.haml
index 30bf1384b22..b4c18374220 100644
--- a/app/views/projects/ci/lints/_create.html.haml
+++ b/app/views/projects/ci/lints/_create.html.haml
@@ -13,20 +13,23 @@
%tbody
- @stages.each do |stage|
- @builds.select { |build| build[:stage] == stage }.each do |build|
+ - job = @jobs[build[:name].to_sym]
%tr
%td #{stage.capitalize} Job - #{build[:name]}
%td
- %pre= build[:commands]
+ %pre= job[:before_script].to_a.join('\n')
+ %pre= job[:script].to_a.join('\n')
+ %pre= job[:after_script].to_a.join('\n')
%br
%b Tag list:
= build[:tag_list].to_a.join(", ")
%br
%b Only policy:
- = @jobs[build[:name].to_sym][:only].to_a.join(", ")
+ = job[:only].to_a.join(", ")
%br
%b Except policy:
- = @jobs[build[:name].to_sym][:except].to_a.join(", ")
+ = job[:except].to_a.join(", ")
%br
%b Environment:
= build[:environment]
diff --git a/config/initializers/ar_mysql_jsonb_support.rb b/config/initializers/ar_mysql_jsonb_support.rb
new file mode 100644
index 00000000000..63a0b05119a
--- /dev/null
+++ b/config/initializers/ar_mysql_jsonb_support.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'active_record/connection_adapters/abstract_mysql_adapter'
+require 'active_record/connection_adapters/mysql/schema_definitions'
+
+# MySQL (5.6) and MariaDB (10.1) are currently supported versions within GitLab,
+# Since they do not support native `json` datatype we force to emulate it as `text`
+
+if Gitlab::Database.mysql?
+ module ActiveRecord
+ module ConnectionAdapters
+ class AbstractMysqlAdapter
+ JSON_DATASIZE = 1.megabyte
+
+ NATIVE_DATABASE_TYPES.merge!(
+ json: { name: "text", limit: JSON_DATASIZE },
+ jsonb: { name: "text", limit: JSON_DATASIZE }
+ )
+ end
+
+ module MySQL
+ module ColumnMethods
+ # We add `jsonb` helper, as `json` is already defined for `MySQL` since Rails 5
+ def jsonb(*args, **options)
+ args.each { |name| column(name, :json, options) }
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb
index bdc0a2db7db..db043e39d2c 100644
--- a/db/fixtures/development/14_pipelines.rb
+++ b/db/fixtures/development/14_pipelines.rb
@@ -102,14 +102,15 @@ class Gitlab::Seeder::Pipelines
[]
end
-
def create_pipeline!(project, ref, commit)
project.ci_pipelines.create!(sha: commit.id, ref: ref, source: :push)
end
def build_create!(pipeline, opts = {})
attributes = job_attributes(pipeline, opts)
- .merge(commands: '$ build command')
+
+ attributes[:options] ||= {}
+ attributes[:options][:script] = 'build command'
Ci::Build.create!(attributes).tap do |build|
# We need to set build trace and artifacts after saving a build
diff --git a/db/migrate/20181219145521_add_options_to_build_metadata.rb b/db/migrate/20181219145521_add_options_to_build_metadata.rb
new file mode 100644
index 00000000000..dc9569babc2
--- /dev/null
+++ b/db/migrate/20181219145521_add_options_to_build_metadata.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddOptionsToBuildMetadata < ActiveRecord::Migration[5.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def change
+ add_column :ci_builds_metadata, :config_options, :jsonb
+ add_column :ci_builds_metadata, :config_variables, :jsonb
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 97daf8ee617..12e4ed6d627 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -374,6 +374,8 @@ ActiveRecord::Schema.define(version: 20190103140724) do
t.integer "project_id", null: false
t.integer "timeout"
t.integer "timeout_source", default: 1, null: false
+ t.jsonb "config_options"
+ t.jsonb "config_variables"
t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id", unique: true, using: :btree
t.index ["project_id"], name: "index_ci_builds_metadata_on_project_id", using: :btree
end
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md
index d0a054c3290..23aa318ef91 100644
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@@ -325,6 +325,31 @@ This ensures all timestamps have a time zone specified. This in turn means exist
suddenly use a different timezone when the system's timezone changes. It also makes it very clear which
timezone was used in the first place.
+## Storing JSON in database
+
+The Rails 5 natively supports `JSONB` (binary JSON) column type.
+Example migration adding this column:
+
+```ruby
+class AddOptionsToBuildMetadata < ActiveRecord::Migration[5.0]
+ DOWNTIME = false
+
+ def change
+ add_column :ci_builds_metadata, :config_options, :jsonb
+ end
+end
+```
+
+On MySQL the `JSON` and `JSONB` is translated to `TEXT 1MB`, as `JSONB` is PostgreSQL only feature.
+
+For above reason you have to use a serializer to provide a translation layer
+in order to support PostgreSQL and MySQL seamlessly:
+
+```ruby
+class BuildMetadata
+ serialize :config_options, Serializers::JSON # rubocop:disable Cop/ActiveRecordSerialize
+end
+```
## Testing
diff --git a/lib/gitlab/ci/build/step.rb b/lib/gitlab/ci/build/step.rb
index d587c896712..7fcabc035ac 100644
--- a/lib/gitlab/ci/build/step.rb
+++ b/lib/gitlab/ci/build/step.rb
@@ -15,7 +15,6 @@ module Gitlab
def from_commands(job)
self.new(:script).tap do |step|
step.script = job.options[:before_script].to_a + job.options[:script].to_a
- step.script = job.commands.split("\n") if step.script.empty?
step.timeout = job.metadata_timeout
step.when = WHEN_ON_SUCCESS
end
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 50942fbdb40..3239743bfff 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -95,7 +95,7 @@ module Gitlab
helpers :before_script, :script, :stage, :type, :after_script,
:cache, :image, :services, :only, :except, :variables,
- :artifacts, :commands, :environment, :coverage, :retry,
+ :artifacts, :environment, :coverage, :retry,
:parallel
attributes :script, :tags, :allow_failure, :when, :dependencies,
@@ -121,10 +121,6 @@ module Gitlab
@config.merge(to_hash.compact)
end
- def commands
- (before_script_value.to_a + script_value.to_a).join("\n")
- end
-
def manual_action?
self.when == 'manual'
end
@@ -156,7 +152,6 @@ module Gitlab
{ name: name,
before_script: before_script_value,
script: script_value,
- commands: commands,
image: image_value,
services: services_value,
stage: stage_value,
diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb
index 172926b8ab0..15097188061 100644
--- a/lib/gitlab/ci/yaml_processor.rb
+++ b/lib/gitlab/ci/yaml_processor.rb
@@ -33,7 +33,6 @@ module Gitlab
{ stage_idx: @stages.index(job[:stage]),
stage: job[:stage],
- commands: job[:commands],
tag_list: job[:tags] || [],
name: job[:name].to_s,
allow_failure: job[:ignore],
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index 9fb1ae9f64b..a1a374cef4a 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -148,6 +148,7 @@ excluded_attributes:
- :when
- :artifacts_file
- :artifacts_metadata
+ - :commands
push_event_payload:
- :event_id
project_badges:
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index a0f4dcfb772..bce12103cce 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -150,6 +150,7 @@ module Gitlab
if BUILD_MODELS.include?(@relation_name)
@relation_hash.delete('trace') # old export files have trace
@relation_hash.delete('token')
+ @relation_hash.delete('commands')
imported_object
elsif @relation_name == :merge_requests
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index a81cee0d6d2..99fa65e0e90 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -115,5 +115,15 @@ module Gitlab
string_or_array.split(',').map(&:strip)
end
+
+ def deep_indifferent_access(data)
+ if data.is_a?(Array)
+ data.map(&method(:deep_indifferent_access))
+ elsif data.is_a?(Hash)
+ data.with_indifferent_access
+ else
+ data
+ end
+ end
end
end
diff --git a/lib/serializers/json.rb b/lib/serializers/json.rb
new file mode 100644
index 00000000000..93cb192087a
--- /dev/null
+++ b/lib/serializers/json.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Serializers
+ # This serializer exports data as JSON,
+ # it is designed to be used with interwork compatibility between MySQL and PostgreSQL
+ # implementations, as used version of MySQL does not support native json type
+ #
+ # Secondly, the loader makes the resulting hash to have deep indifferent access
+ class JSON
+ class << self
+ def dump(obj)
+ # MySQL stores data as text
+ # look at ./config/initializers/ar_mysql_jsonb_support.rb
+ if Gitlab::Database.mysql?
+ obj = ActiveSupport::JSON.encode(obj)
+ end
+
+ obj
+ end
+
+ def load(data)
+ return if data.nil?
+
+ # On MySQL we store data as text
+ # look at ./config/initializers/ar_mysql_jsonb_support.rb
+ if Gitlab::Database.mysql?
+ data = ActiveSupport::JSON.decode(data)
+ end
+
+ Gitlab::Utils.deep_indifferent_access(data)
+ end
+ end
+ end
+end
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 07c1fc31152..bb3c0d6537d 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -7,7 +7,6 @@ FactoryBot.define do
stage_idx 0
ref 'master'
tag false
- commands 'ls -a'
protected false
created_at 'Di 29. Okt 09:50:00 CET 2013'
pending
@@ -15,7 +14,8 @@ FactoryBot.define do
options do
{
image: 'ruby:2.1',
- services: ['postgres']
+ services: ['postgres'],
+ script: ['ls -a']
}
end
@@ -28,7 +28,6 @@ FactoryBot.define do
pipeline factory: :ci_pipeline
trait :degenerated do
- commands nil
options nil
yaml_variables nil
end
@@ -95,33 +94,53 @@ FactoryBot.define do
trait :teardown_environment do
environment 'staging'
- options environment: { name: 'staging',
- action: 'stop',
- url: 'http://staging.example.com/$CI_JOB_NAME' }
+ options do
+ {
+ script: %w(ls),
+ environment: { name: 'staging',
+ action: 'stop',
+ url: 'http://staging.example.com/$CI_JOB_NAME' }
+ }
+ end
end
trait :deploy_to_production do
environment 'production'
- options environment: { name: 'production',
- url: 'http://prd.example.com/$CI_JOB_NAME' }
+ options do
+ {
+ script: %w(ls),
+ environment: { name: 'production',
+ url: 'http://prd.example.com/$CI_JOB_NAME' }
+ }
+ end
end
trait :start_review_app do
environment 'review/$CI_COMMIT_REF_NAME'
- options environment: { name: 'review/$CI_COMMIT_REF_NAME',
- url: 'http://staging.example.com/$CI_JOB_NAME',
- on_stop: 'stop_review_app' }
+ options do
+ {
+ script: %w(ls),
+ environment: { name: 'review/$CI_COMMIT_REF_NAME',
+ url: 'http://staging.example.com/$CI_JOB_NAME',
+ on_stop: 'stop_review_app' }
+ }
+ end
end
trait :stop_review_app do
name 'stop_review_app'
environment 'review/$CI_COMMIT_REF_NAME'
- options environment: { name: 'review/$CI_COMMIT_REF_NAME',
- url: 'http://staging.example.com/$CI_JOB_NAME',
- action: 'stop' }
+ options do
+ {
+ script: %w(ls),
+ environment: { name: 'review/$CI_COMMIT_REF_NAME',
+ url: 'http://staging.example.com/$CI_JOB_NAME',
+ action: 'stop' }
+ }
+ end
end
trait :allowed_to_fail do
@@ -142,7 +161,13 @@ FactoryBot.define do
trait :schedulable do
self.when 'delayed'
- options start_in: '1 minute'
+
+ options do
+ {
+ script: ['ls -a'],
+ start_in: '1 minute'
+ }
+ end
end
trait :actionable do
@@ -265,6 +290,7 @@ FactoryBot.define do
{
image: { name: 'ruby:2.1', entrypoint: '/bin/sh' },
services: ['postgres', { name: 'docker:stable-dind', entrypoint: '/bin/sh', command: 'sleep 30', alias: 'docker' }],
+ script: %w(echo),
after_script: %w(ls date),
artifacts: {
name: 'artifacts_file',
diff --git a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb
index 0272d300e06..0959f1b12f3 100644
--- a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb
+++ b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb
@@ -5,7 +5,7 @@ describe 'Merge request < User sees mini pipeline graph', :js do
let(:user) { project.creator }
let(:merge_request) { create(:merge_request, source_project: project, head_pipeline: pipeline) }
let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: 'master', status: 'running', sha: project.commit.id) }
- let(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') }
+ let(:build) { create(:ci_build, pipeline: pipeline, stage: 'test') }
before do
build.run
diff --git a/spec/features/projects/environments/environments_spec.rb b/spec/features/projects/environments/environments_spec.rb
index 89954d35f91..0c517d5f490 100644
--- a/spec/features/projects/environments/environments_spec.rb
+++ b/spec/features/projects/environments/environments_spec.rb
@@ -272,8 +272,7 @@ describe 'Environments page', :js do
create(:ci_build, :scheduled,
pipeline: pipeline,
name: 'delayed job',
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
let!(:deployment) do
@@ -304,8 +303,7 @@ describe 'Environments page', :js do
create(:ci_build, :expired_scheduled,
pipeline: pipeline,
name: 'delayed job',
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
it "shows 00:00:00 as the remaining time" do
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index a37ad9c3f43..4706c28bb3d 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -18,7 +18,7 @@ describe 'Pipeline', :js do
let!(:build_failed) do
create(:ci_build, :failed,
- pipeline: pipeline, stage: 'test', name: 'test', commands: 'test')
+ pipeline: pipeline, stage: 'test', name: 'test')
end
let!(:build_running) do
diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb
index 17772a35779..b75dee66592 100644
--- a/spec/features/projects/pipelines/pipelines_spec.rb
+++ b/spec/features/projects/pipelines/pipelines_spec.rb
@@ -109,8 +109,7 @@ describe 'Pipelines', :js do
context 'when pipeline is cancelable' do
let!(:build) do
create(:ci_build, pipeline: pipeline,
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
before do
@@ -140,8 +139,7 @@ describe 'Pipelines', :js do
context 'when pipeline is retryable' do
let!(:build) do
create(:ci_build, pipeline: pipeline,
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
before do
@@ -202,8 +200,7 @@ describe 'Pipelines', :js do
create(:ci_build, :manual,
pipeline: pipeline,
name: 'manual build',
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
before do
@@ -237,8 +234,7 @@ describe 'Pipelines', :js do
create(:ci_build, :scheduled,
pipeline: pipeline,
name: 'delayed job',
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
before do
@@ -262,8 +258,7 @@ describe 'Pipelines', :js do
create(:ci_build, :expired_scheduled,
pipeline: pipeline,
name: 'delayed job',
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
it "shows 00:00:00 as the remaining time" do
diff --git a/spec/javascripts/fixtures/jobs.rb b/spec/javascripts/fixtures/jobs.rb
index d6b5349594d..433bb690a1c 100644
--- a/spec/javascripts/fixtures/jobs.rb
+++ b/spec/javascripts/fixtures/jobs.rb
@@ -14,8 +14,7 @@ describe Projects::JobsController, '(JavaScript fixtures)', type: :controller do
create(:ci_build, :scheduled,
pipeline: pipeline,
name: 'delayed job',
- stage: 'test',
- commands: 'test')
+ stage: 'test')
end
render_views
diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb
index c7b272cd6ca..6ab126ad39a 100644
--- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb
+++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb
@@ -5,6 +5,11 @@ require 'spec_helper'
describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration, schema: 20180916011959 do
let(:migration) { described_class.new }
+ before do
+ # This migration was created before we introduced metadata configs
+ stub_feature_flags(ci_build_metadata_config: false)
+ end
+
let!(:internal_pipeline) { create(:ci_pipeline, source: :web) }
let(:pipelines) { [internal_pipeline, unknown_pipeline].map(&:id) }
diff --git a/spec/lib/gitlab/ci/build/step_spec.rb b/spec/lib/gitlab/ci/build/step_spec.rb
index cce4efaa069..e3136fc925e 100644
--- a/spec/lib/gitlab/ci/build/step_spec.rb
+++ b/spec/lib/gitlab/ci/build/step_spec.rb
@@ -18,13 +18,6 @@ describe Gitlab::Ci::Build::Step do
end
end
- context 'when commands are specified' do
- it_behaves_like 'has correct script' do
- let(:job) { create(:ci_build, :no_options, commands: "ls -la\ndate") }
- let(:script) { ['ls -la', 'date'] }
- end
- end
-
context 'when script option is specified' do
it_behaves_like 'has correct script' do
let(:job) { create(:ci_build, :no_options, options: { script: ["ls -la\necho aaa", "date"] }) }
@@ -62,7 +55,7 @@ describe Gitlab::Ci::Build::Step do
end
context 'when after_script is not empty' do
- let(:job) { create(:ci_build, options: { after_script: ['ls -la', 'date'] }) }
+ let(:job) { create(:ci_build, options: { script: ['bash'], after_script: ['ls -la', 'date'] }) }
it 'fabricates an object' do
expect(subject.name).to eq(:after_script)
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
index 61d78f86b51..941ef33c8a4 100644
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -153,7 +153,6 @@ describe Gitlab::Ci::Config::Entry::Global do
rspec: { name: :rspec,
script: %w[rspec ls],
before_script: %w(ls pwd),
- commands: "ls\npwd\nrspec\nls",
image: { name: 'ruby:2.2' },
services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }],
stage: 'test',
@@ -166,7 +165,6 @@ describe Gitlab::Ci::Config::Entry::Global do
spinach: { name: :spinach,
before_script: [],
script: %w[spinach],
- commands: 'spinach',
image: { name: 'ruby:2.2' },
services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }],
stage: 'test',
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index 8e32cede3b5..3d0b98eb238 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -255,7 +255,6 @@ describe Gitlab::Ci::Config::Entry::Job do
.to eq(name: :rspec,
before_script: %w[ls pwd],
script: %w[rspec],
- commands: "ls\npwd\nrspec",
stage: 'test',
ignore: false,
after_script: %w[cleanup],
@@ -264,16 +263,6 @@ describe Gitlab::Ci::Config::Entry::Job do
end
end
end
-
- describe '#commands' do
- let(:config) do
- { before_script: %w[ls pwd], script: 'rspec' }
- end
-
- it 'returns a string of commands concatenated with new line character' do
- expect(entry.commands).to eq "ls\npwd\nrspec"
- end
- end
end
describe '#manual_action?' do
diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb
index 1a2c30d3571..d97be76f0e0 100644
--- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb
@@ -65,14 +65,12 @@ describe Gitlab::Ci::Config::Entry::Jobs do
expect(entry.value).to eq(
rspec: { name: :rspec,
script: %w[rspec],
- commands: 'rspec',
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] },
except: {} },
spinach: { name: :spinach,
script: %w[spinach],
- commands: 'spinach',
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] },
diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index 2cf812b26dc..a700cfd4546 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -6,8 +6,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
let(:attributes) do
{ name: 'rspec',
- ref: 'master',
- commands: 'rspec' }
+ ref: 'master' }
end
subject do
@@ -18,7 +17,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
it 'returns hash attributes of a build' do
expect(subject.attributes).to be_a Hash
expect(subject.attributes)
- .to include(:name, :project, :ref, :commands)
+ .to include(:name, :project, :ref)
end
end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 441e8214181..63e1f167ce2 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -21,7 +21,6 @@ module Gitlab
stage: "test",
stage_idx: 1,
name: "rspec",
- commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [],
options: {
@@ -155,7 +154,6 @@ module Gitlab
builds:
[{ stage_idx: 1,
stage: "test",
- commands: "rspec",
tag_list: [],
name: "rspec",
allow_failure: false,
@@ -171,7 +169,6 @@ module Gitlab
builds:
[{ stage_idx: 2,
stage: "deploy",
- commands: "cap prod",
tag_list: [],
name: "prod",
allow_failure: false,
@@ -271,7 +268,7 @@ module Gitlab
end
it "return commands with scripts concencaced" do
- expect(subject[:commands]).to eq("global script\nscript")
+ expect(subject[:options][:before_script]).to eq(["global script"])
end
end
@@ -284,7 +281,7 @@ module Gitlab
end
it "return commands with scripts concencaced" do
- expect(subject[:commands]).to eq("local script\nscript")
+ expect(subject[:options][:before_script]).to eq(["local script"])
end
end
end
@@ -297,7 +294,7 @@ module Gitlab
end
it "return commands with scripts concencaced" do
- expect(subject[:commands]).to eq("script")
+ expect(subject[:options][:script]).to eq(["script"])
end
end
@@ -347,7 +344,6 @@ module Gitlab
stage: "test",
stage_idx: 1,
name: "rspec",
- commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [],
options: {
@@ -382,7 +378,6 @@ module Gitlab
stage: "test",
stage_idx: 1,
name: "rspec",
- commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [],
options: {
@@ -415,7 +410,6 @@ module Gitlab
stage: "test",
stage_idx: 1,
name: "rspec",
- commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [],
options: {
@@ -444,7 +438,6 @@ module Gitlab
stage: "test",
stage_idx: 1,
name: "rspec",
- commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [],
options: {
@@ -596,7 +589,7 @@ module Gitlab
it 'correctly extends rspec job' do
expect(config_processor.builds).to be_one
- expect(subject.dig(:commands)).to eq 'test'
+ expect(subject.dig(:options, :script)).to eq %w(test)
expect(subject.dig(:options, :image, :name)).to eq 'ruby:alpine'
end
end
@@ -622,7 +615,8 @@ module Gitlab
it 'correctly extends rspec job' do
expect(config_processor.builds).to be_one
- expect(subject.dig(:commands)).to eq "bundle install\nrspec"
+ expect(subject.dig(:options, :before_script)).to eq ["bundle install"]
+ expect(subject.dig(:options, :script)).to eq %w(rspec)
expect(subject.dig(:options, :image, :name)).to eq 'image:test'
expect(subject.dig(:when)).to eq 'always'
end
@@ -769,7 +763,6 @@ module Gitlab
stage: "test",
stage_idx: 1,
name: "rspec",
- commands: "pwd\nrspec",
coverage_regex: nil,
tag_list: [],
options: {
@@ -983,7 +976,6 @@ module Gitlab
stage: "test",
stage_idx: 1,
name: "normal_job",
- commands: "test",
coverage_regex: nil,
tag_list: [],
options: {
@@ -1031,7 +1023,6 @@ module Gitlab
stage: "build",
stage_idx: 0,
name: "job1",
- commands: "execute-script-for-job",
coverage_regex: nil,
tag_list: [],
options: {
@@ -1046,7 +1037,6 @@ module Gitlab
stage: "build",
stage_idx: 0,
name: "job2",
- commands: "execute-script-for-job",
coverage_regex: nil,
tag_list: [],
options: {
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index f5a4b7e2ebf..8f5029b3565 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -197,4 +197,20 @@ describe Gitlab::Utils do
end
end
end
+
+ describe '.deep_indifferent_access' do
+ let(:hash) do
+ { "variables" => [{ "key" => "VAR1", "value" => "VALUE2" }] }
+ end
+
+ subject { described_class.deep_indifferent_access(hash) }
+
+ it 'allows to access hash keys with symbols' do
+ expect(subject[:variables]).to be_a(Array)
+ end
+
+ it 'allows to access array keys with symbols' do
+ expect(subject[:variables].first[:key]).to eq('VAR1')
+ end
+ end
end
diff --git a/spec/lib/serializers/json_spec.rb b/spec/lib/serializers/json_spec.rb
new file mode 100644
index 00000000000..5d59d66e8b8
--- /dev/null
+++ b/spec/lib/serializers/json_spec.rb
@@ -0,0 +1,102 @@
+require 'fast_spec_helper'
+
+describe Serializers::JSON do
+ describe '.dump' do
+ let(:obj) { { key: "value" } }
+
+ subject { described_class.dump(obj) }
+
+ context 'when MySQL is used' do
+ before do
+ allow(Gitlab::Database).to receive(:adapter_name) { 'mysql2' }
+ end
+
+ it 'encodes as string' do
+ is_expected.to eq('{"key":"value"}')
+ end
+ end
+
+ context 'when PostgreSQL is used' do
+ before do
+ allow(Gitlab::Database).to receive(:adapter_name) { 'postgresql' }
+ end
+
+ it 'returns a hash' do
+ is_expected.to eq(obj)
+ end
+ end
+ end
+
+ describe '.load' do
+ let(:data_string) { '{"key":"value","variables":[{"key":"VAR1","value":"VALUE1"}]}' }
+ let(:data_hash) { JSON.parse(data_string) }
+
+ shared_examples 'having consistent accessor' do
+ it 'allows to access with symbols' do
+ expect(subject[:key]).to eq('value')
+ expect(subject[:variables].first[:key]).to eq('VAR1')
+ end
+
+ it 'allows to access with strings' do
+ expect(subject["key"]).to eq('value')
+ expect(subject["variables"].first["key"]).to eq('VAR1')
+ end
+ end
+
+ context 'when MySQL is used' do
+ before do
+ allow(Gitlab::Database).to receive(:adapter_name) { 'mysql2' }
+ end
+
+ context 'when loading a string' do
+ subject { described_class.load(data_string) }
+
+ it 'decodes a string' do
+ is_expected.to be_a(Hash)
+ end
+
+ it_behaves_like 'having consistent accessor'
+ end
+
+ context 'when loading a different type' do
+ subject { described_class.load({ key: 'hash' }) }
+
+ it 'raises an exception' do
+ expect { subject }.to raise_error(TypeError)
+ end
+ end
+
+ context 'when loading a nil' do
+ subject { described_class.load(nil) }
+
+ it 'returns nil' do
+ is_expected.to be_nil
+ end
+ end
+ end
+
+ context 'when PostgreSQL is used' do
+ before do
+ allow(Gitlab::Database).to receive(:adapter_name) { 'postgresql' }
+ end
+
+ context 'when loading a hash' do
+ subject { described_class.load(data_hash) }
+
+ it 'decodes a string' do
+ is_expected.to be_a(Hash)
+ end
+
+ it_behaves_like 'having consistent accessor'
+ end
+
+ context 'when loading a nil' do
+ subject { described_class.load(nil) }
+
+ it 'returns nil' do
+ is_expected.to be_nil
+ end
+ end
+ end
+ end
+end
diff --git a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
index 8c55daf0d37..51291cb362a 100644
--- a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
+++ b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
@@ -90,6 +90,13 @@ describe DeleteInconsistentInternalIdRecords, :migration do
context 'for ci_pipelines' do
let(:scope) { :ci_pipeline }
+
+ let(:create_models) do
+ create_list(:ci_empty_pipeline, 3, project: project1)
+ create_list(:ci_empty_pipeline, 3, project: project2)
+ create_list(:ci_empty_pipeline, 3, project: project3)
+ end
+
it_behaves_like 'deleting inconsistent internal_id records'
end
diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb
index 519968b9e48..016a5899eef 100644
--- a/spec/models/ci/build_metadata_spec.rb
+++ b/spec/models/ci/build_metadata_spec.rb
@@ -13,12 +13,12 @@ describe Ci::BuildMetadata do
end
let(:build) { create(:ci_build, pipeline: pipeline) }
- let(:build_metadata) { build.metadata }
+ let(:metadata) { build.metadata }
it_behaves_like 'having unique enum values'
describe '#update_timeout_state' do
- subject { build_metadata }
+ subject { metadata }
context 'when runner is not assigned to the job' do
it "doesn't change timeout value" do
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 7baf4d93804..7e9f62b7419 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1457,8 +1457,24 @@ describe Ci::Build do
context 'with retries max config option' do
subject { create(:ci_build, options: { retry: { max: 1 } }) }
- it 'returns the number of configured max retries' do
- expect(subject.retries_max).to eq 1
+ context 'when build_metadata_config is set' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: true)
+ end
+
+ it 'returns the number of configured max retries' do
+ expect(subject.retries_max).to eq 1
+ end
+ end
+
+ context 'when build_metadata_config is not set' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: false)
+ end
+
+ it 'returns the number of configured max retries' do
+ expect(subject.retries_max).to eq 1
+ end
end
end
@@ -1679,14 +1695,49 @@ describe Ci::Build do
let(:options) do
{
image: "ruby:2.1",
- services: [
- "postgres"
- ]
+ services: ["postgres"],
+ script: ["ls -a"]
}
end
it 'contains options' do
- expect(build.options).to eq(options)
+ expect(build.options).to eq(options.stringify_keys)
+ end
+
+ it 'allows to access with keys' do
+ expect(build.options[:image]).to eq('ruby:2.1')
+ end
+
+ it 'allows to access with strings' do
+ expect(build.options['image']).to eq('ruby:2.1')
+ end
+
+ context 'when ci_build_metadata_config is set' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: true)
+ end
+
+ it 'persist data in build metadata' do
+ expect(build.metadata.read_attribute(:config_options)).to eq(options.stringify_keys)
+ end
+
+ it 'does not persist data in build' do
+ expect(build.read_attribute(:options)).to be_nil
+ end
+ end
+
+ context 'when ci_build_metadata_config is disabled' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: false)
+ end
+
+ it 'persist data in build' do
+ expect(build.read_attribute(:options)).to eq(options.symbolize_keys)
+ end
+
+ it 'does not persist data in build metadata' do
+ expect(build.metadata.read_attribute(:config_options)).to be_nil
+ end
end
end
@@ -2030,56 +2081,6 @@ describe Ci::Build do
end
end
- describe '#when' do
- subject { build.when }
-
- context 'when `when` is undefined' do
- before do
- build.when = nil
- end
-
- context 'use from gitlab-ci.yml' do
- let(:project) { create(:project, :repository) }
- let(:pipeline) { create(:ci_pipeline, project: project) }
-
- before do
- stub_ci_pipeline_yaml_file(config)
- end
-
- context 'when config is not found' do
- let(:config) { nil }
-
- it { is_expected.to eq('on_success') }
- end
-
- context 'when config does not have a questioned job' do
- let(:config) do
- YAML.dump({
- test_other: {
- script: 'Hello World'
- }
- })
- end
-
- it { is_expected.to eq('on_success') }
- end
-
- context 'when config has `when`' do
- let(:config) do
- YAML.dump({
- test: {
- script: 'Hello World',
- when: 'always'
- }
- })
- end
-
- it { is_expected.to eq('always') }
- end
- end
- end
- end
-
describe '#variables' do
let(:container_registry_enabled) { false }
@@ -2148,62 +2149,6 @@ describe Ci::Build do
it { is_expected.to include(*predefined_variables) }
- context 'when yaml variables are undefined' do
- let(:pipeline) do
- create(:ci_pipeline, project: project,
- sha: project.commit.id,
- ref: project.default_branch)
- end
-
- before do
- build.yaml_variables = nil
- end
-
- context 'use from gitlab-ci.yml' do
- before do
- stub_ci_pipeline_yaml_file(config)
- end
-
- context 'when config is not found' do
- let(:config) { nil }
-
- it { is_expected.to include(*predefined_variables) }
- end
-
- context 'when config does not have a questioned job' do
- let(:config) do
- YAML.dump({
- test_other: {
- script: 'Hello World'
- }
- })
- end
-
- it { is_expected.to include(*predefined_variables) }
- end
-
- context 'when config has variables' do
- let(:config) do
- YAML.dump({
- test: {
- script: 'Hello World',
- variables: {
- KEY: 'value'
- }
- }
- })
- end
-
- let(:variables) do
- [{ key: 'KEY', value: 'value', public: true }]
- end
-
- it { is_expected.to include(*predefined_variables) }
- it { is_expected.to include(*variables) }
- end
- end
- end
-
describe 'variables ordering' do
context 'when variables hierarchy is stubbed' do
let(:build_pre_var) { { key: 'build', value: 'value', public: true } }
@@ -2792,29 +2737,53 @@ describe Ci::Build do
end
describe '#yaml_variables' do
- before do
- build.update_attribute(:yaml_variables, variables)
+ let(:build) { create(:ci_build, pipeline: pipeline, yaml_variables: variables) }
+
+ let(:variables) do
+ [
+ { 'key' => :VARIABLE, 'value' => 'my value' },
+ { 'key' => 'VARIABLE2', 'value' => 'my value 2' }
+ ]
end
- context 'when serialized valu is a symbolized hash' do
- let(:variables) do
- [{ key: :VARIABLE, value: 'my value 1' }]
+ shared_examples 'having consistent representation' do
+ it 'allows to access using symbols' do
+ expect(build.reload.yaml_variables.first[:key]).to eq('VARIABLE')
+ expect(build.reload.yaml_variables.first[:value]).to eq('my value')
+ expect(build.reload.yaml_variables.second[:key]).to eq('VARIABLE2')
+ expect(build.reload.yaml_variables.second[:value]).to eq('my value 2')
end
+ end
+
+ context 'when ci_build_metadata_config is set' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: true)
+ end
+
+ it_behaves_like 'having consistent representation'
- it 'keeps symbolizes keys and stringifies variables names' do
- expect(build.yaml_variables)
- .to eq [{ key: 'VARIABLE', value: 'my value 1' }]
+ it 'persist data in build metadata' do
+ expect(build.metadata.read_attribute(:config_variables)).not_to be_nil
+ end
+
+ it 'does not persist data in build' do
+ expect(build.read_attribute(:yaml_variables)).to be_nil
end
end
- context 'when serialized value is a hash with string keys' do
- let(:variables) do
- [{ 'key' => :VARIABLE, 'value' => 'my value 2' }]
+ context 'when ci_build_metadata_config is disabled' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: false)
end
- it 'symblizes variables hash' do
- expect(build.yaml_variables)
- .to eq [{ key: 'VARIABLE', value: 'my value 2' }]
+ it_behaves_like 'having consistent representation'
+
+ it 'persist data in build' do
+ expect(build.read_attribute(:yaml_variables)).not_to be_nil
+ end
+
+ it 'does not persist data in build metadata' do
+ expect(build.metadata.read_attribute(:config_variables)).to be_nil
end
end
end
@@ -2986,7 +2955,7 @@ describe Ci::Build do
end
context 'when build is configured to be retried' do
- subject { create(:ci_build, :running, options: { retry: { max: 3 } }, project: project, user: user) }
+ subject { create(:ci_build, :running, options: { script: ["ls -al"], retry: 3 }, project: project, user: user) }
it 'retries build and assigns the same user to it' do
expect(described_class).to receive(:retry)
@@ -3475,6 +3444,23 @@ describe Ci::Build do
end
end
+ describe 'degenerate!' do
+ let(:build) { create(:ci_build) }
+
+ subject { build.degenerate! }
+
+ before do
+ build.ensure_metadata
+ end
+
+ it 'drops metadata' do
+ subject
+
+ expect(build.reload).to be_degenerated
+ expect(build.metadata).to be_nil
+ end
+ end
+
describe '#archived?' do
context 'when build is degenerated' do
subject { create(:ci_build, :degenerated) }
@@ -3502,4 +3488,97 @@ describe Ci::Build do
end
end
end
+
+ describe '#read_metadata_attribute' do
+ let(:build) { create(:ci_build, :degenerated) }
+ let(:build_options) { { "key" => "build" } }
+ let(:metadata_options) { { "key" => "metadata" } }
+ let(:default_options) { { "key" => "default" } }
+
+ subject { build.send(:read_metadata_attribute, :options, :config_options, default_options) }
+
+ context 'when build and metadata options is set' do
+ before do
+ build.write_attribute(:options, build_options)
+ build.ensure_metadata.write_attribute(:config_options, metadata_options)
+ end
+
+ it 'prefers build options' do
+ is_expected.to eq(build_options)
+ end
+ end
+
+ context 'when only metadata options is set' do
+ before do
+ build.write_attribute(:options, nil)
+ build.ensure_metadata.write_attribute(:config_options, metadata_options)
+ end
+
+ it 'returns metadata options' do
+ is_expected.to eq(metadata_options)
+ end
+ end
+
+ context 'when none is set' do
+ it 'returns default value' do
+ is_expected.to eq(default_options)
+ end
+ end
+ end
+
+ describe '#write_metadata_attribute' do
+ let(:build) { create(:ci_build, :degenerated) }
+ let(:options) { { "key" => "new options" } }
+ let(:existing_options) { { "key" => "existing options" } }
+
+ subject { build.send(:write_metadata_attribute, :options, :config_options, options) }
+
+ context 'when ci_build_metadata_config is set' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: true)
+ end
+
+ context 'when data in build is already set' do
+ before do
+ build.write_attribute(:options, existing_options)
+ end
+
+ it 'does set metadata options' do
+ subject
+
+ expect(build.metadata.read_attribute(:config_options)).to eq(options)
+ end
+
+ it 'does reset build options' do
+ subject
+
+ expect(build.read_attribute(:options)).to be_nil
+ end
+ end
+ end
+
+ context 'when ci_build_metadata_config is disabled' do
+ before do
+ stub_feature_flags(ci_build_metadata_config: false)
+ end
+
+ context 'when data in build metadata is already set' do
+ before do
+ build.ensure_metadata.write_attribute(:config_options, existing_options)
+ end
+
+ it 'does set metadata options' do
+ subject
+
+ expect(build.read_attribute(:options)).to eq(options)
+ end
+
+ it 'does reset build options' do
+ subject
+
+ expect(build.metadata.read_attribute(:config_options)).to be_nil
+ end
+ end
+ end
+ end
end
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 2f322cc7054..ec48bf60426 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -287,7 +287,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
let(:runner) { create(:ci_runner, :project, projects: [project]) }
let(:job) do
create(:ci_build, :artifacts, :extended_options,
- pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate")
+ pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0)
end
describe 'POST /api/v4/jobs/request' do
@@ -422,7 +422,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
let(:expected_steps) do
[{ 'name' => 'script',
- 'script' => %w(ls date),
+ 'script' => %w(echo),
'timeout' => job.metadata_timeout,
'when' => 'on_success',
'allow_failure' => false },
@@ -588,7 +588,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
let!(:test_job) do
create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'deploy',
stage: 'deploy', stage_idx: 1,
- options: { dependencies: [job2.name] })
+ options: { script: ['bash'], dependencies: [job2.name] })
end
before do
@@ -612,7 +612,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
let!(:empty_dependencies_job) do
create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'empty_dependencies_job',
stage: 'deploy', stage_idx: 1,
- options: { dependencies: [] })
+ options: { script: ['bash'], dependencies: [] })
end
before do
diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb
index 538992b621e..7ce7d2d882a 100644
--- a/spec/services/ci/process_pipeline_service_spec.rb
+++ b/spec/services/ci/process_pipeline_service_spec.rb
@@ -671,9 +671,9 @@ describe Ci::ProcessPipelineService, '#execute' do
context 'when builds with auto-retries are configured' do
before do
- create_build('build:1', stage_idx: 0, user: user, options: { retry: { max: 2 } })
+ create_build('build:1', stage_idx: 0, user: user, options: { script: 'aa', retry: 2 })
create_build('test:1', stage_idx: 1, user: user, when: :on_failure)
- create_build('test:2', stage_idx: 1, user: user, options: { retry: { max: 1 } })
+ create_build('test:2', stage_idx: 1, user: user, options: { script: 'aa', retry: 1 })
end
it 'automatically retries builds in a valid order' do
@@ -770,7 +770,7 @@ describe Ci::ProcessPipelineService, '#execute' do
end
def delayed_options
- { when: 'delayed', options: { start_in: '1 minute' } }
+ { when: 'delayed', options: { script: %w(echo), start_in: '1 minute' } }
end
def unschedule
diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb
index 9d65ac15213..20181387612 100644
--- a/spec/services/ci/register_job_service_spec.rb
+++ b/spec/services/ci/register_job_service_spec.rb
@@ -460,7 +460,12 @@ module Ci
end
let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0) }
- let!(:pending_job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 1, options: { dependencies: ['test'] } ) }
+
+ let!(:pending_job) do
+ create(:ci_build, :pending,
+ pipeline: pipeline, stage_idx: 1,
+ options: { script: ["bash"], dependencies: ['test'] })
+ end
subject { execute(specific_runner) }