Commit ac7552db authored by Matija Čupić's avatar Matija Čupić Committed by Fabio Pitino

Associate subsequent jobs with triggering user

When retrying builds or pipelines, all subsequent build will get
associated to the user which triggered the retry.
parent 17ac37dd
......@@ -159,6 +159,12 @@ class CommitStatus < ApplicationRecord
commit_status.failure_reason = CommitStatus.failure_reasons[failure_reason]
end
before_transition [:skipped, :manual] => :created do |commit_status, transition|
transition.args.first.try do |user|
commit_status.user = user
end
end
after_transition do |commit_status, transition|
next if transition.loopback?
next if commit_status.processed?
......
......@@ -2,6 +2,8 @@
module Ci
class RetryBuildService < ::BaseService
include Gitlab::OptimisticLocking
def self.clone_accessors
%i[pipeline project ref tag options name
allow_failure stage stage_id stage_idx trigger_request
......@@ -65,8 +67,8 @@ module Ci
end
def mark_subsequent_stages_as_processable(build)
build.pipeline.processables.skipped.after_stage(build.stage_idx).find_each do |processable|
Gitlab::OptimisticLocking.retry_lock(processable, &:process)
build.pipeline.processables.skipped.after_stage(build.stage_idx).find_each do |skipped|
retry_optimistic_lock(skipped) { |build| build.process(current_user) }
end
end
end
......
......@@ -23,7 +23,7 @@ module Ci
end
pipeline.builds.latest.skipped.find_each do |skipped|
retry_optimistic_lock(skipped) { |build| build.process }
retry_optimistic_lock(skipped) { |build| build.process(current_user) }
end
pipeline.reset_ancestor_bridges!
......
---
title: When retrying jobs associate subsequent jobs with triggering user.
merge_request: 49833
author:
type: changed
......@@ -61,6 +61,22 @@ RSpec.describe CommitStatus do
expect(commit_status.started_at).to be_present
end
end
describe 'transitioning to created from skipped or manual' do
let(:commit_status) { create(:commit_status, :skipped) }
it 'does not update user without parameter' do
commit_status.process!
expect { commit_status.process }.not_to change { commit_status.reload.user }
end
it 'updates user with user parameter' do
new_user = create(:user)
expect { commit_status.process(new_user) }.to change { commit_status.reload.user }.to(new_user)
end
end
end
describe '#processed' do
......
......@@ -206,6 +206,22 @@ RSpec.describe Ci::RetryBuildService do
expect(subsequent_build.reload).to be_created
expect(subsequent_bridge.reload).to be_created
end
it 'updates ownership for subsequent builds' do
expect { service.execute(build) }.to change { subsequent_build.reload.user }.to(user)
end
it 'updates ownership for subsequent bridges' do
expect { service.execute(build) }.to change { subsequent_bridge.reload.user }.to(user)
end
it 'does not cause n+1 when updaing build ownership' do
control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { service.execute(build) }.count
create_list(:ci_build, 2, :skipped, stage_idx: build.stage_idx + 1, pipeline: pipeline, stage: 'deploy')
expect { service.execute(build) }.not_to exceed_all_query_limit(control_count)
end
end
context 'when pipeline has other builds' do
......
......@@ -64,6 +64,18 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
expect(build('spinach 1')).to be_created
expect(pipeline.reload).to be_running
end
it 'changes ownership of subsequent builds' do
expect(build('rspec 2').user).not_to eq(user)
expect(build('rspec 3').user).not_to eq(user)
expect(build('spinach 1').user).not_to eq(user)
service.execute(pipeline)
expect(build('rspec 2').user).to eq(user)
expect(build('rspec 3').user).to eq(user)
expect(build('spinach 1').user).to eq(user)
end
end
context 'when there is failed build present which was run on failure' do
......@@ -161,6 +173,16 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
expect(build('rspec 2')).to be_created
expect(pipeline.reload).to be_running
end
it 'changes ownership of subsequent builds' do
expect(build('staging').user).not_to eq(user)
expect(build('rspec 2').user).not_to eq(user)
service.execute(pipeline)
expect(build('staging').user).to eq(user)
expect(build('rspec 2').user).to eq(user)
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment