(RE-15162) Update Redis gem to version 5

Gemfile.lock

@@ -3,7 +3,7 @@ PATH
   specs:
     vmpooler (3.1.0)
       concurrent-ruby (~> 1.1)
-      connection_pool (~> 2.2)
+      connection_pool (~> 2.4)
       deep_merge (~> 1.2)
       net-ldap (~> 0.16)
       opentelemetry-exporter-jaeger (= 0.22.0)
@@ -18,7 +18,7 @@ PATH
       puma (>= 5.0.4, < 7)
       rack (>= 2.2, < 4.0)
       rake (~> 13.0)
-      redis (~> 4.1)
+      redis (~> 5.0)
       sinatra (>= 2, < 4)
       spicy-proton (~> 2.1)
       statsd-ruby (~> 1.4)
@@ -29,6 +29,7 @@ GEM
     ast (2.4.2)
     bindata (2.4.15)
     builder (3.2.4)
+    byebug (11.1.3)
     climate_control (1.2.0)
     coderay (1.1.3)
     concurrent-ruby (1.2.2)
@@ -36,15 +37,13 @@ GEM
     deep_merge (1.2.2)
     diff-lcs (1.5.0)
     docile (1.4.0)
-    faraday (2.7.4)
+    faraday (2.7.10)
       faraday-net_http (>= 2.0, < 3.1)
       ruby2_keywords (>= 0.0.4)
     faraday-net_http (3.0.2)
-    ffi (1.15.5-java)
     google-cloud-env (1.6.0)
       faraday (>= 0.17.3, < 3.0)
     json (2.6.3)
-    json (2.6.3-java)
     language_server-protocol (3.17.0.3)
     method_source (1.0.0)
     mock_redis (0.36.0)
@@ -53,9 +52,8 @@ GEM
       ruby2_keywords (~> 0.0.1)
     net-ldap (0.18.0)
     nio4r (2.5.9)
-    nio4r (2.5.9-java)
+    opentelemetry-api (1.2.1)
-    opentelemetry-api (1.1.0)
+    opentelemetry-common (0.19.7)
-    opentelemetry-common (0.19.6)
       opentelemetry-api (~> 1.0)
     opentelemetry-exporter-jaeger (0.22.0)
       opentelemetry-api (~> 1.1)
@@ -80,60 +78,59 @@ GEM
       opentelemetry-api (~> 1.0)
       opentelemetry-common (~> 0.19.3)
       opentelemetry-instrumentation-base (~> 0.19.0)
-    opentelemetry-registry (0.2.0)
+    opentelemetry-registry (0.3.0)
       opentelemetry-api (~> 1.1)
     opentelemetry-resource_detectors (0.23.0)
       google-cloud-env
       opentelemetry-sdk (~> 1.0)
-    opentelemetry-sdk (1.2.0)
+    opentelemetry-sdk (1.2.1)
       opentelemetry-api (~> 1.1)
       opentelemetry-common (~> 0.19.3)
       opentelemetry-registry (~> 0.2)
       opentelemetry-semantic_conventions
-    opentelemetry-semantic_conventions (1.8.0)
+    opentelemetry-semantic_conventions (1.10.0)
       opentelemetry-api (~> 1.0)
     parallel (1.23.0)
     parser (3.2.2.3)
       ast (~> 2.4.1)
       racc
     pickup (0.0.11)
-    prometheus-client (4.1.0)
+    prometheus-client (4.2.1)
     pry (0.14.2)
       coderay (~> 1.1)
       method_source (~> 1.0)
-    pry (0.14.2-java)
+    pry-byebug (3.10.1)
-      coderay (~> 1.1)
+      byebug (~> 11.0)
-      method_source (~> 1.0)
+      pry (>= 0.13, < 0.15)
-      spoon (~> 0.0)
+    puma (6.3.0)
-    puma (6.2.2)
-      nio4r (~> 2.0)
-    puma (6.2.2-java)
       nio4r (~> 2.0)
     racc (1.7.1)
-    racc (1.7.1-java)
+    rack (2.2.8)
-    rack (2.2.7)
+    rack-protection (3.1.0)
-    rack-protection (3.0.6)
+      rack (~> 2.2, >= 2.2.4)
-      rack
     rack-test (2.1.0)
       rack (>= 1.3)
     rainbow (3.1.1)
     rake (13.0.6)
-    redis (4.8.1)
+    redis (5.0.6)
+      redis-client (>= 0.9.0)
+    redis-client (0.15.0)
+      connection_pool
     regexp_parser (2.8.1)
-    rexml (3.2.5)
+    rexml (3.2.6)
     rspec (3.12.0)
       rspec-core (~> 3.12.0)
       rspec-expectations (~> 3.12.0)
       rspec-mocks (~> 3.12.0)
-    rspec-core (3.12.1)
+    rspec-core (3.12.2)
       rspec-support (~> 3.12.0)
-    rspec-expectations (3.12.2)
+    rspec-expectations (3.12.3)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.12.0)
-    rspec-mocks (3.12.3)
+    rspec-mocks (3.12.6)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.12.0)
-    rspec-support (3.12.0)
+    rspec-support (3.12.1)
     rubocop (1.54.2)
       json (~> 2.3)
       language_server-protocol (>= 3.17.0)
@@ -155,33 +152,30 @@ GEM
       simplecov_json_formatter (~> 0.1)
     simplecov-html (0.12.3)
     simplecov_json_formatter (0.1.4)
-    sinatra (3.0.6)
+    sinatra (3.1.0)
       mustermann (~> 3.0)
       rack (~> 2.2, >= 2.2.4)
-      rack-protection (= 3.0.6)
+      rack-protection (= 3.1.0)
       tilt (~> 2.0)
     spicy-proton (2.1.15)
       bindata (~> 2.3)
-    spoon (0.0.6)
-      ffi
     statsd-ruby (1.5.0)
     thor (1.2.2)
     thrift (0.18.1)
-    tilt (2.1.0)
+    tilt (2.2.0)
     unicode-display_width (2.4.2)
     yarjuf (2.0.0)
       builder
       rspec (~> 3)
 
 PLATFORMS
-  universal-java-1.8
+  x86_64-darwin-22
-  universal-java-11
-  x86_64-linux
 
 DEPENDENCIES
   climate_control (>= 0.2.0)
   mock_redis (>= 0.17.0)
   pry
+  pry-byebug
   rack-test (>= 0.6)
   rspec (>= 3.2)
   rubocop (~> 1.54.2)
@@ -191,4 +185,4 @@ DEPENDENCIES
   yarjuf (>= 2.0)
 
 BUNDLED WITH
-   2.3.18
+   2.4.12

lib/vmpooler.rb

@@ -18,7 +18,7 @@ module Vmpooler
   # Dependencies for tracing
   require 'opentelemetry-instrumentation-concurrent_ruby'
   require 'opentelemetry-instrumentation-http_client'
-  require 'opentelemetry-instrumentation-redis'
+  # require 'opentelemetry-instrumentation-redis'
   require 'opentelemetry-instrumentation-sinatra'
   require 'opentelemetry-sdk'
   require 'opentelemetry/exporter/jaeger'
@@ -103,7 +103,7 @@ module Vmpooler
     parsed_config[:redis]['data_ttl']                = string_to_int(ENV['REDIS_DATA_TTL']) || parsed_config[:redis]['data_ttl'] || 168
     parsed_config[:redis]['connection_pool_size']    = string_to_int(ENV['REDIS_CONNECTION_POOL_SIZE']) || parsed_config[:redis]['connection_pool_size'] || 10
     parsed_config[:redis]['connection_pool_timeout'] = string_to_int(ENV['REDIS_CONNECTION_POOL_TIMEOUT']) || parsed_config[:redis]['connection_pool_timeout'] || 5
-    parsed_config[:redis]['reconnect_attempts']      = string_to_int(ENV['REDIS_RECONNECT_ATTEMPTS']) || parsed_config[:redis]['reconnect_attempts'] || 10
+    parsed_config[:redis]['reconnect_attempts']      = string_array_to_array(ENV['REDIS_RECONNECT_ATTEMPTS']) || parsed_config[:redis]['reconnect_attempts'] || 10
 
     parsed_config[:statsd]           = parsed_config[:statsd] || {} if ENV['STATSD_SERVER']
     parsed_config[:statsd]['server'] = ENV['STATSD_SERVER'] if ENV['STATSD_SERVER']
@@ -209,8 +209,7 @@ module Vmpooler
   end
 
   def self.new_redis(host = 'localhost', port = nil, password = nil, redis_reconnect_attempts = 10)
-    Redis.new(host: host, port: port, password: password, reconnect_attempts: redis_reconnect_attempts, reconnect_delay: 1.5,
+    Redis.new(host: host, port: port, password: password, reconnect_attempts: redis_reconnect_attempts, timeout: 5)
-              reconnect_delay_max: 10.0)
   end
 
   def self.pools(conf)
@@ -235,6 +234,13 @@ module Vmpooler
     Integer(s)
   end
 
+  def self.string_array_to_array(s)
+    # Returns an array from an array like string
+    return if s.nil?
+
+    JSON.parse(s)
+  end
+
   def self.true?(obj)
     obj.to_s.downcase == 'true'
   end

lib/vmpooler/pool_manager.rb

@@ -59,7 +59,7 @@ module Vmpooler
         currently_configured_pools = []
         config[:pools].each do |pool|
           currently_configured_pools << pool['name']
-          redis.sadd('vmpooler__pools', pool['name'])
+          redis.sadd('vmpooler__pools', pool['name'].to_s)
           pool_keys = pool.keys
           pool_keys.delete('alias')
           to_set = {}
@@ -68,11 +68,12 @@ module Vmpooler
           end
           to_set['alias'] = pool['alias'].join(',') if to_set.key?('alias')
           to_set['domain'] = Vmpooler::Dns.get_domain_for_pool(config, pool['name'])
-          redis.hmset("vmpooler__pool__#{pool['name']}", to_set.to_a.flatten) unless to_set.empty?
+
+          redis.hmset("vmpooler__pool__#{pool['name']}", *to_set.to_a.flatten) unless to_set.empty?
         end
         previously_configured_pools.each do |pool|
           unless currently_configured_pools.include? pool
-            redis.srem('vmpooler__pools', pool)
+            redis.srem('vmpooler__pools', pool.to_s)
             redis.del("vmpooler__pool__#{pool}")
           end
         end
@@ -129,7 +130,6 @@ module Vmpooler
         if exists
           request_id = redis.hget("vmpooler__vm__#{vm}", 'request_id')
           pool_alias = redis.hget("vmpooler__vm__#{vm}", 'pool_alias') if request_id
-          redis.multi
           redis.smove("vmpooler__pending__#{pool}", "vmpooler__completed__#{pool}", vm)
           if request_id
             ondemandrequest_hash = redis.hgetall("vmpooler__odrequest__#{request_id}")
@@ -138,7 +138,6 @@ module Vmpooler
               redis.zadd('vmpooler__odcreate__task', 1, "#{pool_alias}:#{pool}:1:#{request_id}")
             end
           end
-          redis.exec
           $metrics.increment("errors.markedasfailed.#{pool}")
           $logger.log('d', "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes")
         else
@@ -428,16 +427,15 @@ module Vmpooler
       mutex = vm_mutex(new_vmname)
       mutex.synchronize do
         @redis.with_metrics do |redis|
-          # Add VM to Redis inventory ('pending' pool)
+          redis.multi do |transaction|
-          redis.multi
+            transaction.sadd("vmpooler__pending__#{pool_name}", new_vmname)
-          redis.sadd("vmpooler__pending__#{pool_name}", new_vmname)
+            transaction.hset("vmpooler__vm__#{new_vmname}", 'clone', Time.now.to_s)
-          redis.hset("vmpooler__vm__#{new_vmname}", 'clone', Time.now)
+            transaction.hset("vmpooler__vm__#{new_vmname}", 'template', pool_name) # This value is used to represent the pool.
-          redis.hset("vmpooler__vm__#{new_vmname}", 'template', pool_name) # This value is used to represent the pool.
+            transaction.hset("vmpooler__vm__#{new_vmname}", 'pool', pool_name)
-          redis.hset("vmpooler__vm__#{new_vmname}", 'pool', pool_name)
+            transaction.hset("vmpooler__vm__#{new_vmname}", 'domain', pool_domain)
-          redis.hset("vmpooler__vm__#{new_vmname}", 'domain', pool_domain)
+            transaction.hset("vmpooler__vm__#{new_vmname}", 'request_id', request_id) if request_id
-          redis.hset("vmpooler__vm__#{new_vmname}", 'request_id', request_id) if request_id
+            transaction.hset("vmpooler__vm__#{new_vmname}", 'pool_alias', pool_alias) if pool_alias
-          redis.hset("vmpooler__vm__#{new_vmname}", 'pool_alias', pool_alias) if pool_alias
+          end
-          redis.exec
         end
 
         begin
@@ -502,7 +500,7 @@ module Vmpooler
         @redis.with_metrics do |redis|
           redis.pipelined do |pipeline|
             pipeline.hdel("vmpooler__active__#{pool}", vm)
-            pipeline.hset("vmpooler__vm__#{vm}", 'destroy', Time.now)
+            pipeline.hset("vmpooler__vm__#{vm}", 'destroy', Time.now.to_s)
 
             # Auto-expire metadata key
             pipeline.expire("vmpooler__vm__#{vm}", ($config[:redis]['data_ttl'].to_i * 60 * 60))
@@ -868,12 +866,13 @@ module Vmpooler
     def sleep_with_wakeup_events(loop_delay, wakeup_period = 5, options = {})
       exit_by = Time.now + loop_delay
       wakeup_by = Time.now + wakeup_period
+
       return if time_passed?(:exit_by, exit_by)
 
       @redis.with_metrics do |redis|
         initial_ready_size = redis.scard("vmpooler__ready__#{options[:poolname]}") if options[:pool_size_change]
 
-        initial_clone_target = redis.hget("vmpooler__pool__#{options[:poolname]}", options[:clone_target]) if options[:clone_target_change]
+        initial_clone_target = redis.hget("vmpooler__pool__#{options[:poolname]}", options[:clone_target].to_s) if options[:clone_target_change]
 
         initial_template = redis.hget('vmpooler__template__prepared', options[:poolname]) if options[:pool_template_change]
 
@@ -912,16 +911,15 @@ module Vmpooler
             end
 
             if options[:pending_vm]
-              pending_vm_count = redis.scard("vmpooler__pending__#{options[:poolname]}")
+               pending_vm_count = redis.scard("vmpooler__pending__#{options[:poolname]}")
               break unless pending_vm_count == 0
             end
 
             if options[:ondemand_request]
-              redis.multi
+              od_request = redis.zcard('vmpooler__provisioning__request')
-              redis.zcard('vmpooler__provisioning__request')
+              od_processing = redis.zcard('vmpooler__provisioning__processing')
-              redis.zcard('vmpooler__provisioning__processing')
+              od_createtask = redis.zcard('vmpooler__odcreate__task')
-              redis.zcard('vmpooler__odcreate__task')
+
-              od_request, od_processing, od_createtask = redis.exec
               break unless od_request == 0
               break unless od_processing == 0
               break unless od_createtask == 0
@@ -1093,10 +1091,8 @@ module Vmpooler
 
     def remove_excess_vms(pool)
       @redis.with_metrics do |redis|
-        redis.multi
+        ready = redis.scard("vmpooler__ready__#{pool['name']}")
-        redis.scard("vmpooler__ready__#{pool['name']}")
+        pending = redis.scard("vmpooler__pending__#{pool['name']}")
-        redis.scard("vmpooler__pending__#{pool['name']}")
-        ready, pending = redis.exec
         total = pending.to_i + ready.to_i
         break if total.nil?
         break if total == 0
@@ -1334,12 +1330,11 @@ module Vmpooler
       return if pool_mutex(pool_name).locked?
 
       @redis.with_metrics do |redis|
-        redis.multi
+        ready = redis.scard("vmpooler__ready__#{pool_name}")
-        redis.scard("vmpooler__ready__#{pool_name}")
+        pending = redis.scard("vmpooler__pending__#{pool_name}")
-        redis.scard("vmpooler__pending__#{pool_name}")
+        running = redis.scard("vmpooler__running__#{pool_name}")
-        redis.scard("vmpooler__running__#{pool_name}")
+        
-        ready, pending, running = redis.exec
+          total = pending.to_i + ready.to_i
-        total = pending.to_i + ready.to_i
 
         $metrics.gauge("ready.#{pool_name}", ready)
         $metrics.gauge("running.#{pool_name}", running)
@@ -1594,11 +1589,9 @@ module Vmpooler
 
       return unless vms_ready?(request_id, redis)
 
-      redis.multi
       redis.hset(ondemand_hash_key, 'status', 'ready')
       redis.expire(ondemand_hash_key, default_expiration)
       redis.zrem(processing_key, request_id)
-      redis.exec
     end
 
     def request_expired?(request_id, score, redis)

spec/unit/pool_manager_spec.rb

@@ -1,6 +1,8 @@
 require 'spec_helper'
 require 'time'
 require 'mock_redis'
+require 'pry'
+require 'pry-byebug'
 
 # Custom RSpec :Matchers
 
@@ -3594,8 +3596,9 @@ EOT
       it 'should sleep until the provisioning request is detected' do
         redis_connection_pool.with do |redis|
           expect(subject).to receive(:sleep).exactly(3).times
-          expect(redis).to receive(:multi).and_return('OK').exactly(3).times
+          expect(redis).to receive(:zcard).with('vmpooler__provisioning__request').and_return(0,0,1)
-          expect(redis).to receive(:exec).and_return([0,0,0],[0,0,0],[1,0,0])
+          expect(redis).to receive(:zcard).with('vmpooler__provisioning__processing').and_return(0,0,0)
+          expect(redis).to receive(:zcard).with('vmpooler__odcreate__task').and_return(0,0,0)
         end
 
         subject.sleep_with_wakeup_events(loop_delay, wakeup_period, wakeup_option)
@@ -3604,17 +3607,20 @@ EOT
       it 'should sleep until provisioning processing is detected' do
         redis_connection_pool.with do |redis|
           expect(subject).to receive(:sleep).exactly(3).times
-          expect(redis).to receive(:multi).and_return('OK').exactly(3).times
+          expect(redis).to receive(:zcard).with('vmpooler__provisioning__request').and_return(0,0,0)
-          expect(redis).to receive(:exec).and_return([0,0,0],[0,0,0],[0,1,0])
+          expect(redis).to receive(:zcard).with('vmpooler__provisioning__processing').and_return(0,0,1)
+          expect(redis).to receive(:zcard).with('vmpooler__odcreate__task').and_return(0,0,0)
         end
+
         subject.sleep_with_wakeup_events(loop_delay, wakeup_period, wakeup_option)
       end
 
       it 'should sleep until ondemand creation task is detected' do
         redis_connection_pool.with do |redis|
           expect(subject).to receive(:sleep).exactly(3).times
-          expect(redis).to receive(:multi).and_return('OK').exactly(3).times
+          expect(redis).to receive(:zcard).with('vmpooler__provisioning__request').and_return(0,0,0)
-          expect(redis).to receive(:exec).and_return([0,0,0],[0,0,0],[0,0,1])
+          expect(redis).to receive(:zcard).with('vmpooler__provisioning__processing').and_return(0,0,0)
+          expect(redis).to receive(:zcard).with('vmpooler__odcreate__task').and_return(0,0,1)
         end
 
         subject.sleep_with_wakeup_events(loop_delay, wakeup_period, wakeup_option)

vmpooler.gemspec

@@ -17,7 +17,7 @@ Gem::Specification.new do |s|
   s.executables   = 'vmpooler'
   s.require_paths = ["lib"]
   s.add_dependency 'concurrent-ruby', '~> 1.1'
-  s.add_dependency 'connection_pool', '~> 2.2'
+  s.add_dependency 'connection_pool', '~> 2.4'
   s.add_dependency 'deep_merge', '~> 1.2'
   s.add_dependency 'net-ldap', '~> 0.16'
   s.add_dependency 'opentelemetry-exporter-jaeger', '= 0.22.0'
@@ -32,7 +32,7 @@ Gem::Specification.new do |s|
   s.add_dependency 'puma', '>= 5.0.4', '< 7'
   s.add_dependency 'rack', '>= 2.2', '< 4.0'
   s.add_dependency 'rake', '~> 13.0'
-  s.add_dependency 'redis', '~> 4.1'
+  s.add_dependency 'redis', '~> 5.0'
   s.add_dependency 'sinatra', '>= 2', '< 4'
   s.add_dependency 'spicy-proton', '~> 2.1'
   s.add_dependency 'statsd-ruby', '~> 1.4'
@@ -41,6 +41,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'climate_control', '>= 0.2.0'
   s.add_development_dependency 'mock_redis', '>= 0.17.0'
   s.add_development_dependency 'pry'
+  s.add_development_dependency 'pry-byebug'
   s.add_development_dependency 'rack-test', '>= 0.6'
   s.add_development_dependency 'rspec', '>= 3.2'
   s.add_development_dependency 'rubocop', '~> 1.54.2'