From d738f35727a863697f4a91494392b749d3aa1143 Mon Sep 17 00:00:00 2001
From: "kirby@puppetlabs.com" <mk.kirby@gmail.com>
Date: Fri, 5 Apr 2019 14:01:18 -0700
Subject: [PATCH] (POOLER-140) Ensure a running VM stays in a queue

This commit updates how a VM is checked out to ensure that there is no window where the VM could be considered discovered, and therefore destroyed. Without this change the VM is retrieved by calling 'spop' on the ready queue, and then adding it to the running queue. This change moves to selecting the VM by retrieving the last member of the set, and moving it with 'smove' from ready to running. As a result of this change vmpooler moves from retrieving the VMs from the ready state randomly, to instead retrieve the oldest VM in the queue. This change should reduce churn where it would otherwise not be required to satisfy demand.
---
 lib/vmpooler/api/v1.rb             |  4 ++--
 spec/integration/api/v1/vm_spec.rb | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb
index b10cc7b..d57def9 100644
--- a/lib/vmpooler/api/v1.rb
+++ b/lib/vmpooler/api/v1.rb
@@ -68,9 +68,9 @@ module Vmpooler
       end
 
       template_backends.each do |template_backend|
-        vm = backend.spop("vmpooler__ready__#{template_backend}")
+        vm = backend.smembers("vmpooler__ready__#{template_backend}")[-1]
         if vm
-          backend.sadd("vmpooler__running__#{template_backend}", vm)
+          backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm)
           return [vm, template_backend, template]
         end
       end
diff --git a/spec/integration/api/v1/vm_spec.rb b/spec/integration/api/v1/vm_spec.rb
index 8eac59f..a7712bd 100644
--- a/spec/integration/api/v1/vm_spec.rb
+++ b/spec/integration/api/v1/vm_spec.rb
@@ -213,6 +213,24 @@ describe Vmpooler::API::V1 do
         expect_json(ok = true, http = 200)
       end
 
+      it 'returns the first VM that was moved to the ready state when checking out a VM' do
+        create_ready_vm 'pool1', '1abcdefghijklmnop'
+        create_ready_vm 'pool1', '2abcdefghijklmnop'
+        create_ready_vm 'pool1', '3abcdefghijklmnop'
+
+        post "#{prefix}/vm", '{"pool1":"1"}'
+
+        expected = {
+          ok: true,
+          "pool1": {
+            "hostname": "1abcdefghijklmnop"
+          }
+        }
+
+        expect(last_response.body).to eq(JSON.pretty_generate(expected))
+        expect_json(ok = true, http = 200)
+      end
+
       it 'fails when not all requested vms can be allocated' do
         create_ready_vm 'pool1', '1abcdefghijklmnop'