From cfd6a5f991f41b214884ac6e472becf16a106b63 Mon Sep 17 00:00:00 2001
From: "kirby@puppetlabs.com" <mk.kirby@gmail.com>
Date: Fri, 6 Nov 2020 13:16:38 -0800
Subject: [PATCH 1/2] (POOLER-193) Mark checked out VM as active

This change sets a VM as running in redis as soon as it is checked out. Without this change when allocated several instances it is possible for a machine that has been allocated for a checkout, but not yet marked as active, to be identified as running when it should not be, which was added in POOLER-191. Without this change a machine may be destroyed during checkout by pool_manager if there are several instances being allocated.

Additionally, redis multi is added for vm checkout operations to minimize the round trips to redis during a checkout operation. Without this addition each VM checkout causes several redis interactions.
---
 lib/vmpooler/api/v1.rb | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb
index 4c53d5f..25ac1cf 100644
--- a/lib/vmpooler/api/v1.rb
+++ b/lib/vmpooler/api/v1.rb
@@ -163,26 +163,31 @@ module Vmpooler
     end
 
     def return_vm_to_ready_state(template, vm)
+      backend.multi
+      backend.srem("vmpooler__migrating__#{template}", vm)
+      backend.hdel("vmpooler__active__#{template}", vm)
+      backend.hdel("vmpooler__vm__#{vm}", 'checkout', 'token:token', 'token:user')
       backend.smove("vmpooler__running__#{template}", "vmpooler__ready__#{template}", vm)
+      backend.exec
     end
 
     def account_for_starting_vm(template, vm)
+      user = backend.hget("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}", 'user')
+      has_token_result = has_token?
+      backend.multi
       backend.sadd("vmpooler__migrating__#{template}", vm)
       backend.hset("vmpooler__active__#{template}", vm, Time.now)
       backend.hset("vmpooler__vm__#{vm}", 'checkout', Time.now)
 
-      if Vmpooler::API.settings.config[:auth] and has_token?
-        validate_token(backend)
-
+      if Vmpooler::API.settings.config[:auth] and has_token_result
         backend.hset("vmpooler__vm__#{vm}", 'token:token', request.env['HTTP_X_AUTH_TOKEN'])
-        backend.hset("vmpooler__vm__#{vm}", 'token:user',
-                     backend.hget("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}", 'user')
-        )
+        backend.hset("vmpooler__vm__#{vm}", 'token:user', user)
 
         if config['vm_lifetime_auth'].to_i > 0
           backend.hset("vmpooler__vm__#{vm}", 'lifetime', config['vm_lifetime_auth'].to_i)
         end
       end
+      backend.exec
     end
 
     def update_result_hosts(result, template, vm)
@@ -200,16 +205,19 @@ module Vmpooler
       failed = false
       vms = []
 
+      validate_token(backend) if Vmpooler::API.settings.config[:auth] and has_token?
+
       payload.each do |requested, count|
         count.to_i.times do |_i|
           vmname, vmpool, vmtemplate = fetch_single_vm(requested)
-          if !vmname
+          if vmname
+            account_for_starting_vm(vmpool, vmname)
+            vms << [vmpool, vmname, vmtemplate]
+            metrics.increment("checkout.success.#{vmtemplate}")
+          else
             failed = true
             metrics.increment("checkout.empty.#{requested}")
             break
-          else
-            vms << [vmpool, vmname, vmtemplate]
-            metrics.increment("checkout.success.#{vmpool}")
           end
         end
       end
@@ -220,8 +228,7 @@ module Vmpooler
         end
         status 503
       else
-        vms.each do |(vmpool, vmname, vmtemplate)|
-          account_for_starting_vm(vmpool, vmname)
+        vms.each do |(_vmpool, vmname, vmtemplate)|
           update_result_hosts(result, vmtemplate, vmname)
         end
 
@@ -1094,7 +1101,7 @@ module Vmpooler
         result[params[:hostname]]['lifetime'] = (rdata['lifetime'] || config['vm_lifetime']).to_i
 
         if rdata['destroy']
-          result[params[:hostname]]['running'] = ((Time.parse(rdata['destroy']) - Time.parse(rdata['checkout'])) / 60 / 60).round(2)
+          result[params[:hostname]]['running'] = ((Time.parse(rdata['destroy']) - Time.parse(rdata['checkout'])) / 60 / 60).round(2) if rdata['checkout']
           result[params[:hostname]]['state'] = 'destroyed'
         elsif rdata['checkout']
           result[params[:hostname]]['running'] = ((Time.now - Time.parse(rdata['checkout'])) / 60 / 60).round(2)

From 21bf9dbc1c1a59e09e8e67fa07b83bd3f24cb254 Mon Sep 17 00:00:00 2001
From: "kirby@puppetlabs.com" <mk.kirby@gmail.com>
Date: Fri, 6 Nov 2020 13:17:52 -0800
Subject: [PATCH 2/2] Fix syntax of validate_token in helpers

This change fixes an error in validate_token where hset does not have the correct number of parameters. Without this change validate_token causes a error.
---
 lib/vmpooler/api/helpers.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb
index 3c9f487..e4b2302 100644
--- a/lib/vmpooler/api/helpers.rb
+++ b/lib/vmpooler/api/helpers.rb
@@ -18,7 +18,7 @@ module Vmpooler
 
       def validate_token(backend)
         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)
 
           return true
         end