diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb index 5f03ae773382eeb316a412ac8069b83298a9b949..630be17e64b2b07f48212959a86a2a5bec8f342f 100644 --- a/app/finders/ci/runners_finder.rb +++ b/app/finders/ci/runners_finder.rb @@ -4,7 +4,6 @@ module Ci class RunnersFinder < UnionFinder include Gitlab::Allowable - ALLOWED_SORTS = %w[contacted_asc contacted_desc created_at_asc created_at_desc created_date token_expires_at_asc token_expires_at_desc].freeze DEFAULT_SORT = 'created_at_desc' def initialize(current_user:, params:) @@ -31,11 +30,17 @@ def execute end def sort_key - ALLOWED_SORTS.include?(@params[:sort]) ? @params[:sort] : DEFAULT_SORT + allowed_sorts.include?(@params[:sort]) ? @params[:sort] : DEFAULT_SORT end private + attr_reader :group, :project + + def allowed_sorts + %w[contacted_asc contacted_desc created_at_asc created_at_desc created_date token_expires_at_asc token_expires_at_desc] + end + def search! if @project project_runners @@ -128,3 +133,5 @@ def filter_by!(scope_name, available_scopes) end end end + +Ci::RunnersFinder.prepend_mod diff --git a/app/graphql/resolvers/ci/runners_resolver.rb b/app/graphql/resolvers/ci/runners_resolver.rb index 735e38c1a5c385389d1cac36385462bd203cbb0e..632655d36813f10e0e2669633ddbbca603d47405 100644 --- a/app/graphql/resolvers/ci/runners_resolver.rb +++ b/app/graphql/resolvers/ci/runners_resolver.rb @@ -4,6 +4,7 @@ module Resolvers module Ci class RunnersResolver < BaseResolver include LooksAhead + include Gitlab::Graphql::Authorize::AuthorizeResource type Types::Ci::RunnerType.connection_type, null: true @@ -105,3 +106,5 @@ def nested_preloads end end end + +Resolvers::Ci::RunnersResolver.prepend_mod diff --git a/app/graphql/types/ci/runner_sort_enum.rb b/app/graphql/types/ci/runner_sort_enum.rb index 8f2a13bd69995a347b5fed4fc0ccf3161c572b97..4195eb043ed0dc5ee49465585bb79d3e7d1c3339 100644 --- a/app/graphql/types/ci/runner_sort_enum.rb +++ b/app/graphql/types/ci/runner_sort_enum.rb @@ -15,3 +15,5 @@ class RunnerSortEnum < BaseEnum end end end + +Types::Ci::RunnerSortEnum.prepend_mod diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index c7896bb873dbc8db6467b69a527ba3610cf58cde..4eb5c3c9ed23d98db53fc4f8708a902bcb3450bd 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -72,6 +72,7 @@ class Runner < Ci::ApplicationRecord has_many :runner_managers, inverse_of: :runner has_many :builds + has_many :running_builds, inverse_of: :runner has_many :runner_projects, inverse_of: :runner, autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, through: :runner_projects, disable_joins: true has_many :runner_namespaces, inverse_of: :runner, autosave: true @@ -198,6 +199,7 @@ class Runner < Ci::ApplicationRecord scope :order_created_at_desc, -> { order(created_at: :desc) } scope :order_token_expires_at_asc, -> { order(token_expires_at: :asc) } scope :order_token_expires_at_desc, -> { order(token_expires_at: :desc) } + scope :with_tags, -> { preload(:tags) } scope :with_creator, -> { preload(:creator) } diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 941e077ffaa3dca7aaae0751f0632501116bb9bd..1db6f6b43a71cb417c218f3e4dcd92c15668c3be 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -25019,6 +25019,7 @@ Values for sorting runners. | <a id="cirunnersortcontacted_desc"></a>`CONTACTED_DESC` | Ordered by contacted_at in descending order. | | <a id="cirunnersortcreated_asc"></a>`CREATED_ASC` | Ordered by created_at in ascending order. | | <a id="cirunnersortcreated_desc"></a>`CREATED_DESC` | Ordered by created_at in descending order. | +| <a id="cirunnersortmost_active_desc"></a>`MOST_ACTIVE_DESC` **{warning-solid}** | **Introduced** in 16.2. This feature is an Experiment. It can be changed or removed at any time. Ordered by number of running jobs in descending order (only available on Ultimate plans). | | <a id="cirunnersorttoken_expires_at_asc"></a>`TOKEN_EXPIRES_AT_ASC` | Ordered by token_expires_at in ascending order. | | <a id="cirunnersorttoken_expires_at_desc"></a>`TOKEN_EXPIRES_AT_DESC` | Ordered by token_expires_at in descending order. | diff --git a/ee/app/finders/ee/ci/runners_finder.rb b/ee/app/finders/ee/ci/runners_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..7f4e60b2e0351dd6b9bef57b5e493af59d0d02c4 --- /dev/null +++ b/ee/app/finders/ee/ci/runners_finder.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module EE + module Ci + module RunnersFinder + extend ::Gitlab::Utils::Override + + private + + override :allowed_sorts + def allowed_sorts + super + ['most_active_desc'] + end + + override :sort! + def sort! + if sort_key == 'most_active_desc' && (project || group) + raise ArgumentError, 'most_active_desc can only be used on instance runners' + end + + super + end + end + end +end diff --git a/ee/app/graphql/ee/resolvers/ci/runners_resolver.rb b/ee/app/graphql/ee/resolvers/ci/runners_resolver.rb new file mode 100644 index 0000000000000000000000000000000000000000..5569171625b271fc28f5897c3eaf91f5935a9e5d --- /dev/null +++ b/ee/app/graphql/ee/resolvers/ci/runners_resolver.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module EE + module Resolvers + module Ci + module RunnersResolver + extend ::Gitlab::Utils::Override + + prepended do + include ::Gitlab::Graphql::Authorize::AuthorizeResource + end + + override :ready? + def ready?(sort: nil, type: nil, **args) + check_sort_conditions(type) if sort == :most_active_desc + + super + end + + def resolve(**args) + # keyset pagination doesn't really make sense for most_active_desc sorting + # as it requires counting ci_running_builds anyway + # and it's very hard to implement + return offset_pagination(super) if args[:sort]&.to_s == 'most_active_desc' + + super + end + + private + + def check_sort_conditions(type) + if type != 'instance_type' + raise ::Gitlab::Graphql::Errors::ArgumentError, + 'MOST_ACTIVE_DESC sorting is only available when type is INSTANCE_TYPE' + end + + return if License.feature_available?(:runner_performance_insights) + + raise_resource_not_available_error!( + 'runner_performance_insights feature is required for MOST_ACTIVE_DESC sorting' + ) + end + end + end + end +end diff --git a/ee/app/graphql/ee/types/ci/runner_sort_enum.rb b/ee/app/graphql/ee/types/ci/runner_sort_enum.rb new file mode 100644 index 0000000000000000000000000000000000000000..a13b4da69858922001d3d54419d08827c958d5bf --- /dev/null +++ b/ee/app/graphql/ee/types/ci/runner_sort_enum.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module EE + module Types + module Ci + module RunnerSortEnum + extend ActiveSupport::Concern + + prepended do + value 'MOST_ACTIVE_DESC', + 'Ordered by number of running jobs in descending order (only available on Ultimate plans).', + value: :most_active_desc, + alpha: { milestone: '16.2' } + end + end + end + end +end diff --git a/ee/app/models/ee/ci/runner.rb b/ee/app/models/ee/ci/runner.rb index 34e9d02fc8a75bfe27ee541b574294bbf2243b77..cce852083c7683f11dc21f672c3ba7684c3de308 100644 --- a/ee/app/models/ee/ci/runner.rb +++ b/ee/app/models/ee/ci/runner.rb @@ -5,9 +5,24 @@ module Ci module Runner extend ActiveSupport::Concern + MOST_ACTIVE_RUNNERS_BUILDS_LIMIT = 1000 + prepended do has_one :cost_settings, class_name: 'Ci::Minutes::CostSetting', foreign_key: :runner_id, inverse_of: :runner + scope :order_most_active_desc, -> do + joins( + <<~SQL + INNER JOIN ( + SELECT "runner_id", ROW_NUMBER() OVER(PARTITION BY "runner_id" ORDER BY "runner_id") as rn + FROM "ci_running_builds" + ) as "limited_builds" ON "limited_builds"."runner_id" = "ci_runners"."id" + AND "limited_builds".rn <= #{MOST_ACTIVE_RUNNERS_BUILDS_LIMIT} + SQL + ).group(:id) + .reorder('COUNT(limited_builds.runner_id) DESC NULLS LAST', arel_table['id'].desc) + end + def self.any_shared_runners_with_enabled_cost_factor?(project) if project.public? instance_type.where('public_projects_minutes_cost_factor > 0').exists? @@ -48,6 +63,20 @@ def cost_factor ::Gitlab::Ci::Minutes::CostFactor.new(runner_matcher) end end + + class_methods do + extend ::Gitlab::Utils::Override + + override :order_by + def order_by(order) + case order + when 'most_active_desc' + order_most_active_desc + else + super + end + end + end end end end diff --git a/ee/spec/finders/ee/ci/runners_finder_spec.rb b/ee/spec/finders/ee/ci/runners_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..1265ea26efd757c2820bfa6a32db263664e39bb3 --- /dev/null +++ b/ee/spec/finders/ee/ci/runners_finder_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do + describe '#execute' do + subject(:execute) do + described_class.new(current_user: user, params: params).execute + end + + context 'when sorting' do + let(:params) { { sort: sort_key } } + + context 'with sort param equal to most_active_desc' do + let_it_be(:runners) { create_list(:ci_runner, 6) } + let_it_be(:project) { create(:project) } + + let(:sort_key) { 'most_active_desc' } + + before_all do + runners.map.with_index do |runner, number_of_builds| + create_list(:ci_build, number_of_builds, runner: runner, project: project).each do |build| + create(:ci_running_build, runner: build.runner, build: build, project: project) + end + end + end + + context 'when admin', :enable_admin_mode do + let_it_be(:admin) { create(:user, :admin) } + + let(:user) { admin } + + it 'returns runners with the most running builds' do + is_expected.to eq(runners[1..5].reverse) + end + end + + context 'with user as group owner' do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + + before_all do + group.add_owner(user) + end + + context 'with sort param set to most_active_desc' do + let(:params) do + { group: group, sort: 'most_active_desc' } + end + + it 'raises an error' do + expect { execute }.to raise_error(ArgumentError, 'most_active_desc can only be used on instance runners') + end + end + end + end + end + end +end diff --git a/ee/spec/models/ee/ci/runner_spec.rb b/ee/spec/models/ee/ci/runner_spec.rb index f4688d247855d673410357409d61225c8d074b2b..3aef8ac5e58bfc9a5f4d5bf41e822a05eb2f84a7 100644 --- a/ee/spec/models/ee/ci/runner_spec.rb +++ b/ee/spec/models/ee/ci/runner_spec.rb @@ -208,4 +208,60 @@ it { is_expected.to be_truthy } end end + + describe '.order_most_active_desc' do + let_it_be(:project) { create(:project) } + let_it_be(:runners) { create_list(:ci_runner, 5) } + + before_all do + runners.map.with_index do |runner, idx| + create_list(:ci_build, 10 - idx, runner: runner, project: project).each do |build| + create(:ci_running_build, runner: build.runner, build: build, project: project) + end + end + end + + it 'returns runners sorted by running builds' do + expect(described_class.order_most_active_desc.pluck(:id)).to eq(runners.pluck(:id)) + end + + it 'limits the number of running builds counted and sorts by id desc' do + stub_const("EE::Ci::Runner::MOST_ACTIVE_RUNNERS_BUILDS_LIMIT", 8) + + runner_ids = runners.pluck(:id) + + expected_runner_ids = runner_ids[0..2].reverse + runner_ids[3..5] + + expect(described_class.order_most_active_desc.pluck(:id)).to eq(expected_runner_ids) + end + end + + describe '.order_by' do + subject(:order_by) do + described_class.order_by(sort_key) + end + + context 'when sort_key is most_active_desc' do + let_it_be(:project) { create(:project) } + let_it_be(:runners) { create_list(:ci_runner, 3) } + + let(:sort_key) { 'most_active_desc' } + + context 'with no running builds' do + it { is_expected.to be_empty } + end + + context 'with running builds' do + before_all do + runners.each_with_index do |runner, running_build_count| + create_list(:ci_running_build, running_build_count, runner: runner, project: project) + end + end + + it 'contains the runners in the correct order' do + is_expected.to eq([runners[2], runners[1]]) + end + end + end + end end diff --git a/ee/spec/requests/api/graphql/ci/runners_spec.rb b/ee/spec/requests/api/graphql/ci/runners_spec.rb index eb79c2b55fda92fc396c16c62aafebda9b848d2d..2074be072cfab97f5b20450ee8db44d763b91aa8 100644 --- a/ee/spec/requests/api/graphql/ci/runners_spec.rb +++ b/ee/spec/requests/api/graphql/ci/runners_spec.rb @@ -18,8 +18,6 @@ before do stub_runner_releases(%w[14.0.0 14.0.1]) - - post_graphql(query, current_user: current_user) end context 'with upgradeStatus argument' do @@ -37,6 +35,10 @@ ) end + before do + post_graphql(query, current_user: current_user) + end + context 'with deprecated CiRunnerUpgradeStatusType enum type' do let(:upgrade_status_graphql_type) { 'CiRunnerUpgradeStatusType' } @@ -75,6 +77,10 @@ ) end + before do + post_graphql(query, current_user: current_user) + end + context 'with deprecated RunnerMembershipFilter enum type' do let(:membership_graphql_type) { 'RunnerMembershipFilter' } @@ -91,5 +97,62 @@ end end end + + context 'when sorting by MOST_ACTIVE_DESC' do + let_it_be(:runners) { create_list(:ci_runner, 6) } + + before_all do + runners.map.with_index do |runner, number_of_builds| + create_list(:ci_build, number_of_builds, runner: runner, project: project).each do |build| + create(:ci_running_build, runner: build.runner, build: build, project: project) + end + end + end + + it_behaves_like 'sorted paginated query' do + before do + stub_licensed_features(runner_performance_insights: true) + end + + def pagination_query(params) + graphql_query_for(:runners, params.merge(type: :INSTANCE_TYPE), "#{page_info} nodes { id }") + end + + def pagination_results_data(runners) + runners.map { |runner| GitlabSchema.parse_gid(runner['id'], expected_type: ::Ci::Runner).model_id.to_i } + end + + let(:sort_param) { :MOST_ACTIVE_DESC } + let(:first_param) { 2 } + let(:all_records) { runners[1..5].reverse.map(&:id) } + let(:data_path) { [:runners] } + end + + it 'when requesting not instance_type runners' do + stub_licensed_features(runner_performance_insights: true) + query = graphql_query_for(:runners, { type: :GROUP_TYPE, sort: :MOST_ACTIVE_DESC }, "nodes { id }") + post_graphql(query, current_user: current_user) + + expect(graphql_errors).to include(a_hash_including( + 'message' => 'MOST_ACTIVE_DESC sorting is only available when type is INSTANCE_TYPE')) + end + + it 'when requesting not runners without type' do + stub_licensed_features(runner_performance_insights: true) + query = graphql_query_for(:runners, { sort: :MOST_ACTIVE_DESC }, "nodes { id }") + post_graphql(query, current_user: current_user) + + expect(graphql_errors).to include(a_hash_including( + 'message' => 'MOST_ACTIVE_DESC sorting is only available when type is INSTANCE_TYPE')) + end + + it 'returns error when feature is not enabled' do + query = graphql_query_for(:runners, params.merge(type: :INSTANCE_TYPE, sort: :MOST_ACTIVE_DESC), "nodes { id }") + post_graphql(query, current_user: current_user) + + expect(graphql_errors).to include(a_hash_including( + 'message' => 'runner_performance_insights feature is required for MOST_ACTIVE_DESC sorting')) + end + end end end diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index 77260bb4c5c3856b5db0bfdbf18ed194366d6c89..e57ad5bc76d1a2adc506a969f78ce43a5da648c3 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -302,7 +302,7 @@ def execute end describe '#execute' do - subject { described_class.new(current_user: user, params: params).execute } + subject(:execute) { described_class.new(current_user: user, params: params).execute } shared_examples 'membership equal to :descendants' do it 'returns all descendant runners' do @@ -321,7 +321,7 @@ def execute context 'with :group as target group' do let(:target_group) { group } - context 'passing no params' do + context 'passing no membership params' do it_behaves_like 'membership equal to :descendants' end diff --git a/spec/graphql/resolvers/ci/runners_resolver_spec.rb b/spec/graphql/resolvers/ci/runners_resolver_spec.rb index e4620b96cae6f9152d6656d78ce3371451794560..02fc7d28255a9c1599ca1900d7fa01de0d3ea264 100644 --- a/spec/graphql/resolvers/ci/runners_resolver_spec.rb +++ b/spec/graphql/resolvers/ci/runners_resolver_spec.rb @@ -9,7 +9,7 @@ let(:obj) { nil } let(:args) { {} } - subject do + subject(:resolve_scope) do resolve(described_class, obj: obj, ctx: { current_user: user }, args: args, arg_style: :internal) end