From 197a3d053359e66535c41935eac065ee424cbb07 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 27 May 2019 19:21:36 +0700 Subject: Introduce sidekiq worker for auto merge process As we have a central domain for auto merge process today, we should use a single worker for any auto merge process. --- app/models/ci/pipeline.rb | 10 ++++++++ app/workers/all_queues.yml | 2 ++ app/workers/auto_merge_process_worker.rb | 14 +++++++++++ app/workers/pipeline_success_worker.rb | 9 +------ config/sidekiq_queues.yml | 1 + spec/models/ci/pipeline_spec.rb | 34 ++++++++++++++++++++++++++ spec/workers/auto_merge_process_worker_spec.rb | 31 +++++++++++++++++++++++ spec/workers/pipeline_success_worker_spec.rb | 27 -------------------- 8 files changed, 93 insertions(+), 35 deletions(-) create mode 100644 app/workers/auto_merge_process_worker.rb create mode 100644 spec/workers/auto_merge_process_worker_spec.rb delete mode 100644 spec/workers/pipeline_success_worker_spec.rb diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 80401ca0a1e..3727a9861aa 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -166,6 +166,16 @@ module Ci end end + after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline| + pipeline.run_after_commit do + pipeline.all_merge_requests.each do |merge_request| + next unless merge_request.auto_merge_enabled? + + AutoMergeProcessWorker.perform_async(merge_request.id) + end + end + end + after_transition any => [:success, :failed] do |pipeline| pipeline.run_after_commit do PipelineNotificationWorker.perform_async(pipeline.id) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index e4e85de93da..fd0cc5fb24e 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1,6 +1,8 @@ --- - auto_devops:auto_devops_disable +- auto_merge:auto_merge_process + - cronjob:admin_email - cronjob:expire_build_artifacts - cronjob:gitlab_usage_ping diff --git a/app/workers/auto_merge_process_worker.rb b/app/workers/auto_merge_process_worker.rb new file mode 100644 index 00000000000..cd81cdbc60c --- /dev/null +++ b/app/workers/auto_merge_process_worker.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AutoMergeProcessWorker + include ApplicationWorker + + queue_namespace :auto_merge + + def perform(merge_request_id) + MergeRequest.find_by_id(merge_request_id).try do |merge_request| + AutoMergeService.new(merge_request.project, merge_request.merge_user) + .process(merge_request) + end + end +end diff --git a/app/workers/pipeline_success_worker.rb b/app/workers/pipeline_success_worker.rb index ce6c88c85c1..666331e6cd4 100644 --- a/app/workers/pipeline_success_worker.rb +++ b/app/workers/pipeline_success_worker.rb @@ -6,14 +6,7 @@ class PipelineSuccessWorker queue_namespace :pipeline_processing - # rubocop: disable CodeReuse/ActiveRecord def perform(pipeline_id) - Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| - pipeline.all_merge_requests.preload(:merge_user).each do |merge_request| - AutoMergeService.new(pipeline.project, merge_request.merge_user) - .process(merge_request) - end - end + # no-op end - # rubocop: enable CodeReuse/ActiveRecord end diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 0615da2d241..fd9ce4d3374 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -30,6 +30,7 @@ - [pipeline_default, 3] - [pipeline_cache, 3] - [deployment, 3] + - [auto_merge, 3] - [pipeline_hooks, 2] - [gitlab_shell, 2] - [email_receiver, 2] diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index a0319b3eb0a..a8701f0efa4 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1381,6 +1381,40 @@ describe Ci::Pipeline, :mailer do end end + describe 'auto merge' do + let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + + let(:pipeline) do + create(:ci_pipeline, :running, project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + + before do + merge_request.update_head_pipeline + end + + %w[succeed! drop! cancel! skip!].each do |action| + context "when the pipeline recieved #{action} event" do + it 'performs AutoMergeProcessWorker' do + expect(AutoMergeProcessWorker).to receive(:perform_async).with(merge_request.id) + + pipeline.public_send(action) + end + end + end + + context 'when auto merge is not enabled in the merge request' do + let(:merge_request) { create(:merge_request) } + + it 'performs AutoMergeProcessWorker' do + expect(AutoMergeProcessWorker).not_to receive(:perform_async) + + pipeline.succeed! + end + end + end + def create_build(name, *traits, queued_at: current, started_from: 0, **opts) create(:ci_build, *traits, name: name, diff --git a/spec/workers/auto_merge_process_worker_spec.rb b/spec/workers/auto_merge_process_worker_spec.rb new file mode 100644 index 00000000000..616727ce5ca --- /dev/null +++ b/spec/workers/auto_merge_process_worker_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AutoMergeProcessWorker do + describe '#perform' do + subject { described_class.new.perform(merge_request&.id) } + + context 'when merge request is found' do + let(:merge_request) { create(:merge_request) } + + it 'executes AutoMergeService' do + expect_next_instance_of(AutoMergeService) do |auto_merge| + expect(auto_merge).to receive(:process) + end + + subject + end + end + + context 'when merge request is not found' do + let(:merge_request) { nil } + + it 'does not execute AutoMergeService' do + expect(AutoMergeService).not_to receive(:new) + + subject + end + end + end +end diff --git a/spec/workers/pipeline_success_worker_spec.rb b/spec/workers/pipeline_success_worker_spec.rb deleted file mode 100644 index b511edfa620..00000000000 --- a/spec/workers/pipeline_success_worker_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe PipelineSuccessWorker do - describe '#perform' do - context 'when pipeline exists' do - let(:pipeline) { create(:ci_pipeline, status: 'success', ref: merge_request.source_branch, project: merge_request.source_project) } - let(:merge_request) { create(:merge_request) } - - it 'performs "merge when pipeline succeeds"' do - expect_next_instance_of(AutoMergeService) do |auto_merge| - expect(auto_merge).to receive(:process) - end - - described_class.new.perform(pipeline.id) - end - end - - context 'when pipeline does not exist' do - it 'does not raise exception' do - expect { described_class.new.perform(123) } - .not_to raise_error - end - end - end -end -- cgit v1.2.3