Make clear Redis "default_url" is not an example to be followed

GitLab::Redis::{Cache,Queues,SharedState} have a configuration
fallback mechanism where in the absence of either YAML configuration
file, they fall back to localhost Redis URL's. These default URL's
make no sense because if you need multiple Redis instances, your
GitLab is too big to have Redis on localhost.

Because there is a remote chance someone is using these default URL's
anyway, this commit stops short of removing them. But we do refactor
the default URL lookup code to make it unlikely that the next person
adding a new Redis instance to the app will cargo-cult the default URL
behavior.

lib/gitlab/redis/cache.rb

@@ -5,10 +5,10 @@ module Gitlab
     class Cache < ::Gitlab::Redis::Wrapper
       CACHE_NAMESPACE = 'cache:gitlab'
 
-      class << self
-        def default_url
-          'redis://localhost:6380'
-        end
+      private
+
+      def raw_config_hash
+        super || { url: 'redis://localhost:6380' }
       end
     end
   end

lib/gitlab/redis/queues.rb

@@ -9,10 +9,10 @@ module Gitlab
       SIDEKIQ_NAMESPACE = 'resque:gitlab'
       MAILROOM_NAMESPACE = 'mail_room:gitlab'
 
-      class << self
-        def default_url
-          'redis://localhost:6381'
-        end
+      private
+
+      def raw_config_hash
+        super || { url: 'redis://localhost:6381' }
       end
     end
   end

lib/gitlab/redis/shared_state.rb

@@ -8,10 +8,10 @@ module Gitlab
       USER_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:user:gitlab'
       IP_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:ip:gitlab2'
 
-      class << self
-        def default_url
-          'redis://localhost:6382'
-        end
+      private
+
+      def raw_config_hash
+        super || { url: 'redis://localhost:6382' }
       end
     end
   end

lib/gitlab/redis/wrapper.rb

@@ -51,10 +51,6 @@ module Gitlab
             end
         end
 
-        def default_url
-          raise NotImplementedError
-        end
-
         def config_file_path(filename)
           path = File.join(rails_root, 'config', filename)
           return path if File.file?(path)
@@ -137,8 +133,6 @@ module Gitlab
 
         if config_data
           config_data.is_a?(String) ? { url: config_data } : config_data.deep_symbolize_keys
-        else
-          { url: self.class.default_url }
         end
       end
 

spec/lib/gitlab/redis/cache_spec.rb

@@ -5,7 +5,14 @@ require 'spec_helper'
 RSpec.describe Gitlab::Redis::Cache do
   let(:instance_specific_config_file) { "config/redis.cache.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_CACHE_CONFIG_FILE" }
-  let(:class_redis_url) { 'redis://localhost:6380' }
 
   include_examples "redis_shared_examples"
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config) { false }
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6380' )
+    end
+  end
 end

spec/lib/gitlab/redis/queues_spec.rb

@@ -5,7 +5,14 @@ require 'spec_helper'
 RSpec.describe Gitlab::Redis::Queues do
   let(:instance_specific_config_file) { "config/redis.queues.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_QUEUES_CONFIG_FILE" }
-  let(:class_redis_url) { 'redis://localhost:6381' }
 
   include_examples "redis_shared_examples"
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config) { false }
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6381' )
+    end
+  end
 end

spec/lib/gitlab/redis/shared_state_spec.rb

@@ -5,7 +5,14 @@ require 'spec_helper'
 RSpec.describe Gitlab::Redis::SharedState do
   let(:instance_specific_config_file) { "config/redis.shared_state.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_SHARED_STATE_CONFIG_FILE" }
-  let(:class_redis_url) { 'redis://localhost:6382' }
 
   include_examples "redis_shared_examples"
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config) { false }
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6382' )
+    end
+  end
 end

spec/lib/gitlab/redis/wrapper_spec.rb

@@ -8,10 +8,4 @@ RSpec.describe Gitlab::Redis::Wrapper do
       expect { described_class.instrumentation_class }.to raise_error(NameError)
     end
   end
-
-  describe '.default_url' do
-    it 'is not implemented' do
-      expect { described_class.default_url }.to raise_error(NotImplementedError)
-    end
-  end
 end

spec/support/redis/redis_shared_examples.rb

@@ -92,6 +92,7 @@ RSpec.shared_examples "redis_shared_examples" do
     subject { described_class.new(rails_env).params }
 
     let(:rails_env) { 'development' }
+    let(:config_file_name) { config_old_format_socket }
 
     it 'withstands mutation' do
       params1 = described_class.params
@@ -153,6 +154,8 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '.url' do
+    let(:config_file_name) { config_old_format_socket }
+
     it 'withstands mutation' do
       url1 = described_class.url
       url2 = described_class.url
@@ -201,6 +204,8 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '.with' do
+    let(:config_file_name) { config_old_format_socket }
+
     before do
       clear_pool
     end
@@ -288,12 +293,6 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '#raw_config_hash' do
-    it 'returns default redis url when no config file is present' do
-      expect(subject).to receive(:fetch_config) { false }
-
-      expect(subject.send(:raw_config_hash)).to eq(url: class_redis_url )
-    end
-
     it 'returns old-style single url config in a hash' do
       expect(subject).to receive(:fetch_config) { test_redis_url }
       expect(subject.send(:raw_config_hash)).to eq(url: test_redis_url)