Merge branch 'handle-redis-not-there' into 'master'

Fix database migrations when Redis is not running

If Redis were not running or USE_DB were set to false, the application settings retrieval would fail completely. This change only attempts to use the cache if the system actually
wants to connect to the DB and rescues any failures in talking to Redis.

Closes #17557 


See merge request !4924

2 files changed, 43 insertions, 3 deletions

diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 28c34429c1f..54b46e5d23f 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -9,10 +9,14 @@ module Gitlab
     end
 
     def ensure_application_settings!
-      settings = ::ApplicationSetting.cached
+      if connect_to_db?
+        begin
+          settings = ::ApplicationSetting.current
+        # In case Redis isn't running or the Redis UNIX socket file is not available
+        rescue ::Redis::BaseError, ::Errno::ENOENT
+          settings = ::ApplicationSetting.last
+        end
 
-      if !settings && connect_to_db?
-        settings = ::ApplicationSetting.current
         settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration?
       end
 
diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb
new file mode 100644
index 00000000000..004341ffd02
--- /dev/null
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe Gitlab::CurrentSettings do
+  describe '#current_application_settings' do
+    it 'attempts to use cached values first' do
+      allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true)
+      expect(ApplicationSetting).to receive(:current).and_return(::ApplicationSetting.create_from_defaults)
+      expect(ApplicationSetting).not_to receive(:last)
+
+      expect(current_application_settings).to be_a(ApplicationSetting)
+    end
+
+    it 'does not attempt to connect to DB or Redis' do
+      allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(false)
+      expect(ApplicationSetting).not_to receive(:current)
+      expect(ApplicationSetting).not_to receive(:last)
+
+      expect(current_application_settings).to eq fake_application_settings
+    end
+
+    it 'falls back to DB if Redis returns an empty value' do
+      allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true)
+      expect(ApplicationSetting).to receive(:last).and_call_original
+
+      expect(current_application_settings).to be_a(ApplicationSetting)
+    end
+
+    it 'falls back to DB if Redis fails' do
+      allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true)
+      expect(ApplicationSetting).to receive(:current).and_raise(::Redis::BaseError)
+      expect(ApplicationSetting).to receive(:last).and_call_original
+
+      expect(current_application_settings).to be_a(ApplicationSetting)
+    end
+  end
+end