Merge branch 'ab-cancel-on-delete' into 'master'

Split Builds Service to calculate minutes sync

See merge request gitlab-org/gitlab!65595

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

@@ -3,6 +3,8 @@
 module Ci
   module Minutes
     class UpdateBuildMinutesService < BaseService
+      # 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.complete?
@@ -12,43 +14,14 @@ module Ci
 
         return unless consumption > 0
 
-        consumption_in_seconds = consumption.minutes.to_i
-        legacy_track_usage_of_monthly_minutes(consumption_in_seconds)
-
-        track_usage_of_monthly_minutes(consumption)
-
-        send_minutes_email_notification
+        # TODO(Issue #335338): Introduce async worker UpdateProjectAndNamespaceUsageWorker
+        Ci::Minutes::UpdateProjectAndNamespaceUsageService.new(project, namespace).execute(consumption)
 
         compare_with_live_consumption(build, consumption)
       end
 
       private
 
-      def send_minutes_email_notification
-        # `perform reset` on `project` because otherwise `Namespace#namespace_statistics` will return stale data.
-        ::Ci::Minutes::EmailNotificationService.new(@project.reset).execute if ::Gitlab.com?
-      end
-
-      def legacy_track_usage_of_monthly_minutes(consumption)
-        ProjectStatistics.update_counters(project_statistics,
-          shared_runners_seconds: consumption)
-
-        NamespaceStatistics.update_counters(namespace_statistics,
-          shared_runners_seconds: consumption)
-      end
-
-      def track_usage_of_monthly_minutes(consumption)
-        return unless Feature.enabled?(:ci_minutes_monthly_tracking, project, default_enabled: :yaml)
-
-        namespace_usage = ::Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace)
-        project_usage = ::Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project)
-
-        ActiveRecord::Base.transaction do
-          ::Ci::Minutes::NamespaceMonthlyUsage.increase_usage(namespace_usage, consumption)
-          ::Ci::Minutes::ProjectMonthlyUsage.increase_usage(project_usage, consumption)
-        end
-      end
-
       def compare_with_live_consumption(build, consumption)
         live_consumption = ::Ci::Minutes::TrackLiveConsumptionService.new(build).live_consumption
         return if live_consumption == 0
@@ -57,14 +30,6 @@ module Ci
         observe_ci_minutes_difference(difference, plan: namespace.actual_plan_name)
       end
 
-      def namespace_statistics
-        namespace.namespace_statistics || namespace.create_namespace_statistics
-      end
-
-      def project_statistics
-        project.statistics || project.create_statistics(namespace: project.namespace)
-      end
-
       def namespace
         project.shared_runners_limit_namespace
       end

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

+# frozen_string_literal: true
+
+module Ci
+  module Minutes
+    class UpdateProjectAndNamespaceUsageService
+      def initialize(project, namespace)
+        @project = project
+        @namespace = namespace
+      end
+
+      # Updates the project and namespace usage based on the passed consumption amount
+      def execute(consumption)
+        consumption_in_seconds = consumption.minutes.to_i
+        legacy_track_usage_of_monthly_minutes(consumption_in_seconds)
+
+        track_usage_of_monthly_minutes(consumption)
+        send_minutes_email_notification
+      end
+
+      private
+
+      def send_minutes_email_notification
+        # `perform reset` on `project` because `Namespace#namespace_statistics` will otherwise return stale data.
+        ::Ci::Minutes::EmailNotificationService.new(@project.reset).execute if ::Gitlab.com?
+      end
+
+      def legacy_track_usage_of_monthly_minutes(consumption_in_seconds)
+        ProjectStatistics.update_counters(project_statistics,
+          shared_runners_seconds: consumption_in_seconds)
+
+        NamespaceStatistics.update_counters(namespace_statistics,
+          shared_runners_seconds: consumption_in_seconds)
+      end
+
+      def track_usage_of_monthly_minutes(consumption)
+        return unless Feature.enabled?(:ci_minutes_monthly_tracking, @project, default_enabled: :yaml)
+
+        namespace_usage = ::Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(@namespace)
+        project_usage = ::Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(@project)
+
+        ApplicationRecord.transaction do
+          ::Ci::Minutes::NamespaceMonthlyUsage.increase_usage(namespace_usage, consumption)
+          ::Ci::Minutes::ProjectMonthlyUsage.increase_usage(project_usage, consumption)
+        end
+      end
+
+      def namespace_statistics
+        @namespace.namespace_statistics || @namespace.create_namespace_statistics
+      end
+
+      def project_statistics
+        @project.statistics || @project.create_statistics(namespace: @project.namespace)
+      end
+    end
+  end
+end

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

@@ -31,6 +31,38 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
       end
     end
 
+    shared_examples 'does nothing' do
+      it 'does not update legacy statistics' do
+        subject
+
+        expect(project.statistics.reload.shared_runners_seconds).to eq(0)
+        expect(namespace.namespace_statistics).to be_nil
+      end
+
+      it 'does not update namespace monthly usage' do
+        expect { subject }.not_to change { Ci::Minutes::NamespaceMonthlyUsage.count }
+      end
+
+      it 'does not update project monthly usage' do
+        expect { subject }.not_to change { Ci::Minutes::ProjectMonthlyUsage.count }
+      end
+
+      it 'does not observe the difference between actual vs live consumption' do
+        expect(::Gitlab::Ci::Pipeline::Metrics)
+          .not_to receive(:gitlab_ci_difference_live_vs_actual_minutes)
+
+        subject
+      end
+
+      it 'does not send an email' do
+        allow(Gitlab).to receive(:com?).and_return(true)
+
+        expect(Ci::Minutes::EmailNotificationService).not_to receive(:new)
+
+        subject
+      end
+    end
+
     context 'with shared runner' do
       let(:cost_factor) { 2.0 }
       let(:runner) { create(:ci_runner, :instance, private_projects_minutes_cost_factor: cost_factor) }
@@ -91,7 +123,17 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
         end
       end
 
-      context 'when statistics are created' do
+      context 'when consumption is 0' do
+        let(:build) do
+          create(:ci_build, :success,
+            runner: runner, pipeline: pipeline,
+            started_at: Time.current, finished_at: Time.current)
+        end
+
+        it_behaves_like 'does nothing'
+      end
+
+      context 'when statistics and usage have existing amounts' do
         let(:usage_in_seconds) { 100 }
         let(:usage_in_minutes) { (100.to_f / 60).round(2) }
 
@@ -208,19 +250,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
     context 'for specific runner' do
       let(:runner) { create(:ci_runner, :project) }
 
-      it 'does not create statistics' do
-        subject
-
-        expect(namespace.namespace_statistics).to be_nil
-      end
-
-      it 'does not track namespace monthly usage' do
-        expect { subject }.not_to change { Ci::Minutes::NamespaceMonthlyUsage.count }
-      end
-
-      it 'does not track project monthly usage' do
-        expect { subject }.not_to change { Ci::Minutes::ProjectMonthlyUsage.count }
-      end
+      it_behaves_like 'does nothing'
     end
   end
 end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::Minutes::UpdateProjectAndNamespaceUsageService do
+  let_it_be(:namespace) { create(:namespace, shared_runners_minutes_limit: 100) }
+  let_it_be(:project) { create(:project, :private, namespace: namespace) }
+
+  let(:consumption_minutes) { 120 }
+  let(:consumption_seconds) { consumption_minutes * 60 }
+  let(:namespace_amount_used) { Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace).amount_used }
+  let(:project_amount_used) { Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project).amount_used }
+
+  describe '#execute' do
+    subject { described_class.new(project, namespace).execute(consumption_minutes) }
+
+    context 'with shared runner' do
+      context 'when statistics and usage do not have existing values' do
+        it 'updates legacy statistics with consumption seconds' do
+          subject
+
+          expect(project.statistics.reload.shared_runners_seconds)
+            .to eq(consumption_seconds)
+
+          expect(namespace.namespace_statistics.reload.shared_runners_seconds)
+            .to eq(consumption_seconds)
+        end
+
+        it 'updates monthly usage with consumption minutes' do
+          subject
+
+          expect(namespace_amount_used).to eq(consumption_minutes)
+          expect(project_amount_used).to eq(consumption_minutes)
+        end
+
+        context 'when feature flag ci_minutes_monthly_tracking is disabled' do
+          before do
+            stub_feature_flags(ci_minutes_monthly_tracking: false)
+          end
+
+          it 'does not update the usage on a monthly basis' do
+            subject
+
+            expect(namespace_amount_used).to eq(0)
+            expect(project_amount_used).to eq(0)
+          end
+        end
+
+        context 'when on .com' do
+          before do
+            allow(Gitlab).to receive(:com?).and_return(true)
+          end
+
+          it 'sends a minute notification email' do
+            expect_next_instance_of(Ci::Minutes::EmailNotificationService) do |service|
+              expect(service).to receive(:execute)
+            end
+
+            subject
+          end
+        end
+
+        context 'when not on .com' do
+          before do
+            allow(Gitlab).to receive(:com?).and_return(false)
+          end
+
+          it 'does not send a minute notification email' do
+            expect(Ci::Minutes::EmailNotificationService).not_to receive(:new)
+
+            subject
+          end
+        end
+      end
+
+      context 'when statistics and usage have existing values' do
+        let(:namespace) { create(:namespace, shared_runners_minutes_limit: 100) }
+        let(:project) { create(:project, :private, namespace: namespace) }
+        let(:existing_usage_in_seconds) { 100 }
+        let(:existing_usage_in_minutes) { (100.to_f / 60).round(2) }
+
+        before do
+          project.statistics.update!(shared_runners_seconds: existing_usage_in_seconds)
+          namespace.create_namespace_statistics(shared_runners_seconds: existing_usage_in_seconds)
+          create(:ci_namespace_monthly_usage, namespace: namespace, amount_used: existing_usage_in_minutes)
+          create(:ci_project_monthly_usage, project: project, amount_used: existing_usage_in_minutes)
+        end
+
+        it 'updates legacy statistics with consumption seconds' do
+          subject
+
+          expect(project.statistics.reload.shared_runners_seconds)
+            .to eq(existing_usage_in_seconds + consumption_seconds)
+
+          expect(namespace.namespace_statistics.reload.shared_runners_seconds)
+            .to eq(existing_usage_in_seconds + consumption_seconds)
+        end
+
+        it 'updates monthly usage with consumption minutes' do
+          subject
+
+          expect(namespace_amount_used).to eq(existing_usage_in_minutes + consumption_minutes)
+          expect(project_amount_used).to eq(existing_usage_in_minutes + consumption_minutes)
+        end
+
+        context 'when feature flag ci_minutes_monthly_tracking is disabled' do
+          before do
+            stub_feature_flags(ci_minutes_monthly_tracking: false)
+          end
+
+          it 'does not update usage' do
+            subject
+
+            expect(namespace_amount_used).to eq(existing_usage_in_minutes)
+            expect(project_amount_used).to eq(existing_usage_in_minutes)
+          end
+        end
+      end
+    end
+  end
+end