Merge branch 'ci-introduce-matcher-classes-for-builds-and-runners' into 'master'

Introduce matcher classes for builds and runners

See merge request gitlab-org/gitlab!61471

app/models/ci/build.rb

@@ -11,6 +11,7 @@ module Ci
     include Importable
     include Ci::HasRef
     include IgnorableColumns
+    include TaggableQueries
 
     BuildArchivedError = Class.new(StandardError)
 
@@ -379,6 +380,33 @@ module Ci
       end
     end
 
+    def self.build_matchers(project)
+      unique_params = [
+        :protected,
+        Arel.sql("(#{arel_tag_names_array.to_sql})")
+      ]
+
+      group(*unique_params).pluck('array_agg(id)', *unique_params).map do |values|
+        Gitlab::Ci::Matching::BuildMatcher.new({
+          build_ids: values[0],
+          protected: values[1],
+          tag_list: values[2],
+          project: project
+        })
+      end
+    end
+
+    def build_matcher
+      strong_memoize(:build_matcher) do
+        Gitlab::Ci::Matching::BuildMatcher.new({
+          protected: protected?,
+          tag_list: tag_list,
+          build_ids: [id],
+          project: project
+        })
+      end
+    end
+
     def auto_retry_allowed?
       auto_retry.allowed?
     end

app/models/ci/runner.rb

@@ -10,6 +10,8 @@ module Ci
     include TokenAuthenticatable
     include IgnorableColumns
     include FeatureGate
+    include Gitlab::Utils::StrongMemoize
+    include TaggableQueries
 
     add_authentication_token_field :token, encrypted: -> { Feature.enabled?(:ci_runners_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
 
@@ -197,6 +199,42 @@ module Ci
       end
     end
 
+    def self.runner_matchers
+      unique_params = [
+        :runner_type,
+        :public_projects_minutes_cost_factor,
+        :private_projects_minutes_cost_factor,
+        :run_untagged,
+        :access_level,
+        Arel.sql("(#{arel_tag_names_array.to_sql})")
+      ]
+
+      # we use distinct to de-duplicate data
+      distinct.pluck(*unique_params).map do |values|
+        Gitlab::Ci::Matching::RunnerMatcher.new({
+          runner_type: values[0],
+          public_projects_minutes_cost_factor: values[1],
+          private_projects_minutes_cost_factor: values[2],
+          run_untagged: values[3],
+          access_level: values[4],
+          tag_list: values[5]
+        })
+      end
+    end
+
+    def runner_matcher
+      strong_memoize(:runner_matcher) do
+        Gitlab::Ci::Matching::RunnerMatcher.new({
+          runner_type: runner_type,
+          public_projects_minutes_cost_factor: public_projects_minutes_cost_factor,
+          private_projects_minutes_cost_factor: private_projects_minutes_cost_factor,
+          run_untagged: run_untagged,
+          access_level: access_level,
+          tag_list: tag_list
+        })
+      end
+    end
+
     def assign_to(project, current_user = nil)
       if instance_type?
         self.runner_type = :project_type

app/models/concerns/taggable_queries.rb

+# frozen_string_literal: true
+
+module TaggableQueries
+  extend ActiveSupport::Concern
+
+  class_methods do
+    # context is a name `acts_as_taggable context`
+    def arel_tag_names_array(context = :tags)
+      ActsAsTaggableOn::Tagging
+        .joins(:tag)
+        .where("taggings.taggable_id=#{quoted_table_name}.id") # rubocop:disable GitlabSecurity/SqlInjection
+        .where(taggings: { context: context, taggable_type: polymorphic_name })
+        .select('COALESCE(array_agg(tags.name ORDER BY name), ARRAY[]::text[])')
+    end
+  end
+end

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

@@ -7,6 +7,8 @@
 module Ci
   module Minutes
     class Quota
+      include Gitlab::Utils::StrongMemoize
+
       Report = Struct.new(:used, :limit, :status)
 
       def initialize(namespace)
@@ -61,8 +63,10 @@ module Ci
       end
 
       def namespace_eligible?
+        strong_memoize(:namespace_eligible) do
           namespace.root? && namespace.any_project_with_shared_runners_enabled?
         end
+      end
 
       def current_balance
         total_minutes.to_i - total_minutes_used

ee/lib/ee/gitlab/ci/matching/runner_matcher.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Ci
+      module Matching
+        module RunnerMatcher
+          def matches_quota?(build_matcher)
+            cost_factor = minutes_cost_factor(build_matcher.project.visibility_level)
+
+            cost_factor == 0 || (cost_factor > 0 && !minutes_used_up?(build_matcher))
+          end
+
+          private
+
+          def minutes_cost_factor(visibility_level)
+            return 0.0 unless instance_type?
+
+            case visibility_level
+            when ::Gitlab::VisibilityLevel::PUBLIC
+              public_projects_minutes_cost_factor
+            when ::Gitlab::VisibilityLevel::PRIVATE, ::Gitlab::VisibilityLevel::INTERNAL
+              private_projects_minutes_cost_factor
+            else
+              raise ArgumentError, 'Invalid visibility level'
+            end
+          end
+
+          def minutes_used_up?(build_matcher)
+            build_matcher
+              .project
+              .ci_minutes_quota
+              .minutes_used_up?
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/gitlab/ci/matching/runner_matcher_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Matching::RunnerMatcher do
+  let(:dummy_attributes) do
+    {
+      runner_type: 'instance_type',
+      public_projects_minutes_cost_factor: 0.0,
+      private_projects_minutes_cost_factor: 1.0,
+      run_untagged: false,
+      access_level: 'ref_protected',
+      tag_list: %w[tag1 tag2]
+    }
+  end
+
+  describe '#matches_quota?' do
+    let(:project) { build_stubbed(:project, project_attributes) }
+
+    let(:build) do
+      build_stubbed(:ci_build, project: project)
+    end
+
+    let(:runner_matcher) do
+      described_class.new(dummy_attributes.merge(runner_attributes))
+    end
+
+    let(:visibility_map) do
+      {
+        public:   ::Gitlab::VisibilityLevel::PUBLIC,
+        internal: ::Gitlab::VisibilityLevel::INTERNAL,
+        private:  ::Gitlab::VisibilityLevel::PRIVATE
+      }
+    end
+
+    subject { runner_matcher.matches_quota?(record) }
+
+    shared_examples 'matches quota to runner types' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:runner_type, :project_visibility_level, :quota_minutes_used_up, :result) do
+        :project_type   | :public                  | true                  | true
+        :project_type   | :internal                | true                  | true
+        :project_type   | :private                 | true                  | true
+
+        :instance_type  | :public                  | true                  | true
+        :instance_type  | :public                  | false                 | true
+
+        :instance_type  | :internal                | true                  | false
+        :instance_type  | :internal                | false                 | true
+
+        :instance_type  | :private                 | true                  | false
+        :instance_type  | :private                 | false                 | true
+      end
+      with_them do
+        let(:runner_attributes) do
+          { runner_type: runner_type }
+        end
+
+        let(:project_attributes) do
+          { visibility_level: visibility_map[project_visibility_level] }
+        end
+
+        before do
+          allow(project)
+            .to receive(:ci_minutes_quota)
+            .and_return(double(minutes_used_up?: quota_minutes_used_up))
+        end
+
+        it { is_expected.to eq(result) }
+      end
+    end
+
+    context 'with an instance of BuildMatcher' do
+      it_behaves_like 'matches quota to runner types' do
+        let(:record) { build.build_matcher }
+      end
+    end
+
+    context 'with an instance of Ci::Build' do
+      it_behaves_like 'matches quota to runner types' do
+        let(:record) { build }
+      end
+    end
+
+    context 'N+1 queries check' do
+      let_it_be(:project) { create(:project) }
+      let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+      let_it_be(:build) { create(:ci_build, pipeline: pipeline, project: project, tag_list: %w[tag1 tag2]) }
+
+      let(:runner_attributes) { {} }
+
+      it 'does not generate N+1 queries when loading the quota for project' do
+        project.reload
+        control_count = ActiveRecord::QueryRecorder.new do
+          runner_matcher.matches_quota?(build.build_matcher)
+        end
+
+        create(:ci_build, pipeline: pipeline, project: project, tag_list: %w[tag3 tag4])
+        create(:ci_build, pipeline: pipeline, project: project, tag_list: %w[tag4 tag5])
+        project.reload
+        build_matchers = pipeline.builds.build_matchers(project)
+
+        expect(build_matchers.size).to eq(3)
+
+        expect do
+          ActiveRecord::QueryRecorder.new do
+            build_matchers.each do |matcher|
+              runner_matcher.matches_quota?(matcher)
+            end
+          end
+        end.not_to exceed_query_limit(control_count)
+      end
+    end
+  end
+end

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

@@ -423,5 +423,17 @@ RSpec.describe Ci::Minutes::Quota do
         end
       end
     end
+
+    it 'does not trigger N+1 queries when called multiple times' do
+      # memoizes the result
+      quota.namespace_eligible?
+
+      # count
+      actual = ActiveRecord::QueryRecorder.new do
+        quota.namespace_eligible?
+      end
+
+      expect(actual.count).to eq(0)
+    end
   end
 end

lib/gitlab/ci/matching/build_matcher.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Matching
+      class BuildMatcher
+        ATTRIBUTES = %i[
+          protected
+          tag_list
+          build_ids
+          project
+        ].freeze
+
+        attr_reader(*ATTRIBUTES)
+        alias_method :protected?, :protected
+
+        def initialize(params)
+          ATTRIBUTES.each do |attribute|
+            instance_variable_set("@#{attribute}", params.fetch(attribute))
+          end
+        end
+
+        def has_tags?
+          tag_list.present?
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/matching/runner_matcher.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Matching
+      ###
+      # This class is used to check if a build can be picked by a runner:
+      #
+      # runner = Ci::Runner.find(id)
+      # build  = Ci::Build.find(id)
+      # runner.runner_matcher.matches?(build.build_matcher)
+      #
+      # There are also class level methods to build matchers:
+      #
+      # `project.builds.build_matchers(project)` returns a distinct collection
+      # of build matchers.
+      # `Ci::Runner.runner_matchers` returns a distinct collection of runner matchers.
+      #
+      class RunnerMatcher
+        ATTRIBUTES = %i[
+          runner_type
+          public_projects_minutes_cost_factor
+          private_projects_minutes_cost_factor
+          run_untagged
+          access_level
+          tag_list
+        ].freeze
+
+        attr_reader(*ATTRIBUTES)
+
+        def initialize(params)
+          ATTRIBUTES.each do |attribute|
+            instance_variable_set("@#{attribute}", params.fetch(attribute))
+          end
+        end
+
+        def matches?(build_matcher)
+          ensure_build_matcher_instance!(build_matcher)
+          return false if ref_protected? && !build_matcher.protected?
+
+          accepting_tags?(build_matcher)
+        end
+
+        def instance_type?
+          runner_type.to_sym == :instance_type
+        end
+
+        private
+
+        def ref_protected?
+          access_level.to_sym == :ref_protected
+        end
+
+        def accepting_tags?(build_matcher)
+          (run_untagged || build_matcher.has_tags?) && (build_matcher.tag_list - tag_list).empty?
+        end
+
+        def ensure_build_matcher_instance!(build_matcher)
+          return if build_matcher.is_a?(Matching::BuildMatcher)
+
+          raise ArgumentError, 'only Gitlab::Ci::Matching::BuildMatcher are allowed'
+        end
+      end
+    end
+  end
+end
+
+Gitlab::Ci::Matching::RunnerMatcher.prepend_mod_with('Gitlab::Ci::Matching::RunnerMatcher')

spec/lib/gitlab/ci/matching/build_matcher_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Ci::Matching::BuildMatcher do
+  let(:dummy_attributes) do
+    {
+      protected: true,
+      tag_list: %w[tag1 tag2],
+      build_ids: [1, 2, 3],
+      project: :my_project
+    }
+  end
+
+  subject(:matcher) { described_class.new(attributes) }
+
+  describe '.new' do
+    context 'when attributes are missing' do
+      let(:attributes) { {} }
+
+      it { expect { matcher }.to raise_error(KeyError) }
+    end
+
+    context 'with attributes' do
+      let(:attributes) { dummy_attributes }
+
+      it { expect(matcher.protected).to eq(true) }
+
+      it { expect(matcher.tag_list).to eq(%w[tag1 tag2]) }
+
+      it { expect(matcher.build_ids).to eq([1, 2, 3]) }
+
+      it { expect(matcher.project).to eq(:my_project) }
+    end
+  end
+
+  describe '#protected?' do
+    context 'when protected is set to true' do
+      let(:attributes) { dummy_attributes }
+
+      it { expect(matcher.protected?).to be_truthy }
+    end
+
+    context 'when protected is set to false' do
+      let(:attributes) { dummy_attributes.merge(protected: false) }
+
+      it { expect(matcher.protected?).to be_falsey }
+    end
+  end
+
+  describe '#has_tags?' do
+    context 'when tags are present' do
+      let(:attributes) { dummy_attributes }
+
+      it { expect(matcher.has_tags?).to be_truthy }
+    end
+
+    context 'when tags are empty' do
+      let(:attributes) { dummy_attributes.merge(tag_list: []) }
+
+      it { expect(matcher.has_tags?).to be_falsey }
+    end
+  end
+end

spec/lib/gitlab/ci/matching/runner_matcher_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Matching::RunnerMatcher do
+  let(:dummy_attributes) do
+    {
+      runner_type: 'instance_type',
+      public_projects_minutes_cost_factor: 0,
+      private_projects_minutes_cost_factor: 1,
+      run_untagged: false,
+      access_level: 'ref_protected',
+      tag_list: %w[tag1 tag2]
+    }
+  end
+
+  subject(:matcher) { described_class.new(attributes) }
+
+  describe '.new' do
+    context 'when attributes are missing' do
+      let(:attributes) { {} }
+
+      it { expect { matcher }.to raise_error(KeyError) }
+    end
+
+    context 'with attributes' do
+      let(:attributes) { dummy_attributes }
+
+      it { expect(matcher.runner_type).to eq('instance_type') }
+
+      it { expect(matcher.public_projects_minutes_cost_factor).to eq(0) }
+
+      it { expect(matcher.private_projects_minutes_cost_factor).to eq(1) }
+
+      it { expect(matcher.run_untagged).to eq(false) }
+
+      it { expect(matcher.access_level).to eq('ref_protected') }
+
+      it { expect(matcher.tag_list).to eq(%w[tag1 tag2]) }
+    end
+  end
+
+  describe '#instance_type?' do
+    let(:attributes) { dummy_attributes }
+
+    it { expect(matcher.instance_type?).to be_truthy }
+
+    context 'context with private runners' do
+      let(:attributes) { dummy_attributes.merge(runner_type: 'project_type') }
+
+      it { expect(matcher.instance_type?).to be_falsey }
+    end
+  end
+
+  describe '#matches?' do
+    let(:build) { build_stubbed(:ci_build, build_attributes) }
+    let(:runner_matcher) { described_class.new(dummy_attributes.merge(runner_attributes)) }
+
+    subject { runner_matcher.matches?(record) }
+
+    context 'with an instance of BuildMatcher' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:ref_protected, :build_protected, :run_untagged, :runner_tags, :build_tags, :result) do
+        # the `ref_protected? && !build.protected?` part:
+        true              | true            | true         | []          | []         | true
+        true              | false           | true         | []          | []         | false
+        false             | true            | true         | []          | []         | true
+        false             | false           | true         | []          | []         | true
+        # `accepting_tags?(build)` bit:
+        true              | true            | true         | []          | []         | true
+        true              | true            | true         | []          | ['a']      | false
+        true              | true            | true         | %w[a b]     | ['a']      | true
+        true              | true            | true         | ['a']       | %w[a b]    | false
+        true              | true            | true         | ['a']       | ['a']      | true
+        true              | true            | false        | ['a']       | ['a']      | true
+        true              | true            | false        | ['b']       | ['a']      | false
+        true              | true            | false        | %w[a b]     | ['a']      | true
+      end
+
+      with_them do
+        let(:build_attributes) do
+          {
+            tag_list: build_tags,
+            protected: build_protected
+          }
+        end
+
+        let(:runner_attributes) do
+          {
+            access_level: ref_protected ? 'ref_protected' : 'not_protected',
+            run_untagged: run_untagged,
+            tag_list: runner_tags
+          }
+        end
+
+        let(:record) { build.build_matcher }
+
+        it { is_expected.to eq(result) }
+      end
+    end
+
+    context 'with an instance of Ci::Build' do
+      let(:runner_attributes) { {} }
+      let(:build_attributes) { {} }
+      let(:record) { build }
+
+      it 'raises ArgumentError' do
+        expect { subject }.to raise_error ArgumentError, /BuildMatcher are allowed/
+      end
+    end
+  end
+end

spec/models/ci/build_spec.rb

@@ -4987,4 +4987,67 @@ RSpec.describe Ci::Build do
       it { is_expected.to be_truthy }
     end
   end
+
+  describe '.build_matchers' do
+    let_it_be(:pipeline) { create(:ci_pipeline, :protected) }
+
+    subject(:matchers) { pipeline.builds.build_matchers(pipeline.project) }
+
+    context 'when the pipeline is empty' do
+      it 'does not throw errors' do
+        is_expected.to eq([])
+      end
+    end
+
+    context 'when the pipeline has builds' do
+      let_it_be(:build_without_tags) do
+        create(:ci_build, pipeline: pipeline)
+      end
+
+      let_it_be(:build_with_tags) do
+        create(:ci_build, pipeline: pipeline, tag_list: %w[tag1 tag2])
+      end
+
+      let_it_be(:other_build_with_tags) do
+        create(:ci_build, pipeline: pipeline, tag_list: %w[tag2 tag1])
+      end
+
+      it { expect(matchers.size).to eq(2) }
+
+      it 'groups build ids' do
+        expect(matchers.map(&:build_ids)).to match_array([
+          [build_without_tags.id],
+          match_array([build_with_tags.id, other_build_with_tags.id])
+        ])
+      end
+
+      it { expect(matchers.map(&:tag_list)).to match_array([[], %w[tag1 tag2]]) }
+
+      it { expect(matchers.map(&:protected?)).to all be_falsey }
+
+      context 'when the builds are protected' do
+        before do
+          pipeline.builds.update_all(protected: true)
+        end
+
+        it { expect(matchers).to all be_protected }
+      end
+    end
+  end
+
+  describe '#build_matcher' do
+    let_it_be(:build) do
+      build_stubbed(:ci_build, tag_list: %w[tag1 tag2])
+    end
+
+    subject(:matcher) { build.build_matcher }
+
+    it { expect(matcher.build_ids).to eq([build.id]) }
+
+    it { expect(matcher.tag_list).to match_array(%w[tag1 tag2]) }
+
+    it { expect(matcher.protected?).to eq(build.protected?) }
+
+    it { expect(matcher.project).to eq(build.project) }
+  end
 end

spec/models/ci/runner_spec.rb

@@ -975,6 +975,108 @@ RSpec.describe Ci::Runner do
     end
   end
 
+  describe '.runner_matchers' do
+    subject(:matchers) { described_class.all.runner_matchers }
+
+    context 'deduplicates on runner_type' do
+      before do
+        create_list(:ci_runner, 2, :instance)
+        create_list(:ci_runner, 2, :project)
+      end
+
+      it 'creates two matchers' do
+        expect(matchers.size).to eq(2)
+
+        expect(matchers.map(&:runner_type)).to match_array(%w[instance_type project_type])
+      end
+    end
+
+    context 'deduplicates on public_projects_minutes_cost_factor' do
+      before do
+        create_list(:ci_runner, 2, public_projects_minutes_cost_factor: 5)
+        create_list(:ci_runner, 2, public_projects_minutes_cost_factor: 10)
+      end
+
+      it 'creates two matchers' do
+        expect(matchers.size).to eq(2)
+
+        expect(matchers.map(&:public_projects_minutes_cost_factor)).to match_array([5, 10])
+      end
+    end
+
+    context 'deduplicates on private_projects_minutes_cost_factor' do
+      before do
+        create_list(:ci_runner, 2, private_projects_minutes_cost_factor: 5)
+        create_list(:ci_runner, 2, private_projects_minutes_cost_factor: 10)
+      end
+
+      it 'creates two matchers' do
+        expect(matchers.size).to eq(2)
+
+        expect(matchers.map(&:private_projects_minutes_cost_factor)).to match_array([5, 10])
+      end
+    end
+
+    context 'deduplicates on run_untagged' do
+      before do
+        create_list(:ci_runner, 2, run_untagged: true, tag_list: ['a'])
+        create_list(:ci_runner, 2, run_untagged: false, tag_list: ['a'])
+      end
+
+      it 'creates two matchers' do
+        expect(matchers.size).to eq(2)
+
+        expect(matchers.map(&:run_untagged)).to match_array([true, false])
+      end
+    end
+
+    context 'deduplicates on access_level' do
+      before do
+        create_list(:ci_runner, 2, access_level: :ref_protected)
+        create_list(:ci_runner, 2, access_level: :not_protected)
+      end
+
+      it 'creates two matchers' do
+        expect(matchers.size).to eq(2)
+
+        expect(matchers.map(&:access_level)).to match_array(%w[ref_protected not_protected])
+      end
+    end
+
+    context 'deduplicates on tag_list' do
+      before do
+        create_list(:ci_runner, 2, tag_list: %w[tag1 tag2])
+        create_list(:ci_runner, 2, tag_list: %w[tag3 tag4])
+      end
+
+      it 'creates two matchers' do
+        expect(matchers.size).to eq(2)
+
+        expect(matchers.map(&:tag_list)).to match_array([%w[tag1 tag2], %w[tag3 tag4]])
+      end
+    end
+  end
+
+  describe '#runner_matcher' do
+    let(:runner) do
+      build_stubbed(:ci_runner, :instance_type, tag_list: %w[tag1 tag2])
+    end
+
+    subject(:matcher) { runner.runner_matcher }
+
+    it { expect(matcher.runner_type).to eq(runner.runner_type) }
+
+    it { expect(matcher.public_projects_minutes_cost_factor).to eq(runner.public_projects_minutes_cost_factor) }
+
+    it { expect(matcher.private_projects_minutes_cost_factor).to eq(runner.private_projects_minutes_cost_factor) }
+
+    it { expect(matcher.run_untagged).to eq(runner.run_untagged) }
+
+    it { expect(matcher.access_level).to eq(runner.access_level) }
+
+    it { expect(matcher.tag_list).to match_array(runner.tag_list) }
+  end
+
   describe '#uncached_contacted_at' do
     let(:contacted_at_stored) { 1.hour.ago.change(usec: 0) }
     let(:runner) { create(:ci_runner, contacted_at: contacted_at_stored) }