diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 860ac16eefd..a3a30fe17c2 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -290,16 +290,12 @@ module Ci project.valid_runners_token? token end - def can_be_served?(runner) - runner.can_serve?(self) - end - def has_tags? tag_list.any? end def any_runners_online? - project.any_runners? { |runner| runner.active? && runner.online? && can_be_served?(runner) } + project.any_runners? { |runner| runner.active? && runner.online? && runner.can_pick?(self) } end def stuck? diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index bb5340a0f03..44f820d100b 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -98,7 +98,7 @@ module Ci !shared? end - def can_serve?(build) + def can_pick?(build) not_locked_or_locked_to?(build.project) && run_untagged_or_has_tags?(build) && accepting_tags?(build.tag_list) diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb index 4ff268a6f06..511505bc9a9 100644 --- a/app/services/ci/register_build_service.rb +++ b/app/services/ci/register_build_service.rb @@ -17,7 +17,7 @@ module Ci builds = builds.order('created_at ASC') build = builds.find do |build| - build.can_be_served?(current_runner) + current_runner.can_pick?(build) end if build diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 1d4b8bc4c98..e703a07013b 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -275,122 +275,6 @@ describe Ci::Build, models: true do end end - describe '#can_be_served?' do - let(:runner) { create(:ci_runner) } - - before do - build.project.runners << runner - end - - context 'when runner does not have tags' do - it 'can handle builds without tags' do - expect(build.can_be_served?(runner)).to be_truthy - end - - it 'cannot handle build with tags' do - build.tag_list = ['aa'] - expect(build.can_be_served?(runner)).to be_falsey - end - end - - context 'when runner has tags' do - before do - runner.tag_list = ['bb', 'cc'] - end - - shared_examples 'tagged build picker' do - it 'can handle build with matching tags' do - build.tag_list = ['bb'] - expect(build.can_be_served?(runner)).to be_truthy - end - - it 'cannot handle build without matching tags' do - build.tag_list = ['aa'] - expect(build.can_be_served?(runner)).to be_falsey - end - end - - context 'when runner can pick untagged jobs' do - it 'can handle builds without tags' do - expect(build.can_be_served?(runner)).to be_truthy - end - - it_behaves_like 'tagged build picker' - end - - context 'when runner cannot pick untagged jobs' do - before do - runner.run_untagged = false - end - - it 'cannot handle builds without tags' do - expect(build.can_be_served?(runner)).to be_falsey - end - - it_behaves_like 'tagged build picker' - end - end - - context 'when runner is locked' do - before do - runner.locked = true - end - - shared_examples 'locked build picker' do |serve_matching_tags| - context 'when runner cannot pick untagged jobs' do - before do - runner.run_untagged = false - end - - it 'cannot handle builds without tags' do - expect(build.can_be_served?(runner)).to be_falsey - end - end - - context 'when having runner tags' do - before do - runner.tag_list = ['bb', 'cc'] - end - - it "#{serve_matching_tags} handle it for matching tags" do - build.tag_list = ['bb'] - expected = if serve_matching_tags - be_truthy - else - be_falsey - end - expect(build.can_be_served?(runner)).to expected - end - - it 'cannot handle it for builds without matching tags' do - build.tag_list = ['aa'] - expect(build.can_be_served?(runner)).to be_falsey - end - end - end - - context 'when serving the same project' do - it 'can handle it' do - expect(build.can_be_served?(runner)).to be_truthy - end - - it_behaves_like 'locked build picker', true - end - - context 'serving a different project' do - before do - runner.runner_projects.destroy_all - end - - it 'cannot handle it' do - expect(build.can_be_served?(runner)).to be_falsey - end - - it_behaves_like 'locked build picker', false - end - end - end - describe '#has_tags?' do context 'when build has tags' do subject { create(:ci_build, tag_list: ['tag']) } @@ -431,7 +315,7 @@ describe Ci::Build, models: true do end it 'that cannot handle build' do - expect_any_instance_of(Ci::Build).to receive(:can_be_served?).and_return(false) + expect_any_instance_of(Ci::Runner).to receive(:can_pick?).and_return(false) is_expected.to be_falsey end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index c3b715dcb92..477a32a1f9e 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -90,6 +90,125 @@ describe Ci::Runner, models: true do end end + describe '#can_pick?' do + let(:project) { create(:project) } + let(:commit) { create(:ci_commit, project: project) } + let(:build) { create(:ci_build, commit: commit) } + let(:runner) { create(:ci_runner) } + + before do + build.project.runners << runner + end + + context 'when runner does not have tags' do + it 'can handle builds without tags' do + expect(runner.can_pick?(build)).to be_truthy + end + + it 'cannot handle build with tags' do + build.tag_list = ['aa'] + expect(runner.can_pick?(build)).to be_falsey + end + end + + context 'when runner has tags' do + before do + runner.tag_list = ['bb', 'cc'] + end + + shared_examples 'tagged build picker' do + it 'can handle build with matching tags' do + build.tag_list = ['bb'] + expect(runner.can_pick?(build)).to be_truthy + end + + it 'cannot handle build without matching tags' do + build.tag_list = ['aa'] + expect(runner.can_pick?(build)).to be_falsey + end + end + + context 'when runner can pick untagged jobs' do + it 'can handle builds without tags' do + expect(runner.can_pick?(build)).to be_truthy + end + + it_behaves_like 'tagged build picker' + end + + context 'when runner cannot pick untagged jobs' do + before do + runner.run_untagged = false + end + + it 'cannot handle builds without tags' do + expect(runner.can_pick?(build)).to be_falsey + end + + it_behaves_like 'tagged build picker' + end + end + + context 'when runner is locked' do + before do + runner.locked = true + end + + shared_examples 'locked build picker' do |serve_matching_tags| + context 'when runner cannot pick untagged jobs' do + before do + runner.run_untagged = false + end + + it 'cannot handle builds without tags' do + expect(runner.can_pick?(build)).to be_falsey + end + end + + context 'when having runner tags' do + before do + runner.tag_list = ['bb', 'cc'] + end + + it "#{serve_matching_tags} handle it for matching tags" do + build.tag_list = ['bb'] + expected = if serve_matching_tags + be_truthy + else + be_falsey + end + expect(runner.can_pick?(build)).to expected + end + + it 'cannot handle it for builds without matching tags' do + build.tag_list = ['aa'] + expect(runner.can_pick?(build)).to be_falsey + end + end + end + + context 'when serving the same project' do + it 'can handle it' do + expect(runner.can_pick?(build)).to be_truthy + end + + it_behaves_like 'locked build picker', true + end + + context 'serving a different project' do + before do + runner.runner_projects.destroy_all + end + + it 'cannot handle it' do + expect(runner.can_pick?(build)).to be_falsey + end + + it_behaves_like 'locked build picker', false + end + end + end + describe '#status' do let(:runner) { FactoryGirl.create(:ci_runner, :shared, contacted_at: 1.second.ago) }