Merge branch 'retried-in-database-mysql' into 'master'
Retried in database Closes #25737 See merge request !11115
This commit is contained in:
commit
81df0034f4
|
@ -124,8 +124,8 @@ module Ci
|
||||||
success? || failed? || canceled?
|
success? || failed? || canceled?
|
||||||
end
|
end
|
||||||
|
|
||||||
def retried?
|
def latest?
|
||||||
!self.pipeline.statuses.latest.include?(self)
|
!retried?
|
||||||
end
|
end
|
||||||
|
|
||||||
def expanded_environment_name
|
def expanded_environment_name
|
||||||
|
|
|
@ -19,12 +19,6 @@ class CommitStatus < ActiveRecord::Base
|
||||||
|
|
||||||
alias_attribute :author, :user
|
alias_attribute :author, :user
|
||||||
|
|
||||||
scope :latest, -> do
|
|
||||||
max_id = unscope(:select).select("max(#{quoted_table_name}.id)")
|
|
||||||
|
|
||||||
where(id: max_id.group(:name, :commit_id))
|
|
||||||
end
|
|
||||||
|
|
||||||
scope :failed_but_allowed, -> do
|
scope :failed_but_allowed, -> do
|
||||||
where(allow_failure: true, status: [:failed, :canceled])
|
where(allow_failure: true, status: [:failed, :canceled])
|
||||||
end
|
end
|
||||||
|
@ -37,7 +31,8 @@ class CommitStatus < ActiveRecord::Base
|
||||||
false, all_state_names - [:failed, :canceled, :manual])
|
false, all_state_names - [:failed, :canceled, :manual])
|
||||||
end
|
end
|
||||||
|
|
||||||
scope :retried, -> { where.not(id: latest) }
|
scope :latest, -> { where(retried: [false, nil]) }
|
||||||
|
scope :retried, -> { where(retried: true) }
|
||||||
scope :ordered, -> { order(:name) }
|
scope :ordered, -> { order(:name) }
|
||||||
scope :latest_ordered, -> { latest.ordered.includes(project: :namespace) }
|
scope :latest_ordered, -> { latest.ordered.includes(project: :namespace) }
|
||||||
scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) }
|
scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) }
|
||||||
|
|
|
@ -5,6 +5,8 @@ module Ci
|
||||||
def execute(pipeline)
|
def execute(pipeline)
|
||||||
@pipeline = pipeline
|
@pipeline = pipeline
|
||||||
|
|
||||||
|
update_retried
|
||||||
|
|
||||||
new_builds =
|
new_builds =
|
||||||
stage_indexes_of_created_builds.map do |index|
|
stage_indexes_of_created_builds.map do |index|
|
||||||
process_stage(index)
|
process_stage(index)
|
||||||
|
@ -71,5 +73,23 @@ module Ci
|
||||||
def created_builds
|
def created_builds
|
||||||
pipeline.builds.created
|
pipeline.builds.created
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# This method is for compatibility and data consistency and should be removed with 9.3 version of GitLab
|
||||||
|
# This replicates what is db/post_migrate/20170416103934_upate_retried_for_ci_build.rb
|
||||||
|
# and ensures that functionality will not be broken before migration is run
|
||||||
|
# this updates only when there are data that needs to be updated, there are two groups with no retried flag
|
||||||
|
def update_retried
|
||||||
|
# find the latest builds for each name
|
||||||
|
latest_statuses = pipeline.statuses.latest
|
||||||
|
.group(:name)
|
||||||
|
.having('count(*) > 1')
|
||||||
|
.pluck('max(id)', 'name')
|
||||||
|
|
||||||
|
# mark builds that are retried
|
||||||
|
pipeline.statuses.latest
|
||||||
|
.where(name: latest_statuses.map(&:second))
|
||||||
|
.where.not(id: latest_statuses.map(&:first))
|
||||||
|
.update_all(retried: true) if latest_statuses.any?
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -6,7 +6,7 @@ module Ci
|
||||||
description tag_list].freeze
|
description tag_list].freeze
|
||||||
|
|
||||||
def execute(build)
|
def execute(build)
|
||||||
reprocess(build).tap do |new_build|
|
reprocess!(build).tap do |new_build|
|
||||||
build.pipeline.mark_as_processable_after_stage(build.stage_idx)
|
build.pipeline.mark_as_processable_after_stage(build.stage_idx)
|
||||||
|
|
||||||
new_build.enqueue!
|
new_build.enqueue!
|
||||||
|
@ -17,7 +17,7 @@ module Ci
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def reprocess(build)
|
def reprocess!(build)
|
||||||
unless can?(current_user, :update_build, build)
|
unless can?(current_user, :update_build, build)
|
||||||
raise Gitlab::Access::AccessDeniedError
|
raise Gitlab::Access::AccessDeniedError
|
||||||
end
|
end
|
||||||
|
@ -28,7 +28,14 @@ module Ci
|
||||||
|
|
||||||
attributes.push([:user, current_user])
|
attributes.push([:user, current_user])
|
||||||
|
|
||||||
project.builds.create(Hash[attributes])
|
Ci::Build.transaction do
|
||||||
|
# mark all other builds of that name as retried
|
||||||
|
build.pipeline.builds.latest
|
||||||
|
.where(name: build.name)
|
||||||
|
.update_all(retried: true)
|
||||||
|
|
||||||
|
project.builds.create!(Hash[attributes])
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -11,7 +11,7 @@ module Ci
|
||||||
next unless can?(current_user, :update_build, build)
|
next unless can?(current_user, :update_build, build)
|
||||||
|
|
||||||
Ci::RetryBuildService.new(project, current_user)
|
Ci::RetryBuildService.new(project, current_user)
|
||||||
.reprocess(build)
|
.reprocess!(build)
|
||||||
end
|
end
|
||||||
|
|
||||||
pipeline.builds.latest.skipped.find_each do |skipped|
|
pipeline.builds.latest.skipped.find_each do |skipped|
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Store retried in database for CI Builds
|
||||||
|
merge_request:
|
||||||
|
author:
|
|
@ -0,0 +1,9 @@
|
||||||
|
class AddRetriedToCiBuild < ActiveRecord::Migration
|
||||||
|
include Gitlab::Database::MigrationHelpers
|
||||||
|
|
||||||
|
DOWNTIME = false
|
||||||
|
|
||||||
|
def change
|
||||||
|
add_column(:ci_builds, :retried, :boolean)
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,29 @@
|
||||||
|
class UpateRetriedForCiBuild < ActiveRecord::Migration
|
||||||
|
include Gitlab::Database::MigrationHelpers
|
||||||
|
|
||||||
|
DOWNTIME = false
|
||||||
|
|
||||||
|
disable_ddl_transaction!
|
||||||
|
|
||||||
|
def up
|
||||||
|
disable_statement_timeout
|
||||||
|
|
||||||
|
latest_id = <<-SQL.strip_heredoc
|
||||||
|
SELECT MAX(ci_builds2.id)
|
||||||
|
FROM ci_builds ci_builds2
|
||||||
|
WHERE ci_builds.commit_id=ci_builds2.commit_id
|
||||||
|
AND ci_builds.name=ci_builds2.name
|
||||||
|
SQL
|
||||||
|
|
||||||
|
# This is slow update as it does single-row query
|
||||||
|
# This is designed to be run as idle, or a post deployment migration
|
||||||
|
is_retried = Arel.sql("((#{latest_id}) != ci_builds.id)")
|
||||||
|
|
||||||
|
update_column_in_batches(:ci_builds, :retried, is_retried) do |table, query|
|
||||||
|
query.where(table[:retried].eq(nil))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def down
|
||||||
|
end
|
||||||
|
end
|
|
@ -232,6 +232,7 @@ ActiveRecord::Schema.define(version: 20170508170547) do
|
||||||
t.integer "lock_version"
|
t.integer "lock_version"
|
||||||
t.string "coverage_regex"
|
t.string "coverage_regex"
|
||||||
t.integer "auto_canceled_by_id"
|
t.integer "auto_canceled_by_id"
|
||||||
|
t.boolean "retried"
|
||||||
end
|
end
|
||||||
|
|
||||||
add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
|
add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
|
||||||
|
|
|
@ -231,6 +231,7 @@ CommitStatus:
|
||||||
- lock_version
|
- lock_version
|
||||||
- coverage_regex
|
- coverage_regex
|
||||||
- auto_canceled_by_id
|
- auto_canceled_by_id
|
||||||
|
- retried
|
||||||
Ci::Variable:
|
Ci::Variable:
|
||||||
- id
|
- id
|
||||||
- project_id
|
- project_id
|
||||||
|
|
|
@ -60,8 +60,8 @@ describe Ci::Pipeline, models: true do
|
||||||
subject { pipeline.retried }
|
subject { pipeline.retried }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
@build1 = FactoryGirl.create :ci_build, pipeline: pipeline, name: 'deploy'
|
@build1 = create(:ci_build, pipeline: pipeline, name: 'deploy', retried: true)
|
||||||
@build2 = FactoryGirl.create :ci_build, pipeline: pipeline, name: 'deploy'
|
@build2 = create(:ci_build, pipeline: pipeline, name: 'deploy')
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns old builds' do
|
it 'returns old builds' do
|
||||||
|
@ -70,31 +70,31 @@ describe Ci::Pipeline, models: true do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "coverage" do
|
describe "coverage" do
|
||||||
let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" }
|
let(:project) { create(:empty_project, build_coverage_regex: "/.*/") }
|
||||||
let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
|
let(:pipeline) { create(:ci_empty_pipeline, project: project) }
|
||||||
|
|
||||||
it "calculates average when there are two builds with coverage" do
|
it "calculates average when there are two builds with coverage" do
|
||||||
FactoryGirl.create :ci_build, name: "rspec", coverage: 30, pipeline: pipeline
|
create(:ci_build, name: "rspec", coverage: 30, pipeline: pipeline)
|
||||||
FactoryGirl.create :ci_build, name: "rubocop", coverage: 40, pipeline: pipeline
|
create(:ci_build, name: "rubocop", coverage: 40, pipeline: pipeline)
|
||||||
expect(pipeline.coverage).to eq("35.00")
|
expect(pipeline.coverage).to eq("35.00")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "calculates average when there are two builds with coverage and one with nil" do
|
it "calculates average when there are two builds with coverage and one with nil" do
|
||||||
FactoryGirl.create :ci_build, name: "rspec", coverage: 30, pipeline: pipeline
|
create(:ci_build, name: "rspec", coverage: 30, pipeline: pipeline)
|
||||||
FactoryGirl.create :ci_build, name: "rubocop", coverage: 40, pipeline: pipeline
|
create(:ci_build, name: "rubocop", coverage: 40, pipeline: pipeline)
|
||||||
FactoryGirl.create :ci_build, pipeline: pipeline
|
create(:ci_build, pipeline: pipeline)
|
||||||
expect(pipeline.coverage).to eq("35.00")
|
expect(pipeline.coverage).to eq("35.00")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "calculates average when there are two builds with coverage and one is retried" do
|
it "calculates average when there are two builds with coverage and one is retried" do
|
||||||
FactoryGirl.create :ci_build, name: "rspec", coverage: 30, pipeline: pipeline
|
create(:ci_build, name: "rspec", coverage: 30, pipeline: pipeline)
|
||||||
FactoryGirl.create :ci_build, name: "rubocop", coverage: 30, pipeline: pipeline
|
create(:ci_build, name: "rubocop", coverage: 30, pipeline: pipeline, retried: true)
|
||||||
FactoryGirl.create :ci_build, name: "rubocop", coverage: 40, pipeline: pipeline
|
create(:ci_build, name: "rubocop", coverage: 40, pipeline: pipeline)
|
||||||
expect(pipeline.coverage).to eq("35.00")
|
expect(pipeline.coverage).to eq("35.00")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "calculates average when there is one build without coverage" do
|
it "calculates average when there is one build without coverage" do
|
||||||
FactoryGirl.create :ci_build, pipeline: pipeline
|
FactoryGirl.create(:ci_build, pipeline: pipeline)
|
||||||
expect(pipeline.coverage).to be_nil
|
expect(pipeline.coverage).to be_nil
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -229,6 +229,8 @@ describe Ci::Pipeline, models: true do
|
||||||
name: 'mac',
|
name: 'mac',
|
||||||
stage_idx: 0,
|
stage_idx: 0,
|
||||||
status: 'success')
|
status: 'success')
|
||||||
|
|
||||||
|
pipeline.process!
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'ignores the previous state' do
|
it 'ignores the previous state' do
|
||||||
|
@ -489,6 +491,10 @@ describe Ci::Pipeline, models: true do
|
||||||
context 'there are multiple of the same name' do
|
context 'there are multiple of the same name' do
|
||||||
let!(:manual2) { create(:ci_build, :manual, pipeline: pipeline, name: 'deploy') }
|
let!(:manual2) { create(:ci_build, :manual, pipeline: pipeline, name: 'deploy') }
|
||||||
|
|
||||||
|
before do
|
||||||
|
manual.update(retried: true)
|
||||||
|
end
|
||||||
|
|
||||||
it 'returns latest one' do
|
it 'returns latest one' do
|
||||||
is_expected.to contain_exactly(manual2)
|
is_expected.to contain_exactly(manual2)
|
||||||
end
|
end
|
||||||
|
|
|
@ -102,6 +102,10 @@ describe Ci::Stage, models: true do
|
||||||
context 'and builds are retried' do
|
context 'and builds are retried' do
|
||||||
let!(:new_build) { create_job(:ci_build, status: :success) }
|
let!(:new_build) { create_job(:ci_build, status: :success) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
stage_build.update(retried: true)
|
||||||
|
end
|
||||||
|
|
||||||
it "returns status of latest build" do
|
it "returns status of latest build" do
|
||||||
is_expected.to eq('success')
|
is_expected.to eq('success')
|
||||||
end
|
end
|
||||||
|
|
|
@ -157,9 +157,9 @@ describe CommitStatus, :models do
|
||||||
subject { described_class.latest.order(:id) }
|
subject { described_class.latest.order(:id) }
|
||||||
|
|
||||||
let(:statuses) do
|
let(:statuses) do
|
||||||
[create_status(name: 'aa', ref: 'bb', status: 'running'),
|
[create_status(name: 'aa', ref: 'bb', status: 'running', retried: true),
|
||||||
create_status(name: 'cc', ref: 'cc', status: 'pending'),
|
create_status(name: 'cc', ref: 'cc', status: 'pending', retried: true),
|
||||||
create_status(name: 'aa', ref: 'cc', status: 'success'),
|
create_status(name: 'aa', ref: 'cc', status: 'success', retried: true),
|
||||||
create_status(name: 'cc', ref: 'bb', status: 'success'),
|
create_status(name: 'cc', ref: 'bb', status: 'success'),
|
||||||
create_status(name: 'aa', ref: 'bb', status: 'success')]
|
create_status(name: 'aa', ref: 'bb', status: 'success')]
|
||||||
end
|
end
|
||||||
|
@ -169,6 +169,22 @@ describe CommitStatus, :models do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '.retried' do
|
||||||
|
subject { described_class.retried.order(:id) }
|
||||||
|
|
||||||
|
let(:statuses) do
|
||||||
|
[create_status(name: 'aa', ref: 'bb', status: 'running', retried: true),
|
||||||
|
create_status(name: 'cc', ref: 'cc', status: 'pending', retried: true),
|
||||||
|
create_status(name: 'aa', ref: 'cc', status: 'success', retried: true),
|
||||||
|
create_status(name: 'cc', ref: 'bb', status: 'success'),
|
||||||
|
create_status(name: 'aa', ref: 'bb', status: 'success')]
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns unique statuses' do
|
||||||
|
is_expected.to contain_exactly(*statuses.values_at(0, 1, 2))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '.running_or_pending' do
|
describe '.running_or_pending' do
|
||||||
subject { described_class.running_or_pending.order(:id) }
|
subject { described_class.running_or_pending.order(:id) }
|
||||||
|
|
||||||
|
@ -181,7 +197,7 @@ describe CommitStatus, :models do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns statuses that are running or pending' do
|
it 'returns statuses that are running or pending' do
|
||||||
is_expected.to eq(statuses.values_at(0, 1))
|
is_expected.to contain_exactly(*statuses.values_at(0, 1))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -26,8 +26,8 @@ describe API::CommitStatuses do
|
||||||
create(:commit_status, { pipeline: commit, ref: commit.ref }.merge(opts))
|
create(:commit_status, { pipeline: commit, ref: commit.ref }.merge(opts))
|
||||||
end
|
end
|
||||||
|
|
||||||
let!(:status1) { create_status(master, status: 'running') }
|
let!(:status1) { create_status(master, status: 'running', retried: true) }
|
||||||
let!(:status2) { create_status(master, name: 'coverage', status: 'pending') }
|
let!(:status2) { create_status(master, name: 'coverage', status: 'pending', retried: true) }
|
||||||
let!(:status3) { create_status(develop, status: 'running', allow_failure: true) }
|
let!(:status3) { create_status(develop, status: 'running', allow_failure: true) }
|
||||||
let!(:status4) { create_status(master, name: 'coverage', status: 'success') }
|
let!(:status4) { create_status(master, name: 'coverage', status: 'success') }
|
||||||
let!(:status5) { create_status(develop, name: 'coverage', status: 'success') }
|
let!(:status5) { create_status(develop, name: 'coverage', status: 'success') }
|
||||||
|
|
|
@ -443,6 +443,21 @@ describe Ci::ProcessPipelineService, '#execute', :services do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'updates a list of retried builds' do
|
||||||
|
subject { described_class.retried.order(:id) }
|
||||||
|
|
||||||
|
let!(:build_retried) { create_build('build') }
|
||||||
|
let!(:build) { create_build('build') }
|
||||||
|
let!(:test) { create_build('test') }
|
||||||
|
|
||||||
|
it 'returns unique statuses' do
|
||||||
|
process_pipeline
|
||||||
|
|
||||||
|
expect(all_builds.latest).to contain_exactly(build, test)
|
||||||
|
expect(all_builds.retried).to contain_exactly(build_retried)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def process_pipeline
|
def process_pipeline
|
||||||
described_class.new(pipeline.project, user).execute(pipeline)
|
described_class.new(pipeline.project, user).execute(pipeline)
|
||||||
end
|
end
|
||||||
|
|
|
@ -22,7 +22,7 @@ describe Ci::RetryBuildService, :services do
|
||||||
%i[type lock_version target_url base_tags
|
%i[type lock_version target_url base_tags
|
||||||
commit_id deployments erased_by_id last_deployment project_id
|
commit_id deployments erased_by_id last_deployment project_id
|
||||||
runner_id tag_taggings taggings tags trigger_request_id
|
runner_id tag_taggings taggings tags trigger_request_id
|
||||||
user_id auto_canceled_by_id].freeze
|
user_id auto_canceled_by_id retried].freeze
|
||||||
|
|
||||||
shared_examples 'build duplication' do
|
shared_examples 'build duplication' do
|
||||||
let(:build) do
|
let(:build) do
|
||||||
|
@ -115,7 +115,7 @@ describe Ci::RetryBuildService, :services do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#reprocess' do
|
describe '#reprocess' do
|
||||||
let(:new_build) { service.reprocess(build) }
|
let(:new_build) { service.reprocess!(build) }
|
||||||
|
|
||||||
context 'when user has ability to execute build' do
|
context 'when user has ability to execute build' do
|
||||||
before do
|
before do
|
||||||
|
@ -131,11 +131,16 @@ describe Ci::RetryBuildService, :services do
|
||||||
it 'does not enqueue the new build' do
|
it 'does not enqueue the new build' do
|
||||||
expect(new_build).to be_created
|
expect(new_build).to be_created
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'does mark old build as retried' do
|
||||||
|
expect(new_build).to be_latest
|
||||||
|
expect(build.reload).to be_retried
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when user does not have ability to execute build' do
|
context 'when user does not have ability to execute build' do
|
||||||
it 'raises an error' do
|
it 'raises an error' do
|
||||||
expect { service.reprocess(build) }
|
expect { service.reprocess!(build) }
|
||||||
.to raise_error Gitlab::Access::AccessDeniedError
|
.to raise_error Gitlab::Access::AccessDeniedError
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -13,7 +13,7 @@ describe Ci::RetryPipelineService, '#execute', :services do
|
||||||
|
|
||||||
context 'when there are already retried jobs present' do
|
context 'when there are already retried jobs present' do
|
||||||
before do
|
before do
|
||||||
create_build('rspec', :canceled, 0)
|
create_build('rspec', :canceled, 0, retried: true)
|
||||||
create_build('rspec', :failed, 0)
|
create_build('rspec', :failed, 0)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -39,9 +39,8 @@ describe 'projects/pipelines/_stage', :view do
|
||||||
|
|
||||||
context 'when there are retried builds present' do
|
context 'when there are retried builds present' do
|
||||||
before do
|
before do
|
||||||
create_list(:ci_build, 2, name: 'test:build',
|
create(:ci_build, name: 'test:build', stage: stage.name, pipeline: pipeline, retried: true)
|
||||||
stage: stage.name,
|
create(:ci_build, name: 'test:build', stage: stage.name, pipeline: pipeline)
|
||||||
pipeline: pipeline)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'shows only latest builds' do
|
it 'shows only latest builds' do
|
||||||
|
|
Loading…
Reference in New Issue