From 743c32270e2913a19999bd32d6208e80dd62dc2a Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 5 Oct 2017 13:29:21 +0200 Subject: [PATCH] select group runners also in register_job_service --- app/services/ci/register_job_service.rb | 47 ++++++++--- spec/services/ci/register_job_service_spec.rb | 79 +++++++++++++++++-- 2 files changed, 111 insertions(+), 15 deletions(-) diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 0b087ad73da..000ae3539e3 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -17,6 +17,8 @@ module Ci builds = if runner.shared? builds_for_shared_runner + elsif runner.group? + builds_for_group_runner else builds_for_specific_runner end @@ -69,16 +71,33 @@ module Ci 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, pending_delete: false }) - .joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id') - .where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0'). + builds_for_scheduled_runner( + running_builds_for_shared_runners, + projects: { shared_runners_enabled: true } + ) + end - # 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.project_id=project_builds.project_id") + def builds_for_group_runner + builds_for_scheduled_runner( + running_builds_for_group_runners, + projects: { group_runners_enabled: true } + ) + end + + def builds_for_scheduled_runner(build_join, project_where) + project_where = project_where.deep_merge(projects: { pending_delete: false }) + + # don't run projects which have not enabled group runners and builds + builds = new_builds + .joins(:project).where(project_where) + .joins('LEFT JOIN project_features ON ci_builds.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 group runners at all + builds + .joins("LEFT JOIN (#{build_join.to_sql}) AS project_builds ON ci_builds.project_id=project_builds.project_id") .order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC') end @@ -87,7 +106,15 @@ module Ci end def running_builds_for_shared_runners - Ci::Build.running.where(runner: Ci::Runner.shared) + running_builds_for_runners(Ci::Runner.shared) + end + + def running_builds_for_group_runners + running_builds_for_runners(Ci::Runner.joins(:runner_groups)) + end + + def running_builds_for_runners(runners) + Ci::Build.running.where(runner: runners) .group(:project_id).select(:project_id, 'count(*) AS running_builds') end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 8a537e83d5f..138ef673527 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' module Ci describe RegisterJobService do - let!(:project) { FactoryBot.create :project, shared_runners_enabled: false } - let!(:pipeline) { FactoryBot.create :ci_pipeline, project: project } - let!(:pending_job) { FactoryBot.create :ci_build, pipeline: pipeline } - let!(:shared_runner) { FactoryBot.create(:ci_runner, is_shared: true) } - let!(:specific_runner) { FactoryBot.create(:ci_runner, is_shared: false) } + let!(:project) { create :project, shared_runners_enabled: false } + let!(:group) { create :group } + let!(:pipeline) { create :ci_pipeline, project: project } + let!(:pending_job) { create :ci_build, pipeline: pipeline } + let!(:shared_runner) { create :ci_runner, is_shared: true } + let!(:specific_runner) { create :ci_runner, is_shared: false } + let!(:group_runner) { create :ci_runner, groups: [group] } before do specific_runner.assign_to(project) @@ -167,6 +169,73 @@ module Ci end end + context 'allow group runners' do + before do + project.update!(group_runners_enabled: true, group: group) + end + + context 'for multiple builds' do + let!(:project2) { create :project, group_runners_enabled: true, group: group } + let!(:pipeline2) { create :ci_pipeline, project: project2 } + let!(:project3) { create :project, group_runners_enabled: true, group: group } + let!(:pipeline3) { create :ci_pipeline, project: project3 } + let!(:build1_project1) { pending_job } + let!(:build2_project1) { create :ci_build, pipeline: pipeline } + let!(:build3_project1) { create :ci_build, pipeline: pipeline } + let!(:build1_project2) { create :ci_build, pipeline: pipeline2 } + let!(:build2_project2) { create :ci_build, pipeline: pipeline2 } + let!(:build1_project3) { create :ci_build, pipeline: pipeline3 } + + it 'prefers projects without builds first' do + # it gets for one build from each of the projects + expect(execute(group_runner)).to eq(build1_project1) + expect(execute(group_runner)).to eq(build1_project2) + expect(execute(group_runner)).to eq(build1_project3) + + # then it gets a second build from each of the projects + expect(execute(group_runner)).to eq(build2_project1) + expect(execute(group_runner)).to eq(build2_project2) + + # in the end the third build + expect(execute(group_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(group_runner)).to eq(build1_project1) + build1_project1.reload.success + expect(execute(group_runner)).to eq(build2_project1) + + expect(execute(group_runner)).to eq(build1_project2) + build1_project2.reload.success + expect(execute(group_runner)).to eq(build2_project2) + expect(execute(group_runner)).to eq(build1_project3) + expect(execute(group_runner)).to eq(build3_project1) + end + end + + context 'group runner' do + let(:build) { execute(group_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(group_runner) } + end + end + + context 'disallow group runners' do + before do + project.update(group_runners_enabled: false) + end + + context 'group runner' do + let(:build) { execute(group_runner) } + + it { expect(build).to be_nil } + end + end + context 'when first build is stalled' do before do pending_job.update(lock_version: 0)