Backport changes from EE for RegisterBuildServicecleanup-RegisterBuildService-for-EE

This would reduce conflicts. See:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8084#note_21727977

3 files changed, 41 insertions, 32 deletions

diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb
index 74b5ebf372b..970f2b02fe5 100644
--- a/app/services/ci/register_build_service.rb
+++ b/app/services/ci/register_build_service.rb
@@ -2,11 +2,17 @@ module Ci
   # This class responsible for assigning
   # proper pending build to runner on runner API request
   class RegisterBuildService
-    def execute(current_runner)
+    attr_reader :runner
+
+    def initialize(runner)
+      @runner = runner
+    end
+
+    def execute
       builds = Ci::Build.pending.unstarted
 
       builds =
-        if current_runner.shared?
+        if runner.shared?
           builds.
             # don't run projects which have not enabled shared runners and builds
             joins(:project).where(projects: { shared_runners_enabled: true }).
@@ -19,17 +25,17 @@ module Ci
             order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC')
         else
           # do run projects which are only assigned to this runner (FIFO)
-          builds.where(project: current_runner.projects.with_builds_enabled).order('created_at ASC')
+          builds.where(project: runner.projects.with_builds_enabled).order('created_at ASC')
         end
 
       build = builds.find do |build|
-        current_runner.can_pick?(build)
+        runner.can_pick?(build)
       end
 
       if build
         # In case when 2 runners try to assign the same build, second runner will be declined
         # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method.
-        build.runner_id = current_runner.id
+        build.runner_id = runner.id
         build.run!
       end
 
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index c4bdef781f7..a9da8ea7eeb 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -23,7 +23,7 @@ module Ci
 
           new_update = current_runner.ensure_runner_queue_value
 
-          build = Ci::RegisterBuildService.new.execute(current_runner)
+          build = Ci::RegisterBuildService.new(current_runner).execute
 
           if build
             Gitlab::Metrics.add_event(:build_found,
diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_build_service_spec.rb
index a3fc23ba177..27d7853bbdd 100644
--- a/spec/services/ci/register_build_service_spec.rb
+++ b/spec/services/ci/register_build_service_spec.rb
@@ -2,7 +2,6 @@ require 'spec_helper'
 
 module Ci
   describe RegisterBuildService, services: true do
-    let!(:service) { RegisterBuildService.new }
     let!(:project) { FactoryGirl.create :empty_project, shared_runners_enabled: false }
     let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project }
     let!(:pending_build) { FactoryGirl.create :ci_build, pipeline: pipeline }
@@ -19,29 +18,29 @@ module Ci
           pending_build.tag_list = ["linux"]
           pending_build.save
           specific_runner.tag_list = ["linux"]
-          expect(service.execute(specific_runner)).to eq(pending_build)
+          expect(execute(specific_runner)).to eq(pending_build)
         end
 
         it "does not pick build with different tag" do
           pending_build.tag_list = ["linux"]
           pending_build.save
           specific_runner.tag_list = ["win32"]
-          expect(service.execute(specific_runner)).to be_falsey
+          expect(execute(specific_runner)).to be_falsey
         end
 
         it "picks build without tag" do
-          expect(service.execute(specific_runner)).to eq(pending_build)
+          expect(execute(specific_runner)).to eq(pending_build)
         end
 
         it "does not pick build with tag" do
           pending_build.tag_list = ["linux"]
           pending_build.save
-          expect(service.execute(specific_runner)).to be_falsey
+          expect(execute(specific_runner)).to be_falsey
         end
 
         it "pick build without tag" do
           specific_runner.tag_list = ["win32"]
-          expect(service.execute(specific_runner)).to eq(pending_build)
+          expect(execute(specific_runner)).to eq(pending_build)
         end
       end
 
@@ -56,13 +55,13 @@ module Ci
           end
 
           it 'does not pick a build' do
-            expect(service.execute(shared_runner)).to be_nil
+            expect(execute(shared_runner)).to be_nil
           end
         end
 
         context 'for specific runner' do
           it 'does not pick a build' do
-            expect(service.execute(specific_runner)).to be_nil
+            expect(execute(specific_runner)).to be_nil
           end
         end
       end
@@ -86,34 +85,34 @@ module Ci
 
           it 'prefers projects without builds first' do
             # it gets for one build from each of the projects
-            expect(service.execute(shared_runner)).to eq(build1_project1)
-            expect(service.execute(shared_runner)).to eq(build1_project2)
-            expect(service.execute(shared_runner)).to eq(build1_project3)
+            expect(execute(shared_runner)).to eq(build1_project1)
+            expect(execute(shared_runner)).to eq(build1_project2)
+            expect(execute(shared_runner)).to eq(build1_project3)
 
             # then it gets a second build from each of the projects
-            expect(service.execute(shared_runner)).to eq(build2_project1)
-            expect(service.execute(shared_runner)).to eq(build2_project2)
+            expect(execute(shared_runner)).to eq(build2_project1)
+            expect(execute(shared_runner)).to eq(build2_project2)
 
             # in the end the third build
-            expect(service.execute(shared_runner)).to eq(build3_project1)
+            expect(execute(shared_runner)).to eq(build3_project1)
           end
 
           it 'equalises number of running builds' do
             # after finishing the first build for project 1, get a second build from the same project
-            expect(service.execute(shared_runner)).to eq(build1_project1)
+            expect(execute(shared_runner)).to eq(build1_project1)
             build1_project1.reload.success
-            expect(service.execute(shared_runner)).to eq(build2_project1)
+            expect(execute(shared_runner)).to eq(build2_project1)
 
-            expect(service.execute(shared_runner)).to eq(build1_project2)
+            expect(execute(shared_runner)).to eq(build1_project2)
             build1_project2.reload.success
-            expect(service.execute(shared_runner)).to eq(build2_project2)
-            expect(service.execute(shared_runner)).to eq(build1_project3)
-            expect(service.execute(shared_runner)).to eq(build3_project1)
+            expect(execute(shared_runner)).to eq(build2_project2)
+            expect(execute(shared_runner)).to eq(build1_project3)
+            expect(execute(shared_runner)).to eq(build3_project1)
           end
         end
 
         context 'shared runner' do
-          let(:build) { service.execute(shared_runner) }
+          let(:build) { execute(shared_runner) }
 
           it { expect(build).to be_kind_of(Build) }
           it { expect(build).to be_valid }
@@ -122,7 +121,7 @@ module Ci
         end
 
         context 'specific runner' do
-          let(:build) { service.execute(specific_runner) }
+          let(:build) { execute(specific_runner) }
 
           it { expect(build).to be_kind_of(Build) }
           it { expect(build).to be_valid }
@@ -137,13 +136,13 @@ module Ci
         end
 
         context 'shared runner' do
-          let(:build) { service.execute(shared_runner) }
+          let(:build) { execute(shared_runner) }
 
           it { expect(build).to be_nil }
         end
 
         context 'specific runner' do
-          let(:build) { service.execute(specific_runner) }
+          let(:build) { execute(specific_runner) }
 
           it { expect(build).to be_kind_of(Build) }
           it { expect(build).to be_valid }
@@ -159,17 +158,21 @@ module Ci
         end
 
         context 'and uses shared runner' do
-          let(:build) { service.execute(shared_runner) }
+          let(:build) { execute(shared_runner) }
 
           it { expect(build).to be_nil }
         end
 
         context 'and uses specific runner' do
-          let(:build) { service.execute(specific_runner) }
+          let(:build) { execute(specific_runner) }
 
           it { expect(build).to be_nil }
         end
       end
+
+      def execute(runner)
+        described_class.new(runner).execute
+      end
     end
   end
 end