Support `REDIS_SENTINEL_PORT` variables (#31767)

lib/mastodon/redis_configuration.rb

@@ -57,17 +57,20 @@ class Mastodon::RedisConfiguration
   def setup_config(prefix: nil, defaults: {})
     prefix = "#{prefix}REDIS_"
 
-    url       = ENV.fetch("#{prefix}URL", nil)
+    url           = ENV.fetch("#{prefix}URL", nil)
-    user      = ENV.fetch("#{prefix}USER", nil)
+    user          = ENV.fetch("#{prefix}USER", nil)
-    password  = ENV.fetch("#{prefix}PASSWORD", nil)
+    password      = ENV.fetch("#{prefix}PASSWORD", nil)
-    host      = ENV.fetch("#{prefix}HOST", defaults[:host])
+    host          = ENV.fetch("#{prefix}HOST", defaults[:host])
-    port      = ENV.fetch("#{prefix}PORT", defaults[:port])
+    port          = ENV.fetch("#{prefix}PORT", defaults[:port])
-    db        = ENV.fetch("#{prefix}DB", defaults[:db])
+    db            = ENV.fetch("#{prefix}DB", defaults[:db])
-    name      = ENV.fetch("#{prefix}SENTINEL_MASTER", nil)
+    name          = ENV.fetch("#{prefix}SENTINEL_MASTER", nil)
-    sentinels = parse_sentinels(ENV.fetch("#{prefix}SENTINELS", nil))
+    sentinel_port = ENV.fetch("#{prefix}SENTINEL_PORT", 26_379)
+    sentinel_list = ENV.fetch("#{prefix}SENTINELS", nil)
 
     return { url:, driver: } if url
 
+    sentinels = parse_sentinels(sentinel_list, default_port: sentinel_port)
+
     if name.present? && sentinels.present?
       host = name
       port = nil
@@ -96,10 +99,10 @@ class Mastodon::RedisConfiguration
     end.normalize.to_str
   end
 
-  def parse_sentinels(sentinels_string)
+  def parse_sentinels(sentinels_string, default_port: 26_379)
     (sentinels_string || '').split(',').map do |sentinel|
       host, port = sentinel.split(':')
-      port = port.present? ? port.to_i : 26_379
+      port = (port || default_port).to_i
       { host: host, port: port }
     end.presence
   end

spec/lib/mastodon/redis_configuration_spec.rb

@@ -107,14 +107,40 @@ RSpec.describe Mastodon::RedisConfiguration do
     end
 
     context 'when giving sentinels without port numbers' do
-      around do |example|
+      context "when no default port is given via `#{prefix}REDIS_SENTINEL_PORT`" do
-        ClimateControl.modify "#{prefix}REDIS_SENTINELS": '192.168.0.1,192.168.0.2', "#{prefix}REDIS_SENTINEL_MASTER": 'mainsentinel' do
+        around do |example|
-          example.run
+          ClimateControl.modify "#{prefix}REDIS_SENTINELS": '192.168.0.1,192.168.0.2', "#{prefix}REDIS_SENTINEL_MASTER": 'mainsentinel' do
+            example.run
+          end
+        end
+
+        it 'uses the default sentinel port' do
+          expect(subject[:sentinels]).to contain_exactly({ host: '192.168.0.1', port: 26_379 }, { host: '192.168.0.2', port: 26_379 })
         end
       end
 
-      it 'uses the default sentinel port' do
+      context 'when adding port numbers to some, but not all sentinels' do
-        expect(subject[:sentinels]).to contain_exactly({ host: '192.168.0.1', port: 26_379 }, { host: '192.168.0.2', port: 26_379 })
+        around do |example|
+          ClimateControl.modify "#{prefix}REDIS_SENTINELS": '192.168.0.1:5678,192.168.0.2', "#{prefix}REDIS_SENTINEL_MASTER": 'mainsentinel' do
+            example.run
+          end
+        end
+
+        it 'uses the given port number when available and the default otherwise' do
+          expect(subject[:sentinels]).to contain_exactly({ host: '192.168.0.1', port: 5678 }, { host: '192.168.0.2', port: 26_379 })
+        end
+      end
+
+      context "when a default port is given via `#{prefix}REDIS_SENTINEL_PORT`" do
+        around do |example|
+          ClimateControl.modify "#{prefix}REDIS_SENTINEL_PORT": '1234', "#{prefix}REDIS_SENTINELS": '192.168.0.1,192.168.0.2', "#{prefix}REDIS_SENTINEL_MASTER": 'mainsentinel' do
+            example.run
+          end
+        end
+
+        it 'uses the given port number' do
+          expect(subject[:sentinels]).to contain_exactly({ host: '192.168.0.1', port: 1234 }, { host: '192.168.0.2', port: 1234 })
+        end
       end
     end
   end