Merge branch '332933-track-build-minutes-on-disabled-shared-runners' into 'master'

Track build minutes for disabled shared runners

See merge request gitlab-org/gitlab!67024

ee/app/models/ci/minutes/quota.rb

@@ -16,7 +16,7 @@ module Ci
       end
 
       def enabled?
-        namespace_eligible? && total_minutes.nonzero?
+        namespace_root? && total_minutes.nonzero?
       end
 
       # Status of the monthly allowance being used.
@@ -48,54 +48,53 @@ module Ci
         enabled? && total_minutes_used >= total_minutes
       end
 
-      # TODO: merge this with minutes_used_up? in
-      # https://gitlab.com/gitlab-org/gitlab/-/issues/332933.
-      # This method is agnostic from Project#shared_runners_enabled
-      def actual_minutes_used_up?
-        limit_enabled? && total_minutes_used >= total_minutes
+      def percent_total_minutes_remaining
+        return 0 if total_minutes == 0
+
+        100 * total_minutes_remaining.to_i / total_minutes
       end
 
-      def total_minutes
-        @total_minutes ||= monthly_minutes + purchased_minutes
+      def current_balance
+        total_minutes.to_i - total_minutes_used
       end
 
-      def total_minutes_used
-        @total_minutes_used ||= namespace.shared_runners_seconds.to_i / 60
+      def display_shared_runners_data?
+        namespace_root? && any_project_enabled?
       end
 
-      def percent_total_minutes_remaining
-        return 0 if total_minutes == 0
+      def display_minutes_available_data?
+        display_shared_runners_data? && total_minutes.nonzero?
+      end
 
-        100 * total_minutes_remaining.to_i / total_minutes
+      def total_minutes
+        strong_memoize(:total_minutes) do
+          monthly_minutes + purchased_minutes
+        end
       end
 
-      def namespace_eligible?
-        strong_memoize(:namespace_eligible) do
-          namespace.root? && namespace.any_project_with_shared_runners_enabled?
+      def total_minutes_used
+        strong_memoize(:total_minutes_used) do
+          namespace.shared_runners_seconds.to_i / 60
         end
       end
 
-      def current_balance
-        total_minutes.to_i - total_minutes_used
+      def any_project_enabled?
+        strong_memoize(:any_project_enabled) do
+          namespace.any_project_with_shared_runners_enabled?
+        end
       end
 
       private
 
-      # TODO: rename to `enabled?`
-      # https://gitlab.com/gitlab-org/gitlab/-/issues/332933
-      def limit_enabled?
-        strong_memoize(:limit_enabled) do
-          namespace.root? && !!total_minutes.nonzero?
-        end
-      end
+      attr_reader :namespace
 
       def minutes_limit
-        return monthly_minutes if enabled?
+        return _('Not supported') unless display_shared_runners_data?
 
-        if namespace_eligible?
-          _('Unlimited')
+        if display_minutes_available_data?
+          monthly_minutes
         else
-          _('Not supported')
+          _('Unlimited')
         end
       end
 
@@ -144,14 +143,22 @@ module Ci
       end
 
       def monthly_minutes
-        @monthly_minutes ||= (namespace.shared_runners_minutes_limit || ::Gitlab::CurrentSettings.shared_runners_minutes).to_i
+        strong_memoize(:monthly_minutes) do
+          (namespace.shared_runners_minutes_limit || ::Gitlab::CurrentSettings.shared_runners_minutes).to_i
+        end
       end
 
       def purchased_minutes
-        @purchased_minutes ||= namespace.extra_shared_runners_minutes_limit.to_i
+        strong_memoize(:purchased_minutes) do
+          namespace.extra_shared_runners_minutes_limit.to_i
+        end
       end
 
-      attr_reader :namespace
+      def namespace_root?
+        strong_memoize(:namespace_root) do
+          namespace.root?
+        end
+      end
     end
   end
 end

ee/app/models/ee/ci/build.rb

@@ -67,8 +67,8 @@ module EE
         end
       end
 
-      def shared_runners_minutes_limit_enabled?
-        project.shared_runners_minutes_limit_enabled? && runner&.cost_factor_enabled?(project)
+      def cost_factor_enabled?
+        runner&.cost_factor_enabled?(project)
       end
 
       def log_geo_deleted_event

ee/app/models/ee/ci/pending_build.rb

@@ -28,7 +28,7 @@ module EE
 
         def minutes_exceeded?(project)
           ::Ci::Runner.any_shared_runners_with_enabled_cost_factor?(project) &&
-            project.ci_minutes_quota.actual_minutes_used_up?
+            project.ci_minutes_quota.minutes_used_up?
         end
       end
     end

ee/app/models/ee/namespace.rb

@@ -250,9 +250,9 @@ module EE
       @ci_minutes_quota ||= ::Ci::Minutes::Quota.new(self)
     end
 
-    # The same method name is used also at project and job level
+    # The same method name is used also at project level
     def shared_runners_minutes_limit_enabled?
-      ci_minutes_quota.enabled?
+      any_project_with_shared_runners_enabled? && ci_minutes_quota.enabled?
     end
 
     def any_project_with_shared_runners_enabled?

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

@@ -27,7 +27,7 @@ module Ci
       def update_pending_builds!
         return unless ::Feature.enabled?(:ci_pending_builds_maintain_ci_minutes_data, @root_namespace, type: :development, default_enabled: :yaml)
 
-        minutes_exceeded = @root_namespace.ci_minutes_quota.actual_minutes_used_up?
+        minutes_exceeded = @root_namespace.ci_minutes_quota.minutes_used_up?
         all_namespaces = @root_namespace.self_and_descendant_ids
 
         ::Ci::PendingBuild.where(namespace: all_namespaces).update_all(minutes_exceeded: minutes_exceeded)

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

@@ -77,8 +77,8 @@ module Ci
           ServiceResponse.error(message: 'Feature not enabled')
         elsif !build.running?
           ServiceResponse.error(message: 'Build is not running')
-        elsif !build.shared_runners_minutes_limit_enabled?
-          ServiceResponse.error(message: 'CI minutes limit not enabled for build')
+        elsif !build.cost_factor_enabled?
+          ServiceResponse.error(message: 'Cost factor not enabled for build')
         else
           ServiceResponse.success
         end

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

@@ -6,7 +6,7 @@ module Ci
       # Calculates consumption and updates the project and namespace statistics(legacy)
       # or ProjectMonthlyUsage and NamespaceMonthlyUsage(not legacy) based on the passed build.
       def execute(build)
-        return unless build.shared_runners_minutes_limit_enabled?
+        return unless build.cost_factor_enabled?
         return unless build.complete?
         return unless build.duration&.positive?
 

ee/app/views/namespaces/_additional_minutes_status.html.haml

@@ -5,7 +5,7 @@
 - return unless minutes_quota.enabled?
 
 
-- if minutes_quota.namespace_eligible?
+- if minutes_quota.display_shared_runners_data?
   %li
     %span.light= _('Additional minutes:')
     %strong

ee/app/views/namespaces/_shared_runner_status.html.haml

 - namespace = local_assigns.fetch(:namespace)
 - minutes_quota = namespace.ci_minutes_quota
 
-- if minutes_quota.namespace_eligible?
+- if minutes_quota.display_shared_runners_data?
   %li
     %span.light= _('Pipeline minutes quota:')
     %strong

ee/app/views/namespaces/pipelines_quota/_extra_shared_runners_minutes_quota.html.haml

 - return unless Gitlab.com?
 
 - minutes_quota = namespace.ci_minutes_quota
-- return unless minutes_quota.enabled? && minutes_quota.purchased_minutes_report.limit > 0
+- return unless minutes_quota.display_minutes_available_data? && minutes_quota.purchased_minutes_report.limit > 0
 
 .row
   .col-sm-6

ee/app/views/namespaces/pipelines_quota/_list.haml

@@ -26,9 +26,9 @@
         = link_to sprite_icon('question-o'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'shared-runners-pipeline-minutes-quota'), target: '_blank', 'aria-label': _('Shared runners help link')
 
     .col-sm-6.right
-      - if minutes_quota.enabled?
+      - if minutes_quota.display_minutes_available_data?
         #{minutes_quota.monthly_percent_used}% used
-      - elsif !namespace.any_project_with_shared_runners_enabled?
+      - elsif !minutes_quota.any_project_enabled?
         0% used
       - else
         = s_('UsageQuota|Unlimited')
@@ -48,7 +48,7 @@
         = _('Minutes')
 
   %tbody
-    - if !namespace.any_project_with_shared_runners_enabled?
+    - if !minutes_quota.any_project_enabled?
       %tr
         %td{ colspan: 2 }
           .nothing-here-block

ee/spec/helpers/ee/namespaces_helper_spec.rb

@@ -63,6 +63,7 @@ RSpec.describe EE::NamespacesHelper do
         context 'and the namespace is eligible for unlimited' do
           before do
             allow(quota).to receive(:namespace_eligible?).and_return(true)
+            allow(user_group).to receive(:any_project_with_shared_runners_enabled?).and_return(true)
           end
 
           it 'returns Unlimited for the limit section' do

ee/spec/models/ci/build_spec.rb

@@ -48,48 +48,28 @@ RSpec.describe Ci::Build do
     it { is_expected.to have_one(:dast_scanner_profile).class_name('DastScannerProfile').through(:dast_scanner_profiles_build) }
   end
 
-  describe '#shared_runners_minutes_limit_enabled?' do
-    subject { job.shared_runners_minutes_limit_enabled? }
+  describe '#cost_factor_enabled?' do
+    subject { job.cost_factor_enabled? }
 
-    shared_examples 'depends on runner presence and type' do
-      context 'for shared runner' do
-        before do
-          job.runner = create(:ci_runner, :instance)
-        end
-
-        context 'when project#shared_runners_minutes_limit_enabled? is true' do
-          specify do
-            expect(job.project).to receive(:shared_runners_minutes_limit_enabled?)
-              .and_return(true)
-
-            is_expected.to be_truthy
-          end
-        end
-
-        context 'when project#shared_runners_minutes_limit_enabled? is false' do
-          specify do
-            expect(job.project).to receive(:shared_runners_minutes_limit_enabled?)
-              .and_return(false)
-
-            is_expected.to be_falsey
-          end
-        end
+    context 'for shared runner' do
+      before do
+        job.runner = create(:ci_runner, :instance)
       end
 
-      context 'with specific runner' do
-        before do
-          job.runner = create(:ci_runner, :project)
-        end
+      it { is_expected.to be_truthy }
+    end
 
-        it { is_expected.to be_falsey }
+    context 'with specific runner' do
+      before do
+        job.runner = create(:ci_runner, :project)
       end
 
-      context 'without runner' do
-        it { is_expected.to be_falsey }
-      end
+      it { is_expected.to be_falsey }
     end
 
-    it_behaves_like 'depends on runner presence and type'
+    context 'without runner' do
+      it { is_expected.to be_falsey }
+    end
   end
 
   context 'updates pipeline minutes' do

ee/spec/models/ci/minutes/quota_spec.rb

@@ -5,7 +5,7 @@ RSpec.describe Ci::Minutes::Quota do
   using RSpec::Parameterized::TableSyntax
 
   let_it_be_with_reload(:namespace) do
-    create(:namespace, namespace_statistics: create(:namespace_statistics))
+    create(:group, namespace_statistics: create(:namespace_statistics))
   end
 
   let(:quota) { described_class.new(namespace) }
@@ -38,13 +38,13 @@ RSpec.describe Ci::Minutes::Quota do
         end
       end
 
-      context 'when namespace does not have projects with shared runners enabled' do
+      context 'when namespace has a limit but does not have projects with shared runners enabled' do
         before do
           project.update!(shared_runners_enabled: false)
           allow(namespace).to receive(:shared_runners_minutes_limit).and_return(1000)
         end
 
-        it { is_expected.to be_falsey }
+        it { is_expected.to be_truthy }
       end
     end
 
@@ -67,7 +67,8 @@ RSpec.describe Ci::Minutes::Quota do
     context 'when the quota is not enabled' do
       before do
         allow(quota).to receive(:enabled?).and_return(false)
-        allow(quota).to receive(:namespace_eligible?).and_return(namespace_eligible)
+        allow(namespace).to receive(:root?).and_return(namespace_eligible)
+        allow(namespace).to receive(:any_project_with_shared_runners_enabled?).and_return(true)
       end
 
       context 'when the namespace is not eligible' do
@@ -114,6 +115,7 @@ RSpec.describe Ci::Minutes::Quota do
     context 'when limited' do
       before do
         allow(quota).to receive(:enabled?).and_return(true)
+        allow(namespace).to receive(:any_project_with_shared_runners_enabled?).and_return(true)
         namespace.shared_runners_minutes_limit = 100
       end
 
@@ -257,6 +259,7 @@ RSpec.describe Ci::Minutes::Quota do
     with_them do
       before do
         allow(quota).to receive(:enabled?).and_return(limit_enabled)
+        allow(namespace).to receive(:any_project_with_shared_runners_enabled?).and_return(true)
         namespace.shared_runners_minutes_limit = monthly_limit
         namespace.extra_shared_runners_minutes_limit = purchased_limit
         namespace.namespace_statistics.shared_runners_seconds = minutes_used.minutes
@@ -326,25 +329,6 @@ RSpec.describe Ci::Minutes::Quota do
     end
   end
 
-  describe '#actual_minutes_used_up?' do
-    subject { quota.actual_minutes_used_up? }
-
-    where(:minutes_used, :minutes_limit, :result, :title) do
-      100 | 0   | false | 'limit not enabled'
-      99  | 100 | false | 'total minutes not used'
-      101 | 100 | true  | 'total minutes used'
-    end
-
-    with_them do
-      before do
-        allow(namespace).to receive(:shared_runners_seconds).and_return(minutes_used.minutes)
-        namespace.shared_runners_minutes_limit = minutes_limit
-      end
-
-      it { is_expected.to eq(result) }
-    end
-  end
-
   describe '#total_minutes' do
     subject { quota.total_minutes }
 
@@ -410,49 +394,101 @@ RSpec.describe Ci::Minutes::Quota do
     end
   end
 
-  describe '#namespace_eligible?' do
-    subject { quota.namespace_eligible? }
+  describe '#any_project_enabled?' do
+    let_it_be(:project) { create(:project, namespace: namespace) }
 
-    context 'when namespace is a subgroup' do
-      it 'is false' do
-        allow(namespace).to receive(:root?).and_return(false)
+    context 'when namespace has any project with shared runners enabled' do
+      before do
+        project.update!(shared_runners_enabled: true)
+      end
 
-        expect(subject).to be_falsey
+      it 'returns true' do
+        expect(quota.any_project_enabled?).to be_truthy
       end
     end
 
-    context 'when namespace is root' do
+    context 'when namespace has no projects with shared runners enabled' do
       before do
-        create(:project, namespace: namespace, shared_runners_enabled: shared_runners_enabled)
+        project.update!(shared_runners_enabled: false)
       end
 
-      context 'and it has a project without any shared runner enabled' do
-        let(:shared_runners_enabled) { false }
+      it 'returns false' do
+        expect(quota.any_project_enabled?).to be_falsey
+      end
+    end
 
-        it 'is false' do
-          expect(subject).to be_falsey
-        end
+    it 'does not trigger additional queries when called multiple times' do
+      # memoizes the result
+      quota.any_project_enabled?
+
+      # count
+      actual = ActiveRecord::QueryRecorder.new do
+        quota.any_project_enabled?
+      end
+
+      expect(actual.count).to eq(0)
+    end
+  end
+
+  describe '#display_shared_runners_data?' do
+    let_it_be(:project) { create(:project, namespace: namespace, shared_runners_enabled: true) }
+
+    subject { quota.display_shared_runners_data? }
+
+    context 'when the namespace is root and it has a project with shared runners enabled' do
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when the namespace is not root' do
+      let(:namespace) { create(:group, :nested) }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when the namespaces has no project with shared runners enabled' do
+      before do
+        project.update!(shared_runners_enabled: false)
       end
 
-      context 'and it has a project with shared runner enabled' do
-        let(:shared_runners_enabled) { true }
+      it { is_expected.to be_falsey }
+    end
+  end
+
+  describe '#display_minutes_available_data?' do
+    let_it_be(:project) { create(:project, namespace: namespace, shared_runners_enabled: true) }
 
-        it 'is true' do
-          expect(subject).to be_truthy
+    subject { quota.display_minutes_available_data? }
+
+    context 'when the namespace is root and it has a project with shared runners enabled' do
+      context 'when there is a minutes limit' do
+        before do
+          namespace.update!(shared_runners_minutes_limit: 200)
         end
+
+        it { is_expected.to be_truthy }
+      end
+
+      context 'when there is no minutes limit' do
+        before do
+          namespace.update!(shared_runners_minutes_limit: 0)
+        end
+
+        it { is_expected.to be_falsey }
       end
     end
 
-    it 'does not trigger N+1 queries when called multiple times' do
-      # memoizes the result
-      quota.namespace_eligible?
+    context 'when the namespace is not root' do
+      let(:namespace) { create(:group, :nested) }
 
-      # count
-      actual = ActiveRecord::QueryRecorder.new do
-        quota.namespace_eligible?
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when the namespaces has no project with shared runners enabled' do
+      before do
+        project.update!(shared_runners_enabled: false)
       end
 
-      expect(actual.count).to eq(0)
+      it { is_expected.to be_falsey }
     end
   end
 end

ee/spec/models/ee/ci/pending_build_spec.rb

@@ -50,7 +50,7 @@ RSpec.describe Ci::PendingBuild do
     context 'when ci minutes are not available' do
       before do
         allow_next_instance_of(::Ci::Minutes::Quota) do |instance|
-          allow(instance).to receive(:actual_minutes_used_up?).and_return(true)
+          allow(instance).to receive(:minutes_used_up?).and_return(true)
         end
       end
 

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

@@ -30,7 +30,7 @@ RSpec.describe Ci::Minutes::RefreshCachedDataService do
     context 'when user purchases more ci minutes for a given namespace' do
       before do
         allow_next_instance_of(::Ci::Minutes::Quota) do |instance|
-          allow(instance).to receive(:actual_minutes_used_up?).and_return(false)
+          allow(instance).to receive(:minutes_used_up?).and_return(false)
         end
       end
 

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

@@ -69,15 +69,15 @@ RSpec.describe Ci::Minutes::TrackLiveConsumptionService do
     context 'when runner is not of instance type' do
       let(:runner) { create(:ci_runner, :project) }
 
-      it_behaves_like 'returns early', 'CI minutes limit not enabled for build'
+      it_behaves_like 'returns early', 'Cost factor not enabled for build'
     end
 
-    context 'when shared runners limit is not enabled for build' do
+    context 'when cost factor is not enabled for build' do
       before do
-        allow(build).to receive(:shared_runners_minutes_limit_enabled?).and_return(false)
+        allow(build).to receive(:cost_factor_enabled?).and_return(false)
       end
 
-      it_behaves_like 'returns early', 'CI minutes limit not enabled for build'
+      it_behaves_like 'returns early', 'Cost factor not enabled for build'
     end
 
     context 'when build has not been tracked recently' do

spec/serializers/pipeline_serializer_spec.rb

@@ -202,7 +202,7 @@ RSpec.describe PipelineSerializer do
           # Existing numbers are high and require performance optimization
           # Ongoing issue:
           # https://gitlab.com/gitlab-org/gitlab/-/issues/225156
-          expected_queries = Gitlab.ee? ? 77 : 70
+          expected_queries = Gitlab.ee? ? 74 : 70
 
           expect(recorded.count).to be_within(2).of(expected_queries)
           expect(recorded.cached_count).to eq(0)