From 30820ec115e27fddfffc9a85b75a051c3e6604b9 Mon Sep 17 00:00:00 2001 From: isaac-hammes Date: Mon, 7 Aug 2023 08:37:17 -0700 Subject: [PATCH 1/4] (RE-15162) Update Redis gem to version 5 --- Gemfile.lock | 70 ++++++++++++++++------------------ lib/vmpooler.rb | 14 +++++-- lib/vmpooler/pool_manager.rb | 63 ++++++++++++++---------------- spec/unit/pool_manager_spec.rb | 18 ++++++--- vmpooler.gemspec | 5 ++- 5 files changed, 85 insertions(+), 85 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index da7cb6c..ba19548 100644 --- a/Gemfile.lock +++ b/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.1.0) - opentelemetry-common (0.19.6) + opentelemetry-api (1.2.1) + opentelemetry-common (0.19.7) 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) - coderay (~> 1.1) - method_source (~> 1.0) - spoon (~> 0.0) - puma (6.2.2) - nio4r (~> 2.0) - puma (6.2.2-java) + pry-byebug (3.10.1) + byebug (~> 11.0) + pry (>= 0.13, < 0.15) + puma (6.3.0) nio4r (~> 2.0) racc (1.7.1) - racc (1.7.1-java) - rack (2.2.7) - rack-protection (3.0.6) - rack + rack (2.2.8) + rack-protection (3.1.0) + rack (~> 2.2, >= 2.2.4) 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 - universal-java-11 - x86_64-linux + x86_64-darwin-22 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 diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 54c4ffc..197bf4a 100644 --- a/lib/vmpooler.rb +++ b/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, - reconnect_delay_max: 10.0) + Redis.new(host: host, port: port, password: password, reconnect_attempts: redis_reconnect_attempts, timeout: 5) 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 diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index ae1024a..25afb44 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/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 - redis.sadd("vmpooler__pending__#{pool_name}", new_vmname) - redis.hset("vmpooler__vm__#{new_vmname}", 'clone', Time.now) - redis.hset("vmpooler__vm__#{new_vmname}", 'template', pool_name) # This value is used to represent the pool. - redis.hset("vmpooler__vm__#{new_vmname}", 'pool', pool_name) - redis.hset("vmpooler__vm__#{new_vmname}", 'domain', pool_domain) - redis.hset("vmpooler__vm__#{new_vmname}", 'request_id', request_id) if request_id - redis.hset("vmpooler__vm__#{new_vmname}", 'pool_alias', pool_alias) if pool_alias - redis.exec + redis.multi do |transaction| + transaction.sadd("vmpooler__pending__#{pool_name}", new_vmname) + transaction.hset("vmpooler__vm__#{new_vmname}", 'clone', Time.now.to_s) + transaction.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) + transaction.hset("vmpooler__vm__#{new_vmname}", 'domain', pool_domain) + transaction.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 + end 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 - redis.zcard('vmpooler__provisioning__request') - redis.zcard('vmpooler__provisioning__processing') - redis.zcard('vmpooler__odcreate__task') - od_request, od_processing, od_createtask = redis.exec + od_request = redis.zcard('vmpooler__provisioning__request') + od_processing = redis.zcard('vmpooler__provisioning__processing') + od_createtask = redis.zcard('vmpooler__odcreate__task') + 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 - redis.scard("vmpooler__ready__#{pool['name']}") - redis.scard("vmpooler__pending__#{pool['name']}") - ready, pending = redis.exec + ready = redis.scard("vmpooler__ready__#{pool['name']}") + pending = redis.scard("vmpooler__pending__#{pool['name']}") 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 - redis.scard("vmpooler__ready__#{pool_name}") - redis.scard("vmpooler__pending__#{pool_name}") - redis.scard("vmpooler__running__#{pool_name}") - ready, pending, running = redis.exec - total = pending.to_i + ready.to_i + ready = redis.scard("vmpooler__ready__#{pool_name}") + pending = redis.scard("vmpooler__pending__#{pool_name}") + running = redis.scard("vmpooler__running__#{pool_name}") + + 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) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 0e9c5e6..742ea54 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/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(:exec).and_return([0,0,0],[0,0,0],[1,0,0]) + expect(redis).to receive(:zcard).with('vmpooler__provisioning__request').and_return(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,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(:exec).and_return([0,0,0],[0,0,0],[0,1,0]) + expect(redis).to receive(:zcard).with('vmpooler__provisioning__request').and_return(0,0,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(:exec).and_return([0,0,0],[0,0,0],[0,0,1]) + expect(redis).to receive(:zcard).with('vmpooler__provisioning__request').and_return(0,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,1) end subject.sleep_with_wakeup_events(loop_delay, wakeup_period, wakeup_option) diff --git a/vmpooler.gemspec b/vmpooler.gemspec index cdb865c..e997b11 100644 --- a/vmpooler.gemspec +++ b/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' From 46156fd85f4e0ef5fa0fd7a6273353947a4a66ce Mon Sep 17 00:00:00 2001 From: isaac-hammes Date: Tue, 8 Aug 2023 11:05:55 -0700 Subject: [PATCH 2/4] Convert Times to strings when being added to redis. --- Gemfile.lock | 18 +++++++++++++----- lib/vmpooler.rb | 2 +- lib/vmpooler/api/helpers.rb | 2 +- lib/vmpooler/api/v3.rb | 6 +++--- lib/vmpooler/pool_manager.rb | 16 ++++++++-------- spec/unit/pool_manager_spec.rb | 2 -- vmpooler.gemspec | 1 - 7 files changed, 26 insertions(+), 21 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index ba19548..d59fc9d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -29,7 +29,6 @@ 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) @@ -41,9 +40,11 @@ GEM 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) @@ -52,6 +53,7 @@ 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-common (0.19.7) opentelemetry-api (~> 1.0) @@ -99,12 +101,16 @@ GEM pry (0.14.2) coderay (~> 1.1) method_source (~> 1.0) - pry-byebug (3.10.1) - byebug (~> 11.0) - pry (>= 0.13, < 0.15) + pry (0.14.2-java) + coderay (~> 1.1) + method_source (~> 1.0) + spoon (~> 0.0) puma (6.3.0) nio4r (~> 2.0) + puma (6.3.0-java) + nio4r (~> 2.0) racc (1.7.1) + racc (1.7.1-java) rack (2.2.8) rack-protection (3.1.0) rack (~> 2.2, >= 2.2.4) @@ -159,6 +165,8 @@ GEM 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) @@ -169,13 +177,13 @@ GEM rspec (~> 3) PLATFORMS + universal-java-11 x86_64-darwin-22 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) diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 197bf4a..02a6a85 100644 --- a/lib/vmpooler.rb +++ b/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' diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index e393466..c6351a9 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -25,7 +25,7 @@ module Vmpooler def validate_token(backend) tracer.in_span("Vmpooler::API::Helpers.#{__method__}") do if valid_token?(backend) - backend.hset("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}", 'last', Time.now) + backend.hset("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}", 'last', Time.now.to_s) return true end diff --git a/lib/vmpooler/api/v3.rb b/lib/vmpooler/api/v3.rb index 3b629dc..41c6480 100644 --- a/lib/vmpooler/api/v3.rb +++ b/lib/vmpooler/api/v3.rb @@ -189,8 +189,8 @@ module Vmpooler span.set_attribute('enduser.id', user) has_token_result = has_token? backend.sadd("vmpooler__migrating__#{template}", vm) - backend.hset("vmpooler__active__#{template}", vm, Time.now) - backend.hset("vmpooler__vm__#{vm}", 'checkout', Time.now) + backend.hset("vmpooler__active__#{template}", vm, Time.now.to_s) + backend.hset("vmpooler__vm__#{vm}", 'checkout', Time.now.to_s) if Vmpooler::API.settings.config[:auth] and has_token_result backend.hset("vmpooler__vm__#{vm}", 'token:token', request.env['HTTP_X_AUTH_TOKEN']) @@ -971,7 +971,7 @@ module Vmpooler result['token'] = o[rand(25)] + (0...31).map { o[rand(o.length)] }.join backend.hset("vmpooler__token__#{result['token']}", 'user', @auth.username) - backend.hset("vmpooler__token__#{result['token']}", 'created', Time.now) + backend.hset("vmpooler__token__#{result['token']}", 'created', Time.now.to_s) span = OpenTelemetry::Trace.current_span span.set_attribute('enduser.id', @auth.username) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 25afb44..a4fd3b2 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -167,8 +167,8 @@ module Vmpooler pool_alias = redis.hget("vmpooler__vm__#{vm}", 'pool_alias') redis.pipelined do |pipeline| - pipeline.hset("vmpooler__active__#{pool}", vm, Time.now) - pipeline.hset("vmpooler__vm__#{vm}", 'checkout', Time.now) + pipeline.hset("vmpooler__active__#{pool}", vm, Time.now.to_s) + pipeline.hset("vmpooler__vm__#{vm}", 'checkout', Time.now.to_s) if ondemandrequest_hash['token:token'] pipeline.hset("vmpooler__vm__#{vm}", 'token:token', ondemandrequest_hash['token:token']) pipeline.hset("vmpooler__vm__#{vm}", 'token:user', ondemandrequest_hash['token:user']) @@ -184,10 +184,10 @@ module Vmpooler redis.pipelined do |pipeline| pipeline.hset("vmpooler__boot__#{Date.today}", "#{pool}:#{vm}", finish) # maybe remove as this is never used by vmpooler itself? - pipeline.hset("vmpooler__vm__#{vm}", 'ready', Time.now) + pipeline.hset("vmpooler__vm__#{vm}", 'ready', Time.now.to_s) # last boot time is displayed in API, and used by alarming script - pipeline.hset('vmpooler__lastboot', pool, Time.now) + pipeline.hset('vmpooler__lastboot', pool, Time.now.to_s) end $metrics.timing("time_to_ready_state.#{pool}", finish) @@ -226,7 +226,7 @@ module Vmpooler last_checked_too_soon = ((Time.now - Time.parse(check_stamp)).to_i < $config[:config]['vm_checktime'] * 60) if check_stamp break if check_stamp && last_checked_too_soon - redis.hset("vmpooler__vm__#{vm}", 'check', Time.now) + redis.hset("vmpooler__vm__#{vm}", 'check', Time.now.to_s) # Check if the hosts TTL has expired # if 'boottime' is nil, set bootime to beginning of unix epoch, forces TTL to be assumed expired boottime = redis.hget("vmpooler__vm__#{vm}", 'ready') @@ -911,7 +911,7 @@ 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 @@ -1333,8 +1333,8 @@ module Vmpooler ready = redis.scard("vmpooler__ready__#{pool_name}") pending = redis.scard("vmpooler__pending__#{pool_name}") running = redis.scard("vmpooler__running__#{pool_name}") - - 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) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 742ea54..8561896 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' require 'time' require 'mock_redis' -require 'pry' -require 'pry-byebug' # Custom RSpec :Matchers diff --git a/vmpooler.gemspec b/vmpooler.gemspec index e997b11..c3ba291 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -41,7 +41,6 @@ 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' From ac578eef15415fb1f4d5806837c104fa9d9108bf Mon Sep 17 00:00:00 2001 From: isaac-hammes Date: Thu, 10 Aug 2023 06:15:27 -0700 Subject: [PATCH 3/4] (RE-15162) Update OTEL gems. --- Gemfile.lock | 67 +++++++++++++++++++----------------- lib/vmpooler.rb | 8 ++++- lib/vmpooler/pool_manager.rb | 12 ++++--- vmpooler.gemspec | 16 ++++----- 4 files changed, 58 insertions(+), 45 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d59fc9d..1bd2527 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,13 +6,13 @@ PATH connection_pool (~> 2.4) deep_merge (~> 1.2) net-ldap (~> 0.16) - opentelemetry-exporter-jaeger (= 0.22.0) - opentelemetry-instrumentation-concurrent_ruby (= 0.19.2) - opentelemetry-instrumentation-http_client (= 0.19.4) - opentelemetry-instrumentation-redis (= 0.21.3) - opentelemetry-instrumentation-sinatra (= 0.19.3) - opentelemetry-resource_detectors (= 0.23.0) - opentelemetry-sdk (~> 1.0, >= 1.0.2) + opentelemetry-exporter-jaeger (= 0.23.0) + opentelemetry-instrumentation-concurrent_ruby (= 0.21.1) + opentelemetry-instrumentation-http_client (= 0.22.2) + opentelemetry-instrumentation-redis (= 0.25.3) + opentelemetry-instrumentation-sinatra (= 0.23.2) + opentelemetry-resource_detectors (= 0.24.1) + opentelemetry-sdk (~> 1.3, >= 1.3.0) pickup (~> 0.0.11) prometheus-client (>= 2, < 5) puma (>= 5.0.4, < 7) @@ -47,47 +47,52 @@ GEM json (2.6.3-java) language_server-protocol (3.17.0.3) method_source (1.0.0) - mock_redis (0.36.0) - ruby2_keywords + mock_redis (0.37.0) mustermann (3.0.0) 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-common (0.19.7) + opentelemetry-common (0.20.0) opentelemetry-api (~> 1.0) - opentelemetry-exporter-jaeger (0.22.0) + opentelemetry-exporter-jaeger (0.23.0) opentelemetry-api (~> 1.1) - opentelemetry-common (~> 0.19.6) + opentelemetry-common (~> 0.20) opentelemetry-sdk (~> 1.2) opentelemetry-semantic_conventions thrift - opentelemetry-instrumentation-base (0.19.0) + opentelemetry-instrumentation-base (0.22.2) opentelemetry-api (~> 1.0) - opentelemetry-instrumentation-concurrent_ruby (0.19.2) + opentelemetry-registry (~> 0.1) + opentelemetry-instrumentation-concurrent_ruby (0.21.1) opentelemetry-api (~> 1.0) - opentelemetry-instrumentation-base (~> 0.19.0) - opentelemetry-instrumentation-http_client (0.19.4) + opentelemetry-instrumentation-base (~> 0.22.1) + opentelemetry-instrumentation-http_client (0.22.2) opentelemetry-api (~> 1.0) - opentelemetry-common (~> 0.19.3) - opentelemetry-instrumentation-base (~> 0.19.0) - opentelemetry-instrumentation-redis (0.21.3) + opentelemetry-common (~> 0.20.0) + opentelemetry-instrumentation-base (~> 0.22.1) + opentelemetry-instrumentation-rack (0.23.4) opentelemetry-api (~> 1.0) - opentelemetry-common (~> 0.19.3) - opentelemetry-instrumentation-base (~> 0.19.0) - opentelemetry-instrumentation-sinatra (0.19.3) + opentelemetry-common (~> 0.20.0) + opentelemetry-instrumentation-base (~> 0.22.1) + opentelemetry-instrumentation-redis (0.25.3) opentelemetry-api (~> 1.0) - opentelemetry-common (~> 0.19.3) - opentelemetry-instrumentation-base (~> 0.19.0) + opentelemetry-common (~> 0.20.0) + opentelemetry-instrumentation-base (~> 0.22.1) + opentelemetry-instrumentation-sinatra (0.23.2) + opentelemetry-api (~> 1.0) + opentelemetry-common (~> 0.20.0) + opentelemetry-instrumentation-base (~> 0.22.1) + opentelemetry-instrumentation-rack (~> 0.21) opentelemetry-registry (0.3.0) opentelemetry-api (~> 1.1) - opentelemetry-resource_detectors (0.23.0) + opentelemetry-resource_detectors (0.24.1) google-cloud-env opentelemetry-sdk (~> 1.0) - opentelemetry-sdk (1.2.1) + opentelemetry-sdk (1.3.0) opentelemetry-api (~> 1.1) - opentelemetry-common (~> 0.19.3) + opentelemetry-common (~> 0.20) opentelemetry-registry (~> 0.2) opentelemetry-semantic_conventions opentelemetry-semantic_conventions (1.10.0) @@ -118,7 +123,7 @@ GEM rack (>= 1.3) rainbow (3.1.1) rake (13.0.6) - redis (5.0.6) + redis (5.0.7) redis-client (>= 0.9.0) redis-client (0.15.0) connection_pool @@ -137,7 +142,7 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.12.0) rspec-support (3.12.1) - rubocop (1.54.2) + rubocop (1.55.1) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) @@ -145,7 +150,7 @@ GEM rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.28.0, < 2.0) + rubocop-ast (>= 1.28.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) rubocop-ast (1.29.0) @@ -186,7 +191,7 @@ DEPENDENCIES pry rack-test (>= 0.6) rspec (>= 3.2) - rubocop (~> 1.54.2) + rubocop (~> 1.55.1) simplecov (>= 0.11.2) thor (~> 1.0, >= 1.0.1) vmpooler! diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 02a6a85..5d44d68 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -209,7 +209,13 @@ 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, timeout: 5) + Redis.new( + host: host, + port: port, + password: password, + reconnect_attempts: redis_reconnect_attempts, + connect_timeout: 300, + ) end def self.pools(conf) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index a4fd3b2..3bc020b 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1442,13 +1442,15 @@ module Vmpooler end def create_dns_object(config, logger, metrics, redis_connection_pool, dns_class, dns_name, options) - dns_klass = Vmpooler::PoolManager::Dns - dns_klass.constants.each do |classname| - next unless classname.to_s.casecmp(dns_class) == 0 + if defined?(Vmpooler::PoolManager::Dns) + dns_klass = Vmpooler::PoolManager::Dns + dns_klass.constants.each do |classname| + next unless classname.to_s.casecmp(dns_class) == 0 - return dns_klass.const_get(classname).new(config, logger, metrics, redis_connection_pool, dns_name, options) + return dns_klass.const_get(classname).new(config, logger, metrics, redis_connection_pool, dns_name, options) + end + raise("DNS '#{dns_class}' is unknown for pool with dns name '#{dns_name}'") if dns_klass.nil? end - raise("DNS '#{dns_class}' is unknown for pool with dns name '#{dns_name}'") if dns_klass.nil? end def check_ondemand_requests(maxloop = 0, diff --git a/vmpooler.gemspec b/vmpooler.gemspec index c3ba291..1b367bd 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -20,13 +20,13 @@ Gem::Specification.new do |s| 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' - s.add_dependency 'opentelemetry-instrumentation-concurrent_ruby', '= 0.19.2' - s.add_dependency 'opentelemetry-instrumentation-http_client', '= 0.19.4' - s.add_dependency 'opentelemetry-instrumentation-redis', '= 0.21.3' - s.add_dependency 'opentelemetry-instrumentation-sinatra', '= 0.19.3' - s.add_dependency 'opentelemetry-resource_detectors', '= 0.23.0' - s.add_dependency 'opentelemetry-sdk', '~> 1.0', '>= 1.0.2' + s.add_dependency 'opentelemetry-exporter-jaeger', '= 0.23.0' + s.add_dependency 'opentelemetry-instrumentation-concurrent_ruby', '= 0.21.1' + s.add_dependency 'opentelemetry-instrumentation-http_client', '= 0.22.2' + s.add_dependency 'opentelemetry-instrumentation-redis', '= 0.25.3' + s.add_dependency 'opentelemetry-instrumentation-sinatra', '= 0.23.2' + s.add_dependency 'opentelemetry-resource_detectors', '= 0.24.1' + s.add_dependency 'opentelemetry-sdk', '~> 1.3', '>= 1.3.0' s.add_dependency 'pickup', '~> 0.0.11' s.add_dependency 'prometheus-client', '>= 2', '< 5' s.add_dependency 'puma', '>= 5.0.4', '< 7' @@ -43,7 +43,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'pry' s.add_development_dependency 'rack-test', '>= 0.6' s.add_development_dependency 'rspec', '>= 3.2' - s.add_development_dependency 'rubocop', '~> 1.54.2' + s.add_development_dependency 'rubocop', '~> 1.55.1' s.add_development_dependency 'simplecov', '>= 0.11.2' s.add_development_dependency 'thor', '~> 1.0', '>= 1.0.1' s.add_development_dependency 'yarjuf', '>= 2.0' From e7322577c7c552400231eecbdeb2f7c821fd95c0 Mon Sep 17 00:00:00 2001 From: isaac-hammes Date: Thu, 10 Aug 2023 09:24:17 -0700 Subject: [PATCH 4/4] Fix rubocop --- lib/vmpooler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 5d44d68..985a72b 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -214,7 +214,7 @@ module Vmpooler port: port, password: password, reconnect_attempts: redis_reconnect_attempts, - connect_timeout: 300, + connect_timeout: 300 ) end