Backport changes introduced by https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1083
This commit is contained in:
parent
491f1375fc
commit
b7f4553e3e
11 changed files with 203 additions and 48 deletions
|
@ -4,6 +4,7 @@ class Namespace < ActiveRecord::Base
|
|||
include CacheMarkdownField
|
||||
include Sortable
|
||||
include Gitlab::ShellAdapter
|
||||
include Gitlab::CurrentSettings
|
||||
include Routable
|
||||
|
||||
cache_markdown_field :description, pipeline: :description
|
||||
|
@ -174,6 +175,10 @@ class Namespace < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def shared_runners_enabled?
|
||||
projects.with_shared_runners.any?
|
||||
end
|
||||
|
||||
def full_name
|
||||
@full_name ||=
|
||||
if parent
|
||||
|
|
|
@ -2,34 +2,30 @@ module Ci
|
|||
# This class responsible for assigning
|
||||
# proper pending build to runner on runner API request
|
||||
class RegisterBuildService
|
||||
def execute(current_runner)
|
||||
builds = Ci::Build.pending.unstarted
|
||||
include Gitlab::CurrentSettings
|
||||
|
||||
attr_reader :runner
|
||||
|
||||
def initialize(runner)
|
||||
@runner = runner
|
||||
end
|
||||
|
||||
def execute
|
||||
builds =
|
||||
if current_runner.shared?
|
||||
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').
|
||||
|
||||
# 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')
|
||||
if runner.shared?
|
||||
builds_for_shared_runner
|
||||
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_for_specific_runner
|
||||
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
|
||||
|
||||
|
@ -41,9 +37,35 @@ 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 }).
|
||||
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
|
||||
|
|
|
@ -23,8 +23,8 @@ 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,
|
||||
project: build.project.path_with_namespace)
|
||||
|
|
|
@ -11,6 +11,7 @@ module Gitlab
|
|||
included do
|
||||
scope :public_only, -> { where(visibility_level: PUBLIC) }
|
||||
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 }
|
||||
end
|
||||
|
|
24
lib/prependable.rb
Normal file
24
lib/prependable.rb
Normal file
|
@ -0,0 +1,24 @@
|
|||
# This module is based on: https://gist.github.com/bcardarella/5735987
|
||||
|
||||
module Prependable
|
||||
include ActiveSupport::Concern
|
||||
|
||||
def self.extended(base) #:nodoc:
|
||||
base.instance_variable_set(:@_dependencies, [])
|
||||
end
|
||||
|
||||
def prepend_features(base)
|
||||
if base.instance_variable_defined?(:@_dependencies)
|
||||
base.instance_variable_get(:@_dependencies) << self
|
||||
return false
|
||||
else
|
||||
return false if base < self
|
||||
super
|
||||
base.singleton_class.send(:prepend, const_get('ClassMethods')) if const_defined?(:ClassMethods)
|
||||
@_dependencies.each { |dep| base.send(:include, dep) }
|
||||
base.class_eval(&@_included_block) if instance_variable_defined?(:@_included_block)
|
||||
end
|
||||
end
|
||||
|
||||
alias_method :prepended, :included
|
||||
end
|
|
@ -16,6 +16,10 @@ FactoryGirl.define do
|
|||
is_shared true
|
||||
end
|
||||
|
||||
trait :specific do
|
||||
is_shared false
|
||||
end
|
||||
|
||||
trait :inactive do
|
||||
active false
|
||||
end
|
||||
|
|
|
@ -1,8 +1,9 @@
|
|||
FactoryGirl.define do
|
||||
factory :group do
|
||||
factory :group, class: Group, parent: :namespace do
|
||||
sequence(:name) { |n| "group#{n}" }
|
||||
path { name.downcase.gsub(/\s/, '_') }
|
||||
type 'Group'
|
||||
owner nil
|
||||
|
||||
trait :public do
|
||||
visibility_level Gitlab::VisibilityLevel::PUBLIC
|
||||
|
|
47
spec/lib/prependable_spec.rb
Normal file
47
spec/lib/prependable_spec.rb
Normal file
|
@ -0,0 +1,47 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Prependable do
|
||||
subject { FooObject }
|
||||
|
||||
context 'class methods' do
|
||||
it "has a method" do
|
||||
expect(subject).to respond_to(:class_value)
|
||||
end
|
||||
|
||||
it 'can execute a method' do
|
||||
expect(subject.class_value).to eq(20)
|
||||
end
|
||||
end
|
||||
|
||||
context 'instance methods' do
|
||||
it "has a method" do
|
||||
expect(subject.new).to respond_to(:value)
|
||||
end
|
||||
|
||||
it 'chains a method execution' do
|
||||
expect(subject.new.value).to eq(100)
|
||||
end
|
||||
end
|
||||
|
||||
module Foo
|
||||
extend Prependable
|
||||
|
||||
prepended do
|
||||
def self.class_value
|
||||
20
|
||||
end
|
||||
end
|
||||
|
||||
def value
|
||||
super * 10
|
||||
end
|
||||
end
|
||||
|
||||
class FooObject
|
||||
prepend Foo
|
||||
|
||||
def value
|
||||
10
|
||||
end
|
||||
end
|
||||
end
|
|
@ -81,13 +81,19 @@ describe Group, models: true do
|
|||
describe 'public_only' do
|
||||
subject { described_class.public_only.to_a }
|
||||
|
||||
it{ is_expected.to eq([group]) }
|
||||
it { is_expected.to eq([group]) }
|
||||
end
|
||||
|
||||
describe 'public_and_internal_only' do
|
||||
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
|
||||
|
||||
|
|
|
@ -832,6 +832,26 @@ describe Project, models: true do
|
|||
it { expect(project.builds_enabled?).to be_truthy }
|
||||
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
|
||||
let(:group) { create(:group, :public) }
|
||||
let!(:project1) { create(:empty_project, :public, group: group) }
|
||||
|
@ -974,6 +994,28 @@ describe Project, models: true do
|
|||
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
|
||||
let(:project) { create(:empty_project, :internal) }
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue