Commit 512b4f97 authored by Pedro Pombeiro's avatar Pedro Pombeiro

Only account for recent runners in plan limits

Recent runners are runners that have been created within the
last 3 months or have contacted us within that period.
parent 6f5c1e7c
...@@ -60,6 +60,7 @@ module Ci ...@@ -60,6 +60,7 @@ module Ci
scope :active, -> { where(active: true) } scope :active, -> { where(active: true) }
scope :paused, -> { where(active: false) } scope :paused, -> { where(active: false) }
scope :online, -> { where('contacted_at > ?', online_contact_time_deadline) } scope :online, -> { where('contacted_at > ?', online_contact_time_deadline) }
scope :recent, -> { where('(contacted_at IS ? AND ci_runners.created_at > ?) OR contacted_at > ?', nil, 3.months.ago, 3.months.ago) }
# The following query using negation is cheaper than using `contacted_at <= ?` # The following query using negation is cheaper than using `contacted_at <= ?`
# because there are less runners online than have been created. The # because there are less runners online than have been created. The
# resulting query is quickly finding online ones and then uses the regular # resulting query is quickly finding online ones and then uses the regular
......
...@@ -7,6 +7,7 @@ module Ci ...@@ -7,6 +7,7 @@ module Ci
self.limit_name = 'ci_registered_group_runners' self.limit_name = 'ci_registered_group_runners'
self.limit_scope = :group self.limit_scope = :group
self.limit_relation = :recent_runners
self.limit_feature_flag = :ci_runner_limits self.limit_feature_flag = :ci_runner_limits
belongs_to :runner, inverse_of: :runner_namespaces belongs_to :runner, inverse_of: :runner_namespaces
...@@ -16,6 +17,10 @@ module Ci ...@@ -16,6 +17,10 @@ module Ci
validates :runner_id, uniqueness: { scope: :namespace_id } validates :runner_id, uniqueness: { scope: :namespace_id }
validate :group_runner_type validate :group_runner_type
def recent_runners
::Ci::Runner.belonging_to_group(namespace_id).recent
end
private private
def group_runner_type def group_runner_type
......
...@@ -7,11 +7,16 @@ module Ci ...@@ -7,11 +7,16 @@ module Ci
self.limit_name = 'ci_registered_project_runners' self.limit_name = 'ci_registered_project_runners'
self.limit_scope = :project self.limit_scope = :project
self.limit_relation = :recent_runners
self.limit_feature_flag = :ci_runner_limits self.limit_feature_flag = :ci_runner_limits
belongs_to :runner, inverse_of: :runner_projects belongs_to :runner, inverse_of: :runner_projects
belongs_to :project, inverse_of: :runner_projects belongs_to :project, inverse_of: :runner_projects
def recent_runners
::Ci::Runner.belonging_to_project(project_id).recent
end
validates :runner_id, uniqueness: { scope: :project_id } validates :runner_id, uniqueness: { scope: :project_id }
end end
end end
...@@ -482,10 +482,9 @@ Plan.default.actual_limits.update!(ci_max_artifact_size_junit: 10) ...@@ -482,10 +482,9 @@ Plan.default.actual_limits.update!(ci_max_artifact_size_junit: 10)
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/321368) in GitLab 13.12. > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/321368) in GitLab 13.12.
The total number of registered runners is limited at the group and project The total number of registered runners is limited at the group and project levels. Each time a new runner is registered,
levels. Each time a new runner is registered, GitLab checks these limits. A GitLab checks these limits against runners that have been active in the last 3 months.
runner's registration fails if it exceeds the limit for the scope determined by A runner's registration fails if it exceeds the limit for the scope determined by the runner registration token.
the runner registration token.
- GitLab SaaS subscribers have different limits defined per plan, affecting all projects using that plan. - GitLab SaaS subscribers have different limits defined per plan, affecting all projects using that plan.
- Self-managed GitLab Premium and Ultimate limits are defined by a default plan that affects all projects: - Self-managed GitLab Premium and Ultimate limits are defined by a default plan that affects all projects:
......
...@@ -273,6 +273,20 @@ RSpec.describe Ci::Runner do ...@@ -273,6 +273,20 @@ RSpec.describe Ci::Runner do
end end
end end
describe '.recent' do
subject { described_class.recent }
before do
@runner1 = create(:ci_runner, :instance, contacted_at: nil, created_at: 2.months.ago)
@runner2 = create(:ci_runner, :instance, contacted_at: nil, created_at: 3.months.ago)
@runner3 = create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 2.months.ago)
@runner4 = create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 3.months.ago)
@runner5 = create(:ci_runner, :instance, contacted_at: 3.months.ago, created_at: 5.months.ago)
end
it { is_expected.to eq([@runner1, @runner3, @runner4])}
end
describe '.online' do describe '.online' do
subject { described_class.online } subject { described_class.online }
......
...@@ -94,7 +94,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -94,7 +94,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
context 'when it exceeds the application limits' do context 'when it exceeds the application limits' do
before do before do
create(:ci_runner, runner_type: :project_type, projects: [project]) create(:ci_runner, runner_type: :project_type, projects: [project], contacted_at: 1.second.ago)
create(:plan_limits, :default_plan, ci_registered_project_runners: 1) create(:plan_limits, :default_plan, ci_registered_project_runners: 1)
end end
...@@ -106,6 +106,22 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -106,6 +106,22 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
expect(project.runners.reload.size).to eq(1) expect(project.runners.reload.size).to eq(1)
end end
end end
context 'when abandoned runners cause application limits to not be exceeded' do
before do
create(:ci_runner, runner_type: :project_type, projects: [project], created_at: 14.months.ago, contacted_at: 13.months.ago)
create(:plan_limits, :default_plan, ci_registered_project_runners: 1)
end
it 'creates runner' do
request
expect(response).to have_gitlab_http_status(:created)
expect(json_response['message']).to be_nil
expect(project.runners.reload.size).to eq(2)
expect(project.runners.recent.size).to eq(1)
end
end
end end
context 'when group token is used' do context 'when group token is used' do
...@@ -135,7 +151,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -135,7 +151,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
context 'when it exceeds the application limits' do context 'when it exceeds the application limits' do
before do before do
create(:ci_runner, runner_type: :group_type, groups: [group]) create(:ci_runner, runner_type: :group_type, groups: [group], contacted_at: nil, created_at: 1.month.ago)
create(:plan_limits, :default_plan, ci_registered_group_runners: 1) create(:plan_limits, :default_plan, ci_registered_group_runners: 1)
end end
...@@ -147,6 +163,23 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -147,6 +163,23 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
expect(group.runners.reload.size).to eq(1) expect(group.runners.reload.size).to eq(1)
end end
end end
context 'when abandoned runners cause application limits to not be exceeded' do
before do
create(:ci_runner, runner_type: :group_type, groups: [group], created_at: 4.months.ago, contacted_at: 3.months.ago)
create(:ci_runner, runner_type: :group_type, groups: [group], contacted_at: nil, created_at: 4.months.ago)
create(:plan_limits, :default_plan, ci_registered_group_runners: 1)
end
it 'creates runner' do
request
expect(response).to have_gitlab_http_status(:created)
expect(json_response['message']).to be_nil
expect(group.runners.reload.size).to eq(3)
expect(group.runners.recent.size).to eq(1)
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