Migrate sentinel code to redis-client

Author: byroot

.github/workflows/test.yaml

@@ -179,7 +179,7 @@ jobs:
           path: tmp
           key: "local-tmp-redis-7.0-on-ubuntu-latest"
       - name: Booting up Redis
-        run: make start_all
+        run: make start_sentinel wait_for_sentinel
       - name: Test
         run: bundle exec rake test:sentinel
       - name: Shutting down Redis
@@ -217,7 +217,7 @@ jobs:
           path: tmp
           key: "local-tmp-redis-7.0-on-ubuntu-latest"
       - name: Booting up Redis
-        run: make start_all
+        run: make start_cluster
       - name: Test
         run: bundle exec rake test:cluster
       - name: Shutting down Redis

Gemfile

@@ -8,5 +8,4 @@ gem 'minitest'
 gem 'rake'
 gem 'rubocop', '~> 1.25.1'
 gem 'mocha'
-gem 'hiredis'
 gem 'redis-client', github: 'redis-rb/redis-client' # RESP2 supportnot yet released

README.md

@@ -111,7 +111,7 @@ If you want to [authenticate](https://redis.io/topics/sentinel#configuring-senti
 SENTINELS = [{ host: '127.0.0.1', port: 26380, password: 'mysecret' },
              { host: '127.0.0.1', port: 26381, password: 'mysecret' }]
 
-redis = Redis.new(host: 'mymaster', sentinels: SENTINELS, role: :master)
+redis = Redis.new(name: 'mymaster', sentinels: SENTINELS, role: :master)
 ```
 
 ## Cluster support

bin/cluster_creator

@@ -1,7 +1,8 @@
 #!/usr/bin/env ruby
-
 # frozen_string_literal: true
 
+require 'bundler/setup'
+
 $LOAD_PATH.unshift(File.expand_path('../lib', __dir__))
 require_relative '../test/support/cluster/orchestrator'
 

lib/redis.rb

@@ -46,6 +46,7 @@ def deprecate!(message)
   # @option options [Integer, Array<Integer, Float>] :reconnect_attempts Number of attempts trying to connect,
   #   or a list of sleep duration between attempts.
   # @option options [Boolean] :inherit_socket (false) Whether to use socket in forked process or not
+  # @option options [String] :name The name of the server group to connect to.
   # @option options [Array] :sentinels List of sentinels to contact
   # @option options [Symbol] :role (:master) Role to fetch via Sentinel, either `:master` or `:slave`
   # @option options [Array<String, Hash{Symbol => String, Integer}>] :cluster List of cluster nodes to contact
@@ -56,19 +57,38 @@ def deprecate!(message)
   #
   # @return [Redis] a new client instance
   def initialize(options = {})
+    @monitor = Monitor.new
     @options = options.dup
     @options[:reconnect_attempts] = 1 unless @options.key?(:reconnect_attempts)
     if ENV["REDIS_URL"] && SERVER_URL_OPTIONS.none? { |o| @options.key?(o) }
       @options[:url] = ENV["REDIS_URL"]
     end
     inherit_socket = @options.delete(:inherit_socket)
-    @cluster_mode = options.key?(:cluster)
-    client = @cluster_mode ? Cluster : Client
     @subscription_client = nil
-    @client = client.new(@options)
+
+    @client = if @cluster_mode = options.key?(:cluster)
+      Cluster.new(@options)
+    elsif @options.key?(:sentinels)
+      if url = @options.delete(:url)
+        uri = URI.parse(url)
+        if !@options.key?(:name) && uri.host
+          @options[:name] = uri.host
+        end
+
+        if !@options.key?(:password) && uri.password && !uri.password.empty?
+          @options[:password] = uri.password
+        end
+
+        if !@options.key?(:username) && uri.user && !uri.user.empty?
+          @options[:username] = uri.user
+        end
+      end
+
+      Client.sentinel(**@options).new_client
+    else
+      Client.new(@options)
+    end
     @client.inherit_socket! if inherit_socket
-    @queue = Hash.new { |h, k| h[k] = [] }
-    @monitor = Monitor.new
   end
 
   # Run code without the client reconnecting

lib/redis/client.rb

@@ -10,14 +10,19 @@ class Client < ::RedisClient
       RedisClient::ReadTimeoutError => Redis::TimeoutError,
       RedisClient::CannotConnectError => Redis::CannotConnectError,
       RedisClient::AuthenticationError => Redis::CannotConnectError,
+      RedisClient::FailoverError => Redis::CannotConnectError,
       RedisClient::PermissionError => Redis::PermissionError,
       RedisClient::WrongTypeError => Redis::WrongTypeError,
       RedisClient::RESP3::UnknownType => Redis::ProtocolError,
     }.freeze
 
     class << self
       def config(**kwargs)
-        ::RedisClient.config(protocol: 2, **kwargs)
+        super(protocol: 2, **kwargs)
+      end
+
+      def sentinel(**kwargs)
+        super(protocol: 2, **kwargs)
       end
     end
 

makefile

@@ -54,19 +54,19 @@ start: ${BINARY}
 stop_slave:
 	@$(call kill-redis,${SLAVE_PID_PATH})
 
-start_slave: ${BINARY}
+start_slave: start
 	@${BINARY}\
 		--daemonize  yes\
 		--pidfile    ${SLAVE_PID_PATH}\
 		--port       ${SLAVE_PORT}\
 		--unixsocket ${SLAVE_SOCKET_PATH}\
 		--slaveof    127.0.0.1 ${PORT}
 
-stop_sentinel:
+stop_sentinel: stop_slave stop
 	@$(call kill-redis,${SENTINEL_PID_PATHS})
 	@rm -f ${TMP}/sentinel*.conf || true
 
-start_sentinel: ${BINARY}
+start_sentinel: start start_slave
 	@for port in ${SENTINEL_PORTS}; do\
 		conf=${TMP}/sentinel$$port.conf;\
 		touch $$conf;\

test/sentinel/sentinel_test.rb

@@ -124,17 +124,17 @@ def test_sentinel_failover_prioritize_healthy_sentinel
       end
     end
 
-    assert_equal commands[:s1], [%w[get-master-addr-by-name master1]]
-    assert_equal commands[:s2], [%w[get-master-addr-by-name master1], %w[get-master-addr-by-name master1]]
+    assert_equal [%w[get-master-addr-by-name master1]], commands[:s1]
+    assert_equal [%w[get-master-addr-by-name master1]], commands[:s2]
   end
 
   def test_sentinel_with_non_sentinel_options
     commands = { s1: [], m1: [] }
 
     sentinel = lambda do |port|
       {
-        auth: lambda do |pass|
-          commands[:s1] << ['auth', pass]
+        auth: lambda do |*args|
+          commands[:s1] << ['auth', *args]
           '+OK'
         end,
         select: lambda do |db|
@@ -149,8 +149,8 @@ def test_sentinel_with_non_sentinel_options
     end
 
     master = {
-      auth: lambda do |pass|
-        commands[:m1] << ['auth', pass]
+      auth: lambda do |*args|
+        commands[:m1] << ['auth', *args]
         '+OK'
       end,
       role: lambda do
@@ -176,8 +176,8 @@ def test_authentication_for_sentinel
 
     sentinel = lambda do |port|
       {
-        auth: lambda do |pass|
-          commands[:s1] << ['auth', pass]
+        auth: lambda do |*args|
+          commands[:s1] << ['auth', *args]
           '+OK'
         end,
         select: lambda do |db|
@@ -192,8 +192,8 @@ def test_authentication_for_sentinel
     end
 
     master = {
-      auth: lambda do |pass|
-        commands[:m1] << ['auth', pass]
+      auth: lambda do |*args|
+        commands[:s1] << ['auth', *args]
         '-ERR Client sent AUTH, but no password is set'
       end,
       role: lambda do
@@ -205,7 +205,7 @@ def test_authentication_for_sentinel
     RedisMock.start(master) do |master_port|
       RedisMock.start(sentinel.call(master_port)) do |sen_port|
         s = [{ host: '127.0.0.1', port: sen_port, password: 'foo' }]
-        r = Redis.new(host: 'master1', sentinels: s, role: :master)
+        r = Redis.new(name: 'master1', sentinels: s, role: :master)
         assert r.ping
       end
     end
@@ -219,8 +219,8 @@ def test_authentication_for_sentinel_and_redis
 
     sentinel = lambda do |port|
       {
-        auth: lambda do |pass|
-          commands[:s1] << ['auth', pass]
+        auth: lambda do |*args|
+          commands[:s1] << ['auth', *args]
           '+OK'
         end,
         select: lambda do |db|
@@ -235,8 +235,8 @@ def test_authentication_for_sentinel_and_redis
     end
 
     master = {
-      auth: lambda do |pass|
-        commands[:m1] << ['auth', pass]
+      auth: lambda do |*args|
+        commands[:m1] << ['auth', *args]
         '+OK'
       end,
       role: lambda do
@@ -248,7 +248,7 @@ def test_authentication_for_sentinel_and_redis
     RedisMock.start(master) do |master_port|
       RedisMock.start(sentinel.call(master_port)) do |sen_port|
         s = [{ host: '127.0.0.1', port: sen_port, password: 'foo' }]
-        r = Redis.new(host: 'master1', sentinels: s, role: :master, password: 'bar')
+        r = Redis.new(name: 'master1', sentinels: s, role: :master, password: 'bar')
         assert r.ping
       end
     end
@@ -291,7 +291,7 @@ def test_authentication_with_acl
     RedisMock.start(master) do |master_port|
       RedisMock.start(sentinel.call(master_port)) do |sen_port|
         s = [{ host: '127.0.0.1', port: sen_port, username: 'bob', password: 'foo' }]
-        r = Redis.new(host: 'master1', sentinels: s, role: :master, username: 'alice', password: 'bar')
+        r = Redis.new(name: 'master1', sentinels: s, role: :master, username: 'alice', password: 'bar')
         assert r.ping
       end
     end
@@ -317,18 +317,18 @@ def test_sentinel_role_mismatch
       end
     }
 
-    ex = assert_raises(Redis::ConnectionError) do
+    ex = assert_raises(Redis::CannotConnectError) do
       RedisMock.start(master) do |master_port|
         RedisMock.start(sentinel.call(master_port)) do |sen_port|
           sentinels[0][:port] = sen_port
-          redis = Redis.new(url: "redis://master1", sentinels: sentinels, role: :master)
+          redis = Redis.new(url: "redis://master1", sentinels: sentinels, role: :master, reconnect_attempts: 0)
 
           assert redis.ping
         end
       end
     end
 
-    assert_match(/Instance role mismatch/, ex.message)
+    assert_match(/Expected to connect to a master, but the server is a replica/, ex.message)
   end
 
   def test_sentinel_retries
@@ -389,35 +389,4 @@ def test_sentinel_retries
 
     assert_match(/No sentinels available/, ex.message)
   end
-
-  def test_sentinel_with_string_option_keys
-    commands = []
-
-    master = {
-      role: lambda do
-        ['master']
-      end
-    }
-
-    sentinel = lambda do |port|
-      {
-        sentinel: lambda do |command, *args|
-          commands << [command, *args]
-          ['127.0.0.1', port.to_s]
-        end
-      }
-    end
-
-    RedisMock.start(master) do |master_port|
-      RedisMock.start(sentinel.call(master_port)) do |sen_port|
-        sentinels = [{ 'host' => '127.0.0.1', 'port' => sen_port }]
-
-        redis = Redis.new(url: 'redis://master1', 'sentinels' => sentinels, 'role' => :master)
-
-        assert redis.ping
-      end
-    end
-
-    assert_equal [%w[get-master-addr-by-name master1]], commands
-  end
 end