From 3eafffcef0f8932bab4e06b465f9b63327e87942 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 16 Feb 2017 01:05:44 +0100 Subject: Refactor JobRequest response structure --- app/models/ci/build.rb | 29 +++ app/services/ci/register_build_service.rb | 85 --------- app/services/ci/register_job_service.rb | 85 +++++++++ lib/api/entities.rb | 94 +++++++--- lib/api/runner.rb | 6 +- lib/ci/api/builds.rb | 2 +- lib/gitlab/ci/build/response/image.rb | 20 +++ lib/gitlab/ci/build/response/step.rb | 46 +++++ spec/factories/ci/builds.rb | 26 ++- spec/requests/api/runner_spec.rb | 51 ++++-- spec/services/ci/register_build_service_spec.rb | 223 ------------------------ spec/services/ci/register_job_service_spec.rb | 223 ++++++++++++++++++++++++ 12 files changed, 539 insertions(+), 351 deletions(-) delete mode 100644 app/services/ci/register_build_service.rb create mode 100644 app/services/ci/register_job_service.rb create mode 100644 lib/gitlab/ci/build/response/image.rb create mode 100644 lib/gitlab/ci/build/response/step.rb delete mode 100644 spec/services/ci/register_build_service_spec.rb create mode 100644 spec/services/ci/register_job_service_spec.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index f2989eff22d..0c0bbb9d41d 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -508,6 +508,35 @@ module Ci ] end + def steps + [ + Gitlab::Ci::Build::Response::Step.from_commands(self), + Gitlab::Ci::Build::Response::Step.from_after_script(self) + ].compact + end + + def image + image = Gitlab::Ci::Build::Response::Image.new(options[:image]) + return unless image.valid? + image + end + + def services + services = options[:services].map do |service| + Gitlab::Ci::Build::Response::Image.new(service) + end + + services.select(&:valid?).compact + end + + def artifacts + options[:artifacts] + end + + def cache + options[:cache] + end + def credentials Gitlab::Ci::Build::Credentials::Factory.new(self).create! end diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb deleted file mode 100644 index 5b52a0425de..00000000000 --- a/app/services/ci/register_build_service.rb +++ /dev/null @@ -1,85 +0,0 @@ -module Ci - # This class responsible for assigning - # proper pending build to runner on runner API request - class RegisterBuildService - include Gitlab::CurrentSettings - - attr_reader :runner - - Result = Struct.new(:build, :valid?) - - def initialize(runner) - @runner = runner - end - - def execute - builds = - if runner.shared? - builds_for_shared_runner - else - builds_for_specific_runner - end - - valid = true - - builds.find do |build| - next unless runner.can_pick?(build) - - begin - # 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 = runner.id - build.run! - - return Result.new(build, true) - rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError - # We are looping to find another build that is not conflicting - # It also indicates that this build can be picked and passed to runner. - # If we don't do it, basically a bunch of runners would be competing for a build - # and thus we will generate a lot of 409. This will increase - # the number of generated requests, also will reduce significantly - # how many builds can be picked by runner in a unit of time. - # In case we hit the concurrency-access lock, - # we still have to return 409 in the end, - # to make sure that this is properly handled by runner. - valid = false - end - end - - Result.new(nil, valid) - end - - private - - def builds_for_shared_runner - new_builds. - # don't run projects which have not enabled shared runners and builds - joins(:project).where(projects: { shared_runners_enabled: true }). - joins('LEFT JOIN project_features ON ci_builds.gl_project_id = project_features.project_id'). - where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0'). - - # Implement fair scheduling - # this returns builds that are ordered by number of running builds - # we prefer projects that don't use shared runners at all - joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.gl_project_id=project_builds.gl_project_id"). - order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC') - end - - def builds_for_specific_runner - new_builds.where(project: runner.projects.with_builds_enabled).order('created_at ASC') - end - - def running_builds_for_shared_runners - Ci::Build.running.where(runner: Ci::Runner.shared). - group(:gl_project_id).select(:gl_project_id, 'count(*) AS running_builds') - end - - def new_builds - Ci::Build.pending.unstarted - end - - def shared_runner_build_limits_feature_enabled? - ENV['DISABLE_SHARED_RUNNER_BUILD_MINUTES_LIMIT'].to_s != 'true' - end - end -end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb new file mode 100644 index 00000000000..0ab9042bf24 --- /dev/null +++ b/app/services/ci/register_job_service.rb @@ -0,0 +1,85 @@ +module Ci + # This class responsible for assigning + # proper pending build to runner on runner API request + class RegisterJobService + include Gitlab::CurrentSettings + + attr_reader :runner + + Result = Struct.new(:build, :valid?) + + def initialize(runner) + @runner = runner + end + + def execute + builds = + if runner.shared? + builds_for_shared_runner + else + builds_for_specific_runner + end + + valid = true + + builds.find do |build| + next unless runner.can_pick?(build) + + begin + # 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 = runner.id + build.run! + + return Result.new(build, true) + rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError + # We are looping to find another build that is not conflicting + # It also indicates that this build can be picked and passed to runner. + # If we don't do it, basically a bunch of runners would be competing for a build + # and thus we will generate a lot of 409. This will increase + # the number of generated requests, also will reduce significantly + # how many builds can be picked by runner in a unit of time. + # In case we hit the concurrency-access lock, + # we still have to return 409 in the end, + # to make sure that this is properly handled by runner. + valid = false + end + end + + Result.new(nil, valid) + end + + private + + def builds_for_shared_runner + new_builds. + # don't run projects which have not enabled shared runners and builds + joins(:project).where(projects: { shared_runners_enabled: true }). + joins('LEFT JOIN project_features ON ci_builds.gl_project_id = project_features.project_id'). + where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0'). + + # Implement fair scheduling + # this returns builds that are ordered by number of running builds + # we prefer projects that don't use shared runners at all + joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.gl_project_id=project_builds.gl_project_id"). + order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC') + end + + def builds_for_specific_runner + new_builds.where(project: runner.projects.with_builds_enabled).order('created_at ASC') + end + + def running_builds_for_shared_runners + Ci::Build.running.where(runner: Ci::Runner.shared). + group(:gl_project_id).select(:gl_project_id, 'count(*) AS running_builds') + end + + def new_builds + Ci::Build.pending.unstarted + end + + def shared_runner_build_limits_feature_enabled? + ENV['DISABLE_SHARED_RUNNER_BUILD_MINUTES_LIMIT'].to_s != 'true' + end + end +end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index cb3b409b8d0..7e07154600a 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -698,42 +698,82 @@ module API expose :active?, as: :active end - class ArtifactFile < Grape::Entity - expose :filename, :size - end + module JobRequest + class JobInfo < Grape::Entity + expose :name, :stage + expose :project_id, :project_name + end + + class GitInfo < Grape::Entity + expose :repo_url, :ref, :sha, :before_sha + expose :ref_type do |model| + if model.tag + 'tag' + else + 'branch' + end + end + end - class JobCredentials < Grape::Entity - expose :type, :url, :username, :password - end + class RunnerInfo < Grape::Entity + expose :timeout + end - class JobResponse < Grape::Entity - expose :id, :ref, :tag, :sha, :status - expose :name, :token, :stage - expose :project_id - expose :project_name - expose :artifacts_file, using: ArtifactFile, if: ->(build, _) { build.artifacts? } - end + class Step < Grape::Entity + expose :name, :script, :timeout, :condition, :result + end - class RequestJobResponse < JobResponse - expose :commands - expose :repo_url - expose :before_sha - expose :allow_git_fetch - expose :token - expose :artifacts_expire_at, if: ->(build, _) { build.artifacts? } + class Image < Grape::Entity + expose :name + end - expose :options do |model| - model.options + class Artifacts < Grape::Entity + expose :name, :untracked, :paths, :when, :expire_in end - expose :timeout do |model| - model.timeout + class Cache < Grape::Entity + expose :key, :untracked, :paths end - expose :variables - expose :depends_on_builds, using: JobResponse + class Credentials < Grape::Entity + expose :type, :url, :username, :password + end - expose :credentials, using: JobCredentials + class ArtifactFile < Grape::Entity + expose :filename, :size + end + + class Dependency < Grape::Entity + expose :id, :name + expose :artifacts_file, using: ArtifactFile, if: ->(job, _) { job.artifacts? } + end + + class Response < Grape::Entity + expose :id + expose :token + expose :allow_git_fetch + + expose :job_info, using: JobInfo do |model| + model + end + + expose :git_info, using: GitInfo do |model| + model + end + + expose :runner_info, using: RunnerInfo do |model| + model + end + + expose :variables + expose :steps, using: Step + expose :image, using: Image + expose :services, using: Image + expose :artifacts, using: Artifacts + expose :cache, using: Cache + expose :credentials, using: Credentials + expose :depends_on_builds, as: :dependencies, using: Dependency + end end end end diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 2e7b96e5169..8372b794739 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -51,7 +51,7 @@ module API resource :jobs do desc 'Request a job' do - success Entities::RequestJobResponse + success Entities::JobRequest::Response end params do requires :token, type: String, desc: %q(Runner's authentication token) @@ -68,13 +68,13 @@ module API end new_update = current_runner.ensure_runner_queue_value - result = ::Ci::RegisterBuildService.new(current_runner).execute + result = ::Ci::RegisterJobService.new(current_runner).execute if result.valid? if result.build Gitlab::Metrics.add_event(:build_found, project: result.build.project.path_with_namespace) - present result.build, with: Entities::RequestJobResponse + present result.build, with: Entities::JobRequest::Response else Gitlab::Metrics.add_event(:build_not_found) header 'X-GitLab-Last-Update', new_update diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index b51e76d93f2..746e76a1b1f 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -24,7 +24,7 @@ module Ci new_update = current_runner.ensure_runner_queue_value - result = Ci::RegisterBuildService.new(current_runner).execute + result = Ci::RegisterJobService.new(current_runner).execute if result.valid? if result.build diff --git a/lib/gitlab/ci/build/response/image.rb b/lib/gitlab/ci/build/response/image.rb new file mode 100644 index 00000000000..342a249aee8 --- /dev/null +++ b/lib/gitlab/ci/build/response/image.rb @@ -0,0 +1,20 @@ +module Gitlab + module Ci + module Build + module Response + class Image + attr_reader :name + + def initialize(image) + type = image.class + @name = image if type == String + end + + def valid? + @name != nil + end + end + end + end + end +end \ No newline at end of file diff --git a/lib/gitlab/ci/build/response/step.rb b/lib/gitlab/ci/build/response/step.rb new file mode 100644 index 00000000000..7b35852a510 --- /dev/null +++ b/lib/gitlab/ci/build/response/step.rb @@ -0,0 +1,46 @@ +module Gitlab + module Ci + module Build + module Response + class Step + CONDITION_IF_SUCCEEDED = 'run_if_succeeded' + CONDITION_ALWAYS = 'run_always' + + RESULT_FAILS_JOB = 'fails_job' + RESULT_DOESNT_FAIL_JOB = 'doesnt_fail_job' + + attr_reader :name, :script, :condition, :result, :timeout + + class << self + def from_commands(build) + self.new(:script, + build.commands, + build.timeout, + CONDITION_IF_SUCCEEDED, + RESULT_FAILS_JOB) + end + + def from_after_script(build) + after_script = build.options[:after_script] + return unless after_script + + self.new(:after_script, + after_script, + build.timeout, + CONDITION_ALWAYS, + RESULT_DOESNT_FAIL_JOB) + end + end + + def initialize(name, script, timeout, condition = CONDITION_IF_SUCCEEDED, result = RESULT_FAILS_JOB) + @name = name + @script = script.split("\n") + @timeout = timeout + @condition = condition + @result = result + end + end + end + end + end +end \ No newline at end of file diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index a90534d10ba..a77b3356b9a 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -15,8 +15,8 @@ FactoryGirl.define do options do { - image: "ruby:2.1", - services: ["postgres"] + image: 'ruby:2.1', + services: ['postgres'] } end @@ -151,5 +151,27 @@ FactoryGirl.define do allow(build).to receive(:commit).and_return build(:commit) end end + + trait :extended_options do + options do + { + image: 'ruby:2.1', + services: ['postgres'], + after_script: "ls\ndate", + artifacts: { + name: 'artifacts_file', + untracked: false, + paths: ['out/'], + when: 'always', + expire_in: '7d' + }, + cache: { + key: 'cache_key', + untracked: false, + paths: ['vendor/*'] + } + } + end + end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 3b94be55974..bd9dc422703 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -152,8 +152,8 @@ describe API::Runner do describe '/api/v4/jobs' do let(:project) { create(:empty_project, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } - let!(:job) { create(:ci_build, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } let(:runner) { create(:ci_runner) } + let!(:job) { create(:ci_build, :artifacts, :extended_options, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate") } before { project.runners << runner } @@ -271,14 +271,44 @@ describe API::Runner do expect(response).to have_http_status(201) expect(response.headers).not_to have_key('X-GitLab-Last-Update') - expect(json_response['sha']).to eq(job.sha) - expect(json_response['options']).to eq({'image' => 'ruby:2.1', 'services' => ['postgres']}) - expect(json_response['variables']).to include( - {'key' => 'CI_BUILD_NAME', 'value' => 'spinach', 'public' => true}, - {'key' => 'CI_BUILD_STAGE', 'value' => 'test', 'public' => true}, - {'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true} - ) expect(runner.reload.platform).to eq('darwin') + + expect(json_response['id']).to eq(job.id) + expect(json_response['token']).to eq(job.token) + expect(json_response['job_info']).to include({ 'name' => job.name }, + { 'stage' => job.stage }) + expect(json_response['git_info']).to include({ 'sha' => job.sha }, + { 'repo_url' => job.repo_url }) + expect(json_response['image']).to include({ 'name' => 'ruby:2.1' }) + expect(json_response['services']).to include({ 'name' => 'postgres' }) + expect(json_response['steps']).to include({ 'name' => 'after_script', + 'script' => ['ls', 'date'], + 'timeout' => job.timeout, + 'condition' => Gitlab::Ci::Build::Response::Step::CONDITION_ALWAYS, + 'result' => Gitlab::Ci::Build::Response::Step::RESULT_DOESNT_FAIL_JOB }) + expect(json_response['variables']).to include({ 'key' => 'CI_BUILD_NAME', 'value' => 'spinach', 'public' => true }, + { 'key' => 'CI_BUILD_STAGE', 'value' => 'test', 'public' => true }, + { 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true }) + expect(json_response['artifacts']).to include({ 'name' => 'artifacts_file' }, + { 'paths' => ['out/'] }) + end + + context 'when job is made for tag' do + let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + + it 'sets branch as ref_type' do + request_job + expect(response).to have_http_status(201) + expect(json_response['git_info']['ref_type']).to eq('tag') + end + end + + context 'when job is made for branch' do + it 'sets tag as ref_type' do + request_job + expect(response).to have_http_status(201) + expect(json_response['git_info']['ref_type']).to eq('branch') + end end it 'updates runner info' do @@ -322,8 +352,8 @@ describe API::Runner do expect(response).to have_http_status(201) expect(json_response['id']).to eq(test_job.id) - expect(json_response['depends_on_builds'].count).to eq(1) - expect(json_response['depends_on_builds'][0]).to include('id' => job.id, 'name' => 'spinach') + expect(json_response['dependencies'].count).to eq(1) + expect(json_response['dependencies'][0]).to include('id' => job.id, 'name' => 'spinach') end end @@ -381,6 +411,7 @@ describe API::Runner do it 'sends registry credentials key' do request_job + expect(json_response).to have_key('credentials') expect(json_response['credentials']).to include(registry_credentials) end diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_build_service_spec.rb deleted file mode 100644 index cd7dd53025c..00000000000 --- a/spec/services/ci/register_build_service_spec.rb +++ /dev/null @@ -1,223 +0,0 @@ -require 'spec_helper' - -module Ci - describe RegisterBuildService, services: true do - 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 } - let!(:shared_runner) { FactoryGirl.create(:ci_runner, is_shared: true) } - let!(:specific_runner) { FactoryGirl.create(:ci_runner, is_shared: false) } - - before do - specific_runner.assign_to(project) - end - - describe '#execute' do - context 'runner follow tag list' do - it "picks build with the same tag" do - pending_build.tag_list = ["linux"] - pending_build.save - specific_runner.tag_list = ["linux"] - 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(execute(specific_runner)).to be_falsey - end - - it "picks build without tag" do - 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(execute(specific_runner)).to be_falsey - end - - it "pick build without tag" do - specific_runner.tag_list = ["win32"] - expect(execute(specific_runner)).to eq(pending_build) - end - end - - context 'deleted projects' do - before do - project.update(pending_delete: true) - end - - context 'for shared runners' do - before do - project.update(shared_runners_enabled: true) - end - - it 'does not pick a build' do - expect(execute(shared_runner)).to be_nil - end - end - - context 'for specific runner' do - it 'does not pick a build' do - expect(execute(specific_runner)).to be_nil - end - end - end - - context 'allow shared runners' do - before do - project.update(shared_runners_enabled: true) - end - - context 'for multiple builds' do - let!(:project2) { create :empty_project, shared_runners_enabled: true } - let!(:pipeline2) { create :ci_pipeline, project: project2 } - let!(:project3) { create :empty_project, shared_runners_enabled: true } - let!(:pipeline3) { create :ci_pipeline, project: project3 } - let!(:build1_project1) { pending_build } - let!(:build2_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } - let!(:build3_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } - let!(:build1_project2) { FactoryGirl.create :ci_build, pipeline: pipeline2 } - let!(:build2_project2) { FactoryGirl.create :ci_build, pipeline: pipeline2 } - let!(:build1_project3) { FactoryGirl.create :ci_build, pipeline: pipeline3 } - - it 'prefers projects without builds first' do - # it gets for one build from each of the projects - 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(execute(shared_runner)).to eq(build2_project1) - expect(execute(shared_runner)).to eq(build2_project2) - - # in the end the third build - 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(execute(shared_runner)).to eq(build1_project1) - build1_project1.reload.success - expect(execute(shared_runner)).to eq(build2_project1) - - expect(execute(shared_runner)).to eq(build1_project2) - build1_project2.reload.success - 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) { execute(shared_runner) } - - it { expect(build).to be_kind_of(Build) } - it { expect(build).to be_valid } - it { expect(build).to be_running } - it { expect(build.runner).to eq(shared_runner) } - end - - context 'specific runner' do - let(:build) { execute(specific_runner) } - - it { expect(build).to be_kind_of(Build) } - it { expect(build).to be_valid } - it { expect(build).to be_running } - it { expect(build.runner).to eq(specific_runner) } - end - end - - context 'disallow shared runners' do - before do - project.update(shared_runners_enabled: false) - end - - context 'shared runner' do - let(:build) { execute(shared_runner) } - - it { expect(build).to be_nil } - end - - context 'specific runner' do - let(:build) { execute(specific_runner) } - - it { expect(build).to be_kind_of(Build) } - it { expect(build).to be_valid } - it { expect(build).to be_running } - it { expect(build.runner).to eq(specific_runner) } - end - end - - context 'disallow when builds are disabled' do - before do - project.update(shared_runners_enabled: true) - project.project_feature.update_attribute(:builds_access_level, ProjectFeature::DISABLED) - end - - context 'and uses shared runner' do - let(:build) { execute(shared_runner) } - - it { expect(build).to be_nil } - end - - context 'and uses specific runner' do - let(:build) { execute(specific_runner) } - - it { expect(build).to be_nil } - end - end - - context 'when first build is stalled' do - before do - pending_build.lock_version = 10 - end - - subject { described_class.new(specific_runner).execute } - - context 'with multiple builds are in queue' do - let!(:other_build) { create :ci_build, pipeline: pipeline } - - before do - allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) - .and_return([pending_build, other_build]) - end - - it "receives second build from the queue" do - expect(subject).to be_valid - expect(subject.build).to eq(other_build) - end - end - - context 'when single build is in queue' do - before do - allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) - .and_return([pending_build]) - end - - it "does not receive any valid result" do - expect(subject).not_to be_valid - end - end - - context 'when there is no build in queue' do - before do - allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) - .and_return([]) - end - - it "does not receive builds but result is valid" do - expect(subject).to be_valid - expect(subject.build).to be_nil - end - end - end - - def execute(runner) - described_class.new(runner).execute.build - end - end - end -end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb new file mode 100644 index 00000000000..ea8b996d481 --- /dev/null +++ b/spec/services/ci/register_job_service_spec.rb @@ -0,0 +1,223 @@ +require 'spec_helper' + +module Ci + describe RegisterJobService, services: true do + 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 } + let!(:shared_runner) { FactoryGirl.create(:ci_runner, is_shared: true) } + let!(:specific_runner) { FactoryGirl.create(:ci_runner, is_shared: false) } + + before do + specific_runner.assign_to(project) + end + + describe '#execute' do + context 'runner follow tag list' do + it "picks build with the same tag" do + pending_build.tag_list = ["linux"] + pending_build.save + specific_runner.tag_list = ["linux"] + 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(execute(specific_runner)).to be_falsey + end + + it "picks build without tag" do + 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(execute(specific_runner)).to be_falsey + end + + it "pick build without tag" do + specific_runner.tag_list = ["win32"] + expect(execute(specific_runner)).to eq(pending_build) + end + end + + context 'deleted projects' do + before do + project.update(pending_delete: true) + end + + context 'for shared runners' do + before do + project.update(shared_runners_enabled: true) + end + + it 'does not pick a build' do + expect(execute(shared_runner)).to be_nil + end + end + + context 'for specific runner' do + it 'does not pick a build' do + expect(execute(specific_runner)).to be_nil + end + end + end + + context 'allow shared runners' do + before do + project.update(shared_runners_enabled: true) + end + + context 'for multiple builds' do + let!(:project2) { create :empty_project, shared_runners_enabled: true } + let!(:pipeline2) { create :ci_pipeline, project: project2 } + let!(:project3) { create :empty_project, shared_runners_enabled: true } + let!(:pipeline3) { create :ci_pipeline, project: project3 } + let!(:build1_project1) { pending_build } + let!(:build2_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } + let!(:build3_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } + let!(:build1_project2) { FactoryGirl.create :ci_build, pipeline: pipeline2 } + let!(:build2_project2) { FactoryGirl.create :ci_build, pipeline: pipeline2 } + let!(:build1_project3) { FactoryGirl.create :ci_build, pipeline: pipeline3 } + + it 'prefers projects without builds first' do + # it gets for one build from each of the projects + 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(execute(shared_runner)).to eq(build2_project1) + expect(execute(shared_runner)).to eq(build2_project2) + + # in the end the third build + 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(execute(shared_runner)).to eq(build1_project1) + build1_project1.reload.success + expect(execute(shared_runner)).to eq(build2_project1) + + expect(execute(shared_runner)).to eq(build1_project2) + build1_project2.reload.success + 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) { execute(shared_runner) } + + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(shared_runner) } + end + + context 'specific runner' do + let(:build) { execute(specific_runner) } + + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(specific_runner) } + end + end + + context 'disallow shared runners' do + before do + project.update(shared_runners_enabled: false) + end + + context 'shared runner' do + let(:build) { execute(shared_runner) } + + it { expect(build).to be_nil } + end + + context 'specific runner' do + let(:build) { execute(specific_runner) } + + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(specific_runner) } + end + end + + context 'disallow when builds are disabled' do + before do + project.update(shared_runners_enabled: true) + project.project_feature.update_attribute(:builds_access_level, ProjectFeature::DISABLED) + end + + context 'and uses shared runner' do + let(:build) { execute(shared_runner) } + + it { expect(build).to be_nil } + end + + context 'and uses specific runner' do + let(:build) { execute(specific_runner) } + + it { expect(build).to be_nil } + end + end + + context 'when first build is stalled' do + before do + pending_build.lock_version = 10 + end + + subject { described_class.new(specific_runner).execute } + + context 'with multiple builds are in queue' do + let!(:other_build) { create :ci_build, pipeline: pipeline } + + before do + allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) + .and_return([pending_build, other_build]) + end + + it "receives second build from the queue" do + expect(subject).to be_valid + expect(subject.build).to eq(other_build) + end + end + + context 'when single build is in queue' do + before do + allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) + .and_return([pending_build]) + end + + it "does not receive any valid result" do + expect(subject).not_to be_valid + end + end + + context 'when there is no build in queue' do + before do + allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) + .and_return([]) + end + + it "does not receive builds but result is valid" do + expect(subject).to be_valid + expect(subject.build).to be_nil + end + end + end + + def execute(runner) + described_class.new(runner).execute.build + end + end + end +end -- cgit v1.2.1