summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Bizon <grzegorz@gitlab.com>2019-01-06 09:47:29 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2019-01-06 09:47:29 +0000
commit1aa2ac13b95b9fa9527596610bb07e132dc1a6f0 (patch)
treea04dba27d51620b49b93e2d25a8ed6222cefa914
parent52d0c0edb796f21498fb4b88c99ffa6020c048a9 (diff)
parent0103d5be960e620342c67436ddd64ca9e729d7a8 (diff)
downloadgitlab-ce-1aa2ac13b95b9fa9527596610bb07e132dc1a6f0.tar.gz
Merge branch 'kamil-refactor-ci-builds-v5' into 'master'
Use BuildMetadata to store build configuration in JSONB form See merge request gitlab-org/gitlab-ce!21499
-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 28b1a1e37e5..1afc2436bb5 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 }
@@ -2149,62 +2150,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 } }
@@ -2793,29 +2738,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
@@ -2987,7 +2956,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)
@@ -3476,6 +3445,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) }
@@ -3503,4 +3489,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) }