diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb index 8bc2a47a024fd9fc13b66a668fb0db04e7e4d92c..b38db61abdde80671875c8c7ede862c533cb6926 100644 --- a/app/finders/ci/runners_finder.rb +++ b/app/finders/ci/runners_finder.rb @@ -53,8 +53,7 @@ def group_runners Ci::Runner.belonging_to_group(@group.id) when :descendants, nil # Getting all runners from the group itself and all its descendant groups/projects - descendant_projects = Project.for_group_and_its_subgroups(@group) - Ci::Runner.belonging_to_group_or_project(@group.self_and_descendants, descendant_projects) + Ci::Runner.belonging_to_group_or_descendants(@group) else raise ArgumentError, 'Invalid membership filter' end diff --git a/app/models/ci/namespace_hierarchy.rb b/app/models/ci/namespace_hierarchy.rb index 14482b9ea5ad8ea2f749736d653226b2b0c95acc..17ced95a3563b58f3e66e65f4e72be1e3fa5a567 100644 --- a/app/models/ci/namespace_hierarchy.rb +++ b/app/models/ci/namespace_hierarchy.rb @@ -4,6 +4,10 @@ module Ci # This model represents a record in a shadow table of the main database's namespaces table. # It allows us to navigate the namespace hierarchy on the ci database without resorting to a JOIN. class NamespaceHierarchy < ApplicationRecord + scope :belonging_to_namespace, -> (namespace_id) { + where('traversal_ids && ARRAY[?]::int[]', namespace_id) + } + class << self def update_traversal_ids(old_self_and_ancestor_ids, new_self_and_ancestor_ids) # Update the traversal IDs with the new_self_and_ancestor_ids plus any descendants in the record diff --git a/app/models/ci/project_hierarchy.rb b/app/models/ci/project_hierarchy.rb index 392ef9a0b45cb4d1e62d530eb83f3dae6370ab8d..923e62287a2e768999aec62193ed3d104cc5ee56 100644 --- a/app/models/ci/project_hierarchy.rb +++ b/app/models/ci/project_hierarchy.rb @@ -6,6 +6,10 @@ module Ci class ProjectHierarchy < ApplicationRecord belongs_to :project, class_name: 'Project' + scope :belonging_to_namespace, -> (namespace_id) { + where('traversal_ids && ARRAY[?]::int[]', namespace_id) + } + class << self def update_traversal_ids(project_id, new_ancestor_ids) where(project_id: project_id).update_all(traversal_ids: new_ancestor_ids) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 8a3025e56081f320c7030ede6356b5dc5d4b9eb4..1bc2c35c151ea51613a3bef07f54bb271248f4bc 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -88,16 +88,15 @@ class Runner < Ci::ApplicationRecord .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') } - scope :belonging_to_group_or_project, -> (group_id, project_id) { - groups = ::Group.where(id: group_id) + scope :belonging_to_group_or_descendants, -> (group_id) { + groups = ::Group.where(id: group_id).ids - group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups }) - project_runners = joins(:runner_projects).where(ci_runner_projects: { project_id: project_id }) + runners_on_groups = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: Ci::NamespaceHierarchy.belonging_to_namespace(groups).select(:namespace_id) }) + runners_on_projects = belonging_to_project(Ci::ProjectHierarchy.belonging_to_namespace(groups).select(:project_id)) - union_sql = ::Gitlab::SQL::Union.new([group_runners, project_runners]).to_sql + union_sql = ::Gitlab::SQL::Union.new([runners_on_groups, runners_on_projects]).to_sql from("(#{union_sql}) #{table_name}") - .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') } scope :belonging_to_parent_group_of_project, -> (project_id) { diff --git a/ee/lib/analytics/devops_adoption/snapshot_calculator.rb b/ee/lib/analytics/devops_adoption/snapshot_calculator.rb index 0ad5d021102a2150941352f57ff1325836cf6983..620640fef14f3f222e57640e2235725bfdfc3587 100644 --- a/ee/lib/analytics/devops_adoption/snapshot_calculator.rb +++ b/ee/lib/analytics/devops_adoption/snapshot_calculator.rb @@ -61,9 +61,7 @@ def merge_request_approved # rubocop: enable CodeReuse/ActiveRecord def runner_configured - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/337541') do - Ci::Runner.active.belonging_to_group_or_project(snapshot_groups, snapshot_project_ids).exists? - end + Ci::Runner.active.belonging_to_group_or_descendants(enabled_namespace.namespace).exists? end def pipeline_succeeded diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 7aaec55a9f4c30a7c22cab12316153fd71fa1f0d..88791f9c88e130c7f796b343f926c6132f46c22e 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1226,4 +1226,42 @@ def does_db_update end end end + + describe '.belonging_to_group_or_descendants' do + subject do + # TODO: Think about a better way to ensure that the Sidekiq jobs generated by the creation of namespaces/projects are executed before the subject + Ci::ProcessNamespaceSyncEventsWorker.new.perform + Ci::ProcessProjectSyncEventsWorker.new.perform + + described_class.belonging_to_group_or_descendants(group.id) + end + + let_it_be(:group) { create(:group) } + + context 'runner belonging to group' do + let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) } + + it 'returns the group runner' do + is_expected.to contain_exactly(group_runner) + end + + context 'runner belonging to descendant group' do + let_it_be(:child_group) { create(:group, parent: group) } + let_it_be(:child_group_runner) { create(:ci_runner, :group, groups: [child_group]) } + + it 'returns the group runner from the child group' do + is_expected.to contain_exactly(group_runner, child_group_runner) + end + + context 'runner belonging to project on descendant group' do + let_it_be(:project) { create(:project, group: child_group) } + let_it_be(:child_project_runner) { create(:ci_runner, :project, projects: [project]) } + + it 'returns the project runner from the child group project' do + is_expected.to contain_exactly(group_runner, child_group_runner, child_project_runner) + end + end + end + end + end end