From 3b03c0dfdc5535192aa82b0a9979a3c2a397f70e Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Wed, 2 Oct 2024 16:02:19 +0100
Subject: [PATCH 01/11] Remove error message if replication not enabled

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/491296

Changelog: changed
EE: true
---
 .../system_check/geo/database_replication_enabled_check.rb | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/ee/lib/system_check/geo/database_replication_enabled_check.rb b/ee/lib/system_check/geo/database_replication_enabled_check.rb
index 5879f8d4e77892..2b602638fbfe80 100644
--- a/ee/lib/system_check/geo/database_replication_enabled_check.rb
+++ b/ee/lib/system_check/geo/database_replication_enabled_check.rb
@@ -15,12 +15,7 @@ def check?
       end
 
       def show_error
-        try_fixing_it(
-          'Follow Geo setup instructions to configure primary and secondary nodes for replication'
-        )
-
-        docs_link = construct_help_page_url('administration/geo/setup/database')
-        for_more_information(docs_link)
+        nil
       end
 
       private
-- 
GitLab


From 9de533922638f43f168afcca0745157bdbfd6cdb Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Fri, 4 Oct 2024 15:33:04 +0100
Subject: [PATCH 02/11] Add feature flag yml

---
 .../wip/geo_postgresql_replication_agnostic.yml          | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 ee/config/feature_flags/wip/geo_postgresql_replication_agnostic.yml

diff --git a/ee/config/feature_flags/wip/geo_postgresql_replication_agnostic.yml b/ee/config/feature_flags/wip/geo_postgresql_replication_agnostic.yml
new file mode 100644
index 00000000000000..2278919d2b8814
--- /dev/null
+++ b/ee/config/feature_flags/wip/geo_postgresql_replication_agnostic.yml
@@ -0,0 +1,9 @@
+---
+name: geo_postgresql_replication_agnostic
+feature_issue_url: https://gitlab.com/groups/gitlab-org/-/epics/13721
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167925
+rollout_issue_url:
+milestone: '17.5'
+group: group::geo
+type: wip
+default_enabled: false
-- 
GitLab


From ea697ab874c99089b1152dd1061229ae2b59c93b Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Fri, 4 Oct 2024 15:45:29 +0100
Subject: [PATCH 03/11] Add feature flag logic

---
 .../geo/database_replication_enabled_check.rb            | 9 ++++++++-
 .../geo/database_replication_working_check.rb            | 3 ++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ee/lib/system_check/geo/database_replication_enabled_check.rb b/ee/lib/system_check/geo/database_replication_enabled_check.rb
index 2b602638fbfe80..c017b5465f8284 100644
--- a/ee/lib/system_check/geo/database_replication_enabled_check.rb
+++ b/ee/lib/system_check/geo/database_replication_enabled_check.rb
@@ -15,7 +15,14 @@ def check?
       end
 
       def show_error
-        nil
+        return if Feature.enabled?(:geo_postgresql_replication_agnostic, project)
+
+        try_fixing_it(
+          'Follow Geo setup instructions to configure primary and secondary nodes for replication'
+        )
+
+        docs_link = construct_help_page_url('administration/geo/setup/database')
+        for_more_information(docs_link)
       end
 
       private
diff --git a/ee/lib/system_check/geo/database_replication_working_check.rb b/ee/lib/system_check/geo/database_replication_working_check.rb
index 220c1f730a117d..f0baafcc6fe1ba 100644
--- a/ee/lib/system_check/geo/database_replication_working_check.rb
+++ b/ee/lib/system_check/geo/database_replication_working_check.rb
@@ -7,7 +7,8 @@ class DatabaseReplicationWorkingCheck < SystemCheck::BaseCheck
       set_skip_reason 'not a secondary node'
 
       def skip?
-        !Gitlab::Geo.secondary?
+        !Gitlab::Geo.secondary? ||
+          (Feature.enabled?(:geo_postgresql_replication_agnostic, project) && !geo_health_check.replication_enabled?)
       end
 
       def check?
-- 
GitLab


From d8a381fd4499a4f06af2f730f511bd6825fbbdef Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Sat, 5 Oct 2024 11:51:53 +0100
Subject: [PATCH 04/11] Add specs

---
 .../system_check/rake_task/geo_task_spec.rb   | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/ee/spec/lib/system_check/rake_task/geo_task_spec.rb b/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
index 49dc597f623db2..cc9995f2b55533 100644
--- a/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
+++ b/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
@@ -62,5 +62,30 @@
         expect(described_class.checks).to eq(secondary_checks)
       end
     end
+
+    context 'feature geo_postgresql_replication_agnostic is enabled' do
+      before do
+        Feature.enable(:geo_postgresql_replication_agnostic)
+      end
+
+      context 'replication is not enabled' do
+        before do
+          allow_next_instance_of(::Gitlab::Geo::HealthCheck).to receive(:replication_enabled?).and_return(false)
+        end
+
+        context 'DatabaseReplicationEnabledCheck' do
+          it 'does not log an error message' do
+            expect(SystemCheck::Helpers).not_to receive(:try_fixing_it)
+            expect(SystemCheck::Helpers).not_to receive(:for_more_information)
+          end
+        end
+
+        context 'DatabaseReplicationWorkingCheck' do
+          it 'skips checks' do
+            expect(SystemCheck::Helpers).not_to receive(:check?)
+          end
+        end
+      end
+    end
   end
 end
-- 
GitLab


From e9f3949ef857b6d6d51cd508ac9a0968e5a88193 Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Mon, 7 Oct 2024 18:43:19 +0100
Subject: [PATCH 05/11] Add specs

---
 .../geo/database_replication_enabled_check.rb |  2 +-
 .../geo/database_replication_working_check.rb |  3 +-
 .../system_check/rake_task/geo_task_spec.rb   | 59 +++++++++++++++++--
 3 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/ee/lib/system_check/geo/database_replication_enabled_check.rb b/ee/lib/system_check/geo/database_replication_enabled_check.rb
index c017b5465f8284..bf6e1ba0107f7d 100644
--- a/ee/lib/system_check/geo/database_replication_enabled_check.rb
+++ b/ee/lib/system_check/geo/database_replication_enabled_check.rb
@@ -15,7 +15,7 @@ def check?
       end
 
       def show_error
-        return if Feature.enabled?(:geo_postgresql_replication_agnostic, project)
+        return if Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip)
 
         try_fixing_it(
           'Follow Geo setup instructions to configure primary and secondary nodes for replication'
diff --git a/ee/lib/system_check/geo/database_replication_working_check.rb b/ee/lib/system_check/geo/database_replication_working_check.rb
index f0baafcc6fe1ba..47717930d548fd 100644
--- a/ee/lib/system_check/geo/database_replication_working_check.rb
+++ b/ee/lib/system_check/geo/database_replication_working_check.rb
@@ -8,7 +8,8 @@ class DatabaseReplicationWorkingCheck < SystemCheck::BaseCheck
 
       def skip?
         !Gitlab::Geo.secondary? ||
-          (Feature.enabled?(:geo_postgresql_replication_agnostic, project) && !geo_health_check.replication_enabled?)
+          (Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip) &&
+            !geo_health_check.replication_enabled?)
       end
 
       def check?
diff --git a/ee/spec/lib/system_check/rake_task/geo_task_spec.rb b/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
index cc9995f2b55533..23aebcb1220b0e 100644
--- a/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
+++ b/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
@@ -70,19 +70,66 @@
 
       context 'replication is not enabled' do
         before do
-          allow_next_instance_of(::Gitlab::Geo::HealthCheck).to receive(:replication_enabled?).and_return(false)
+          secondary = create(:geo_node)
+          stub_current_geo_node(secondary)
         end
 
         context 'DatabaseReplicationEnabledCheck' do
-          it 'does not log an error message' do
-            expect(SystemCheck::Helpers).not_to receive(:try_fixing_it)
-            expect(SystemCheck::Helpers).not_to receive(:for_more_information)
+          subject(:database_replication_enabled_check) { SystemCheck::Geo::DatabaseReplicationEnabledCheck.new }
+
+          it 'show_error does not display an error message' do
+            expect { database_replication_enabled_check.show_error }.not_to output(/Try fixing it/).to_stdout
+            expect { database_replication_enabled_check.show_error }.not_to output(/For more information/).to_stdout
           end
         end
 
         context 'DatabaseReplicationWorkingCheck' do
-          it 'skips checks' do
-            expect(SystemCheck::Helpers).not_to receive(:check?)
+          before do
+            allow_next_instance_of(Gitlab::Geo::HealthCheck) do |health_check|
+              allow(health_check).to receive(:replication_enabled?).and_return(false)
+            end
+          end
+
+          subject(:database_replication_working_check) { SystemCheck::Geo::DatabaseReplicationWorkingCheck.new }
+
+          it '.skip? returns true' do
+            expect(database_replication_working_check.skip?).to be(true)
+          end
+        end
+      end
+    end
+
+    context 'feature geo_postgresql_replication_agnostic is disabled' do
+      before do
+        Feature.disable(:geo_postgresql_replication_agnostic)
+      end
+
+      context 'replication is not enabled' do
+        before do
+          secondary = create(:geo_node)
+          stub_current_geo_node(secondary)
+        end
+
+        context 'DatabaseReplicationEnabledCheck' do
+          subject(:database_replication_enabled_check) { SystemCheck::Geo::DatabaseReplicationEnabledCheck.new }
+
+          it 'show_error does not display an error message' do
+            expect { database_replication_enabled_check.show_error }.to output(/Try fixing it/).to_stdout
+            expect { database_replication_enabled_check.show_error }.to output(/For more information/).to_stdout
+          end
+        end
+
+        context 'DatabaseReplicationWorkingCheck' do
+          before do
+            allow_next_instance_of(Gitlab::Geo::HealthCheck) do |health_check|
+              allow(health_check).to receive(:replication_enabled?).and_return(false)
+            end
+          end
+
+          subject(:database_replication_working_check) { SystemCheck::Geo::DatabaseReplicationWorkingCheck.new }
+
+          it '.skip? returns false' do
+            expect(database_replication_working_check.skip?).to be(false)
           end
         end
       end
-- 
GitLab


From 36fb93293fc4ff3918b4be63640de6448670789a Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Tue, 8 Oct 2024 18:01:12 +0100
Subject: [PATCH 06/11] Add additional skip reason

---
 .../geo/database_replication_working_check.rb         | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ee/lib/system_check/geo/database_replication_working_check.rb b/ee/lib/system_check/geo/database_replication_working_check.rb
index 47717930d548fd..77bcf43677ef5d 100644
--- a/ee/lib/system_check/geo/database_replication_working_check.rb
+++ b/ee/lib/system_check/geo/database_replication_working_check.rb
@@ -4,7 +4,7 @@ module SystemCheck
   module Geo
     class DatabaseReplicationWorkingCheck < SystemCheck::BaseCheck
       set_name 'Database replication working?'
-      set_skip_reason 'not a secondary node'
+      set_skip_reason skip_reason
 
       def skip?
         !Gitlab::Geo.secondary? ||
@@ -25,6 +25,15 @@ def show_error
         for_more_information(help_page)
       end
 
+      def skip_reason
+        if !Gitlab::Geo.secondary?
+          'not a secondary node'
+        elsif Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip) &&
+            !geo_health_check.replication_enabled?
+          'replication is disabled'
+        end
+      end
+
       private
 
       def geo_health_check
-- 
GitLab


From 77f9d3533b8d8baf823b4679071325c287c847cc Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Thu, 10 Oct 2024 18:38:33 +0100
Subject: [PATCH 07/11] Update tests

---
 ...database_replication_enabled_check_spec.rb | 33 ++++++++++++++++++
 ...database_replication_working_check_spec.rb | 34 +++++++++++++++++++
 2 files changed, 67 insertions(+)
 create mode 100644 ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
 create mode 100644 ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb

diff --git a/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
new file mode 100644
index 00000000000000..957759659ff75d
--- /dev/null
+++ b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SystemCheck::Geo::DatabaseReplicationEnabledCheck, :silence_stdout, feature_category: :geo_replication do
+  subject(:database_replication_enabled_check) { described_class.new }
+
+  describe '#check?', :reestablished_active_record_base do
+    before do
+      Feature.disable(:geo_postgresql_replication_agnostic)
+    end
+
+    it "checks database replication enabled" do
+      stub_database_state(replication_enabled: false)
+
+      expect(database_replication_enabled_check.check?).to be_falsey
+    end
+
+    it "returns true when all checks passed" do
+      stub_database_state
+
+      expect(database_replication_enabled_check).not_to receive(:try_fixing_it)
+
+      expect(database_replication_enabled_check.check?).to be_truthy
+    end
+  end
+
+  def stub_database_state(replication_enabled: true)
+    allow_next_instance_of(::Gitlab::Geo::HealthCheck) do |health_check|
+      allow(health_check).to receive(:replication_enabled?).and_return(replication_enabled)
+    end
+  end
+end
diff --git a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
new file mode 100644
index 00000000000000..428e15cbf26a3e
--- /dev/null
+++ b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SystemCheck::Geo::DatabaseReplicationWorkingCheck, :silence_stdout, feature_category: :geo_replication do
+  subject(:database_replication_working_check) { described_class.new }
+
+  describe '#check?', :reestablished_active_record_base do
+    before do
+      Feature.disable(:geo_postgresql_replication_agnostic)
+    end
+
+    it "checks database replication enabled" do
+      stub_database_state(replication_working: false)
+
+      expect(database_replication_working_check.check?).to be_falsey
+    end
+
+    it "returns true when all checks passed" do
+      stub_database_state
+
+      expect(database_replication_working_check).not_to receive(:try_fixing_it)
+
+      expect(database_replication_working_check.check?).to be_truthy
+    end
+  end
+
+  def stub_database_state(replication_enabled: true, replication_working: true)
+    allow_next_instance_of(::Gitlab::Geo::HealthCheck) do |health_check|
+      allow(health_check).to receive(:replication_enabled?).and_return(replication_enabled)
+      allow(health_check).to receive(:replication_working?).and_return(replication_working)
+    end
+  end
+end
-- 
GitLab


From dad718e3de814ead8dac58778a473289e4d66a53 Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Thu, 10 Oct 2024 19:42:47 +0100
Subject: [PATCH 08/11] Update tests

---
 ...database_replication_enabled_check_spec.rb | 21 ++++++++++++++++
 ...database_replication_working_check_spec.rb | 25 +++++++++++++++++--
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
index 957759659ff75d..a5caec68bce054 100644
--- a/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
+++ b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
@@ -23,6 +23,27 @@
 
       expect(database_replication_enabled_check.check?).to be_truthy
     end
+
+    context 'when flag geo_postgresql_replication_agnostic is enabled' do
+      before do
+        Feature.enable(:geo_postgresql_replication_agnostic)
+      end
+
+      it "checks database replication enabled without raising error messages" do
+        stub_database_state(replication_enabled: false)
+
+        expect(database_replication_enabled_check).not_to receive(:try_fixing_it)
+        expect(database_replication_enabled_check).not_to receive(:for_more_information)
+
+        expect(database_replication_enabled_check.check?).to be_falsey
+      end
+
+      it "returns true when all checks passed" do
+        stub_database_state
+
+        expect(database_replication_enabled_check.check?).to be_truthy
+      end
+    end
   end
 
   def stub_database_state(replication_enabled: true)
diff --git a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
index 428e15cbf26a3e..d2a925fff411ff 100644
--- a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
+++ b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
@@ -10,8 +10,8 @@
       Feature.disable(:geo_postgresql_replication_agnostic)
     end
 
-    it "checks database replication enabled" do
-      stub_database_state(replication_working: false)
+    it "checks database replication is working" do
+      stub_database_state(replication_enabled: false, replication_working: false)
 
       expect(database_replication_working_check.check?).to be_falsey
     end
@@ -23,6 +23,27 @@
 
       expect(database_replication_working_check.check?).to be_truthy
     end
+
+    context 'when flag geo_postgresql_replication_agnostic is enabled' do
+      before do
+        Feature.enable(:geo_postgresql_replication_agnostic)
+      end
+
+      it "checks database replication enabled without raising error messages" do
+        stub_database_state(replication_enabled: false, replication_working: false)
+
+        expect(database_replication_working_check).not_to receive(:try_fixing_it)
+        expect(database_replication_working_check).not_to receive(:for_more_information)
+
+        expect(database_replication_working_check.check?).to be_falsey
+      end
+
+      it "returns true when all checks passed" do
+        stub_database_state
+
+        expect(database_replication_working_check.check?).to be_truthy
+      end
+    end
   end
 
   def stub_database_state(replication_enabled: true, replication_working: true)
-- 
GitLab


From 1d17cbb2ad1c9e65c884bff7fe6edb197726c50e Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Fri, 11 Oct 2024 13:03:31 +0100
Subject: [PATCH 09/11] Update tests

---
 .../geo/database_replication_working_check.rb |  2 +-
 ...database_replication_enabled_check_spec.rb | 51 +++++++++----
 ...database_replication_working_check_spec.rb | 63 +++++++++++++---
 .../system_check/rake_task/geo_task_spec.rb   | 72 -------------------
 4 files changed, 93 insertions(+), 95 deletions(-)

diff --git a/ee/lib/system_check/geo/database_replication_working_check.rb b/ee/lib/system_check/geo/database_replication_working_check.rb
index 77bcf43677ef5d..64f360e2b3856a 100644
--- a/ee/lib/system_check/geo/database_replication_working_check.rb
+++ b/ee/lib/system_check/geo/database_replication_working_check.rb
@@ -30,7 +30,7 @@ def skip_reason
           'not a secondary node'
         elsif Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip) &&
             !geo_health_check.replication_enabled?
-          'replication is disabled'
+          'database replication is disabled'
         end
       end
 
diff --git a/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
index a5caec68bce054..bf9727d13e94d1 100644
--- a/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
+++ b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
@@ -5,40 +5,35 @@
 RSpec.describe SystemCheck::Geo::DatabaseReplicationEnabledCheck, :silence_stdout, feature_category: :geo_replication do
   subject(:database_replication_enabled_check) { described_class.new }
 
-  describe '#check?', :reestablished_active_record_base do
+  describe '#check?' do
     before do
-      Feature.disable(:geo_postgresql_replication_agnostic)
+      stub_feature_flags(geo_postgresql_replication_agnostic: false)
     end
 
-    it "checks database replication enabled" do
+    it "returns false when replication is disabled" do
       stub_database_state(replication_enabled: false)
 
       expect(database_replication_enabled_check.check?).to be_falsey
     end
 
-    it "returns true when all checks passed" do
+    it "returns true when replication is enabled" do
       stub_database_state
 
-      expect(database_replication_enabled_check).not_to receive(:try_fixing_it)
-
       expect(database_replication_enabled_check.check?).to be_truthy
     end
 
-    context 'when flag geo_postgresql_replication_agnostic is enabled' do
+    context 'with geo_postgresql_replication_agnostic enabled' do
       before do
-        Feature.enable(:geo_postgresql_replication_agnostic)
+        stub_feature_flags(geo_postgresql_replication_agnostic: true)
       end
 
-      it "checks database replication enabled without raising error messages" do
+      it "returns false when replication is disabled" do
         stub_database_state(replication_enabled: false)
 
-        expect(database_replication_enabled_check).not_to receive(:try_fixing_it)
-        expect(database_replication_enabled_check).not_to receive(:for_more_information)
-
         expect(database_replication_enabled_check.check?).to be_falsey
       end
 
-      it "returns true when all checks passed" do
+      it "returns true when replication is enabled" do
         stub_database_state
 
         expect(database_replication_enabled_check.check?).to be_truthy
@@ -46,6 +41,36 @@
     end
   end
 
+  describe '#show_error' do
+    before do
+      stub_feature_flags(geo_postgresql_replication_agnostic: false)
+    end
+
+    it 'shows errors when replication is disabled' do
+      stub_database_state(replication_enabled: false)
+
+      expect(database_replication_enabled_check).to receive(:try_fixing_it)
+      expect(database_replication_enabled_check).to receive(:for_more_information)
+
+      database_replication_enabled_check.show_error
+    end
+
+    context 'with geo_postgresql_replication_agnostic enabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: true)
+      end
+
+      it 'does not show errors when replication is disabled' do
+        stub_database_state(replication_enabled: false)
+
+        expect(database_replication_enabled_check).not_to receive(:try_fixing_it)
+        expect(database_replication_enabled_check).not_to receive(:for_more_information)
+
+        database_replication_enabled_check.show_error
+      end
+    end
+  end
+
   def stub_database_state(replication_enabled: true)
     allow_next_instance_of(::Gitlab::Geo::HealthCheck) do |health_check|
       allow(health_check).to receive(:replication_enabled?).and_return(replication_enabled)
diff --git a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
index d2a925fff411ff..9adef448b9412b 100644
--- a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
+++ b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
@@ -3,11 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe SystemCheck::Geo::DatabaseReplicationWorkingCheck, :silence_stdout, feature_category: :geo_replication do
+  include ::EE::GeoHelpers
+
   subject(:database_replication_working_check) { described_class.new }
 
-  describe '#check?', :reestablished_active_record_base do
+  describe '#check?' do
     before do
-      Feature.disable(:geo_postgresql_replication_agnostic)
+      stub_feature_flags(geo_postgresql_replication_agnostic: false)
     end
 
     it "checks database replication is working" do
@@ -19,22 +21,17 @@
     it "returns true when all checks passed" do
       stub_database_state
 
-      expect(database_replication_working_check).not_to receive(:try_fixing_it)
-
       expect(database_replication_working_check.check?).to be_truthy
     end
 
-    context 'when flag geo_postgresql_replication_agnostic is enabled' do
+    context 'with geo_postgresql_replication_agnostic enabled' do
       before do
-        Feature.enable(:geo_postgresql_replication_agnostic)
+        stub_feature_flags(geo_postgresql_replication_agnostic: true)
       end
 
       it "checks database replication enabled without raising error messages" do
         stub_database_state(replication_enabled: false, replication_working: false)
 
-        expect(database_replication_working_check).not_to receive(:try_fixing_it)
-        expect(database_replication_working_check).not_to receive(:for_more_information)
-
         expect(database_replication_working_check.check?).to be_falsey
       end
 
@@ -46,6 +43,54 @@
     end
   end
 
+  describe '#skip?' do
+    before do
+      stub_feature_flags(geo_postgresql_replication_agnostic: false)
+    end
+
+    it 'returns false when node is not a secondary' do
+      expect(database_replication_working_check.skip?).to be_truthy
+    end
+
+    context 'with geo_postgresql_replication_agnostic enabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: true)
+      end
+
+      it 'returns true when replication is not enabled' do
+        stub_database_state(replication_enabled: false)
+
+        expect(database_replication_working_check.skip?).to be_truthy
+      end
+    end
+  end
+
+  describe '#skip_reason' do
+    before do
+      stub_feature_flags(geo_postgresql_replication_agnostic: false)
+    end
+
+    it 'returns a message when not a secondary' do
+      stub_database_state(replication_enabled: false)
+
+      expect(database_replication_working_check.skip_reason).to eq('not a secondary node')
+    end
+
+    context 'with geo_postgresql_replication_agnostic enabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: true)
+      end
+
+      it 'returns a message when replication is not enabled' do
+        secondary = create(:geo_node)
+        stub_current_geo_node(secondary)
+        stub_database_state(replication_enabled: false)
+
+        expect(database_replication_working_check.skip_reason).to eq('database replication is disabled')
+      end
+    end
+  end
+
   def stub_database_state(replication_enabled: true, replication_working: true)
     allow_next_instance_of(::Gitlab::Geo::HealthCheck) do |health_check|
       allow(health_check).to receive(:replication_enabled?).and_return(replication_enabled)
diff --git a/ee/spec/lib/system_check/rake_task/geo_task_spec.rb b/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
index 23aebcb1220b0e..49dc597f623db2 100644
--- a/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
+++ b/ee/spec/lib/system_check/rake_task/geo_task_spec.rb
@@ -62,77 +62,5 @@
         expect(described_class.checks).to eq(secondary_checks)
       end
     end
-
-    context 'feature geo_postgresql_replication_agnostic is enabled' do
-      before do
-        Feature.enable(:geo_postgresql_replication_agnostic)
-      end
-
-      context 'replication is not enabled' do
-        before do
-          secondary = create(:geo_node)
-          stub_current_geo_node(secondary)
-        end
-
-        context 'DatabaseReplicationEnabledCheck' do
-          subject(:database_replication_enabled_check) { SystemCheck::Geo::DatabaseReplicationEnabledCheck.new }
-
-          it 'show_error does not display an error message' do
-            expect { database_replication_enabled_check.show_error }.not_to output(/Try fixing it/).to_stdout
-            expect { database_replication_enabled_check.show_error }.not_to output(/For more information/).to_stdout
-          end
-        end
-
-        context 'DatabaseReplicationWorkingCheck' do
-          before do
-            allow_next_instance_of(Gitlab::Geo::HealthCheck) do |health_check|
-              allow(health_check).to receive(:replication_enabled?).and_return(false)
-            end
-          end
-
-          subject(:database_replication_working_check) { SystemCheck::Geo::DatabaseReplicationWorkingCheck.new }
-
-          it '.skip? returns true' do
-            expect(database_replication_working_check.skip?).to be(true)
-          end
-        end
-      end
-    end
-
-    context 'feature geo_postgresql_replication_agnostic is disabled' do
-      before do
-        Feature.disable(:geo_postgresql_replication_agnostic)
-      end
-
-      context 'replication is not enabled' do
-        before do
-          secondary = create(:geo_node)
-          stub_current_geo_node(secondary)
-        end
-
-        context 'DatabaseReplicationEnabledCheck' do
-          subject(:database_replication_enabled_check) { SystemCheck::Geo::DatabaseReplicationEnabledCheck.new }
-
-          it 'show_error does not display an error message' do
-            expect { database_replication_enabled_check.show_error }.to output(/Try fixing it/).to_stdout
-            expect { database_replication_enabled_check.show_error }.to output(/For more information/).to_stdout
-          end
-        end
-
-        context 'DatabaseReplicationWorkingCheck' do
-          before do
-            allow_next_instance_of(Gitlab::Geo::HealthCheck) do |health_check|
-              allow(health_check).to receive(:replication_enabled?).and_return(false)
-            end
-          end
-
-          subject(:database_replication_working_check) { SystemCheck::Geo::DatabaseReplicationWorkingCheck.new }
-
-          it '.skip? returns false' do
-            expect(database_replication_working_check.skip?).to be(false)
-          end
-        end
-      end
-    end
   end
 end
-- 
GitLab


From 3102095e928eca63c111c78000f052ac6d31ff2f Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Fri, 11 Oct 2024 15:04:34 +0100
Subject: [PATCH 10/11] Update tests

---
 ee/lib/gitlab/geo.rb                                      | 6 ++++++
 .../geo/database_replication_enabled_check.rb             | 2 +-
 .../geo/database_replication_working_check.rb             | 6 ++----
 ee/spec/lib/gitlab/geo_spec.rb                            | 8 ++++++++
 .../geo/database_replication_working_check_spec.rb        | 8 ++++++--
 5 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/ee/lib/gitlab/geo.rb b/ee/lib/gitlab/geo.rb
index 0318bf83267d41..3f236c60c1f3bc 100644
--- a/ee/lib/gitlab/geo.rb
+++ b/ee/lib/gitlab/geo.rb
@@ -344,5 +344,11 @@ def self.primary_pipeline_refs(project_id)
 
       results['pipeline_refs']
     end
+
+    # Checks if the Feature flag `geo_postgresql_replication_agnostic` is enabled.
+    # @return [Boolean] whether the feature flag is enabled or not.
+    def self.postgresql_replication_agnostic_enabled?
+      Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip)
+    end
   end
 end
diff --git a/ee/lib/system_check/geo/database_replication_enabled_check.rb b/ee/lib/system_check/geo/database_replication_enabled_check.rb
index bf6e1ba0107f7d..1ac3d86befbbf5 100644
--- a/ee/lib/system_check/geo/database_replication_enabled_check.rb
+++ b/ee/lib/system_check/geo/database_replication_enabled_check.rb
@@ -15,7 +15,7 @@ def check?
       end
 
       def show_error
-        return if Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip)
+        return if Gitlab::Geo.postgresql_replication_agnostic_enabled?
 
         try_fixing_it(
           'Follow Geo setup instructions to configure primary and secondary nodes for replication'
diff --git a/ee/lib/system_check/geo/database_replication_working_check.rb b/ee/lib/system_check/geo/database_replication_working_check.rb
index 64f360e2b3856a..b207e2d3b9e71c 100644
--- a/ee/lib/system_check/geo/database_replication_working_check.rb
+++ b/ee/lib/system_check/geo/database_replication_working_check.rb
@@ -8,8 +8,7 @@ class DatabaseReplicationWorkingCheck < SystemCheck::BaseCheck
 
       def skip?
         !Gitlab::Geo.secondary? ||
-          (Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip) &&
-            !geo_health_check.replication_enabled?)
+          (Gitlab::Geo.postgresql_replication_agnostic_enabled? && !geo_health_check.replication_enabled?)
       end
 
       def check?
@@ -28,8 +27,7 @@ def show_error
       def skip_reason
         if !Gitlab::Geo.secondary?
           'not a secondary node'
-        elsif Feature.enabled?(:geo_postgresql_replication_agnostic, :instance, type: :wip) &&
-            !geo_health_check.replication_enabled?
+        elsif Gitlab::Geo.postgresql_replication_agnostic_enabled? && !geo_health_check.replication_enabled?
           'database replication is disabled'
         end
       end
diff --git a/ee/spec/lib/gitlab/geo_spec.rb b/ee/spec/lib/gitlab/geo_spec.rb
index b37e8ec904da93..a1dc6d94637a39 100644
--- a/ee/spec/lib/gitlab/geo_spec.rb
+++ b/ee/spec/lib/gitlab/geo_spec.rb
@@ -911,4 +911,12 @@
       expect(described_class.primary_pipeline_refs(project_id)).to eq([])
     end
   end
+
+  describe '.postgresql_replication_agnostic_enabled?' do
+    it 'returns true when the feature flag is enabled' do
+      stub_feature_flags(geo_postgresql_replication_agnostic: true)
+
+      expect(described_class.postgresql_replication_agnostic_enabled?).to be_truthy
+    end
+  end
 end
diff --git a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
index 9adef448b9412b..d340128f674ec3 100644
--- a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
+++ b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
@@ -24,7 +24,7 @@
       expect(database_replication_working_check.check?).to be_truthy
     end
 
-    context 'with geo_postgresql_replication_agnostic enabled' do
+    context 'with geo_postgresql_replication_agnostic feature flag enabled' do
       before do
         stub_feature_flags(geo_postgresql_replication_agnostic: true)
       end
@@ -49,10 +49,12 @@
     end
 
     it 'returns false when node is not a secondary' do
+      primary = create(:geo_node, :primary)
+      stub_current_geo_node(primary)
       expect(database_replication_working_check.skip?).to be_truthy
     end
 
-    context 'with geo_postgresql_replication_agnostic enabled' do
+    context 'with geo_postgresql_replication_agnostic feature flag enabled' do
       before do
         stub_feature_flags(geo_postgresql_replication_agnostic: true)
       end
@@ -71,6 +73,8 @@
     end
 
     it 'returns a message when not a secondary' do
+      primary = create(:geo_node, :primary)
+      stub_current_geo_node(primary)
       stub_database_state(replication_enabled: false)
 
       expect(database_replication_working_check.skip_reason).to eq('not a secondary node')
-- 
GitLab


From e2e8c252d83e8252134a92e45a853e83b2a102d0 Mon Sep 17 00:00:00 2001
From: Scott Murray <smurray@gitlab.com>
Date: Tue, 15 Oct 2024 15:07:01 +0100
Subject: [PATCH 11/11] Update spec contexts

---
 .../geo/database_replication_working_check.rb |  9 ++-
 ee/spec/lib/gitlab/geo_spec.rb                |  6 +-
 ...database_replication_enabled_check_spec.rb | 40 +++++++------
 ...database_replication_working_check_spec.rb | 56 ++++++++++---------
 4 files changed, 62 insertions(+), 49 deletions(-)

diff --git a/ee/lib/system_check/geo/database_replication_working_check.rb b/ee/lib/system_check/geo/database_replication_working_check.rb
index b207e2d3b9e71c..0b6ba087c58f3c 100644
--- a/ee/lib/system_check/geo/database_replication_working_check.rb
+++ b/ee/lib/system_check/geo/database_replication_working_check.rb
@@ -7,8 +7,7 @@ class DatabaseReplicationWorkingCheck < SystemCheck::BaseCheck
       set_skip_reason skip_reason
 
       def skip?
-        !Gitlab::Geo.secondary? ||
-          (Gitlab::Geo.postgresql_replication_agnostic_enabled? && !geo_health_check.replication_enabled?)
+        !Gitlab::Geo.secondary? || database_replication_disabled?
       end
 
       def check?
@@ -27,13 +26,17 @@ def show_error
       def skip_reason
         if !Gitlab::Geo.secondary?
           'not a secondary node'
-        elsif Gitlab::Geo.postgresql_replication_agnostic_enabled? && !geo_health_check.replication_enabled?
+        elsif database_replication_disabled?
           'database replication is disabled'
         end
       end
 
       private
 
+      def database_replication_disabled?
+        Gitlab::Geo.postgresql_replication_agnostic_enabled? && !geo_health_check.replication_enabled?
+      end
+
       def geo_health_check
         @geo_health_check ||= Gitlab::Geo::HealthCheck.new
       end
diff --git a/ee/spec/lib/gitlab/geo_spec.rb b/ee/spec/lib/gitlab/geo_spec.rb
index a1dc6d94637a39..cbdc2bd4ede604 100644
--- a/ee/spec/lib/gitlab/geo_spec.rb
+++ b/ee/spec/lib/gitlab/geo_spec.rb
@@ -913,10 +913,10 @@
   end
 
   describe '.postgresql_replication_agnostic_enabled?' do
-    it 'returns true when the feature flag is enabled' do
-      stub_feature_flags(geo_postgresql_replication_agnostic: true)
+    it 'returns false when the feature flag is disabled' do
+      stub_feature_flags(geo_postgresql_replication_agnostic: false)
 
-      expect(described_class.postgresql_replication_agnostic_enabled?).to be_truthy
+      expect(described_class.postgresql_replication_agnostic_enabled?).to be_falsey
     end
   end
 end
diff --git a/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
index bf9727d13e94d1..a77606b12cf1d7 100644
--- a/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
+++ b/ee/spec/lib/system_check/geo/database_replication_enabled_check_spec.rb
@@ -6,20 +6,22 @@
   subject(:database_replication_enabled_check) { described_class.new }
 
   describe '#check?' do
-    before do
-      stub_feature_flags(geo_postgresql_replication_agnostic: false)
-    end
+    context  'with geo_postgresql_replication_agnostic disabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: false)
+      end
 
-    it "returns false when replication is disabled" do
-      stub_database_state(replication_enabled: false)
+      it "returns false when replication is disabled" do
+        stub_database_state(replication_enabled: false)
 
-      expect(database_replication_enabled_check.check?).to be_falsey
-    end
+        expect(database_replication_enabled_check.check?).to be_falsey
+      end
 
-    it "returns true when replication is enabled" do
-      stub_database_state
+      it "returns true when replication is enabled" do
+        stub_database_state
 
-      expect(database_replication_enabled_check.check?).to be_truthy
+        expect(database_replication_enabled_check.check?).to be_truthy
+      end
     end
 
     context 'with geo_postgresql_replication_agnostic enabled' do
@@ -42,17 +44,19 @@
   end
 
   describe '#show_error' do
-    before do
-      stub_feature_flags(geo_postgresql_replication_agnostic: false)
-    end
+    context 'with geo_postgresql_replication_agnostic disabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: false)
+      end
 
-    it 'shows errors when replication is disabled' do
-      stub_database_state(replication_enabled: false)
+      it 'shows errors when replication is disabled' do
+        stub_database_state(replication_enabled: false)
 
-      expect(database_replication_enabled_check).to receive(:try_fixing_it)
-      expect(database_replication_enabled_check).to receive(:for_more_information)
+        expect(database_replication_enabled_check).to receive(:try_fixing_it)
+        expect(database_replication_enabled_check).to receive(:for_more_information)
 
-      database_replication_enabled_check.show_error
+        database_replication_enabled_check.show_error
+      end
     end
 
     context 'with geo_postgresql_replication_agnostic enabled' do
diff --git a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
index d340128f674ec3..d41a46b4d2b850 100644
--- a/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
+++ b/ee/spec/lib/system_check/geo/database_replication_working_check_spec.rb
@@ -8,20 +8,22 @@
   subject(:database_replication_working_check) { described_class.new }
 
   describe '#check?' do
-    before do
-      stub_feature_flags(geo_postgresql_replication_agnostic: false)
-    end
+    context  'with geo_postgresql_replication_agnostic disabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: false)
+      end
 
-    it "checks database replication is working" do
-      stub_database_state(replication_enabled: false, replication_working: false)
+      it "checks database replication is working" do
+        stub_database_state(replication_enabled: false, replication_working: false)
 
-      expect(database_replication_working_check.check?).to be_falsey
-    end
+        expect(database_replication_working_check.check?).to be_falsey
+      end
 
-    it "returns true when all checks passed" do
-      stub_database_state
+      it "returns true when all checks passed" do
+        stub_database_state
 
-      expect(database_replication_working_check.check?).to be_truthy
+        expect(database_replication_working_check.check?).to be_truthy
+      end
     end
 
     context 'with geo_postgresql_replication_agnostic feature flag enabled' do
@@ -44,14 +46,16 @@
   end
 
   describe '#skip?' do
-    before do
-      stub_feature_flags(geo_postgresql_replication_agnostic: false)
-    end
+    context 'with geo_postgresql_replication_agnostic disabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: false)
+      end
 
-    it 'returns false when node is not a secondary' do
-      primary = create(:geo_node, :primary)
-      stub_current_geo_node(primary)
-      expect(database_replication_working_check.skip?).to be_truthy
+      it 'returns false when node is not a secondary' do
+        primary = create(:geo_node, :primary)
+        stub_current_geo_node(primary)
+        expect(database_replication_working_check.skip?).to be_truthy
+      end
     end
 
     context 'with geo_postgresql_replication_agnostic feature flag enabled' do
@@ -68,16 +72,18 @@
   end
 
   describe '#skip_reason' do
-    before do
-      stub_feature_flags(geo_postgresql_replication_agnostic: false)
-    end
+    context 'with geo_postgresql_replication_agnostic disabled' do
+      before do
+        stub_feature_flags(geo_postgresql_replication_agnostic: false)
+      end
 
-    it 'returns a message when not a secondary' do
-      primary = create(:geo_node, :primary)
-      stub_current_geo_node(primary)
-      stub_database_state(replication_enabled: false)
+      it 'returns a message when not a secondary' do
+        primary = create(:geo_node, :primary)
+        stub_current_geo_node(primary)
+        stub_database_state(replication_enabled: false)
 
-      expect(database_replication_working_check.skip_reason).to eq('not a secondary node')
+        expect(database_replication_working_check.skip_reason).to eq('not a secondary node')
+      end
     end
 
     context 'with geo_postgresql_replication_agnostic enabled' do
-- 
GitLab