Merge branch 'fix-ci-live-trace-unlimited-minutes-checks' into 'master'

Fix namespace checks for live quota consumption

See merge request gitlab-org/gitlab!68646

app/models/namespace.rb

@@ -274,11 +274,6 @@ class Namespace < ApplicationRecord
     projects.with_shared_runners.any?
   end
 
-  # Internal Gitlab owned namespaces only (example: gitlab-org)
-  def unlimited_minutes?
-    shared_runners_minutes_limit == 0
-  end
-
   def user_ids_for_project_authorizations
     [owner_id]
   end

ee/app/services/ci/minutes/track_live_consumption_service.rb

@@ -77,8 +77,6 @@ module Ci
           ServiceResponse.error(message: 'Feature not enabled')
         elsif !build.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?
           ServiceResponse.error(message: 'Cost factor not enabled for build')
         else

ee/lib/gitlab/ci/minutes/cost_factor.rb

@@ -22,6 +22,7 @@ module Gitlab
 
         def for_project(project)
           return 0.0 unless @runner_matcher.instance_type?
+          return 0.0 unless project.ci_minutes_quota.enabled?
 
           cost_factor = for_visibility(project.visibility_level)
 

ee/spec/lib/ee/gitlab/ci/matching/runner_matcher_spec.rb

@@ -65,7 +65,7 @@ RSpec.describe Gitlab::Ci::Matching::RunnerMatcher do
         before do
           allow(project)
             .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
 
         it { is_expected.to eq(result) }

ee/spec/lib/gitlab/ci/minutes/build_consumption_spec.rb

@@ -33,6 +33,7 @@ RSpec.describe Gitlab::Ci::Minutes::BuildConsumption do
         project.update!(visibility_level: visibility_level)
 
         allow(build).to receive(:duration).and_return(duration)
+        allow(::Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { 400 }
       end
 
       it 'returns the expected consumption' do

ee/spec/lib/gitlab/ci/minutes/cost_factor_spec.rb

@@ -5,6 +5,10 @@ require 'spec_helper'
 RSpec.describe Gitlab::Ci::Minutes::CostFactor do
   using RSpec::Parameterized::TableSyntax
 
+  let(:runner_type) {}
+  let(:public_cost_factor) {}
+  let(:private_cost_factor) {}
+
   let(:runner) do
     build_stubbed(:ci_runner,
       runner_type,
@@ -22,104 +26,153 @@ RSpec.describe Gitlab::Ci::Minutes::CostFactor do
   end
 
   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
-      :project  | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | false
-      :project  | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | false
-      :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
+    context 'when the cost factor is zero' do
+      before do
+        expect(cost_factor).to receive(:for_project).with(project) { 0 }
+      end
+
+      it { is_expected.to be_falsey }
     end
 
-    with_them do
-      it { is_expected.to eq(result) }
+    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
 
   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
-      :project  | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | true
-      :project  | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | true
-      :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
+    context 'when the cost factor is zero' do
+      before do
+        expect(cost_factor).to receive(:for_project).with(project) { 0 }
+      end
+
+      it { is_expected.to be_truthy }
     end
 
-    with_them do
-      it { is_expected.to eq(result) }
+    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
 
   describe '#for_project' do
-    let(:project) { build_stubbed(:project, namespace: namespace, visibility_level: visibility_level) }
-
-    subject { described_class.new(runner.runner_matcher).for_project(project) }
-
     context 'before the public project cost factor release date' do
-      let_it_be(:namespace) do
-        create(:group, created_at: Date.new(2021, 7, 16))
-      end
-
-      where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do
-        :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
-        :instance | Gitlab::VisibilityLevel::INTERNAL | 0 | 5 | 5
-        :instance | Gitlab::VisibilityLevel::PRIVATE  | 0 | 5 | 5
+      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
 
       with_them do
+        let(:namespace) do
+          create(:group, created_at: Date.new(2021, 7, 16), shared_runners_minutes_limit: namespace_limit)
+        end
+
+        let(:project) do
+          create(:project, namespace: namespace, visibility_level: visibility_level)
+        end
+
+        before do
+          allow(Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { instance_limit }
+        end
+
+        subject { described_class.new(runner.runner_matcher).for_project(project) }
+
         it { is_expected.to eq(result) }
       end
     end
 
     context 'after the public project cost factor release date' do
-      let_it_be(:namespace) do
-        create(:group, created_at: Date.new(2021, 7, 17))
+      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.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
 
-      before do
-        allow(Gitlab).to receive(:com?).and_return(true)
-      end
+      with_them do
+        let(:namespace) do
+          create(:group, created_at: Date.new(2021, 7, 17), shared_runners_minutes_limit: namespace_limit)
+        end
 
-      where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do
-        :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
+        let(:project) do
+          create(:project, namespace: namespace, visibility_level: visibility_level)
+        end
+
+        before do
+          allow(Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { instance_limit }
+          allow(Gitlab).to receive(:com?).and_return(true)
+        end
+
+        subject { described_class.new(runner.runner_matcher).for_project(project) }
 
-      with_them do
         it { is_expected.to eq(result) }
       end
     end

ee/spec/models/ci/build_spec.rb

@@ -51,6 +51,10 @@ RSpec.describe Ci::Build do
   describe '#cost_factor_enabled?' do
     subject { job.cost_factor_enabled? }
 
+    before do
+      allow(::Gitlab::CurrentSettings).to receive(:shared_runners_minutes) { 400 }
+    end
+
     context 'for shared runner' do
       before do
         job.runner = create(:ci_runner, :instance)

ee/spec/models/ee/ci/runner_spec.rb

@@ -3,6 +3,12 @@
 require 'spec_helper'
 
 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
     subject { runner.cost_factor_for_project(project) }
 
@@ -42,6 +48,12 @@ RSpec.describe EE::Ci::Runner do
         let(:project) { create(:project, visibility_level: ::Gitlab::VisibilityLevel::PRIVATE) }
 
         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
 
       context 'with public visibility level' do
@@ -85,18 +97,26 @@ RSpec.describe EE::Ci::Runner do
   end
 
   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))
       create(:project, namespace: namespace)
     end
 
     context 'when the project has any cost factor' do
-      it 'returns true' do
-        runner = create(:ci_runner, :instance,
-                        private_projects_minutes_cost_factor: 1,
-                        public_projects_minutes_cost_factor: 0)
+      let(:runner) do
+        create(:ci_runner, :instance,
+          private_projects_minutes_cost_factor: 1,
+          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
 

ee/spec/services/ci/minutes/track_live_consumption_service_spec.rb

@@ -82,10 +82,11 @@ RSpec.describe Ci::Minutes::TrackLiveConsumptionService do
 
     context 'when namespace has unlimited minutes' 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
 
-      it_behaves_like 'returns early', 'Namespace has unlimited minutes'
+      it_behaves_like 'returns early', 'Cost factor not enabled for build'
     end
 
     context 'when build has not been tracked recently' do

ee/spec/services/ci/pipeline_creation/drop_not_runnable_builds_service_spec.rb

@@ -40,7 +40,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do
     shared_examples 'limit exceeded' do
       before do
         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
 
       it 'drops the job with ci_quota_exceeded reason' do
@@ -96,7 +96,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do
       context 'when the CI quota is exceeded' do
         before do
           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
 
         it_behaves_like 'jobs allowed to run'