Merge branch 'ensure_project_iid_before_drop_pipeline' into 'master'

Ensure project_iid before calling drop! on the pipeline [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57783

changelogs/unreleased/ensure_project_iid_before_drop_pipeline.yml

+---
+title: Ensure a project iid is set before transitioning on pipeline error
+merge_request: 57783
+author:
+type: performance

config/feature_flags/development/ci_pipeline_ensure_iid_on_drop.yml

+---
+name: ci_pipeline_ensure_iid_on_drop
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57783
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326886
+milestone: '13.11'
+type: development
+group: group::code review
+default_enabled: false

lib/gitlab/ci/pipeline/chain/helpers.rb

@@ -12,7 +12,17 @@ module Gitlab
             end
 
             pipeline.add_error_message(message)
-            pipeline.drop!(drop_reason) if drop_reason && persist_pipeline?
+
+            if drop_reason && persist_pipeline?
+              if Feature.enabled?(:ci_pipeline_ensure_iid_on_drop, pipeline.project, default_enabled: :yaml)
+                # Project iid must be called outside a transaction, so we ensure it is set here
+                # otherwise it may be set within the state transition transaction of the drop! call
+                # which it will lock the InternalId row for the whole transaction
+                pipeline.ensure_project_iid!
+              end
+
+              pipeline.drop!(drop_reason)
+            end
 
             # TODO: consider not to rely on AR errors directly as they can be
             # polluted with other unrelated errors (e.g. state machine)

spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do
+  let(:helper_class) do
+    Class.new do
+      include Gitlab::Ci::Pipeline::Chain::Helpers
+
+      attr_accessor :pipeline, :command
+
+      def initialize(pipeline, command)
+        self.pipeline = pipeline
+        self.command = command
+      end
+    end
+  end
+
+  subject(:helper) { helper_class.new(pipeline, command) }
+
+  let(:pipeline) { build(:ci_empty_pipeline) }
+  let(:command) { double(save_incompleted: true) }
+  let(:message) { 'message' }
+
+  describe '.error' do
+    shared_examples 'error function' do
+      specify do
+        expect(pipeline).to receive(:drop!).with(drop_reason).and_call_original
+        expect(pipeline).to receive(:add_error_message).with(message).and_call_original
+        expect(pipeline).to receive(:ensure_project_iid!).twice.and_call_original
+
+        subject.error(message, config_error: config_error, drop_reason: drop_reason)
+
+        expect(pipeline.yaml_errors).to eq(yaml_error)
+        expect(pipeline.errors[:base]).to include(message)
+      end
+    end
+
+    context 'when given a drop reason' do
+      context 'when config error is true' do
+        context 'sets the yaml error and overrides the drop reason' do
+          let(:drop_reason) { :config_error }
+          let(:config_error) { true }
+          let(:yaml_error) { message }
+
+          it_behaves_like "error function"
+        end
+      end
+
+      context 'when config error is false' do
+        context 'does not set the yaml error or override the drop reason' do
+          let(:drop_reason) { :size_limit_exceeded }
+          let(:config_error) { false }
+          let(:yaml_error) { nil }
+
+          it_behaves_like "error function"
+        end
+      end
+    end
+
+    context 'when the ci_pipeline_ensure_iid_on_drop feature flag is false' do
+      it 'does not ensure the project iid' do
+        stub_feature_flags(ci_pipeline_ensure_iid_on_drop: false)
+        expect(pipeline).to receive(:ensure_project_iid!).once
+
+        subject.error(message, config_error: true)
+      end
+    end
+  end
+end