Commit 6cfe60df authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch 'backport-ee-changes-for-build-minutes' into 'master'

Backport changes introduced by https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1078

See merge request !8657
parents b55c1bc4 b368447c
...@@ -4,6 +4,7 @@ class Namespace < ActiveRecord::Base ...@@ -4,6 +4,7 @@ class Namespace < ActiveRecord::Base
include CacheMarkdownField include CacheMarkdownField
include Sortable include Sortable
include Gitlab::ShellAdapter include Gitlab::ShellAdapter
include Gitlab::CurrentSettings
include Routable include Routable
cache_markdown_field :description, pipeline: :description cache_markdown_field :description, pipeline: :description
...@@ -176,6 +177,10 @@ class Namespace < ActiveRecord::Base ...@@ -176,6 +177,10 @@ class Namespace < ActiveRecord::Base
end end
end end
def shared_runners_enabled?
projects.with_shared_runners.any?
end
def full_name def full_name
@full_name ||= @full_name ||=
if parent if parent
......
...@@ -224,6 +224,7 @@ class Project < ActiveRecord::Base ...@@ -224,6 +224,7 @@ class Project < ActiveRecord::Base
scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') } scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }
scope :with_statistics, -> { includes(:statistics) } scope :with_statistics, -> { includes(:statistics) }
scope :with_shared_runners, -> { where(shared_runners_enabled: true) }
# "enabled" here means "not disabled". It includes private features! # "enabled" here means "not disabled". It includes private features!
scope :with_feature_enabled, ->(feature) { scope :with_feature_enabled, ->(feature) {
...@@ -1096,12 +1097,20 @@ class Project < ActiveRecord::Base ...@@ -1096,12 +1097,20 @@ class Project < ActiveRecord::Base
project_feature.update_attribute(:builds_access_level, ProjectFeature::ENABLED) project_feature.update_attribute(:builds_access_level, ProjectFeature::ENABLED)
end end
def shared_runners_available?
shared_runners_enabled?
end
def shared_runners
shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none
end
def any_runners?(&block) def any_runners?(&block)
if runners.active.any?(&block) if runners.active.any?(&block)
return true return true
end end
shared_runners_enabled? && Ci::Runner.shared.active.any?(&block) shared_runners.active.any?(&block)
end end
def valid_runners_token?(token) def valid_runners_token?(token)
......
...@@ -2,34 +2,30 @@ module Ci ...@@ -2,34 +2,30 @@ module Ci
# This class responsible for assigning # This class responsible for assigning
# proper pending build to runner on runner API request # proper pending build to runner on runner API request
class RegisterBuildService class RegisterBuildService
def execute(current_runner) include Gitlab::CurrentSettings
builds = Ci::Build.pending.unstarted
attr_reader :runner
def initialize(runner)
@runner = runner
end
def execute
builds = builds =
if current_runner.shared? if runner.shared?
builds. builds_for_shared_runner
# 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').
# 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").
where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0').
order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC')
else else
# do run projects which are only assigned to this runner (FIFO) builds_for_specific_runner
builds.where(project: current_runner.projects.with_builds_enabled).order('created_at ASC')
end end
build = builds.find do |build| build = builds.find do |build|
current_runner.can_pick?(build) runner.can_pick?(build)
end end
if build if build
# In case when 2 runners try to assign the same build, second runner will be declined # 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. # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method.
build.runner_id = current_runner.id build.runner_id = runner.id
build.run! build.run!
end end
...@@ -41,9 +37,35 @@ module Ci ...@@ -41,9 +37,35 @@ module Ci
private 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 def running_builds_for_shared_runners
Ci::Build.running.where(runner: Ci::Runner.shared). Ci::Build.running.where(runner: Ci::Runner.shared).
group(:gl_project_id).select(:gl_project_id, 'count(*) AS running_builds') group(:gl_project_id).select(:gl_project_id, 'count(*) AS running_builds')
end 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
end end
...@@ -23,7 +23,7 @@ module Ci ...@@ -23,7 +23,7 @@ module Ci
new_update = current_runner.ensure_runner_queue_value new_update = current_runner.ensure_runner_queue_value
build = Ci::RegisterBuildService.new.execute(current_runner) build = Ci::RegisterBuildService.new(current_runner).execute
if build if build
Gitlab::Metrics.add_event(:build_found, Gitlab::Metrics.add_event(:build_found,
......
...@@ -11,6 +11,7 @@ module Gitlab ...@@ -11,6 +11,7 @@ module Gitlab
included do included do
scope :public_only, -> { where(visibility_level: PUBLIC) } scope :public_only, -> { where(visibility_level: PUBLIC) }
scope :public_and_internal_only, -> { where(visibility_level: [PUBLIC, INTERNAL] ) } scope :public_and_internal_only, -> { where(visibility_level: [PUBLIC, INTERNAL] ) }
scope :non_public_only, -> { where.not(visibility_level: PUBLIC) }
scope :public_to_user, -> (user) { user && !user.external ? public_and_internal_only : public_only } scope :public_to_user, -> (user) { user && !user.external ? public_and_internal_only : public_only }
end end
......
...@@ -16,6 +16,10 @@ FactoryGirl.define do ...@@ -16,6 +16,10 @@ FactoryGirl.define do
is_shared true is_shared true
end end
trait :specific do
is_shared false
end
trait :inactive do trait :inactive do
active false active false
end end
......
FactoryGirl.define do FactoryGirl.define do
factory :group do factory :group, class: Group, parent: :namespace do
sequence(:name) { |n| "group#{n}" } sequence(:name) { |n| "group#{n}" }
path { name.downcase.gsub(/\s/, '_') } path { name.downcase.gsub(/\s/, '_') }
type 'Group' type 'Group'
owner nil
trait :public do trait :public do
visibility_level Gitlab::VisibilityLevel::PUBLIC visibility_level Gitlab::VisibilityLevel::PUBLIC
......
...@@ -81,13 +81,19 @@ describe Group, models: true do ...@@ -81,13 +81,19 @@ describe Group, models: true do
describe 'public_only' do describe 'public_only' do
subject { described_class.public_only.to_a } subject { described_class.public_only.to_a }
it{ is_expected.to eq([group]) } it { is_expected.to eq([group]) }
end end
describe 'public_and_internal_only' do describe 'public_and_internal_only' do
subject { described_class.public_and_internal_only.to_a } subject { described_class.public_and_internal_only.to_a }
it{ is_expected.to match_array([group, internal_group]) } it { is_expected.to match_array([group, internal_group]) }
end
describe 'non_public_only' do
subject { described_class.non_public_only.to_a }
it { is_expected.to match_array([private_group, internal_group]) }
end end
end end
......
...@@ -832,6 +832,26 @@ describe Project, models: true do ...@@ -832,6 +832,26 @@ describe Project, models: true do
it { expect(project.builds_enabled?).to be_truthy } it { expect(project.builds_enabled?).to be_truthy }
end end
describe '.with_shared_runners' do
subject { Project.with_shared_runners }
context 'when shared runners are enabled for project' do
let!(:project) { create(:empty_project, shared_runners_enabled: true) }
it "returns a project" do
is_expected.to eq([project])
end
end
context 'when shared runners are disabled for project' do
let!(:project) { create(:empty_project, shared_runners_enabled: false) }
it "returns an empty array" do
is_expected.to be_empty
end
end
end
describe '.cached_count', caching: true do describe '.cached_count', caching: true do
let(:group) { create(:group, :public) } let(:group) { create(:group, :public) }
let!(:project1) { create(:empty_project, :public, group: group) } let!(:project1) { create(:empty_project, :public, group: group) }
...@@ -974,6 +994,28 @@ describe Project, models: true do ...@@ -974,6 +994,28 @@ describe Project, models: true do
end end
end end
describe '#shared_runners' do
let!(:runner) { create(:ci_runner, :shared) }
subject { project.shared_runners }
context 'when shared runners are enabled for project' do
let!(:project) { create(:empty_project, shared_runners_enabled: true) }
it "returns a list of shared runners" do
is_expected.to eq([runner])
end
end
context 'when shared runners are disabled for project' do
let!(:project) { create(:empty_project, shared_runners_enabled: false) }
it "returns a empty list" do
is_expected.to be_empty
end
end
end
describe '#visibility_level_allowed?' do describe '#visibility_level_allowed?' do
let(:project) { create(:empty_project, :internal) } let(:project) { create(:empty_project, :internal) }
......
...@@ -2,7 +2,6 @@ require 'spec_helper' ...@@ -2,7 +2,6 @@ require 'spec_helper'
module Ci module Ci
describe RegisterBuildService, services: true do describe RegisterBuildService, services: true do
let!(:service) { RegisterBuildService.new }
let!(:project) { FactoryGirl.create :empty_project, shared_runners_enabled: false } let!(:project) { FactoryGirl.create :empty_project, shared_runners_enabled: false }
let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project }
let!(:pending_build) { FactoryGirl.create :ci_build, pipeline: pipeline } let!(:pending_build) { FactoryGirl.create :ci_build, pipeline: pipeline }
...@@ -19,29 +18,29 @@ module Ci ...@@ -19,29 +18,29 @@ module Ci
pending_build.tag_list = ["linux"] pending_build.tag_list = ["linux"]
pending_build.save pending_build.save
specific_runner.tag_list = ["linux"] specific_runner.tag_list = ["linux"]
expect(service.execute(specific_runner)).to eq(pending_build) expect(execute(specific_runner)).to eq(pending_build)
end end
it "does not pick build with different tag" do it "does not pick build with different tag" do
pending_build.tag_list = ["linux"] pending_build.tag_list = ["linux"]
pending_build.save pending_build.save
specific_runner.tag_list = ["win32"] specific_runner.tag_list = ["win32"]
expect(service.execute(specific_runner)).to be_falsey expect(execute(specific_runner)).to be_falsey
end end
it "picks build without tag" do it "picks build without tag" do
expect(service.execute(specific_runner)).to eq(pending_build) expect(execute(specific_runner)).to eq(pending_build)
end end
it "does not pick build with tag" do it "does not pick build with tag" do
pending_build.tag_list = ["linux"] pending_build.tag_list = ["linux"]
pending_build.save pending_build.save
expect(service.execute(specific_runner)).to be_falsey expect(execute(specific_runner)).to be_falsey
end end
it "pick build without tag" do it "pick build without tag" do
specific_runner.tag_list = ["win32"] specific_runner.tag_list = ["win32"]
expect(service.execute(specific_runner)).to eq(pending_build) expect(execute(specific_runner)).to eq(pending_build)
end end
end end
...@@ -56,13 +55,13 @@ module Ci ...@@ -56,13 +55,13 @@ module Ci
end end
it 'does not pick a build' do it 'does not pick a build' do
expect(service.execute(shared_runner)).to be_nil expect(execute(shared_runner)).to be_nil
end end
end end
context 'for specific runner' do context 'for specific runner' do
it 'does not pick a build' 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 end
end end
...@@ -86,34 +85,34 @@ module Ci ...@@ -86,34 +85,34 @@ module Ci
it 'prefers projects without builds first' do it 'prefers projects without builds first' do
# it gets for one build from each of the projects # it gets for one build from each of the projects
expect(service.execute(shared_runner)).to eq(build1_project1) expect(execute(shared_runner)).to eq(build1_project1)
expect(service.execute(shared_runner)).to eq(build1_project2) expect(execute(shared_runner)).to eq(build1_project2)
expect(service.execute(shared_runner)).to eq(build1_project3) expect(execute(shared_runner)).to eq(build1_project3)
# then it gets a second build from each of the projects # then it gets a second build from each of the projects
expect(service.execute(shared_runner)).to eq(build2_project1) expect(execute(shared_runner)).to eq(build2_project1)
expect(service.execute(shared_runner)).to eq(build2_project2) expect(execute(shared_runner)).to eq(build2_project2)
# in the end the third build # in the end the third build
expect(service.execute(shared_runner)).to eq(build3_project1) expect(execute(shared_runner)).to eq(build3_project1)
end end
it 'equalises number of running builds' do it 'equalises number of running builds' do
# after finishing the first build for project 1, get a second build from the same project # 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 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 build1_project2.reload.success
expect(service.execute(shared_runner)).to eq(build2_project2) expect(execute(shared_runner)).to eq(build2_project2)
expect(service.execute(shared_runner)).to eq(build1_project3) expect(execute(shared_runner)).to eq(build1_project3)
expect(service.execute(shared_runner)).to eq(build3_project1) expect(execute(shared_runner)).to eq(build3_project1)
end end
end end
context 'shared runner' do 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_kind_of(Build) }
it { expect(build).to be_valid } it { expect(build).to be_valid }
...@@ -122,7 +121,7 @@ module Ci ...@@ -122,7 +121,7 @@ module Ci
end end
context 'specific runner' do 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_kind_of(Build) }
it { expect(build).to be_valid } it { expect(build).to be_valid }
...@@ -137,13 +136,13 @@ module Ci ...@@ -137,13 +136,13 @@ module Ci
end end
context 'shared runner' do context 'shared runner' do
let(:build) { service.execute(shared_runner) } let(:build) { execute(shared_runner) }
it { expect(build).to be_nil } it { expect(build).to be_nil }
end end
context 'specific runner' do 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_kind_of(Build) }
it { expect(build).to be_valid } it { expect(build).to be_valid }
...@@ -159,17 +158,21 @@ module Ci ...@@ -159,17 +158,21 @@ module Ci
end end
context 'and uses shared runner' do context 'and uses shared runner' do
let(:build) { service.execute(shared_runner) } let(:build) { execute(shared_runner) }
it { expect(build).to be_nil } it { expect(build).to be_nil }
end end
context 'and uses specific runner' do context 'and uses specific runner' do
let(:build) { service.execute(specific_runner) } let(:build) { execute(specific_runner) }
it { expect(build).to be_nil } it { expect(build).to be_nil }
end end
end end
def execute(runner)
described_class.new(runner).execute
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment