From c9d578a0f7300b85947906784efe495802034123 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Tue, 7 Dec 2021 11:47:21 +0300
Subject: [PATCH 1/8] Eliminate cross-db-join in RunnersFinder

This starts to use ci_namespace_mirrors and ci_project_mirrors
Behind a FF ci_find_runners_by_ci_mirrors
---
 app/finders/ci/runners_finder.rb              | 16 ++++++++---
 app/models/ci/namespace_mirror.rb             |  2 ++
 app/models/ci/project_mirror.rb               |  3 ++
 app/models/ci/runner.rb                       | 28 +++++++++++++++++--
 app/models/ci/runner_namespace.rb             |  2 +-
 .../ci_find_runners_by_ci_mirrors.yml         |  8 ++++++
 .../devops_adoption/snapshot_calculator.rb    |  2 +-
 lib/api/ci/runners.rb                         |  2 +-
 .../groups/runners_controller_spec.rb         |  2 +-
 .../groups/settings/ci_cd_controller_spec.rb  |  2 +-
 spec/features/runners_spec.rb                 |  2 +-
 spec/finders/ci/runners_finder_spec.rb        | 14 ++++++++--
 spec/frontend/fixtures/runner.rb              |  2 +-
 .../ci/group_runners_resolver_spec.rb         |  2 +-
 spec/models/ci/runner_spec.rb                 | 19 ++++++++++---
 spec/requests/api/graphql/ci/runner_spec.rb   |  2 +-
 spec/spec_helper.rb                           |  5 ++++
 17 files changed, 92 insertions(+), 21 deletions(-)
 create mode 100644 config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml

diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb
index 5d597f94f72c04..21cd9d814e0e31 100644
--- a/app/finders/ci/runners_finder.rb
+++ b/app/finders/ci/runners_finder.rb
@@ -51,11 +51,19 @@ def group_runners
 
       @runners = case @params[:membership]
                  when :direct
-                   Ci::Runner.belonging_to_group(@group.id)
+                   if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, @group, default_enabled: :yaml)
+                     Ci::Runner.belonging_to_group(@group.id)
+                   else
+                     Ci::Runner.legacy_belonging_to_group(@group.id)
+                   end
                  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)
+                   if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, @group, default_enabled: :yaml)
+                     Ci::Runner.belonging_to_group_or_project(@group.id)
+                   else
+                     # Getting all runners from the group itself and all its descendant groups/projects
+                     descendant_projects = Project.for_group_and_its_subgroups(@group)
+                     Ci::Runner.legacy_belonging_to_group_or_project(@group.self_and_descendants, descendant_projects)
+                   end
                  else
                    raise ArgumentError, 'Invalid membership filter'
                  end
diff --git a/app/models/ci/namespace_mirror.rb b/app/models/ci/namespace_mirror.rb
index 8a4be3139e83da..cff63c00e07b53 100644
--- a/app/models/ci/namespace_mirror.rb
+++ b/app/models/ci/namespace_mirror.rb
@@ -10,6 +10,8 @@ class NamespaceMirror < ApplicationRecord
       where('traversal_ids @> ARRAY[?]::int[]', id)
     end
 
+    scope :by_namespace_id, -> (namespace_id) { where(namespace_id: namespace_id) }
+
     class << self
       def sync!(event)
         namespace = event.namespace
diff --git a/app/models/ci/project_mirror.rb b/app/models/ci/project_mirror.rb
index d6aaa3f50c142c..9000d1791a6c7e 100644
--- a/app/models/ci/project_mirror.rb
+++ b/app/models/ci/project_mirror.rb
@@ -6,6 +6,9 @@ module Ci
   class ProjectMirror < ApplicationRecord
     belongs_to :project
 
+    scope :by_namespace_id, -> (namespace_id) { where(namespace_id: namespace_id) }
+    scope :by_project_id, -> (project_id) { where(project_id: project_id) }
+
     class << self
       def sync!(event)
         upsert({ project_id: event.project_id, namespace_id: event.project.namespace_id },
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 6ba0a00ddae7ae..4d0c1d6f017c59 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -95,7 +95,28 @@ class Runner < Ci::ApplicationRecord
       joins(:runner_projects).where(ci_runner_projects: { project_id: project_id })
     }
 
-    scope :belonging_to_group, -> (group_id, include_ancestors: false) {
+    scope :belonging_to_group, -> (group_id) {
+      namespaces = Ci::NamespaceMirror.by_namespace_id(group_id).select(:namespace_id)
+
+      joins(:runner_namespaces)
+        .where(ci_runner_namespaces: { namespace_id: namespaces })
+    }
+
+    scope :belonging_to_group_or_project, -> (group_id) {
+      group_ids = Ci::NamespaceMirror.contains_namespace(group_id).select(:namespace_id)
+      project_ids = Ci::ProjectMirror.by_namespace_id(group_ids).select(:project_id)
+
+      group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: group_ids })
+      project_runners = joins(:runner_projects).where(ci_runner_projects: { project_id: project_ids })
+
+      union_sql = ::Gitlab::SQL::Union.new([group_runners, project_runners]).to_sql
+
+      from("(#{union_sql}) #{table_name}")
+    }
+
+    # deprecated
+    # split this into: belonging_to_group & belonging_to_group_and_ancestors
+    scope :legacy_belonging_to_group, -> (group_id, include_ancestors: false) {
       groups = ::Group.where(id: group_id)
       groups = groups.self_and_ancestors if include_ancestors
 
@@ -104,7 +125,8 @@ 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) {
+    # deprecated
+    scope :legacy_belonging_to_group_or_project, -> (group_id, project_id) {
       groups = ::Group.where(id: group_id)
 
       group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups })
@@ -116,6 +138,7 @@ class Runner < Ci::ApplicationRecord
         .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
     }
 
+    # deprecated
     scope :belonging_to_parent_group_of_project, -> (project_id) {
       project_groups = ::Group.joins(:projects).where(projects: { id: project_id })
 
@@ -124,6 +147,7 @@ class Runner < Ci::ApplicationRecord
         .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
     }
 
+    # deprecated
     scope :owned_or_instance_wide, -> (project_id) do
       from_union(
         [
diff --git a/app/models/ci/runner_namespace.rb b/app/models/ci/runner_namespace.rb
index 82390ccc538da3..a005c84d1e1171 100644
--- a/app/models/ci/runner_namespace.rb
+++ b/app/models/ci/runner_namespace.rb
@@ -16,7 +16,7 @@ class RunnerNamespace < Ci::ApplicationRecord
     validate :group_runner_type
 
     def recent_runners
-      ::Ci::Runner.belonging_to_group(namespace_id).recent
+      ::Ci::Runner.legacy_belonging_to_group(namespace_id).recent
     end
 
     private
diff --git a/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml b/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml
new file mode 100644
index 00000000000000..fd6d5d529e22ff
--- /dev/null
+++ b/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml
@@ -0,0 +1,8 @@
+---
+name: ci_find_runners_by_ci_mirrors
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74900
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/347226
+milestone: '14.6'
+type: development
+group: group::runner
+default_enabled: false
diff --git a/ee/lib/analytics/devops_adoption/snapshot_calculator.rb b/ee/lib/analytics/devops_adoption/snapshot_calculator.rb
index 0ad5d021102a21..7c5b0c59d982a2 100644
--- a/ee/lib/analytics/devops_adoption/snapshot_calculator.rb
+++ b/ee/lib/analytics/devops_adoption/snapshot_calculator.rb
@@ -62,7 +62,7 @@ def merge_request_approved
 
       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?
+          Ci::Runner.active.legacy_belonging_to_group_or_project(snapshot_groups, snapshot_project_ids).exists?
         end
       end
 
diff --git a/lib/api/ci/runners.rb b/lib/api/ci/runners.rb
index ef712c84804ec2..f39c139b4facf1 100644
--- a/lib/api/ci/runners.rb
+++ b/lib/api/ci/runners.rb
@@ -229,7 +229,7 @@ class Runners < ::API::Base
           use :pagination
         end
         get ':id/runners' do
-          runners = ::Ci::Runner.belonging_to_group(user_group.id, include_ancestors: true)
+          runners = ::Ci::Runner.legacy_belonging_to_group(user_group.id, include_ancestors: true)
           runners = apply_filter(runners, params)
 
           present paginate(runners), with: Entities::Ci::Runner
diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb
index a8830efe65394c..cb83f8573cecc6 100644
--- a/spec/controllers/groups/runners_controller_spec.rb
+++ b/spec/controllers/groups/runners_controller_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Groups::RunnersController do
+RSpec.describe Groups::RunnersController, :drain_sync_events do
   let_it_be(:user)   { create(:user) }
   let_it_be(:group)  { create(:group) }
   let_it_be(:project) { create(:project, group: group) }
diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
index f225d798886bdf..582145de922121 100644
--- a/spec/controllers/groups/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Groups::Settings::CiCdController do
+RSpec.describe Groups::Settings::CiCdController, :drain_sync_events do
   include ExternalAuthorizationServiceHelpers
 
   let_it_be(:group) { create(:group) }
diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb
index 22de77f7cd0149..36cd549a92afa0 100644
--- a/spec/features/runners_spec.rb
+++ b/spec/features/runners_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Runners' do
+RSpec.describe 'Runners', :sidekiq_inline do
   let(:user) { create(:user) }
 
   before do
diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index 7e3c1abd6d1e94..3133a567106e2a 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -166,7 +166,7 @@
     end
   end
 
-  context 'group' do
+  context 'group', :drain_sync_events do
     let_it_be(:user) { create(:user) }
     let_it_be(:group) { create(:group) }
     let_it_be(:sub_group_1) { create(:group, parent: group) }
@@ -206,7 +206,7 @@
       sub_group_4.runners << runner_sub_group_4
     end
 
-    describe '#execute' do
+    shared_examples '#execute' do
       subject { described_class.new(current_user: user, params: params).execute }
 
       shared_examples 'membership equal to :descendants' do
@@ -349,6 +349,16 @@
       end
     end
 
+    it_behaves_like '#execute'
+
+    context 'when the FF ci_find_runners_by_ci_mirrors is disabled' do
+      before do
+        stub_feature_flags(ci_find_runners_by_ci_mirrors: false)
+      end
+
+      it_behaves_like '#execute'
+    end
+
     describe '#sort_key' do
       subject { described_class.new(current_user: user, params: params.merge(group: group)).sort_key }
 
diff --git a/spec/frontend/fixtures/runner.rb b/spec/frontend/fixtures/runner.rb
index fa150fbf57c730..27466e5faa2f4b 100644
--- a/spec/frontend/fixtures/runner.rb
+++ b/spec/frontend/fixtures/runner.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Runner (JavaScript fixtures)' do
+RSpec.describe 'Runner (JavaScript fixtures)', :drain_sync_events do
   include AdminModeHelper
   include ApiHelpers
   include JavaScriptFixturesHelpers
diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index 89a2437a189f65..b2ea59bc04dd30 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Resolvers::Ci::GroupRunnersResolver do
+RSpec.describe Resolvers::Ci::GroupRunnersResolver, :drain_sync_events do
   include GraphqlHelpers
 
   describe '#resolve' do
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 5142f70fa2c233..f6207a7b9579bf 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -1358,7 +1358,7 @@ def does_db_update
     it { is_expected.to eq(contacted_at_stored) }
   end
 
-  describe '.belonging_to_group' do
+  describe '.legacy_belonging_to_group' do
     shared_examples 'returns group runners' do
       it 'returns the specific group runner' do
         group = create(:group)
@@ -1366,7 +1366,7 @@ def does_db_update
         unrelated_group = create(:group)
         create(:ci_runner, :group, groups: [unrelated_group])
 
-        expect(described_class.belonging_to_group(group.id)).to contain_exactly(runner)
+        expect(described_class.legacy_belonging_to_group(group.id)).to contain_exactly(runner)
       end
 
       context 'runner belonging to parent group' do
@@ -1376,13 +1376,13 @@ def does_db_update
 
         context 'when include_parent option is passed' do
           it 'returns the group runner from the parent group' do
-            expect(described_class.belonging_to_group(group.id, include_ancestors: true)).to contain_exactly(parent_runner)
+            expect(described_class.legacy_belonging_to_group(group.id, include_ancestors: true)).to contain_exactly(parent_runner)
           end
         end
 
         context 'when include_parent option is not passed' do
           it 'does not return the group runner from the parent group' do
-            expect(described_class.belonging_to_group(group.id)).to be_empty
+            expect(described_class.legacy_belonging_to_group(group.id)).to be_empty
           end
         end
       end
@@ -1398,4 +1398,15 @@ def does_db_update
       it_behaves_like 'returns group runners'
     end
   end
+
+  describe '.belonging_to_group' do
+    it 'returns the specific group runner', :sidekiq_inline do
+      group = create(:group)
+      runner = create(:ci_runner, :group, groups: [group])
+      unrelated_group = create(:group)
+      create(:ci_runner, :group, groups: [unrelated_group])
+
+      expect(described_class.belonging_to_group(group.id)).to contain_exactly(runner)
+    end
+  end
 end
diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb
index 3cb54c8ef4b2ef..5d01a096a8ad7d 100644
--- a/spec/requests/api/graphql/ci/runner_spec.rb
+++ b/spec/requests/api/graphql/ci/runner_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Query.runner(id)' do
+RSpec.describe 'Query.runner(id)', :drain_sync_events do
   include GraphqlHelpers
 
   let_it_be(:user) { create(:user, :admin) }
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index c497f8245feef2..a5f9cdf6bd2366 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -464,6 +464,11 @@
   end
 
   config.disable_monkey_patching!
+
+  config.before(:each, :drain_sync_events) do |example|
+    ::Namespaces::ProcessSyncEventsWorker.new.perform
+    ::Projects::ProcessSyncEventsWorker.new.perform
+  end
 end
 
 ActiveRecord::Migration.maintain_test_schema!
-- 
GitLab


From 51d57064dc573acc4a659e4543b5b03ba7c0fd8e Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Tue, 14 Dec 2021 13:59:54 +0300
Subject: [PATCH 2/8] Backend review updates

---
 app/finders/ci/runners_finder.rb              |  2 +-
 app/models/ci/runner.rb                       |  2 +-
 .../groups/runners_controller_spec.rb         |  2 +-
 .../groups/settings/ci_cd_controller_spec.rb  |  2 +-
 spec/factories/ci/namespace_mirrors.rb        |  8 ++
 spec/factories/ci/project_mirrors.rb          |  8 ++
 spec/factories/groups.rb                      |  3 +
 spec/factories/projects.rb                    |  3 +
 spec/features/runners_spec.rb                 |  2 +-
 spec/finders/ci/runners_finder_spec.rb        |  2 +-
 spec/frontend/fixtures/runner.rb              |  2 +-
 .../ci/group_runners_resolver_spec.rb         |  2 +-
 spec/models/ci/namespace_mirror_spec.rb       | 84 +++++++++----------
 spec/models/ci/project_mirror_spec.rb         | 34 ++++++--
 spec/models/ci/runner_spec.rb                 | 25 +++++-
 spec/requests/api/graphql/ci/runner_spec.rb   |  2 +-
 spec/spec_helper.rb                           |  5 --
 17 files changed, 124 insertions(+), 64 deletions(-)
 create mode 100644 spec/factories/ci/namespace_mirrors.rb
 create mode 100644 spec/factories/ci/project_mirrors.rb

diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb
index 21cd9d814e0e31..2ceed69135031f 100644
--- a/app/finders/ci/runners_finder.rb
+++ b/app/finders/ci/runners_finder.rb
@@ -58,7 +58,7 @@ def group_runners
                    end
                  when :descendants, nil
                    if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, @group, default_enabled: :yaml)
-                     Ci::Runner.belonging_to_group_or_project(@group.id)
+                     Ci::Runner.belonging_to_group_or_project_descendants(@group.id)
                    else
                      # Getting all runners from the group itself and all its descendant groups/projects
                      descendant_projects = Project.for_group_and_its_subgroups(@group)
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 4d0c1d6f017c59..3fe65e240a49a5 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -102,7 +102,7 @@ class Runner < Ci::ApplicationRecord
         .where(ci_runner_namespaces: { namespace_id: namespaces })
     }
 
-    scope :belonging_to_group_or_project, -> (group_id) {
+    scope :belonging_to_group_or_project_descendants, -> (group_id) {
       group_ids = Ci::NamespaceMirror.contains_namespace(group_id).select(:namespace_id)
       project_ids = Ci::ProjectMirror.by_namespace_id(group_ids).select(:project_id)
 
diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb
index cb83f8573cecc6..a8830efe65394c 100644
--- a/spec/controllers/groups/runners_controller_spec.rb
+++ b/spec/controllers/groups/runners_controller_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Groups::RunnersController, :drain_sync_events do
+RSpec.describe Groups::RunnersController do
   let_it_be(:user)   { create(:user) }
   let_it_be(:group)  { create(:group) }
   let_it_be(:project) { create(:project, group: group) }
diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
index 582145de922121..f225d798886bdf 100644
--- a/spec/controllers/groups/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Groups::Settings::CiCdController, :drain_sync_events do
+RSpec.describe Groups::Settings::CiCdController do
   include ExternalAuthorizationServiceHelpers
 
   let_it_be(:group) { create(:group) }
diff --git a/spec/factories/ci/namespace_mirrors.rb b/spec/factories/ci/namespace_mirrors.rb
new file mode 100644
index 00000000000000..033cdcf67b41d0
--- /dev/null
+++ b/spec/factories/ci/namespace_mirrors.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ci_namespace_mirror, class: 'Ci::NamespaceMirror' do
+    namespace
+    traversal_ids { namespace.self_and_ancestor_ids(hierarchy_order: :desc) }
+  end
+end
diff --git a/spec/factories/ci/project_mirrors.rb b/spec/factories/ci/project_mirrors.rb
new file mode 100644
index 00000000000000..271a6a80c5fff9
--- /dev/null
+++ b/spec/factories/ci/project_mirrors.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ci_project_mirror, class: 'Ci::ProjectMirror' do
+    project
+    namespace_id { project.namespace_id }
+  end
+end
diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb
index 859f381e4c1553..d8708a59b426d3 100644
--- a/spec/factories/groups.rb
+++ b/spec/factories/groups.rb
@@ -16,6 +16,9 @@
       end
 
       create(:namespace_settings, namespace: group) unless group.namespace_settings
+
+      # simulating ::Namespaces::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
+      create(:ci_namespace_mirror, namespace: group) unless group.ci_namespace_mirror
     end
 
     trait :public do
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 981f10e8260ec3..3d59cc681d4ff6 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -101,6 +101,9 @@
         import_state.last_error = evaluator.import_last_error
         import_state.save!
       end
+
+      # simulating ::Projects::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
+      create(:ci_project_mirror, project: project) unless project.ci_project_mirror
     end
 
     trait :public do
diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb
index 36cd549a92afa0..22de77f7cd0149 100644
--- a/spec/features/runners_spec.rb
+++ b/spec/features/runners_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Runners', :sidekiq_inline do
+RSpec.describe 'Runners' do
   let(:user) { create(:user) }
 
   before do
diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index 3133a567106e2a..dac244e4300d39 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -166,7 +166,7 @@
     end
   end
 
-  context 'group', :drain_sync_events do
+  context 'group' do
     let_it_be(:user) { create(:user) }
     let_it_be(:group) { create(:group) }
     let_it_be(:sub_group_1) { create(:group, parent: group) }
diff --git a/spec/frontend/fixtures/runner.rb b/spec/frontend/fixtures/runner.rb
index 27466e5faa2f4b..fa150fbf57c730 100644
--- a/spec/frontend/fixtures/runner.rb
+++ b/spec/frontend/fixtures/runner.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Runner (JavaScript fixtures)', :drain_sync_events do
+RSpec.describe 'Runner (JavaScript fixtures)' do
   include AdminModeHelper
   include ApiHelpers
   include JavaScriptFixturesHelpers
diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index b2ea59bc04dd30..89a2437a189f65 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Resolvers::Ci::GroupRunnersResolver, :drain_sync_events do
+RSpec.describe Resolvers::Ci::GroupRunnersResolver do
   include GraphqlHelpers
 
   describe '#resolve' do
diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb
index b4c71f51377f1d..237b8a5d3fe54b 100644
--- a/spec/models/ci/namespace_mirror_spec.rb
+++ b/spec/models/ci/namespace_mirror_spec.rb
@@ -8,50 +8,68 @@
   let!(:group3) { create(:group, parent: group2) }
   let!(:group4) { create(:group, parent: group3) }
 
-  describe '.sync!' do
-    let!(:event) { namespace.sync_events.create! }
+  context 'scopes' do
+    describe '.contains_namespace' do
+      let_it_be(:another_group) { create(:group) }
 
-    subject(:sync) { described_class.sync!(event.reload) }
+      subject(:result) { described_class.contains_namespace(group2.id) }
 
-    context 'when namespace hierarchy does not exist in the first place' do
-      let(:namespace) { group3 }
+      it 'returns groups having group2.id in traversal_ids' do
+        expect(result.pluck(:namespace_id)).to contain_exactly(group2.id, group3.id, group4.id)
+      end
+    end
 
-      it 'creates the hierarchy' do
-        expect { sync }.to change { described_class.count }.from(0).to(1)
+    describe '.by_namespace_id' do
+      subject(:result) { described_class.by_namespace_id(group2.id) }
 
-        expect(namespace.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
+      it 'returns namesapce mirrors of namespace id' do
+        expect(result).to contain_exactly(group2.reload.ci_namespace_mirror)
       end
     end
+  end
 
-    context 'when namespace hierarchy does already exist' do
+  describe '.sync!' do
+    subject(:sync) { described_class.sync!(namespace.sync_events.create!) }
+
+    context 'when namespace mirror does not exist in the first place' do
       let(:namespace) { group3 }
 
       before do
-        described_class.create!(namespace: namespace, traversal_ids: [namespace.id])
+        namespace.reload.ci_namespace_mirror.destroy!
       end
 
-      it 'updates the hierarchy' do
-        expect { sync }.not_to change { described_class.count }
+      it 'creates the mirror' do
+        expect { sync }.to change { described_class.count }.from(3).to(4)
 
         expect(namespace.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
       end
     end
 
-    # I did not extract this context to a `shared_context` because the behavior will change
-    # after implementing the TODO in `Ci::NamespaceMirror.sync!`
-    context 'changing the middle namespace' do
+    context 'when namespace mirror does already exist' do
+      let(:namespace) { group3 }
+
+      it 'updates the mirror' do
+        expect { sync }.not_to change { described_class.count }
+
+        expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
+      end
+    end
+
+    shared_context 'changing the middle namespace' do
       let(:namespace) { group2 }
 
       before do
-        described_class.create!(namespace_id: group1.id, traversal_ids: [group1.id])
-        described_class.create!(namespace_id: group2.id, traversal_ids: [group1.id, group2.id])
-        described_class.create!(namespace_id: group3.id, traversal_ids: [group1.id, group2.id, group3.id])
-        described_class.create!(namespace_id: group4.id, traversal_ids: [group1.id, group2.id, group3.id, group4.id])
+        # check initial situation
+        expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
+        expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id])
+        expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
+        expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id, group4.id])
 
-        group2.update!(parent: nil)
+        group2.update!(parent: nil) # creates a sync event
+        group2.reload # guarantees traversal_ids updated
       end
 
-      it 'updates hierarchies for the base but wait for events for the children' do
+      it 'updates traversal_ids for the base and descendants' do
         expect { sync }.not_to change { described_class.count }
 
         expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
@@ -61,6 +79,8 @@
       end
     end
 
+    it_behaves_like 'changing the middle namespace'
+
     context 'when the FFs sync_traversal_ids, use_traversal_ids and use_traversal_ids_for_ancestors are disabled' do
       before do
         stub_feature_flags(sync_traversal_ids: false,
@@ -68,27 +88,7 @@
                            use_traversal_ids_for_ancestors: false)
       end
 
-      context 'changing the middle namespace' do
-        let(:namespace) { group2 }
-
-        before do
-          described_class.create!(namespace_id: group1.id, traversal_ids: [group1.id])
-          described_class.create!(namespace_id: group2.id, traversal_ids: [group1.id, group2.id])
-          described_class.create!(namespace_id: group3.id, traversal_ids: [group1.id, group2.id, group3.id])
-          described_class.create!(namespace_id: group4.id, traversal_ids: [group1.id, group2.id, group3.id, group4.id])
-
-          group2.update!(parent: nil)
-        end
-
-        it 'updates hierarchies for the base and descendants' do
-          expect { sync }.not_to change { described_class.count }
-
-          expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
-          expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id])
-          expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id])
-          expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id, group4.id])
-        end
-      end
+      it_behaves_like 'changing the middle namespace'
     end
   end
 end
diff --git a/spec/models/ci/project_mirror_spec.rb b/spec/models/ci/project_mirror_spec.rb
index 199285b036c3ec..46e74a45f6201f 100644
--- a/spec/models/ci/project_mirror_spec.rb
+++ b/spec/models/ci/project_mirror_spec.rb
@@ -8,12 +8,36 @@
 
   let!(:project) { create(:project, namespace: group2) }
 
+  context 'scopes' do
+    let_it_be(:another_project) { create(:project, namespace: group1) }
+
+    describe '.by_project_id' do
+      subject(:result) { described_class.by_project_id(project.id) }
+
+      it 'returns project mirrors of project' do
+        expect(result.pluck(:project_id)).to contain_exactly(project.id)
+      end
+    end
+
+    describe '.by_namespace_id' do
+      subject(:result) { described_class.by_namespace_id(group2.id) }
+
+      it 'returns project mirrors of namespace id' do
+        expect(result).to contain_exactly(project.reload.ci_project_mirror)
+      end
+    end
+  end
+
   describe '.sync!' do
     let!(:event) { Projects::SyncEvent.create!(project: project) }
 
     subject(:sync) { described_class.sync!(event.reload) }
 
-    context 'when project hierarchy does not exist in the first place' do
+    context 'when project mirror does not exist in the first place' do
+      before do
+        project.reload.ci_project_mirror.destroy!
+      end
+
       it 'creates a ci_projects record' do
         expect { sync }.to change { described_class.count }.from(0).to(1)
 
@@ -21,15 +45,11 @@
       end
     end
 
-    context 'when project hierarchy does already exist' do
-      before do
-        described_class.create!(project_id: project.id, namespace_id: group1.id)
-      end
-
+    context 'when project mirror does already exist' do
       it 'updates the related ci_projects record' do
         expect { sync }.not_to change { described_class.count }
 
-        expect(project.ci_project_mirror).to have_attributes(namespace_id: group2.id)
+        expect(project.reload.ci_project_mirror).to have_attributes(namespace_id: group2.id)
       end
     end
   end
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index f6207a7b9579bf..868551d31e835f 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -1400,7 +1400,7 @@ def does_db_update
   end
 
   describe '.belonging_to_group' do
-    it 'returns the specific group runner', :sidekiq_inline do
+    it 'returns the specific group runner' do
       group = create(:group)
       runner = create(:ci_runner, :group, groups: [group])
       unrelated_group = create(:group)
@@ -1409,4 +1409,27 @@ def does_db_update
       expect(described_class.belonging_to_group(group.id)).to contain_exactly(runner)
     end
   end
+
+  describe '.belonging_to_group_or_project_descendants' do
+    it 'returns the specific group runners' do
+      group1 = create(:group)
+      group2 = create(:group, parent: group1)
+      group3 = create(:group)
+
+      project1 = create(:project, namespace: group1)
+      project2 = create(:project, namespace: group2)
+      project3 = create(:project, namespace: group3)
+
+      runner1 = create(:ci_runner, :group, groups: [group1])
+      runner2 = create(:ci_runner, :group, groups: [group2])
+      _runner3 = create(:ci_runner, :group, groups: [group3])
+      runner4 = create(:ci_runner, :project, projects: [project1])
+      runner5 = create(:ci_runner, :project, projects: [project2])
+      _runner6 = create(:ci_runner, :project, projects: [project3])
+
+      expect(described_class.belonging_to_group_or_project_descendants(group1.id)).to contain_exactly(
+        runner1, runner2, runner4, runner5
+      )
+    end
+  end
 end
diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb
index 5d01a096a8ad7d..3cb54c8ef4b2ef 100644
--- a/spec/requests/api/graphql/ci/runner_spec.rb
+++ b/spec/requests/api/graphql/ci/runner_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Query.runner(id)', :drain_sync_events do
+RSpec.describe 'Query.runner(id)' do
   include GraphqlHelpers
 
   let_it_be(:user) { create(:user, :admin) }
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index a5f9cdf6bd2366..c497f8245feef2 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -464,11 +464,6 @@
   end
 
   config.disable_monkey_patching!
-
-  config.before(:each, :drain_sync_events) do |example|
-    ::Namespaces::ProcessSyncEventsWorker.new.perform
-    ::Projects::ProcessSyncEventsWorker.new.perform
-  end
 end
 
 ActiveRecord::Migration.maintain_test_schema!
-- 
GitLab


From d62bf78d38f35677619cc762d24625d081bb9144 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Tue, 14 Dec 2021 14:23:49 +0300
Subject: [PATCH 3/8] DB review changes

---
 app/models/ci/runner.rb | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 3fe65e240a49a5..193583d7c5dac1 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -96,10 +96,8 @@ class Runner < Ci::ApplicationRecord
     }
 
     scope :belonging_to_group, -> (group_id) {
-      namespaces = Ci::NamespaceMirror.by_namespace_id(group_id).select(:namespace_id)
-
       joins(:runner_namespaces)
-        .where(ci_runner_namespaces: { namespace_id: namespaces })
+        .where(ci_runner_namespaces: { namespace_id: group_id })
     }
 
     scope :belonging_to_group_or_project_descendants, -> (group_id) {
-- 
GitLab


From 9ece4f5c0836b9858f7f6555832467ab1bf6c86c Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Tue, 14 Dec 2021 15:54:12 +0300
Subject: [PATCH 4/8] Fix the tests

---
 spec/factories/ci/namespace_mirrors.rb               | 2 +-
 spec/factories/ci/project_mirrors.rb                 | 2 +-
 spec/factories/groups.rb                             | 3 ---
 spec/factories/namespaces.rb                         | 9 +++++++++
 spec/factories/projects.rb                           | 4 +++-
 spec/factories_spec.rb                               | 2 ++
 spec/services/ci/process_sync_events_service_spec.rb | 8 ++++----
 7 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/spec/factories/ci/namespace_mirrors.rb b/spec/factories/ci/namespace_mirrors.rb
index 033cdcf67b41d0..7bc7bb6d2a955b 100644
--- a/spec/factories/ci/namespace_mirrors.rb
+++ b/spec/factories/ci/namespace_mirrors.rb
@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :ci_namespace_mirror, class: 'Ci::NamespaceMirror' do
-    namespace
+    association :namespace, factory: :namespace, creating_via_ci_mirror: true
     traversal_ids { namespace.self_and_ancestor_ids(hierarchy_order: :desc) }
   end
 end
diff --git a/spec/factories/ci/project_mirrors.rb b/spec/factories/ci/project_mirrors.rb
index 271a6a80c5fff9..904af61017e736 100644
--- a/spec/factories/ci/project_mirrors.rb
+++ b/spec/factories/ci/project_mirrors.rb
@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :ci_project_mirror, class: 'Ci::ProjectMirror' do
-    project
+    association :project, factory: :project, creating_via_ci_mirror: true
     namespace_id { project.namespace_id }
   end
 end
diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb
index d8708a59b426d3..859f381e4c1553 100644
--- a/spec/factories/groups.rb
+++ b/spec/factories/groups.rb
@@ -16,9 +16,6 @@
       end
 
       create(:namespace_settings, namespace: group) unless group.namespace_settings
-
-      # simulating ::Namespaces::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
-      create(:ci_namespace_mirror, namespace: group) unless group.ci_namespace_mirror
     end
 
     trait :public do
diff --git a/spec/factories/namespaces.rb b/spec/factories/namespaces.rb
index 2b3dabc07d86af..2980b0e4a42468 100644
--- a/spec/factories/namespaces.rb
+++ b/spec/factories/namespaces.rb
@@ -11,6 +11,15 @@
 
     owner { association(:user, strategy: :build, namespace: instance, username: path) }
 
+    transient do
+      creating_via_ci_mirror { false } # prevents circular creation with :ci_namespace_mirror
+    end
+
+    after(:create) do |namespace, evaluator|
+      # simulating ::Namespaces::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
+      create(:ci_namespace_mirror, namespace: namespace) unless namespace.ci_namespace_mirror || evaluator.creating_via_ci_mirror
+    end
+
     trait :with_aggregation_schedule do
       after(:create) do |namespace|
         create(:namespace_aggregation_schedules, namespace: namespace)
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 3d59cc681d4ff6..d01493c19cc7d3 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -49,6 +49,8 @@
       forward_deployment_enabled { nil }
       restrict_user_defined_variables { nil }
       ci_job_token_scope_enabled { nil }
+
+      creating_via_ci_mirror { false } # prevents circular creation with :ci_project_mirror
     end
 
     after(:build) do |project, evaluator|
@@ -103,7 +105,7 @@
       end
 
       # simulating ::Projects::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
-      create(:ci_project_mirror, project: project) unless project.ci_project_mirror
+      create(:ci_project_mirror, project: project) unless project.ci_project_mirror || evaluator.creating_via_ci_mirror
     end
 
     trait :public do
diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb
index 811ed18dce3e5c..0b9c4647c2dcd0 100644
--- a/spec/factories_spec.rb
+++ b/spec/factories_spec.rb
@@ -69,6 +69,8 @@ def skipped_traits
   # is being mutated.
   skip_factory_defaults = %i[
     ci_job_token_project_scope_link
+    ci_namespace_mirror
+    ci_project_mirror
     evidence
     exported_protected_branch
     fork_network_member
diff --git a/spec/services/ci/process_sync_events_service_spec.rb b/spec/services/ci/process_sync_events_service_spec.rb
index 00b670ff54f28b..c4d4a2a4776e98 100644
--- a/spec/services/ci/process_sync_events_service_spec.rb
+++ b/spec/services/ci/process_sync_events_service_spec.rb
@@ -28,10 +28,10 @@
       it 'consumes events' do
         expect { execute }.to change(Projects::SyncEvent, :count).from(2).to(0)
 
-        expect(project1.ci_project_mirror).to have_attributes(
+        expect(project1.reload.ci_project_mirror).to have_attributes(
           namespace_id: parent_group_1.id
         )
-        expect(project2.ci_project_mirror).to have_attributes(
+        expect(project2.reload.ci_project_mirror).to have_attributes(
           namespace_id: parent_group_2.id
         )
       end
@@ -88,10 +88,10 @@
         it 'consumes events' do
           expect { execute }.to change(Namespaces::SyncEvent, :count).from(2).to(0)
 
-          expect(group.ci_namespace_mirror).to have_attributes(
+          expect(group.reload.ci_namespace_mirror).to have_attributes(
             traversal_ids: [parent_group_1.id, parent_group_2.id, group.id]
           )
-          expect(parent_group_2.ci_namespace_mirror).to have_attributes(
+          expect(parent_group_2.reload.ci_namespace_mirror).to have_attributes(
             traversal_ids: [parent_group_1.id, parent_group_2.id]
           )
         end
-- 
GitLab


From 24ae2c531be6082bf754e0e58900e44c7960ab4f Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Fri, 17 Dec 2021 13:25:45 +0300
Subject: [PATCH 5/8] Remove FF usage for belonging_to_group

---
 app/finders/ci/runners_finder.rb  | 6 +-----
 app/models/ci/runner_namespace.rb | 2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb
index 2ceed69135031f..a514d2e8d03434 100644
--- a/app/finders/ci/runners_finder.rb
+++ b/app/finders/ci/runners_finder.rb
@@ -51,11 +51,7 @@ def group_runners
 
       @runners = case @params[:membership]
                  when :direct
-                   if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, @group, default_enabled: :yaml)
-                     Ci::Runner.belonging_to_group(@group.id)
-                   else
-                     Ci::Runner.legacy_belonging_to_group(@group.id)
-                   end
+                   Ci::Runner.belonging_to_group(@group.id)
                  when :descendants, nil
                    if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, @group, default_enabled: :yaml)
                      Ci::Runner.belonging_to_group_or_project_descendants(@group.id)
diff --git a/app/models/ci/runner_namespace.rb b/app/models/ci/runner_namespace.rb
index a005c84d1e1171..82390ccc538da3 100644
--- a/app/models/ci/runner_namespace.rb
+++ b/app/models/ci/runner_namespace.rb
@@ -16,7 +16,7 @@ class RunnerNamespace < Ci::ApplicationRecord
     validate :group_runner_type
 
     def recent_runners
-      ::Ci::Runner.legacy_belonging_to_group(namespace_id).recent
+      ::Ci::Runner.belonging_to_group(namespace_id).recent
     end
 
     private
-- 
GitLab


From 724a67d61004ec5b7d871b482e2a6027ef45b192 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Wed, 22 Dec 2021 11:19:47 +0300
Subject: [PATCH 6/8] Simplify the tests

---
 spec/factories/ci/namespace_mirrors.rb  |  8 -------
 spec/factories/ci/project_mirrors.rb    |  8 -------
 spec/factories/namespaces.rb            |  6 +----
 spec/factories/projects.rb              |  4 +---
 spec/factories_spec.rb                  |  2 --
 spec/models/ci/namespace_mirror_spec.rb | 31 ++++++++++++++++---------
 spec/models/ci/project_mirror_spec.rb   |  8 +++----
 7 files changed, 26 insertions(+), 41 deletions(-)
 delete mode 100644 spec/factories/ci/namespace_mirrors.rb
 delete mode 100644 spec/factories/ci/project_mirrors.rb

diff --git a/spec/factories/ci/namespace_mirrors.rb b/spec/factories/ci/namespace_mirrors.rb
deleted file mode 100644
index 7bc7bb6d2a955b..00000000000000
--- a/spec/factories/ci/namespace_mirrors.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-# frozen_string_literal: true
-
-FactoryBot.define do
-  factory :ci_namespace_mirror, class: 'Ci::NamespaceMirror' do
-    association :namespace, factory: :namespace, creating_via_ci_mirror: true
-    traversal_ids { namespace.self_and_ancestor_ids(hierarchy_order: :desc) }
-  end
-end
diff --git a/spec/factories/ci/project_mirrors.rb b/spec/factories/ci/project_mirrors.rb
deleted file mode 100644
index 904af61017e736..00000000000000
--- a/spec/factories/ci/project_mirrors.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-# frozen_string_literal: true
-
-FactoryBot.define do
-  factory :ci_project_mirror, class: 'Ci::ProjectMirror' do
-    association :project, factory: :project, creating_via_ci_mirror: true
-    namespace_id { project.namespace_id }
-  end
-end
diff --git a/spec/factories/namespaces.rb b/spec/factories/namespaces.rb
index 2980b0e4a42468..9e929909d8406f 100644
--- a/spec/factories/namespaces.rb
+++ b/spec/factories/namespaces.rb
@@ -11,13 +11,9 @@
 
     owner { association(:user, strategy: :build, namespace: instance, username: path) }
 
-    transient do
-      creating_via_ci_mirror { false } # prevents circular creation with :ci_namespace_mirror
-    end
-
     after(:create) do |namespace, evaluator|
       # simulating ::Namespaces::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
-      create(:ci_namespace_mirror, namespace: namespace) unless namespace.ci_namespace_mirror || evaluator.creating_via_ci_mirror
+      namespace.create_ci_namespace_mirror!(traversal_ids: namespace.traversal_ids) unless namespace.ci_namespace_mirror
     end
 
     trait :with_aggregation_schedule do
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index d01493c19cc7d3..f34f6b67ef88f2 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -49,8 +49,6 @@
       forward_deployment_enabled { nil }
       restrict_user_defined_variables { nil }
       ci_job_token_scope_enabled { nil }
-
-      creating_via_ci_mirror { false } # prevents circular creation with :ci_project_mirror
     end
 
     after(:build) do |project, evaluator|
@@ -105,7 +103,7 @@
       end
 
       # simulating ::Projects::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
-      create(:ci_project_mirror, project: project) unless project.ci_project_mirror || evaluator.creating_via_ci_mirror
+      project.create_ci_project_mirror!(namespace_id: project.namespace_id) unless project.ci_project_mirror
     end
 
     trait :public do
diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb
index 0b9c4647c2dcd0..811ed18dce3e5c 100644
--- a/spec/factories_spec.rb
+++ b/spec/factories_spec.rb
@@ -69,8 +69,6 @@ def skipped_traits
   # is being mutated.
   skip_factory_defaults = %i[
     ci_job_token_project_scope_link
-    ci_namespace_mirror
-    ci_project_mirror
     evidence
     exported_protected_branch
     fork_network_member
diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb
index 237b8a5d3fe54b..fd0581341b73f8 100644
--- a/spec/models/ci/namespace_mirror_spec.rb
+++ b/spec/models/ci/namespace_mirror_spec.rb
@@ -8,6 +8,17 @@
   let!(:group3) { create(:group, parent: group2) }
   let!(:group4) { create(:group, parent: group3) }
 
+  before do
+    # refreshing ci mirrors according to the parent tree above
+    Namespaces::SyncEvent.find_each { |event| Ci::NamespaceMirror.sync!(event) }
+
+    # checking initial situation. we need to reload to reflect the changes of event sync
+    expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
+    expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id])
+    expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
+    expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id, group4.id])
+  end
+
   context 'scopes' do
     describe '.contains_namespace' do
       let_it_be(:another_group) { create(:group) }
@@ -23,31 +34,36 @@
       subject(:result) { described_class.by_namespace_id(group2.id) }
 
       it 'returns namesapce mirrors of namespace id' do
-        expect(result).to contain_exactly(group2.reload.ci_namespace_mirror)
+        expect(result).to contain_exactly(group2.ci_namespace_mirror)
       end
     end
   end
 
   describe '.sync!' do
-    subject(:sync) { described_class.sync!(namespace.sync_events.create!) }
+    subject(:sync) { described_class.sync!(Namespaces::SyncEvent.last) }
 
     context 'when namespace mirror does not exist in the first place' do
       let(:namespace) { group3 }
 
       before do
-        namespace.reload.ci_namespace_mirror.destroy!
+        namespace.ci_namespace_mirror.destroy!
+        namespace.sync_events.create!
       end
 
       it 'creates the mirror' do
         expect { sync }.to change { described_class.count }.from(3).to(4)
 
-        expect(namespace.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
+        expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
       end
     end
 
     context 'when namespace mirror does already exist' do
       let(:namespace) { group3 }
 
+      before do
+        namespace.sync_events.create!
+      end
+
       it 'updates the mirror' do
         expect { sync }.not_to change { described_class.count }
 
@@ -59,14 +75,7 @@
       let(:namespace) { group2 }
 
       before do
-        # check initial situation
-        expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
-        expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id])
-        expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
-        expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id, group4.id])
-
         group2.update!(parent: nil) # creates a sync event
-        group2.reload # guarantees traversal_ids updated
       end
 
       it 'updates traversal_ids for the base and descendants' do
diff --git a/spec/models/ci/project_mirror_spec.rb b/spec/models/ci/project_mirror_spec.rb
index 46e74a45f6201f..5ef520b4230c13 100644
--- a/spec/models/ci/project_mirror_spec.rb
+++ b/spec/models/ci/project_mirror_spec.rb
@@ -23,7 +23,7 @@
       subject(:result) { described_class.by_namespace_id(group2.id) }
 
       it 'returns project mirrors of namespace id' do
-        expect(result).to contain_exactly(project.reload.ci_project_mirror)
+        expect(result).to contain_exactly(project.ci_project_mirror)
       end
     end
   end
@@ -31,11 +31,11 @@
   describe '.sync!' do
     let!(:event) { Projects::SyncEvent.create!(project: project) }
 
-    subject(:sync) { described_class.sync!(event.reload) }
+    subject(:sync) { described_class.sync!(event) }
 
     context 'when project mirror does not exist in the first place' do
       before do
-        project.reload.ci_project_mirror.destroy!
+        project.ci_project_mirror.destroy!
       end
 
       it 'creates a ci_projects record' do
@@ -49,7 +49,7 @@
       it 'updates the related ci_projects record' do
         expect { sync }.not_to change { described_class.count }
 
-        expect(project.reload.ci_project_mirror).to have_attributes(namespace_id: group2.id)
+        expect(project.ci_project_mirror).to have_attributes(namespace_id: group2.id)
       end
     end
   end
-- 
GitLab


From 36f8bdd9a7f2aae4d43f707d3afcef8b3675703e Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Wed, 22 Dec 2021 11:24:53 +0300
Subject: [PATCH 7/8] Change FF milestone

---
 .../feature_flags/development/ci_find_runners_by_ci_mirrors.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml b/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml
index fd6d5d529e22ff..337e6b11408047 100644
--- a/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml
+++ b/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml
@@ -2,7 +2,7 @@
 name: ci_find_runners_by_ci_mirrors
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74900
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/347226
-milestone: '14.6'
+milestone: '14.7'
 type: development
 group: group::runner
 default_enabled: false
-- 
GitLab


From 63932daada141dd2f00229387e2d8a122f23d40b Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Wed, 22 Dec 2021 13:52:41 +0300
Subject: [PATCH 8/8] Fix namespace factory callback

---
 spec/factories/namespaces.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/spec/factories/namespaces.rb b/spec/factories/namespaces.rb
index 9e929909d8406f..e88bb634898b1a 100644
--- a/spec/factories/namespaces.rb
+++ b/spec/factories/namespaces.rb
@@ -13,7 +13,10 @@
 
     after(:create) do |namespace, evaluator|
       # simulating ::Namespaces::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
-      namespace.create_ci_namespace_mirror!(traversal_ids: namespace.traversal_ids) unless namespace.ci_namespace_mirror
+      # Note: we need to get refreshed `traversal_ids` it is updated via SQL query
+      #       in `Namespaces::Traversal::Linear#sync_traversal_ids` (see the NOTE in that method).
+      #       We cannot use `.reload` because it cleans other on-the-fly attributes.
+      namespace.create_ci_namespace_mirror!(traversal_ids: Namespace.find(namespace.id).traversal_ids) unless namespace.ci_namespace_mirror
     end
 
     trait :with_aggregation_schedule do
-- 
GitLab