Deduplicated resque.yml loading from several places

We will trust redis configuration params loading to Gitlab::RedisConfig.

config/application.rb

@@ -107,7 +107,8 @@ module Gitlab
       end
     end
-    redis_config_hash = Gitlab::Redis.redis_store_options
+    # Use Redis caching across all environments
+    redis_config_hash = Gitlab::Redis.params
     redis_config_hash[:namespace] = Gitlab::Redis::CACHE_NAMESPACE
     redis_config_hash[:expires_in] = 2.weeks # Cache should not grow forever
     config.cache_store = :redis_store, redis_config_hash

config/initializers/session_store.rb

@@ -13,9 +13,9 @@ end
 if Rails.env.test?
   Gitlab::Application.config.session_store :cookie_store, key: "_gitlab_session"
 else
-  redis_config = Gitlab::Redis.redis_store_options
+  redis_config = Gitlab::Redis.params
   redis_config[:namespace] = Gitlab::Redis::SESSION_NAMESPACE
-  
+
   Gitlab::Application.config.session_store(
     :redis_store, # Using the cookie_store would enable session replay attacks.
     servers: redis_config,

config/mail_room.yml

 :mailboxes:
-<%
-require "yaml"
-require "json"
-require_relative "lib/gitlab/redis" unless defined?(Gitlab::Redis)
-rails_env = ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "development"
-
-config_file = ENV["MAIL_ROOM_GITLAB_CONFIG_FILE"] || "config/gitlab.yml"
-if File.exists?(config_file)
-  all_config = YAML.load_file(config_file)[rails_env]
-
-  config = all_config["incoming_email"] || {}
-  config['enabled']    = false    if config['enabled'].nil?
-  config['port']       = 143      if config['port'].nil?
-  config['ssl']        = false    if config['ssl'].nil?
-  config['start_tls']  = false    if config['start_tls'].nil?
-  config['mailbox']    = "inbox"  if config['mailbox'].nil?
-
-  if config['enabled'] && config['address']
-    redis_url = Gitlab::Redis.new(rails_env).url
-    %>
+  <%
+    require_relative "lib/gitlab/mail_room" unless defined?(Gitlab::MailRoom)
+    config = Gitlab::MailRoom.config
+    if Gitlab::Mailroom.enabled?
+  %>
     -
-      :host: <%= config['host'].to_json %>
-      :port: <%= config['port'].to_json %>
-      :ssl: <%= config['ssl'].to_json %>
-      :start_tls: <%= config['start_tls'].to_json %>
-      :email: <%= config['user'].to_json %>
-      :password: <%= config['password'].to_json %>
-      :name: <%= config['mailbox'].to_json %>
+      :host: <%= config[:host].to_json %>
+      :port: <%= config[:port].to_json %>
+      :ssl: <%= config[:ssl].to_json %>
+      :start_tls: <%= config[:start_tls].to_json %>
+      :email: <%= config[:user].to_json %>
+      :password: <%= config[:password].to_json %>
+      :name: <%= config[:mailbox].to_json %>
       :delete_after_delivery: true
       :delivery_method: sidekiq
       :delivery_options:
-        :redis_url: <%= redis_url.to_json %>
-        :namespace: resque:gitlab
+        :redis_url: <%= config[:redis_url].to_json %>
+        :namespace: <%= Gitlab::Redis::SIDEKIQ_NAMESPACE %>
         :queue: incoming_email
         :worker: EmailReceiverWorker
       :arbitration_method: redis
       :arbitration_options:
-        :redis_url: <%= redis_url.to_json %>
-        :namespace: mail_room:gitlab
+        :redis_url: <%= config[:redis_url].to_json %>
+        :namespace: <%= Gitlab::Redis::MAILROOM_NAMESPACE %>
+
   <% end %>
-<% end %>

config/resque.yml.example

 # If you change this file in a Merge Request, please also create
 # a Merge Request on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests
 #
-development: redis://localhost:6379
-test: redis://localhost:6379
-production: unix:/var/run/redis/redis.sock
+development:
+  url: redis://localhost:6379
+  sentinels:
+    -
+      host: localhost
+      port: 26380 # point to sentinel, not to redis port
+    -
+      host: slave2
+      port: 26381 # point to sentinel, not to redis port
+test:
+  url: redis://localhost:6379
+production:
+  # Redis (single instance)
+  url: unix:/var/run/redis/redis.sock
+  ##
+  # Redis + Sentinel (for HA)
+  #
+  # Please read instructions carefully before using it as you may loose data:
+  # http://redis.io/topics/sentinel
+  #
+  # You must specify a list of a few sentinels that will handle client connection
+  # please read here for more information: https://github.com/redis/redis-rb#sentinel-support
+  ##
+  #url: redis://master:6379
+  #  sentinels:
+  #    -
+  #      host: slave1
+  #      port: 26379 # point to sentinel, not to redis port
+  #    -
+  #      host: slave2
+  #      port: 26379 # point to sentinel, not to redis port

doc/install/installation.md

@@ -591,12 +591,14 @@ for the changes to take effect.
 If you'd like Resque to connect to a Redis server on a non-standard port or on a different host, you can configure its connection string via the `config/resque.yml` file.
     # example
-    production: redis://redis.example.tld:6379
+    production: 
+      url: redis://redis.example.tld:6379
 If you want to connect the Redis server via socket, then use the "unix:" URL scheme and the path to the Redis socket file in the `config/resque.yml` file.
     # example
-    production: unix:/path/to/redis/socket
+    production: 
+      url: unix:/path/to/redis/socket
 ### Custom SSH Connection

lib/gitlab/mail_room.rb

+require 'yaml'
+require 'json'
+require_relative 'lib/gitlab/redis' unless defined?(Gitlab::Redis)
+
+module Gitlab
+  module MailRoom
+
+    class << self
+
+      def enabled?
+        config[:enabled] && config[:address]
+      end
+
+      def config
+        @config ||= fetch_config
+      end
+
+      private
+
+      def fetch_config
+        return nil unless File.exists?(config_file)
+
+        rails_env = ENV['RAILS_ENV'] || ENV['RACK_ENV'] || 'development'
+        all_config = YAML.load_file(config_file)[rails_env].deep_symbolize_keys
+
+        config = all_config[:incoming_email] || {}
+        config[:enabled] = false if config[:enabled].nil?
+        config[:port] = 143 if config[:port].nil?
+        config[:ssl] = false if config[:ssl].nil?
+        config[:start_tls] = false if config[:start_tls].nil?
+        config[:mailbox] = 'inbox' if config[:mailbox].nil?
+
+        if config[:enabled] && config[:address]
+          config[:redis_url] = Gitlab::Redis.new(rails_env).url
+        end
+      end
+
+      def config_file
+        file = ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || 'config/gitlab.yml'
+        File.expand_path("../../../#{file}", __FILE__)
+      end
+    end
+  end
+end

lib/gitlab/redis.rb

+# This file should not have any direct dependency on Rails environment
+# please require all dependencies below:
+require 'active_support/core_ext/hash/keys'
+
 module Gitlab
   class Redis
     CACHE_NAMESPACE = 'cache:gitlab'
     SESSION_NAMESPACE = 'session:gitlab'
     SIDEKIQ_NAMESPACE = 'resque:gitlab'
-
-    attr_reader :url
+    MAILROOM_NAMESPACE = 'mail_room:gitlab'
+    DEFAULT_REDIS_URL = 'redis://localhost:6379'
     # To be thread-safe we must be careful when writing the class instance
     # variables @url and @pool. Because @pool depends on @url we need two
     # mutexes to prevent deadlock.
-    URL_MUTEX = Mutex.new
+    PARAMS_MUTEX = Mutex.new
     POOL_MUTEX = Mutex.new
-    private_constant :URL_MUTEX, :POOL_MUTEX
-    def self.url
-      @url || URL_MUTEX.synchronize { @url = new.url }
-    end
-    def self.with
-      if @pool.nil?
-        POOL_MUTEX.synchronize do
-          @pool = ConnectionPool.new { ::Redis.new(url: url) }
+    private_constant :PARAMS_MUTEX, :POOL_MUTEX
+    class << self
+      def params
+        @params || PARAMS_MUTEX.synchronize { @params = new.params }
+      end
+
+      # @deprecated Use .params instead to get sentinel support
+      def url
+        raw_config_hash[:url]
+      end
+      def with
+        if @pool.nil?
+          POOL_MUTEX.synchronize do
+            @pool = ConnectionPool.new { ::Redis.new(params) }
+          end
         end
+        @pool.with { |redis| yield redis }
       end
-      @pool.with { |redis| yield redis }
     end
-    def self.redis_store_options
-      url = new.url
-      redis_config_hash = ::Redis::Store::Factory.extract_host_options_from_uri(url)
-      # Redis::Store does not handle Unix sockets well, so let's do it for them
-      redis_uri = URI.parse(url)
+    def initialize(rails_env=nil)
+      @rails_env = rails_env || Rails.env
+    end
+
+    def params
+      redis_store_options
+    end
+
+    private
+
+    def redis_store_options
+      config = raw_config_hash
+
+      redis_uri = URI.parse(config[:url])
       if redis_uri.scheme == 'unix'
-        redis_config_hash[:path] = redis_uri.path
+        # Redis::Store does not handle Unix sockets well, so let's do it for them
+        config[:path] = redis_uri.path
+        config.delete(:url)
+      else
+        redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(redis_uri)
+        config.merge!(redis_hash)
       end
-      redis_config_hash
+
+      config
     end
-    def initialize(rails_env=nil)
-      rails_env ||= Rails.env
-      config_file = File.expand_path('../../../config/resque.yml', __FILE__)
-      @url = "redis://localhost:6379"
-      if File.exist?(config_file)
-        @url = YAML.load_file(config_file)[rails_env]
+    def raw_config_hash
+      config_data = fetch_config
+      if config_data
+        config_data.is_a?(String) ? { url: config_data } : config_data.deep_symbolize_keys
+      else
+        { url: DEFAULT_REDIS_URL }
       end
     end
+
+    def fetch_config
+      File.exists?(config_file) ? YAML.load_file(config_file)[@rails_env] : false
+    end
+
+    def config_file
+      File.expand_path('../../../config/resque.yml', __FILE__)
+    end
   end
 end