Merge branch 'ci-minutes-fix-failure-reason-for-dropped-builds' into 'master'

Fix failure reason for dropped builds on CI quota exhaustion

See merge request gitlab-org/gitlab!62562

app/models/project.rb

@@ -1717,7 +1717,11 @@ class Project < ApplicationRecord
   end
 
   def shared_runners
-    @shared_runners ||= shared_runners_available? ? Ci::Runner.instance_type : Ci::Runner.none
+    @shared_runners ||= shared_runners_enabled? ? Ci::Runner.instance_type : Ci::Runner.none
+  end
+
+  def available_shared_runners
+    @available_shared_runners ||= shared_runners_available? ? shared_runners : Ci::Runner.none
   end
 
   def group_runners
@@ -1728,9 +1732,13 @@ class Project < ApplicationRecord
     Ci::Runner.from_union([runners, group_runners, shared_runners])
   end
 
+  def all_available_runners
+    Ci::Runner.from_union([runners, group_runners, available_shared_runners])
+  end
+
   def active_runners
     strong_memoize(:active_runners) do
-      all_runners.active
+      all_available_runners.active
     end
   end
 

app/services/ci/pipeline_creation/drop_not_runnable_builds_service.rb

@@ -47,7 +47,7 @@ module Ci
 
       def validate_build_matcher(build_matcher)
         return if matching_private_runners?(build_matcher)
-        return if matching_instance_runners_available?(build_matcher)
+        return if matching_instance_runners?(build_matcher)
 
         matching_failure_reason(build_matcher)
       end
@@ -69,17 +69,17 @@ module Ci
           .present?
       end
 
-      # Overridden in EE to include more conditions
-      def matching_instance_runners_available?(build_matcher)
-        matching_instance_runners?(build_matcher)
-      end
-
       def matching_instance_runners?(build_matcher)
         instance_runners
-          .find { |matcher| matcher.matches?(build_matcher) }
+          .find { |matcher| matching_criteria(matcher, build_matcher) }
           .present?
       end
 
+      # Overridden in EE
+      def matching_criteria(runner_matcher, build_matcher)
+        runner_matcher.matches?(build_matcher)
+      end
+
       # Overridden in EE
       def matching_failure_reason(build_matcher)
         :no_matching_runner

app/services/ci/update_build_queue_service.rb

@@ -52,7 +52,7 @@ module Ci
     # Unblock runner associated with given project / build
     #
     def tick(build)
-      tick_for(build, build.project.all_runners)
+      tick_for(build, build.project.all_available_runners)
     end
 
     private

ee/app/models/ee/project.rb

@@ -359,7 +359,11 @@ module EE
     end
 
     def shared_runners_available?
-      super && !::Ci::Minutes::Quota.new(shared_runners_limit_namespace).minutes_used_up?
+      super && !ci_minutes_quota.minutes_used_up?
+    end
+
+    def shared_runners_enabled_but_unavailable?
+      shared_runners_enabled? && !shared_runners_available?
     end
 
     def link_pool_repository

ee/app/services/ee/ci/pipeline_creation/drop_not_runnable_builds_service.rb

@@ -8,16 +8,14 @@ module EE
 
         private
 
-        override :matching_instance_runners_available?
-        def matching_instance_runners_available?(build_matcher)
-          instance_runners
-            .find { |matcher| matcher.matches?(build_matcher) && matcher.matches_quota?(build_matcher) }
-            .present?
+        override :matching_criteria
+        def matching_criteria(runner_matcher, build_matcher)
+          super && runner_matcher.matches_quota?(build_matcher)
         end
 
         override :matching_failure_reason
         def matching_failure_reason(build_matcher)
-          if matching_instance_runners?(build_matcher)
+          if build_matcher.project.shared_runners_enabled_but_unavailable?
             :ci_quota_exceeded
           else
             :no_matching_runner

ee/spec/models/project_spec.rb

@@ -2809,4 +2809,92 @@ RSpec.describe Project do
       end
     end
   end
+
+  describe '#available_shared_runners' do
+    let_it_be(:runner) { create(:ci_runner, :instance) }
+
+    let(:project) { build_stubbed(:project, shared_runners_enabled: true) }
+
+    subject { project.available_shared_runners }
+
+    before do
+      allow(project).to receive(:ci_minutes_quota)
+        .and_return(double('quota', minutes_used_up?: minutes_used_up))
+    end
+
+    context 'when CI minutes are available for project' do
+      let(:minutes_used_up) { false }
+
+      it 'returns a list of shared runners' do
+        is_expected.to eq([runner])
+      end
+    end
+
+    context 'when out of CI minutes for project' do
+      let(:minutes_used_up) { true }
+
+      it 'returns a empty list' do
+        is_expected.to be_empty
+      end
+    end
+  end
+
+  describe '#all_available_runners' do
+    let_it_be_with_refind(:project) do
+      create(:project, group: create(:group), shared_runners_enabled: true)
+    end
+
+    let_it_be(:instance_runner) { create(:ci_runner, :instance) }
+    let_it_be(:group_runner) { create(:ci_runner, :group, groups: [project.group]) }
+    let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project]) }
+
+    subject { project.all_available_runners }
+
+    before do
+      allow(project).to receive(:ci_minutes_quota)
+        .and_return(double('quota', minutes_used_up?: minutes_used_up))
+    end
+
+    context 'when CI minutes are available for project' do
+      let(:minutes_used_up) { false }
+
+      it 'returns a list with all runners' do
+        is_expected.to match_array([instance_runner, group_runner, project_runner])
+      end
+    end
+
+    context 'when out of CI minutes for project' do
+      let(:minutes_used_up) { true }
+
+      it 'returns a list with specific runners' do
+        is_expected.to match_array([group_runner, project_runner])
+      end
+    end
+  end
+
+  describe '#shared_runners_enabled_but_unavailable?' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:project) do
+      build_stubbed(:project, shared_runners_enabled: shared_runners_enabled)
+    end
+
+    before do
+      allow(project).to receive(:ci_minutes_quota)
+        .and_return(double('quota', minutes_used_up?: minutes_used_up))
+    end
+
+    where(:shared_runners_enabled, :minutes_used_up, :result) do
+      true  | true  | true
+      true  | false | false
+      false | false | false
+      false | true  | false
+    end
+
+    with_them do
+      it 'returns the correct value' do
+        expect(project.shared_runners_enabled_but_unavailable?).to eq(result)
+      end
+    end
+  end
 end

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

@@ -29,13 +29,27 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do
     end
 
     shared_examples 'limit exceeded' do
-      it 'drops the job' do
+      it 'drops the job with ci_quota_exceeded reason' do
         execute
         job.reload
 
         expect(job).to be_failed
         expect(job.failure_reason).to eq('ci_quota_exceeded')
       end
+
+      context 'when shared runners are disabled' do
+        before do
+          pipeline.project.update!(shared_runners_enabled: false)
+        end
+
+        it 'drops the job with no_matching_runner reason' do
+          execute
+          job.reload
+
+          expect(job).to be_failed
+          expect(job.failure_reason).to eq('no_matching_runner')
+        end
+      end
     end
 
     context 'with public projects' do
@@ -66,7 +80,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do
 
       context 'when the Ci quota is exceeded' do
         before do
-          expect(pipeline.project).to receive(:ci_minutes_quota)
+          allow(pipeline.project).to receive(:ci_minutes_quota)
             .and_return(double('quota', minutes_used_up?: true))
         end
 
@@ -83,7 +97,7 @@ RSpec.describe Ci::PipelineCreation::DropNotRunnableBuildsService do
 
       context 'when the Ci quota is exceeded' do
         before do
-          expect(pipeline.project).to receive(:ci_minutes_quota)
+          allow(pipeline.project).to receive(:ci_minutes_quota)
             .and_return(double('quota', minutes_used_up?: true))
         end
 

spec/models/project_spec.rb

@@ -1770,13 +1770,13 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
-  describe '#shared_runners' do
-    let!(:runner) { create(:ci_runner, :instance) }
+  shared_examples 'shared_runners' do
+    let_it_be(:runner) { create(:ci_runner, :instance) }
 
     subject { project.shared_runners }
 
     context 'when shared runners are enabled for project' do
-      let!(:project) { create(:project, shared_runners_enabled: true) }
+      let(:project) { build_stubbed(:project, shared_runners_enabled: true) }
 
       it "returns a list of shared runners" do
         is_expected.to eq([runner])
@@ -1784,7 +1784,7 @@ RSpec.describe Project, factory_default: :keep do
     end
 
     context 'when shared runners are disabled for project' do
-      let!(:project) { create(:project, shared_runners_enabled: false) }
+      let(:project) { build_stubbed(:project, shared_runners_enabled: false) }
 
       it "returns a empty list" do
         is_expected.to be_empty
@@ -1792,6 +1792,16 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#shared_runners' do
+    it_behaves_like 'shared_runners'
+  end
+
+  describe '#available_shared_runners' do
+    it_behaves_like 'shared_runners' do
+      subject { project.available_shared_runners }
+    end
+  end
+
   describe '#visibility_level' do
     let(:project) { build(:project) }
 
@@ -6916,6 +6926,67 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  shared_examples 'all_runners' do
+    let_it_be_with_refind(:project) { create(:project, group: create(:group)) }
+    let_it_be(:instance_runner) { create(:ci_runner, :instance) }
+    let_it_be(:group_runner) { create(:ci_runner, :group, groups: [project.group]) }
+    let_it_be(:other_group_runner) { create(:ci_runner, :group) }
+    let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project]) }
+    let_it_be(:other_project_runner) { create(:ci_runner, :project) }
+
+    subject { project.all_runners }
+
+    context 'when shared runners are enabled for project' do
+      before do
+        project.update!(shared_runners_enabled: true)
+      end
+
+      it 'returns a list with all runners' do
+        is_expected.to match_array([instance_runner, group_runner, project_runner])
+      end
+    end
+
+    context 'when shared runners are disabled for project' do
+      before do
+        project.update!(shared_runners_enabled: false)
+      end
+
+      it 'returns a list without shared runners' do
+        is_expected.to match_array([group_runner, project_runner])
+      end
+    end
+
+    context 'when group runners are enabled for project' do
+      before do
+        project.update!(group_runners_enabled: true)
+      end
+
+      it 'returns a list with all runners' do
+        is_expected.to match_array([instance_runner, group_runner, project_runner])
+      end
+    end
+
+    context 'when group runners are disabled for project' do
+      before do
+        project.update!(group_runners_enabled: false)
+      end
+
+      it 'returns a list without group runners' do
+        is_expected.to match_array([instance_runner, project_runner])
+      end
+    end
+  end
+
+  describe '#all_runners' do
+    it_behaves_like 'all_runners'
+  end
+
+  describe '#all_available_runners' do
+    it_behaves_like 'all_runners' do
+      subject { project.all_available_runners }
+    end
+  end
+
   def finish_job(export_job)
     export_job.start
     export_job.finish