Commit fba6a41b authored by Fabio Pitino's avatar Fabio Pitino

Merge branch 'mc/feature/associate-restarted-pipeline-triggerer' into 'master'

Associate subsequent jobs with triggering user

See merge request gitlab-org/gitlab!49833
parents 074f231c ac7552db
...@@ -159,6 +159,12 @@ class CommitStatus < ApplicationRecord ...@@ -159,6 +159,12 @@ class CommitStatus < ApplicationRecord
commit_status.failure_reason = CommitStatus.failure_reasons[failure_reason] commit_status.failure_reason = CommitStatus.failure_reasons[failure_reason]
end 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| after_transition do |commit_status, transition|
next if transition.loopback? next if transition.loopback?
next if commit_status.processed? next if commit_status.processed?
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Ci module Ci
class RetryBuildService < ::BaseService class RetryBuildService < ::BaseService
include Gitlab::OptimisticLocking
def self.clone_accessors def self.clone_accessors
%i[pipeline project ref tag options name %i[pipeline project ref tag options name
allow_failure stage stage_id stage_idx trigger_request allow_failure stage stage_id stage_idx trigger_request
...@@ -65,8 +67,8 @@ module Ci ...@@ -65,8 +67,8 @@ module Ci
end end
def mark_subsequent_stages_as_processable(build) def mark_subsequent_stages_as_processable(build)
build.pipeline.processables.skipped.after_stage(build.stage_idx).find_each do |processable| build.pipeline.processables.skipped.after_stage(build.stage_idx).find_each do |skipped|
Gitlab::OptimisticLocking.retry_lock(processable, &:process) retry_optimistic_lock(skipped) { |build| build.process(current_user) }
end end
end end
end end
......
...@@ -23,7 +23,7 @@ module Ci ...@@ -23,7 +23,7 @@ module Ci
end end
pipeline.builds.latest.skipped.find_each do |skipped| 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 end
pipeline.reset_ancestor_bridges! 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 ...@@ -61,6 +61,22 @@ RSpec.describe CommitStatus do
expect(commit_status.started_at).to be_present expect(commit_status.started_at).to be_present
end end
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 end
describe '#processed' do describe '#processed' do
......
...@@ -206,6 +206,22 @@ RSpec.describe Ci::RetryBuildService do ...@@ -206,6 +206,22 @@ RSpec.describe Ci::RetryBuildService do
expect(subsequent_build.reload).to be_created expect(subsequent_build.reload).to be_created
expect(subsequent_bridge.reload).to be_created expect(subsequent_bridge.reload).to be_created
end 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 end
context 'when pipeline has other builds' do context 'when pipeline has other builds' do
......
...@@ -64,6 +64,18 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do ...@@ -64,6 +64,18 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
expect(build('spinach 1')).to be_created expect(build('spinach 1')).to be_created
expect(pipeline.reload).to be_running expect(pipeline.reload).to be_running
end 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 end
context 'when there is failed build present which was run on failure' do context 'when there is failed build present which was run on failure' do
...@@ -161,6 +173,16 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do ...@@ -161,6 +173,16 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
expect(build('rspec 2')).to be_created expect(build('rspec 2')).to be_created
expect(pipeline.reload).to be_running expect(pipeline.reload).to be_running
end 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
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