Merge pull request #489 from puppetlabs/fix-deprecation-redis

(maint) Fix deprecation warning for redis ruby library

lib/vmpooler/api/helpers.rb

@@ -147,12 +147,12 @@ module Vmpooler
 
       def export_tags(backend, hostname, tags)
         tracer.in_span("Vmpooler::API::Helpers.#{__method__}") do
-          backend.pipelined do
+          backend.pipelined do |pipeline|
             tags.each_pair do |tag, value|
               next if value.nil? or value.empty?
 
-              backend.hset("vmpooler__vm__#{hostname}", "tag:#{tag}", value)
+              pipeline.hset("vmpooler__vm__#{hostname}", "tag:#{tag}", value)
-              backend.hset("vmpooler__tag__#{Date.today}", "#{hostname}:#{tag}", value)
+              pipeline.hset("vmpooler__tag__#{Date.today}", "#{hostname}:#{tag}", value)
             end
           end
         end
@@ -201,9 +201,9 @@ module Vmpooler
         tracer.in_span("Vmpooler::API::Helpers.#{__method__}") do
           # using pipelined is much faster than querying each of the pools and adding them
           # as we get the result.
-          res = backend.pipelined do
+          res = backend.pipelined do |pipeline|
             pools.each do |pool|
-              backend.scard(key + pool['name'])
+              pipeline.scard(key + pool['name'])
             end
           end
           res.inject(0) { |m, x| m + x }.to_i
@@ -217,9 +217,9 @@ module Vmpooler
           # using pipelined is much faster than querying each of the pools and adding them
           # as we get the result.
           temp_hash = {}
-          res = backend.pipelined do
+          res = backend.pipelined do |pipeline|
             pools.each do |pool|
-              backend.scard(key + pool['name'])
+              pipeline.scard(key + pool['name'])
             end
           end
           pools.each_with_index do |pool, i|
@@ -236,9 +236,9 @@ module Vmpooler
           # using pipelined is much faster than querying each of the pools and adding them
           # as we get the result.
           temp_hash = {}
-          res = backend.pipelined do
+          res = backend.pipelined do |pipeline|
             pools.each do |pool|
-              backend.hget(key, pool['name'])
+              pipeline.hget(key, pool['name'])
             end
           end
           pools.each_with_index do |pool, i|

lib/vmpooler/pool_manager.rb

@@ -148,15 +148,15 @@ module Vmpooler
         end
         pool_alias = redis.hget("vmpooler__vm__#{vm}", 'pool_alias')
 
-        redis.pipelined do
+        redis.pipelined do |pipeline|
-          redis.hset("vmpooler__active__#{pool}", vm, Time.now)
+          pipeline.hset("vmpooler__active__#{pool}", vm, Time.now)
-          redis.hset("vmpooler__vm__#{vm}", 'checkout', Time.now)
+          pipeline.hset("vmpooler__vm__#{vm}", 'checkout', Time.now)
           if ondemandrequest_hash['token:token']
-            redis.hset("vmpooler__vm__#{vm}", 'token:token', ondemandrequest_hash['token:token'])
+            pipeline.hset("vmpooler__vm__#{vm}", 'token:token', ondemandrequest_hash['token:token'])
-            redis.hset("vmpooler__vm__#{vm}", 'token:user', ondemandrequest_hash['token:user'])
+            pipeline.hset("vmpooler__vm__#{vm}", 'token:user', ondemandrequest_hash['token:user'])
-            redis.hset("vmpooler__vm__#{vm}", 'lifetime', $config[:config]['vm_lifetime_auth'].to_i)
+            pipeline.hset("vmpooler__vm__#{vm}", 'lifetime', $config[:config]['vm_lifetime_auth'].to_i)
           end
-          redis.sadd("vmpooler__#{request_id}__#{pool_alias}__#{pool}", vm)
+          pipeline.sadd("vmpooler__#{request_id}__#{pool_alias}__#{pool}", vm)
         end
         move_vm_queue(pool, vm, 'pending', 'running', redis)
         check_ondemand_request_ready(request_id, redis)
@@ -164,12 +164,12 @@ module Vmpooler
         redis.smove("vmpooler__pending__#{pool}", "vmpooler__ready__#{pool}", vm)
       end
 
-      redis.pipelined do
+      redis.pipelined do |pipeline|
-        redis.hset("vmpooler__boot__#{Date.today}", "#{pool}:#{vm}", finish) # maybe remove as this is never used by vmpooler itself?
+        pipeline.hset("vmpooler__boot__#{Date.today}", "#{pool}:#{vm}", finish) # maybe remove as this is never used by vmpooler itself?
-        redis.hset("vmpooler__vm__#{vm}", 'ready', Time.now)
+        pipeline.hset("vmpooler__vm__#{vm}", 'ready', Time.now)
 
         # last boot time is displayed in API, and used by alarming script
-        redis.hset('vmpooler__lastboot', pool, Time.now)
+        pipeline.hset('vmpooler__lastboot', pool, Time.now)
       end
 
       $metrics.timing("time_to_ready_state.#{pool}", finish)
@@ -418,9 +418,9 @@ module Vmpooler
           finish = format('%<time>.2f', time: Time.now - start)
 
           @redis.with_metrics do |redis|
-            redis.pipelined do
+            redis.pipelined do |pipeline|
-              redis.hset("vmpooler__clone__#{Date.today}", "#{pool_name}:#{new_vmname}", finish)
+              pipeline.hset("vmpooler__clone__#{Date.today}", "#{pool_name}:#{new_vmname}", finish)
-              redis.hset("vmpooler__vm__#{new_vmname}", 'clone_time', finish)
+              pipeline.hset("vmpooler__vm__#{new_vmname}", 'clone_time', finish)
             end
           end
           $logger.log('s', "[+] [#{pool_name}] '#{new_vmname}' cloned in #{finish} seconds")
@@ -428,10 +428,10 @@ module Vmpooler
           $metrics.timing("clone.#{pool_name}", finish)
         rescue StandardError
           @redis.with_metrics do |redis|
-            redis.pipelined do
+            redis.pipelined do |pipeline|
-              redis.srem("vmpooler__pending__#{pool_name}", new_vmname)
+              pipeline.srem("vmpooler__pending__#{pool_name}", new_vmname)
               expiration_ttl = $config[:redis]['data_ttl'].to_i * 60 * 60
-              redis.expire("vmpooler__vm__#{new_vmname}", expiration_ttl)
+              pipeline.expire("vmpooler__vm__#{new_vmname}", expiration_ttl)
             end
           end
           raise
@@ -462,12 +462,12 @@ module Vmpooler
 
       mutex.synchronize do
         @redis.with_metrics do |redis|
-          redis.pipelined do
+          redis.pipelined do |pipeline|
-            redis.hdel("vmpooler__active__#{pool}", vm)
+            pipeline.hdel("vmpooler__active__#{pool}", vm)
-            redis.hset("vmpooler__vm__#{vm}", 'destroy', Time.now)
+            pipeline.hset("vmpooler__vm__#{vm}", 'destroy', Time.now)
 
             # Auto-expire metadata key
-            redis.expire("vmpooler__vm__#{vm}", ($config[:redis]['data_ttl'].to_i * 60 * 60))
+            pipeline.expire("vmpooler__vm__#{vm}", ($config[:redis]['data_ttl'].to_i * 60 * 60))
           end
 
           start = Time.now
@@ -1204,19 +1204,19 @@ module Vmpooler
               pool_check_response[:destroyed_vms] += 1
               destroy_vm(vm, pool_name, provider)
             rescue StandardError => e
-              redis.pipelined do
+              redis.pipelined do |pipeline|
-                redis.srem("vmpooler__completed__#{pool_name}", vm)
+                pipeline.srem("vmpooler__completed__#{pool_name}", vm)
-                redis.hdel("vmpooler__active__#{pool_name}", vm)
+                pipeline.hdel("vmpooler__active__#{pool_name}", vm)
-                redis.del("vmpooler__vm__#{vm}")
+                pipeline.del("vmpooler__vm__#{vm}")
               end
               $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating completed VMs: #{e}")
             end
           else
             $logger.log('s', "[!] [#{pool_name}] '#{vm}' not found in inventory, removed from 'completed' queue")
-            redis.pipelined do
+            redis.pipelined do |pipeline|
-              redis.srem("vmpooler__completed__#{pool_name}", vm)
+              pipeline.srem("vmpooler__completed__#{pool_name}", vm)
-              redis.hdel("vmpooler__active__#{pool_name}", vm)
+              pipeline.hdel("vmpooler__active__#{pool_name}", vm)
-              redis.del("vmpooler__vm__#{vm}")
+              pipeline.del("vmpooler__vm__#{vm}")
             end
           end
         end
@@ -1432,12 +1432,12 @@ module Vmpooler
       score = redis.zscore('vmpooler__provisioning__request', request_id)
       requested = requested.split(',')
 
-      redis.pipelined do
+      redis.pipelined do |pipeline|
         requested.each do |request|
-          redis.zadd('vmpooler__odcreate__task', Time.now.to_i, "#{request}:#{request_id}")
+          pipeline.zadd('vmpooler__odcreate__task', Time.now.to_i, "#{request}:#{request_id}")
         end
-        redis.zrem('vmpooler__provisioning__request', request_id)
+        pipeline.zrem('vmpooler__provisioning__request', request_id)
-        redis.zadd('vmpooler__provisioning__processing', score, request_id)
+        pipeline.zadd('vmpooler__provisioning__processing', score, request_id)
       end
     end
 
@@ -1467,9 +1467,9 @@ module Vmpooler
             redis.incr('vmpooler__tasks__ondemandclone')
             clone_vm(pool, provider, request_id, pool_alias)
           end
-          redis.pipelined do
+          redis.pipelined do |pipeline|
-            redis.zrem(queue_key, request)
+            pipeline.zrem(queue_key, request)
-            redis.zadd(queue_key, score, "#{pool_alias}:#{pool}:#{remaining_count}:#{request_id}")
+            pipeline.zadd(queue_key, score, "#{pool_alias}:#{pool}:#{remaining_count}:#{request_id}")
           end
         end
       end
@@ -1520,10 +1520,10 @@ module Vmpooler
 
       $logger.log('s', "Ondemand request for '#{request_id}' failed to provision all instances within the configured ttl '#{ondemand_request_ttl}'")
       expiration_ttl = $config[:redis]['data_ttl'].to_i * 60 * 60
-      redis.pipelined do
+      redis.pipelined do |pipeline|
-        redis.zrem('vmpooler__provisioning__processing', request_id)
+        pipeline.zrem('vmpooler__provisioning__processing', request_id)
-        redis.hset("vmpooler__odrequest__#{request_id}", 'status', 'failed')
+        pipeline.hset("vmpooler__odrequest__#{request_id}", 'status', 'failed')
-        redis.expire("vmpooler__odrequest__#{request_id}", expiration_ttl)
+        pipeline.expire("vmpooler__odrequest__#{request_id}", expiration_ttl)
       end
       remove_vms_for_failed_request(request_id, expiration_ttl, redis)
       true
@@ -1533,11 +1533,11 @@ module Vmpooler
       request_hash = redis.hgetall("vmpooler__odrequest__#{request_id}")
       Parsing.get_platform_pool_count(request_hash['requested']) do |platform_alias, pool, _count|
         pools_filled = redis.smembers("vmpooler__#{request_id}__#{platform_alias}__#{pool}")
-        redis.pipelined do
+        redis.pipelined do |pipeline|
           pools_filled&.each do |vm|
-            move_vm_queue(pool, vm, 'running', 'completed', redis, "moved to completed queue. '#{request_id}' could not be filled in time")
+            move_vm_queue(pool, vm, 'running', 'completed', pipeline, "moved to completed queue. '#{request_id}' could not be filled in time")
           end
-          redis.expire("vmpooler__#{request_id}__#{platform_alias}__#{pool}", expiration_ttl)
+          pipeline.expire("vmpooler__#{request_id}__#{platform_alias}__#{pool}", expiration_ttl)
         end
       end
     end

spec/unit/pool_manager_spec.rb

@@ -478,10 +478,10 @@ EOT
         it 'should specify auth data on the vm' do
           redis_connection_pool.with do |redis|
             allow(redis).to receive(:hset)
-            expect(redis).to receive(:hset).with("vmpooler__vm__#{vm}", 'token:token', token)
-            expect(redis).to receive(:hset).with("vmpooler__vm__#{vm}", 'token:user', user)
 
             subject.move_pending_vm_to_ready(vm, pool, redis, request_id)
+            expect(redis.hget("vmpooler__vm__#{vm}", 'token:token')).to eq(token)
+            expect(redis.hget("vmpooler__vm__#{vm}", 'token:user')).to eq(user)
           end
         end
       end
@@ -970,7 +970,9 @@ EOT
 
       it 'should expire the vm metadata' do
         redis_connection_pool.with do |redis|
-          expect(redis).to receive(:expire)
+          redis.pipelined do |pipe|
+            expect(pipe).to receive(:expire)
+          end
           expect{subject._clone_vm(pool,provider)}.to raise_error(/MockError/)
         end
       end
@@ -1074,7 +1076,9 @@ EOT
 
       it 'should call redis expire with 0' do
         redis_connection_pool.with do |redis|
-          expect(redis).to receive(:expire).with("vmpooler__vm__#{vm}", 0)
+          redis.pipelined do |pipe|
+            expect(pipe).to receive(:expire).with("vmpooler__vm__#{vm}", 0)
+          end
           subject._destroy_vm(vm,pool,provider)
         end
       end
@@ -4648,16 +4652,20 @@ EOT
 
       it 'creates tasks for instances to be provisioned' do
         redis_connection_pool.with do |redis|
-          allow(redis).to receive(:zadd)
+          redis.pipelined do |pipe|
-          expect(redis).to receive(:zadd).with('vmpooler__odcreate__task', current_time.to_i, "#{request_string}:#{request_id}")
+            allow(pipe).to receive(:zadd)
+            expect(pipe).to receive(:zadd).with('vmpooler__odcreate__task', current_time.to_i, "#{request_string}:#{request_id}")
+          end
           subject.create_ondemand_vms(request_id, redis)
         end
       end
 
       it 'adds a member to provisioning__processing' do
         redis_connection_pool.with do |redis|
-          allow(redis).to receive(:zadd)
+          redis.pipelined do |pipe|
-          expect(redis).to receive(:zadd).with('vmpooler__provisioning__processing', current_time.to_i, request_id)
+            allow(pipe).to receive(:zadd)
+            expect(pipe).to receive(:zadd).with('vmpooler__provisioning__processing', current_time.to_i, request_id)
+          end
           subject.create_ondemand_vms(request_id, redis)
         end
       end
@@ -4712,7 +4720,9 @@ EOT
 
       it 'should add the remaining number back as a new create task with the same score' do
         redis_connection_pool.with do |redis|
-          expect(redis).to receive(:zadd).with('vmpooler__odcreate__task', current_time.to_i, "#{request_string_remaining}:#{request_id}")
+          redis.pipelined do |pipe|
+            expect(pipe).to receive(:zadd).with('vmpooler__odcreate__task', current_time.to_i, "#{request_string_remaining}:#{request_id}")
+          end
           subject.process_ondemand_vms(redis)
         end
       end
@@ -4922,21 +4932,27 @@ EOT
 
       it 'removes the request from processing requests' do
         redis_connection_pool.with do |redis|
-          expect(redis).to receive(:zrem).with('vmpooler__provisioning__processing', request_id)
+          redis.pipelined do |pipe|
+            expect(pipe).to receive(:zrem).with('vmpooler__provisioning__processing', request_id)
+          end
           subject.request_expired?(request_id, expired_time, redis)
         end
       end
 
       it 'sets the status as failed on the request hash' do
         redis_connection_pool.with do |redis|
-          expect(redis).to receive(:hset).with("vmpooler__odrequest__#{request_id}", 'status', 'failed')
+          redis.pipelined do |pipe|
+            expect(pipe).to receive(:hset).with("vmpooler__odrequest__#{request_id}", 'status', 'failed')
+          end
           subject.request_expired?(request_id, expired_time, redis)
         end
       end
 
       it 'marks the request hash for expiration' do
         redis_connection_pool.with do |redis|
-          expect(redis).to receive(:expire).with("vmpooler__odrequest__#{request_id}", expiration_ttl * 60 * 60)
+          redis.pipelined do |pipe|
+            expect(pipe).to receive(:expire).with("vmpooler__odrequest__#{request_id}", expiration_ttl * 60 * 60)
+          end
           subject.request_expired?(request_id, expired_time, redis)
         end
       end
@@ -4975,14 +4991,18 @@ EOT
 
       it 'should remove the associated vms' do
         redis_connection_pool.with do |redis|
-          expect(subject).to receive(:move_vm_queue).with(pool, String, 'running', 'completed', redis, "moved to completed queue. '#{request_id}' could not be filled in time").twice
+          redis.pipelined do |pipe|
+            expect(subject).to receive(:move_vm_queue).with(pool, String, 'running', 'completed', pipe, "moved to completed queue. '#{request_id}' could not be filled in time").twice
+          end
           subject.remove_vms_for_failed_request(request_id, expiration_ttl, redis)
         end
       end
 
       it 'should mark the ready set for expiration' do
         redis_connection_pool.with do |redis|
-          expect(redis).to receive(:expire).with("vmpooler__#{request_id}__#{platform_alias}__#{pool}", expiration_ttl)
+          redis.pipelined do |pipe|
+            expect(pipe).to receive(:expire).with("vmpooler__#{request_id}__#{platform_alias}__#{pool}", expiration_ttl)
+          end
           subject.remove_vms_for_failed_request(request_id, expiration_ttl, redis)
         end
       end