diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2017-01-20 14:47:20 +0300 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2017-01-20 14:47:20 +0300 |
commit | 894f52cf5fd5afd21dacfe4affc707fb5829c499 (patch) | |
tree | 4486ba2b8b7c5070f40581f3934cd0f4989fedba | |
parent | 491f1375fc055805c623a3079a383de988689f3d (diff) |
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
-rw-r--r-- | app/services/ci/register_build_service.rb | 16 | ||||
-rw-r--r-- | lib/ci/api/builds.rb | 2 | ||||
-rw-r--r-- | spec/services/ci/register_build_service_spec.rb | 55 |
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 |