From a2d613782a5cc4b1c4c1307e1f59b9d6ce2c0b87 Mon Sep 17 00:00:00 2001
From: Samuel Beaulieu <samuel+14401380@puppet.com>
Date: Tue, 29 Mar 2022 12:01:17 -0500
Subject: [PATCH] remove 'last minute' tcp vm_ready check from the api for
 pooled vms the vm_ready methods should be implemented per provider and
 checking in the api creates issues since provider code is not available.
 Removing this for the V2 api

---
 lib/vmpooler/api/v2.rb                      | 17 +++++---------
 spec/integration/api/v1/vm_spec.rb          |  3 +--
 spec/integration/api/v2/vm_spec.rb          | 25 +--------------------
 spec/integration/api/v2/vm_template_spec.rb |  4 ++--
 4 files changed, 9 insertions(+), 40 deletions(-)

diff --git a/lib/vmpooler/api/v2.rb b/lib/vmpooler/api/v2.rb
index 4cd1a66..d2c17f9 100644
--- a/lib/vmpooler/api/v2.rb
+++ b/lib/vmpooler/api/v2.rb
@@ -73,19 +73,12 @@ module Vmpooler
               next if vms.empty?
 
               vms.reverse.each do |vm|
-                vm_domain = Parsing.get_domain_for_pool(full_config, template_backend)
-                ready = vm_ready?(vm, vm_domain)
-                if ready
-                  smoved = backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm)
-                  if smoved
-                    return [vm, template_backend, template]
-                  else
-                    metrics.increment("checkout.smove.failed.#{template_backend}")
-                    return [nil, nil, nil]
-                  end
+                smoved = backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm)
+                if smoved
+                  return [vm, template_backend, template]
                 else
-                  backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__completed__#{template_backend}", vm)
-                  metrics.increment("checkout.nonresponsive.#{template_backend}")
+                  metrics.increment("checkout.smove.failed.#{template_backend}")
+                  return [nil, nil, nil]
                 end
               end
             end
diff --git a/spec/integration/api/v1/vm_spec.rb b/spec/integration/api/v1/vm_spec.rb
index 0d06585..88d8b57 100644
--- a/spec/integration/api/v1/vm_spec.rb
+++ b/spec/integration/api/v1/vm_spec.rb
@@ -124,8 +124,7 @@ describe Vmpooler::API::V1 do
       end
 
       it 'returns 503 for empty pool referenced by alias' do
-        create_ready_vm 'pool1', vmname, redis
-        post "#{prefix}/vm/poolone"
+        create_ready_vm 'pool2', vmname, redis
         post "#{prefix}/vm/poolone"
 
         expected = { ok: false }
diff --git a/spec/integration/api/v2/vm_spec.rb b/spec/integration/api/v2/vm_spec.rb
index d76f38a..b971ba8 100644
--- a/spec/integration/api/v2/vm_spec.rb
+++ b/spec/integration/api/v2/vm_spec.rb
@@ -129,8 +129,7 @@ describe Vmpooler::API::V2 do
       end
 
       it 'returns 503 for empty pool referenced by alias' do
-        create_ready_vm 'pool1', vmname, redis
-        post "#{prefix}/vm/poolone"
+        create_ready_vm 'pool2', vmname, redis
         post "#{prefix}/vm/poolone"
 
         expected = { ok: false }
@@ -342,28 +341,6 @@ describe Vmpooler::API::V2 do
         expect(pool_has_ready_vm?('pool1', '2abcdefghijklmnop', redis)).to eq(true)
       end
 
-      it 'returns the second VM when the first fails to respond' do
-        create_ready_vm 'pool1', vmname, redis
-        create_ready_vm 'pool1', "2#{vmname}", redis
-
-        allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).with(vmname, 'one.example.com').and_raise('mockerror')
-        allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).with("2#{vmname}", 'one.example.com').and_return(socket)
-
-        post "#{prefix}/vm", '{"pool1":"1"}'
-        expect_json(ok = true, http = 200)
-
-        expected = {
-          ok: true,
-          pool1: {
-            hostname: "2#{vmname}.one.example.com"
-          }
-        }
-
-        expect(last_response.body).to eq(JSON.pretty_generate(expected))
-
-        expect(pool_has_ready_vm?('pool1', vmname, redis)).to be false
-      end
-
       context '(auth not configured)' do
         it 'does not extend VM lifetime if auth token is provided' do
           app.settings.set :config, auth: false
diff --git a/spec/integration/api/v2/vm_template_spec.rb b/spec/integration/api/v2/vm_template_spec.rb
index 1a487a4..862df25 100644
--- a/spec/integration/api/v2/vm_template_spec.rb
+++ b/spec/integration/api/v2/vm_template_spec.rb
@@ -28,7 +28,7 @@ describe Vmpooler::API::V2 do
         pools: [
           {'name' => 'pool1', 'size' => 5},
           {'name' => 'pool2', 'size' => 10},
-          {'name' => 'poolone', 'size' => 0}
+          {'name' => 'poolone', 'size' => 1}
         ],
         statsd: { 'prefix' => 'stats_prefix'},
         alias: { 'poolone' => 'pool1' },
@@ -105,7 +105,7 @@ describe Vmpooler::API::V2 do
       end
 
       it 'returns 503 for empty pool referenced by alias' do
-        create_ready_vm 'pool1', 'abcdefghijklmnop', redis
+        create_ready_vm 'pool2', 'abcdefghijklmnop', redis
         post "#{prefix}/vm/poolone"
 
         expected = { ok: false }