Merge branch...

Merge branch '2031-geo-gdk-doctor-is-throwing-a-warning-when-geo-is-enabled-in-primary-gdk' into 'main'

Geo: `gdk doctor` is throwing a warning when Geo is enabled in primary GDK

Closes #2031

See merge request https://gitlab.com/gitlab-org/gitlab-development-kit/-/merge_requests/3543



Merged-by: Tiger Watson <twatson@gitlab.com>
Approved-by: David Dieulivol <ddieulivol@gitlab.com>
Approved-by: Tiger Watson <twatson@gitlab.com>
Reviewed-by: Tiger Watson <twatson@gitlab.com>
Co-authored-by: Javiera Tapia <jtapia@gitlab.com>

lib/gdk/diagnostic/geo.rb

@@ -6,21 +6,36 @@ class Geo < Base
       TITLE = 'Geo'
 
       def success?
-        @success ||= (geo_database_exists? && geo_enabled?) || (!geo_database_exists? && !geo_enabled?)
+        @success ||= if geo_primary?
+                       geo_enabled?
+                     elsif geo_secondary?
+                       geo_enabled? && geo_database_exists?
+                     else
+                       (!geo_enabled? && !geo_database_exists?)
+                     end
       end
 
       def detail
         return if success?
 
-        <<~MESSAGE
-          #{database_yml_file} contains the geo database settings but
-          geo.enabled is not set to true in your gdk.yml.
+        if geo_primary?
+          <<~MESSAGE
+            GDK could be a Geo primary node. However, geo.enabled is not set to true in your gdk.yml.
+            Update your gdk.yml to set geo.enabled to true.
 
-          Either update your gdk.yml to set geo.enabled to true or remove
-          the geo database settings from #{database_yml_file}
+            #{geo_howto_url}
+          MESSAGE
+        elsif geo_secondary?
+          <<~MESSAGE
+            GDK is a Geo secondary node. #{database_yml_file} contains the geo database settings but
+            geo.enabled is not set to true in your gdk.yml.
 
-          #{geo_howto_url}
-        MESSAGE
+            Either update your gdk.yml to set geo.enabled to true or remove
+            the geo database settings from #{database_yml_file}
+
+            #{geo_howto_url}
+          MESSAGE
+        end
       end
 
       private
@@ -29,6 +44,14 @@ def geo_enabled?
         config.geo.enabled
       end
 
+      def geo_secondary?
+        config.geo.secondary
+      end
+
+      def geo_primary?
+        !geo_secondary?
+      end
+
       def database_yml_file
         @database_yml_file ||= config.gitlab.dir.join('config', 'database.yml').expand_path.to_s
       end

spec/lib/gdk/diagnostic/geo_spec.rb

 # frozen_string_literal: true
 
 RSpec.describe GDK::Diagnostic::Geo do
+  subject(:geo_diagnostic) { described_class.new }
+
   let(:database_yml_file) { '/home/git/gdk/gitlab/config/database.yml' }
 
   let(:default_content) do
@@ -43,48 +45,55 @@
     CONTENT
   end
 
+  shared_examples 'with Geo diagnostic success' do |geo_enabled, expected_result, database_settings = nil|
+    it "returns #{expected_result}" do
+      stub_geo_enabled(geo_enabled)
+      stub_database_yml_content(database_settings) if database_settings
+
+      expect(geo_diagnostic.success?).to eq(expected_result)
+    end
+  end
+
   describe '#success?' do
-    context "when Geo database settings doesn't exist" do
+    context 'with GDK primary' do
       before do
-        stub_database_yml_content(default_content)
+        stub_geo_primary
       end
 
-      context 'and geo.enabled is set to false' do
-        it 'returns true' do
-          stub_geo_enabled(false)
-
-          expect(subject.success?).to be_truthy
-        end
+      context 'when geo.enabled is set to true' do
+        include_examples 'with Geo diagnostic success', true, true
       end
 
-      context 'and geo.enabled is set to true' do
-        it 'returns false' do
-          stub_geo_enabled(true)
-
-          expect(subject.success?).to be_falsy
-        end
+      context 'when geo.enabled is set to false' do
+        include_examples 'with Geo diagnostic success', false, false
       end
     end
 
-    context 'when Geo database settings does exist' do
+    context 'with GDK secondary' do
       before do
-        stub_database_yml_content(geo_content)
+        stub_geo_secondary
       end
 
-      context 'and geo.enabled is set to false' do
-        it 'returns false' do
-          stub_geo_enabled(false)
+      context 'when geo.enabled is set to true' do
+        context 'with Geo database settings' do
+          it_behaves_like 'with Geo diagnostic success' do
+            let(:geo_enabled) { true }
+            let(:expected_result) { true }
+            let(:database_settings) { geo_content }
+          end
+        end
 
-          expect(subject.success?).to be_falsy
+        context 'without Geo database settings' do
+          it_behaves_like 'with Geo diagnostic success' do
+            let(:geo_enabled) { true }
+            let(:expected_result) { false }
+            let(:database_settings) { default_content }
+          end
         end
       end
 
-      context 'and geo.enabled is set to true' do
-        it 'returns true' do
-          stub_geo_enabled(true)
-
-          expect(subject.success?).to be_truthy
-        end
+      context 'when geo.enabled is set to false' do
+        include_examples 'with Geo diagnostic success', false, false
       end
     end
   end
@@ -93,32 +102,57 @@
     let(:success) { nil }
 
     before do
-      allow(subject).to receive(:success?).and_return(success)
+      allow(geo_diagnostic).to receive(:success?).and_return(success)
     end
 
     context 'when #success? returns true' do
       let(:success) { true }
 
       it 'returns nil' do
-        expect(subject.detail).to be_nil
+        expect(geo_diagnostic.detail).to be_nil
       end
     end
 
     context 'when #success? returns false' do
-      let(:success) { false }
+      context 'with GDK primary' do
+        let(:success) { false }
 
-      it 'returns a message advising how to detail with the situation' do
-        expected_detail = <<~MESSAGE
-          #{database_yml_file} contains the geo database settings but
-          geo.enabled is not set to true in your gdk.yml.
+        before do
+          stub_geo_primary
+        end
 
-          Either update your gdk.yml to set geo.enabled to true or remove
-          the geo database settings from #{database_yml_file}
+        it 'returns a message advising how to detail with the situation' do
+          expected_detail = <<~MESSAGE
+            GDK could be a Geo primary node. However, geo.enabled is not set to true in your gdk.yml.
+            Update your gdk.yml to set geo.enabled to true.
 
-          https://gitlab.com/gitlab-org/gitlab-development-kit/blob/main/doc/howto/geo.md
-        MESSAGE
+            https://gitlab.com/gitlab-org/gitlab-development-kit/blob/main/doc/howto/geo.md
+          MESSAGE
 
-        expect(subject.detail).to eq(expected_detail)
+          expect(geo_diagnostic.detail).to eq(expected_detail)
+        end
+      end
+
+      context 'with GDK secondary' do
+        let(:success) { false }
+
+        before do
+          stub_geo_secondary
+        end
+
+        it 'returns a message advising how to detail with the situation' do
+          expected_detail = <<~MESSAGE
+            GDK is a Geo secondary node. #{database_yml_file} contains the geo database settings but
+            geo.enabled is not set to true in your gdk.yml.
+
+            Either update your gdk.yml to set geo.enabled to true or remove
+            the geo database settings from #{database_yml_file}
+
+            https://gitlab.com/gitlab-org/gitlab-development-kit/blob/main/doc/howto/geo.md
+          MESSAGE
+
+          expect(geo_diagnostic.detail).to eq(expected_detail)
+        end
       end
     end
   end
@@ -132,7 +166,14 @@ def stub_database_yml_content(content)
   end
 
   def stub_geo_enabled(enabled)
-    gdk_geo_config = double('geo config', enabled: enabled) # rubocop:todo RSpec/VerifiedDoubles
-    allow_any_instance_of(GDK::Config).to receive(:geo).and_return(gdk_geo_config)
+    allow_any_instance_of(GDK::Config).to receive_message_chain('geo.enabled').and_return(enabled)
+  end
+
+  def stub_geo_primary
+    allow_any_instance_of(GDK::Config).to receive_message_chain('geo.secondary').and_return(nil)
+  end
+
+  def stub_geo_secondary
+    allow_any_instance_of(GDK::Config).to receive_message_chain('geo.secondary').and_return(true)
   end
 end