Merge branch 'accumulate-shared-runners-duration' into 'master'

Support tracking of shared runners duration together with CI minutes

See merge request gitlab-org/gitlab!72315

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

@@ -27,13 +27,14 @@ module Ci
         current_month.safe_find_or_create_by(namespace_id: namespace_id)
       end
 
-      def self.increase_usage(usage, amount)
-        return unless amount > 0
+      def self.increase_usage(usage, increments)
+        increment_params = increments.select { |_attribute, value| value > 0 }
+        return if increment_params.empty?
 
         # The use of `update_counters` ensures we do a SQL update rather than
         # incrementing the counter for the object in memory and then save it.
         # This is better for concurrent updates.
-        update_counters(usage, amount_used: amount)
+        update_counters(usage, increment_params)
       end
 
       def self.reset_current_usage(namespace)

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

@@ -30,13 +30,14 @@ module Ci
         current_month.safe_find_or_create_by(project_id: project_id)
       end
 
-      def self.increase_usage(usage, amount)
-        return unless amount > 0
+      def self.increase_usage(usage, increments)
+        increment_params = increments.select { |_attribute, value| value > 0 }
+        return if increment_params.empty?
 
         # The use of `update_counters` ensures we do a SQL update rather than
         # incrementing the counter for the object in memory and then save it.
         # This is better for concurrent updates.
-        update_counters(usage, amount_used: amount)
+        update_counters(usage, increment_params)
       end
     end
   end

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

@@ -16,10 +16,10 @@ module Ci
       end
 
       # Updates the project and namespace usage based on the passed consumption amount
-      def execute(consumption)
+      def execute(consumption, duration = nil)
         legacy_track_usage_of_monthly_minutes(consumption)
 
-        ensure_idempotency { track_usage_of_monthly_minutes(consumption) }
+        ensure_idempotency { track_monthly_usage(consumption, duration.to_i) }
 
         send_minutes_email_notification
       end
@@ -53,14 +53,23 @@ module Ci
         update_legacy_namespace_minutes(consumption_in_seconds)
       end
 
-      def track_usage_of_monthly_minutes(consumption)
+      def track_monthly_usage(consumption, duration)
         # preload minutes usage data outside of transaction
         project_usage
         namespace_usage
 
         ::Ci::Minutes::NamespaceMonthlyUsage.transaction do
-          ::Ci::Minutes::NamespaceMonthlyUsage.increase_usage(namespace_usage, consumption) if namespace_usage
-          ::Ci::Minutes::ProjectMonthlyUsage.increase_usage(project_usage, consumption) if project_usage
+          if namespace_usage
+            ::Ci::Minutes::NamespaceMonthlyUsage.increase_usage(namespace_usage,
+              amount_used: consumption,
+              shared_runners_duration: duration)
+          end
+
+          if project_usage
+            ::Ci::Minutes::ProjectMonthlyUsage.increase_usage(project_usage,
+              amount_used: consumption,
+              shared_runners_duration: duration)
+          end
         end
       end
 

ee/app/workers/ci/minutes/update_project_and_namespace_usage_worker.rb

@@ -13,10 +13,10 @@ module Ci
       # used by the service object.
       sidekiq_options retry: 3
 
-      def perform(consumption, project_id, namespace_id, build_id)
+      def perform(consumption, project_id, namespace_id, build_id, params = {})
         ::Ci::Minutes::UpdateProjectAndNamespaceUsageService
           .new(project_id, namespace_id, build_id)
-          .execute(consumption)
+          .execute(consumption, params[:duration].to_i)
       end
     end
   end

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

@@ -71,27 +71,7 @@ RSpec.describe Ci::Minutes::NamespaceMonthlyUsage do
   end
 
   describe '.increase_usage' do
-    subject { described_class.increase_usage(current_usage, amount) }
-
-    context 'when amount is greater than 0' do
-      let(:amount) { 10.5 }
-
-      it 'updates the current month usage' do
-        subject
-
-        expect(current_usage.reload.amount_used).to eq(110.5)
-      end
-    end
-
-    context 'when amount is less or equal to 0' do
-      let(:amount) { -2.0 }
-
-      it 'does not update the current month usage' do
-        subject
-
-        expect(current_usage.reload.amount_used).to eq(100.0)
-      end
-    end
+    it_behaves_like 'CI minutes increase usage'
   end
 
   describe '.for_namespace' do

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

@@ -66,29 +66,13 @@ RSpec.describe Ci::Minutes::ProjectMonthlyUsage do
   end
 
   describe '.increase_usage' do
-    subject { described_class.increase_usage(usage, amount) }
-
-    let(:usage) { create(:ci_project_monthly_usage, project: project, amount_used: 100.0) }
-
-    context 'when amount is greater than 0' do
-      let(:amount) { 10.5 }
-
-      it 'updates the current month usage' do
-        subject
-
-        expect(usage.reload.amount_used).to eq(110.5)
-      end
+    let_it_be_with_refind(:current_usage) do
+      create(:ci_project_monthly_usage,
+        project: project,
+        amount_used: 100)
     end
 
-    context 'when amount is less or equal to 0' do
-      let(:amount) { -2.0 }
-
-      it 'does not update the current month usage' do
-        subject
-
-        expect(usage.reload.amount_used).to eq(100.0)
-      end
-    end
+    it_behaves_like 'CI minutes increase usage'
   end
 
   describe '.for_namespace_monthly_usage' do

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

@@ -9,13 +9,14 @@ RSpec.describe Ci::Minutes::UpdateProjectAndNamespaceUsageService do
   let(:namespace) { project.namespace }
   let(:build) { create(:ci_build) }
   let(:consumption_minutes) { 120 }
+  let(:duration) { 1_000 }
   let(:consumption_seconds) { consumption_minutes * 60 }
   let(:namespace_amount_used) { Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace_id: namespace.id).amount_used }
   let(:project_amount_used) { Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project_id: project.id).amount_used }
   let(:service) { described_class.new(project.id, namespace.id, build.id) }
 
   describe '#execute', :clean_gitlab_redis_shared_state do
-    subject { service.execute(consumption_minutes) }
+    subject { service.execute(consumption_minutes, duration) }
 
     shared_examples 'updates legacy consumption' do
       it 'updates legacy statistics with consumption seconds' do
@@ -26,10 +27,20 @@ RSpec.describe Ci::Minutes::UpdateProjectAndNamespaceUsageService do
     end
 
     shared_examples 'updates monthly consumption' do
-      it 'updates monthly usage with consumption minutes' do
+      it 'updates monthly usage for namespace' do
+        current_usage = Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace_id: namespace.id)
+
+        expect { subject }
+          .to change { current_usage.reload.amount_used }.by(consumption_minutes)
+          .and change { current_usage.reload.shared_runners_duration }.by(duration)
+      end
+
+      it 'updates monthly usage for project' do
+        current_usage = Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project_id: project.id)
+
         expect { subject }
-          .to change { Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace_id: namespace.id).amount_used }.by(consumption_minutes)
-          .and change { Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project_id: project.id).amount_used }.by(consumption_minutes)
+          .to change { current_usage.reload.amount_used }.by(consumption_minutes)
+          .and change { current_usage.reload.shared_runners_duration }.by(duration)
       end
     end
 

ee/spec/support/shared_examples/models/ci/increase_usage_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples_for 'CI minutes increase usage' do
+  subject { described_class.increase_usage(current_usage, increments) }
+
+  let(:increments) { { amount_used: amount } }
+
+  context 'when amount is greater than 0' do
+    let(:amount) { 10.5 }
+
+    it 'updates the current month usage' do
+      subject
+
+      expect(current_usage.reload.amount_used).to eq(110.5)
+    end
+  end
+
+  context 'when amount is less or equal to 0' do
+    let(:amount) { -2.0 }
+
+    it 'does not update the current month usage' do
+      subject
+
+      expect(current_usage.reload.amount_used).to eq(100.0)
+    end
+  end
+
+  context 'when shared_runners_duration is incremented' do
+    let(:increments) { { amount_used: amount, shared_runners_duration: duration } }
+    let(:amount) { 10.5 }
+
+    context 'when duration is positive' do
+      let(:duration) { 10 }
+
+      it 'updates the duration and amount used' do
+        subject
+
+        expect(current_usage.reload.amount_used).to eq(110.5)
+        expect(current_usage.shared_runners_duration).to eq(10)
+      end
+
+      context 'when amount_used is zero' do
+        let(:amount) { 0 }
+
+        it 'updates only the duration' do
+          subject
+
+          expect(current_usage.reload.amount_used).to eq(100.0)
+          expect(current_usage.shared_runners_duration).to eq(10)
+        end
+      end
+    end
+
+    context 'when duration is zero' do
+      let(:duration) { 0 }
+
+      it 'updates only the amount used' do
+        subject
+
+        expect(current_usage.reload.amount_used).to eq(110.5)
+        expect(current_usage.shared_runners_duration).to eq(0)
+      end
+
+      context 'when amount_used is zero' do
+        let(:amount) { 0 }
+
+        it 'does not perform updates' do
+          subject
+
+          expect(current_usage.reload.amount_used).to eq(100.0)
+          expect(current_usage.shared_runners_duration).to eq(0)
+        end
+      end
+    end
+  end
+end

ee/spec/workers/ci/minutes/update_project_and_namespace_usage_worker_spec.rb

@@ -9,35 +9,78 @@ RSpec.describe Ci::Minutes::UpdateProjectAndNamespaceUsageWorker do
 
   let(:consumption) { 100 }
   let(:consumption_seconds) { consumption * 60 }
+  let(:duration) { 60_000 }
   let(:worker) { described_class.new }
 
   describe '#perform', :clean_gitlab_redis_shared_state do
-    subject { perform_multiple([consumption, project.id, namespace.id, build.id]) }
+    context 'when duration param is not passed in' do
+      subject { perform_multiple([consumption, project.id, namespace.id, build.id]) }
 
-    context 'behaves idempotently for monthly usage update' do
-      it 'executes UpdateProjectAndNamespaceUsageService' do
-        service_instance = double
-        expect(::Ci::Minutes::UpdateProjectAndNamespaceUsageService).to receive(:new).at_least(:once).and_return(service_instance)
-        expect(service_instance).to receive(:execute).at_least(:once).with(consumption)
+      context 'behaves idempotently for monthly usage update' do
+        it 'executes UpdateProjectAndNamespaceUsageService' do
+          service_instance = double
+          expect(::Ci::Minutes::UpdateProjectAndNamespaceUsageService).to receive(:new).at_least(:once).and_return(service_instance)
+          expect(service_instance).to receive(:execute).at_least(:once).with(consumption, 0)
 
-        subject
+          subject
+        end
+
+        it 'updates monthly usage but not shared_runners_duration' do
+          subject
+
+          namespace_usage = Ci::Minutes::NamespaceMonthlyUsage.find_by(namespace: namespace)
+          expect(namespace_usage.amount_used).to eq(consumption)
+          expect(namespace_usage.shared_runners_duration).to eq(0)
+
+          project_usage = Ci::Minutes::ProjectMonthlyUsage.find_by(project: project)
+          expect(project_usage.amount_used).to eq(consumption)
+          expect(project_usage.shared_runners_duration).to eq(0)
+        end
       end
 
-      it 'updates monthly usage' do
+      it 'does not behave idempotently for legacy statistics update' do
+        expect(::Ci::Minutes::UpdateProjectAndNamespaceUsageService).to receive(:new).twice.and_call_original
+
         subject
 
-        expect(Ci::Minutes::NamespaceMonthlyUsage.find_by(namespace: namespace).amount_used).to eq(consumption)
-        expect(Ci::Minutes::ProjectMonthlyUsage.find_by(project: project).amount_used).to eq(consumption)
+        expect(project.statistics.reload.shared_runners_seconds).to eq(2 * consumption_seconds)
+        expect(namespace.reload.namespace_statistics.shared_runners_seconds).to eq(2 * consumption_seconds)
       end
     end
 
-    it 'does not behave idempotently for legacy statistics update' do
-      expect(::Ci::Minutes::UpdateProjectAndNamespaceUsageService).to receive(:new).twice.and_call_original
+    context 'when duration param is passed in' do
+      subject { perform_multiple([consumption, project.id, namespace.id, build.id, { duration: duration }]) }
+
+      context 'behaves idempotently for monthly usage update' do
+        it 'executes UpdateProjectAndNamespaceUsageService' do
+          service_instance = double
+          expect(::Ci::Minutes::UpdateProjectAndNamespaceUsageService).to receive(:new).at_least(:once).and_return(service_instance)
+          expect(service_instance).to receive(:execute).at_least(:once).with(consumption, duration)
+
+          subject
+        end
 
-      subject
+        it 'updates monthly usage and shared_runners_duration' do
+          subject
 
-      expect(project.statistics.reload.shared_runners_seconds).to eq(2 * consumption_seconds)
-      expect(namespace.reload.namespace_statistics.shared_runners_seconds).to eq(2 * consumption_seconds)
+          namespace_usage = Ci::Minutes::NamespaceMonthlyUsage.find_by(namespace: namespace)
+          expect(namespace_usage.amount_used).to eq(consumption)
+          expect(namespace_usage.shared_runners_duration).to eq(duration)
+
+          project_usage = Ci::Minutes::ProjectMonthlyUsage.find_by(project: project)
+          expect(project_usage.amount_used).to eq(consumption)
+          expect(project_usage.shared_runners_duration).to eq(duration)
+        end
+      end
+
+      it 'does not behave idempotently for legacy statistics update' do
+        expect(::Ci::Minutes::UpdateProjectAndNamespaceUsageService).to receive(:new).twice.and_call_original
+
+        subject
+
+        expect(project.statistics.reload.shared_runners_seconds).to eq(2 * consumption_seconds)
+        expect(namespace.reload.namespace_statistics.shared_runners_seconds).to eq(2 * consumption_seconds)
+      end
     end
   end
 end