Commit 4f2e7ab3 authored by Marius Bobin's avatar Marius Bobin

Fix namespace checks for live quota consumption

Changelog: fixed
EE: true
parent 457ef961
...@@ -274,11 +274,6 @@ class Namespace < ApplicationRecord ...@@ -274,11 +274,6 @@ class Namespace < ApplicationRecord
projects.with_shared_runners.any? projects.with_shared_runners.any?
end end
# Internal Gitlab owned namespaces only (example: gitlab-org)
def unlimited_minutes?
shared_runners_minutes_limit == 0
end
def user_ids_for_project_authorizations def user_ids_for_project_authorizations
[owner_id] [owner_id]
end end
......
...@@ -77,8 +77,6 @@ module Ci ...@@ -77,8 +77,6 @@ module Ci
ServiceResponse.error(message: 'Feature not enabled') ServiceResponse.error(message: 'Feature not enabled')
elsif !build.running? elsif !build.running?
ServiceResponse.error(message: 'Build is not running') ServiceResponse.error(message: 'Build is not running')
elsif root_namespace.unlimited_minutes?
ServiceResponse.error(message: 'Namespace has unlimited minutes')
elsif !build.cost_factor_enabled? elsif !build.cost_factor_enabled?
ServiceResponse.error(message: 'Cost factor not enabled for build') ServiceResponse.error(message: 'Cost factor not enabled for build')
else else
......
...@@ -22,6 +22,7 @@ module Gitlab ...@@ -22,6 +22,7 @@ module Gitlab
def for_project(project) def for_project(project)
return 0.0 unless @runner_matcher.instance_type? return 0.0 unless @runner_matcher.instance_type?
return 0.0 unless project.ci_minutes_quota.enabled?
cost_factor = for_visibility(project.visibility_level) cost_factor = for_visibility(project.visibility_level)
......
...@@ -65,7 +65,7 @@ RSpec.describe Gitlab::Ci::Matching::RunnerMatcher do ...@@ -65,7 +65,7 @@ RSpec.describe Gitlab::Ci::Matching::RunnerMatcher do
before do before do
allow(project) allow(project)
.to receive(:ci_minutes_quota) .to receive(:ci_minutes_quota)
.and_return(double(minutes_used_up?: quota_minutes_used_up)) .and_return(double(minutes_used_up?: quota_minutes_used_up, enabled?: true))
end end
it { is_expected.to eq(result) } it { is_expected.to eq(result) }
......
...@@ -33,6 +33,7 @@ RSpec.describe Gitlab::Ci::Minutes::BuildConsumption do ...@@ -33,6 +33,7 @@ RSpec.describe Gitlab::Ci::Minutes::BuildConsumption do
project.update!(visibility_level: visibility_level) project.update!(visibility_level: visibility_level)
allow(build).to receive(:duration).and_return(duration) allow(build).to receive(:duration).and_return(duration)
allow(::Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { 400 }
end end
it 'returns the expected consumption' do it 'returns the expected consumption' do
......
...@@ -5,6 +5,10 @@ require 'spec_helper' ...@@ -5,6 +5,10 @@ require 'spec_helper'
RSpec.describe Gitlab::Ci::Minutes::CostFactor do RSpec.describe Gitlab::Ci::Minutes::CostFactor do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
let(:runner_type) {}
let(:public_cost_factor) {}
let(:private_cost_factor) {}
let(:runner) do let(:runner) do
build_stubbed(:ci_runner, build_stubbed(:ci_runner,
runner_type, runner_type,
...@@ -22,104 +26,153 @@ RSpec.describe Gitlab::Ci::Minutes::CostFactor do ...@@ -22,104 +26,153 @@ RSpec.describe Gitlab::Ci::Minutes::CostFactor do
end end
describe '#enabled?' do describe '#enabled?' do
let(:project) { build_stubbed(:project, visibility_level: visibility_level) } let(:project) { build_stubbed(:project) }
let(:cost_factor) { described_class.new(runner.runner_matcher) }
subject { described_class.new(runner.runner_matcher).enabled?(project) } subject { cost_factor.enabled?(project) }
where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do context 'when the cost factor is zero' do
:project | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | false before do
:project | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | false expect(cost_factor).to receive(:for_project).with(project) { 0 }
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | false
:group | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | false
:group | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | false
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | false
:instance | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | true
:instance | Gitlab::VisibilityLevel::PRIVATE | 1 | 0 | false
:instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | true
:instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 0 | false
:instance | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | true
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 1 | false
end end
with_them do it { is_expected.to be_falsey }
it { is_expected.to eq(result) } end
context 'when the cost factor is positive' do
before do
expect(cost_factor).to receive(:for_project).with(project) { 0.5 }
end
it { is_expected.to be_truthy }
end end
end end
describe '#disabled?' do describe '#disabled?' do
let(:project) { build_stubbed(:project, visibility_level: visibility_level) } let(:project) { build_stubbed(:project) }
let(:cost_factor) { described_class.new(runner.runner_matcher) }
subject { described_class.new(runner.runner_matcher).disabled?(project) } subject { cost_factor.disabled?(project) }
where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do context 'when the cost factor is zero' do
:project | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | true before do
:project | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | true expect(cost_factor).to receive(:for_project).with(project) { 0 }
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | true
:group | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | true
:group | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | true
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | true
:instance | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | false
:instance | Gitlab::VisibilityLevel::PRIVATE | 1 | 0 | true
:instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | false
:instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 0 | true
:instance | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | false
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 1 | true
end end
with_them do it { is_expected.to be_truthy }
it { is_expected.to eq(result) } end
context 'when the cost factor is positive' do
before do
expect(cost_factor).to receive(:for_project).with(project) { 0.5 }
end
it { is_expected.to be_falsey }
end end
end end
describe '#for_project' do describe '#for_project' do
let(:project) { build_stubbed(:project, namespace: namespace, visibility_level: visibility_level) } context 'before the public project cost factor release date' do
where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :namespace_limit, :instance_limit, :result) do
:project | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | nil | 400 | 0
:project | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | nil | 400 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | 400 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0 | 0 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | nil | 0
:group | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | nil | 400 | 0
:group | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | nil | 400 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | 400 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0 | 0 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | nil | 400 | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | nil | 0 | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | 0 | 400 | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | 400 | 0 | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | nil | 400 | 5
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | nil | 0 | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | 0 | 400 | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | 400 | 0 | 5
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | nil | 400 | 5
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | nil | 0 | 0
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | 0 | 400 | 0
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | 400 | 0 | 5
end
subject { described_class.new(runner.runner_matcher).for_project(project) } with_them do
let(:namespace) do
create(:group, created_at: Date.new(2021, 7, 16), shared_runners_minutes_limit: namespace_limit)
end
context 'before the public project cost factor release date' do let(:project) do
let_it_be(:namespace) do create(:project, namespace: namespace, visibility_level: visibility_level)
create(:group, created_at: Date.new(2021, 7, 16))
end end
where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do before do
:project | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | 0 allow(Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { instance_limit }
:project | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0
:group | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | 0
:group | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | 5
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | 5
end end
with_them do subject { described_class.new(runner.runner_matcher).for_project(project) }
it { is_expected.to eq(result) } it { is_expected.to eq(result) }
end end
end end
context 'after the public project cost factor release date' do context 'after the public project cost factor release date' do
let_it_be(:namespace) do where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :namespace_limit, :instance_limit, :result) do
create(:group, created_at: Date.new(2021, 7, 17)) :project | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | nil | 400 | 0
:project | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | nil | 400 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | 400 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0 | 0 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | nil | 0
:group | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | nil | 400 | 0
:group | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | nil | 400 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | 400 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0 | 0 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | nil | 400 | 0.008
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | nil | 0 | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | 0 | 400 | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | 400 | 0 | 0.008
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | nil | 400 | 5
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | nil | 0 | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | 0 | 400 | 0
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | 400 | 0 | 5
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | nil | 400 | 5
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | nil | nil | 0
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | nil | 0 | 0
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | 0 | 400 | 0
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | 400 | 0 | 5
end
with_them do
let(:namespace) do
create(:group, created_at: Date.new(2021, 7, 17), shared_runners_minutes_limit: namespace_limit)
end
let(:project) do
create(:project, namespace: namespace, visibility_level: visibility_level)
end end
before do before do
allow(Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { instance_limit }
allow(Gitlab).to receive(:com?).and_return(true) allow(Gitlab).to receive(:com?).and_return(true)
end end
where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do subject { described_class.new(runner.runner_matcher).for_project(project) }
:project | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | 0
:project | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | 0
:project | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0
:group | Gitlab::VisibilityLevel::PRIVATE | 1 | 1 | 0
:group | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | 0
:group | Gitlab::VisibilityLevel::PUBLIC | 1 | 1 | 0
:instance | Gitlab::VisibilityLevel::PUBLIC | 0 | 5 | 0.008
:instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | 5
:instance | Gitlab::VisibilityLevel::PRIVATE | 0 | 5 | 5
end
with_them do
it { is_expected.to eq(result) } it { is_expected.to eq(result) }
end end
end end
......
...@@ -51,6 +51,10 @@ RSpec.describe Ci::Build do ...@@ -51,6 +51,10 @@ RSpec.describe Ci::Build do
describe '#cost_factor_enabled?' do describe '#cost_factor_enabled?' do
subject { job.cost_factor_enabled? } subject { job.cost_factor_enabled? }
before do
allow(::Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { 400 }
end
context 'for shared runner' do context 'for shared runner' do
before do before do
job.runner = create(:ci_runner, :instance) job.runner = create(:ci_runner, :instance)
......
...@@ -3,6 +3,12 @@ ...@@ -3,6 +3,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe EE::Ci::Runner do RSpec.describe EE::Ci::Runner do
let(:shared_runners_minutes) { 400 }
before do
allow(::Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { shared_runners_minutes }
end
describe '#cost_factor_for_project' do describe '#cost_factor_for_project' do
subject { runner.cost_factor_for_project(project) } subject { runner.cost_factor_for_project(project) }
...@@ -42,6 +48,12 @@ RSpec.describe EE::Ci::Runner do ...@@ -42,6 +48,12 @@ RSpec.describe EE::Ci::Runner do
let(:project) { create(:project, visibility_level: ::Gitlab::VisibilityLevel::PRIVATE) } let(:project) { create(:project, visibility_level: ::Gitlab::VisibilityLevel::PRIVATE) }
it { is_expected.to eq(1.1) } it { is_expected.to eq(1.1) }
context 'with unlimited minutes' do
let(:shared_runners_minutes) { 400 }
it { is_expected.to eq(1.1) }
end
end end
context 'with public visibility level' do context 'with public visibility level' do
...@@ -85,18 +97,26 @@ RSpec.describe EE::Ci::Runner do ...@@ -85,18 +97,26 @@ RSpec.describe EE::Ci::Runner do
end end
describe '#cost_factor_enabled?' do describe '#cost_factor_enabled?' do
let_it_be(:project) do let_it_be_with_reload(:project) do
namespace = create(:group, created_at: Date.new(2021, 7, 16)) namespace = create(:group, created_at: Date.new(2021, 7, 16))
create(:project, namespace: namespace) create(:project, namespace: namespace)
end end
context 'when the project has any cost factor' do context 'when the project has any cost factor' do
it 'returns true' do let(:runner) do
runner = create(:ci_runner, :instance, create(:ci_runner, :instance,
private_projects_minutes_cost_factor: 1, private_projects_minutes_cost_factor: 1,
public_projects_minutes_cost_factor: 0) public_projects_minutes_cost_factor: 0)
end
subject { runner.cost_factor_enabled?(project) }
it { is_expected.to be_truthy }
context 'with unlimited minutes' do
let(:shared_runners_minutes) { 0 }
expect(runner.cost_factor_enabled?(project)).to be_truthy it { is_expected.to be_falsy }
end end
end end
......
...@@ -82,10 +82,11 @@ RSpec.describe Ci::Minutes::TrackLiveConsumptionService do ...@@ -82,10 +82,11 @@ RSpec.describe Ci::Minutes::TrackLiveConsumptionService do
context 'when namespace has unlimited minutes' do context 'when namespace has unlimited minutes' do
before do before do
namespace.update!(shared_runners_minutes_limit: 0) quota = double('quota', enabled?: false)
allow(project).to receive(:ci_minutes_quota).and_return(quota)
end end
it_behaves_like 'returns early', 'Namespace has unlimited minutes' it_behaves_like 'returns early', 'Cost factor not enabled for build'
end end
context 'when build has not been tracked recently' do context 'when build has not been tracked recently' do
......
...@@ -40,7 +40,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do ...@@ -40,7 +40,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do
shared_examples 'limit exceeded' do shared_examples 'limit exceeded' do
before do before do
allow(pipeline.project).to receive(:ci_minutes_quota) allow(pipeline.project).to receive(:ci_minutes_quota)
.and_return(double('quota', minutes_used_up?: true)) .and_return(double('quota', minutes_used_up?: true, enabled?: true))
end end
it 'drops the job with ci_quota_exceeded reason' do it 'drops the job with ci_quota_exceeded reason' do
...@@ -96,7 +96,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do ...@@ -96,7 +96,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do
context 'when the CI quota is exceeded' do context 'when the CI quota is exceeded' do
before do before do
allow(pipeline.project).to receive(:ci_minutes_quota) allow(pipeline.project).to receive(:ci_minutes_quota)
.and_return(double('quota', minutes_used_up?: true)) .and_return(double('quota', minutes_used_up?: true, enabled?: true))
end end
it_behaves_like 'jobs allowed to run' it_behaves_like 'jobs allowed to run'
......
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