From d6e948d34d5ee9fb20caee78658b0644ddb44c1f Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Tue, 16 Jul 2019 09:19:47 -0700 Subject: [PATCH] (POOLER-140) Ensure a VM is alive at checkout This commit duplicates the vm_ready? check to the API layer to allow for API to validate that a VM is alive at checkout. Without this change API relies upon the checks in pool_manager validating pools. This change should allow for additional insight into whether a machine is in a ready state and resopnding at checkout time. --- docker/Dockerfile_local | 2 +- lib/vmpooler/api/helpers.rb | 23 +++++++ lib/vmpooler/api/v1.rb | 15 +++-- spec/integration/api/v1/vm_spec.rb | 74 +++++++++++++++++---- spec/integration/api/v1/vm_template_spec.rb | 21 ++++++ 5 files changed, 118 insertions(+), 17 deletions(-) diff --git a/docker/Dockerfile_local b/docker/Dockerfile_local index ce1d3d501..fc6627056 100644 --- a/docker/Dockerfile_local +++ b/docker/Dockerfile_local @@ -15,7 +15,7 @@ COPY ./ ./ ENV RACK_ENV=production -RUN gem install bundler && bundle && gem build vmpooler.gemspec && gem install vmpooler*.gem && \ +RUN gem install bundler -v '2.0.1' && bundle install && gem build vmpooler.gemspec && gem install vmpooler*.gem && \ chmod +x /usr/local/bin/docker-entrypoint.sh ENTRYPOINT ["docker-entrypoint.sh"] diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index 943e5645b..54636ca6d 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -470,6 +470,29 @@ def is_integer?(x) rescue false end + + def open_socket(host, domain = nil, timeout = 1, port = 22, &_block) + Timeout.timeout(timeout) do + target_host = host + target_host = "#{host}.#{domain}" if domain + sock = TCPSocket.new target_host, port + begin + yield sock if block_given? + ensure + sock.close + end + end + end + + def vm_ready?(vm_name, domain = nil) + begin + open_socket(vm_name, domain) + rescue => _err + return false + end + + true + end end end end diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 9a115cc7e..accf5cca8 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -68,10 +68,17 @@ def fetch_single_vm(template) end template_backends.each do |template_backend| - vm = backend.smembers("vmpooler__ready__#{template_backend}")[-1] - if vm - backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm) - return [vm, template_backend, template] + vms = backend.smembers("vmpooler__ready__#{template_backend}") + next if vms.empty? + vms.reverse.each do |vm| + ready = vm_ready?(vm, config[:domain]) + if ready + backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm) + return [vm, template_backend, template] + else + backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__completed__#{template_backend}", vm) + metrics.increment("checkout.nonresponsive.#{template_backend}") + end end end [nil, nil, nil] diff --git a/spec/integration/api/v1/vm_spec.rb b/spec/integration/api/v1/vm_spec.rb index a7712bd46..de1b254a8 100644 --- a/spec/integration/api/v1/vm_spec.rb +++ b/spec/integration/api/v1/vm_spec.rb @@ -15,7 +15,7 @@ def app() { config: { 'site_name' => 'test pooler', - 'vm_lifetime_auth' => 2, + 'vm_lifetime_auth' => 2 }, pools: [ {'name' => 'pool1', 'size' => 5}, @@ -28,6 +28,7 @@ def app() } } let(:current_time) { Time.now } + let(:vmname) { 'abcdefghijkl' } before(:each) do app.settings.set :config, config @@ -39,10 +40,10 @@ def app() describe 'GET /vm/:hostname' do it 'returns correct information on a running vm' do - create_running_vm 'pool1', 'abcdefghijklmnop' - get "#{prefix}/vm/abcdefghijklmnop" + create_running_vm 'pool1', vmname + get "#{prefix}/vm/#{vmname}" expect_json(ok = true, http = 200) - response_body = (JSON.parse(last_response.body)["abcdefghijklmnop"]) + response_body = (JSON.parse(last_response.body)[vmname]) expect(response_body["template"]).to eq("pool1") expect(response_body["lifetime"]).to eq(0) @@ -56,8 +57,11 @@ def app() end describe 'POST /vm' do + + let(:socket) { double('socket') } it 'returns a single VM' do - create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool1', vmname + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) post "#{prefix}/vm", '{"pool1":"1"}' expect_json(ok = true, http = 200) @@ -65,7 +69,7 @@ def app() expected = { ok: true, pool1: { - hostname: 'abcdefghijklmnop' + hostname: vmname } } @@ -73,7 +77,9 @@ def app() end it 'returns a single VM for an alias' do - create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool1', vmname + + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) post "#{prefix}/vm", '{"poolone":"1"}' expect_json(ok = true, http = 200) @@ -81,7 +87,7 @@ def app() expected = { ok: true, poolone: { - hostname: 'abcdefghijklmnop' + hostname: vmname } } @@ -97,7 +103,7 @@ def app() Vmpooler::API.settings.config.delete(:alias) Vmpooler::API.settings.config[:pool_names] = ['pool1', 'pool2'] - create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool1', vmname post "#{prefix}/vm/pool1" post "#{prefix}/vm/pool1" @@ -108,7 +114,7 @@ def app() end it 'returns 503 for empty pool referenced by alias' do - create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool1', vmname post "#{prefix}/vm/poolone" post "#{prefix}/vm/poolone" @@ -119,16 +125,18 @@ def app() end it 'returns multiple VMs' do - create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool1', vmname create_ready_vm 'pool2', 'qrstuvwxyz012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' expect_json(ok = true, http = 200) expected = { ok: true, pool1: { - hostname: 'abcdefghijklmnop' + hostname: vmname }, pool2: { hostname: 'qrstuvwxyz012345' @@ -143,6 +151,8 @@ def app() create_ready_vm 'pool1', '2abcdefghijklmnop' create_ready_vm 'pool2', 'qrstuvwxyz012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"2","pool2":"1"}' expected = { @@ -170,6 +180,8 @@ def app() create_ready_vm 'pool2', '2qrstuvwxyz012345' create_ready_vm 'pool2', '3qrstuvwxyz012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"2","pool2":"3"}' expected = { @@ -197,6 +209,8 @@ def app() create_ready_vm 'pool2', '2abcdefghijklmnop' create_ready_vm 'pool3', '1qrstuvwxyz012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"genericpool":"3"}' expected = { @@ -218,6 +232,8 @@ def app() create_ready_vm 'pool1', '2abcdefghijklmnop' create_ready_vm 'pool1', '3abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"1"}' expected = { @@ -245,6 +261,8 @@ def app() it 'returns any checked out vms to their pools when not all requested vms can be allocated' do create_ready_vm 'pool1', '1abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' expected = { ok: false } @@ -269,6 +287,8 @@ def app() it 'returns any checked out vms to their pools when not all requested vms can be allocated, when requesting multiple instances from a pool' do create_ready_vm 'pool1', '1abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"2","pool2":"1"}' expected = { ok: false } @@ -294,6 +314,8 @@ def app() create_ready_vm 'pool1', '1abcdefghijklmnop' create_ready_vm 'pool1', '2abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"2","pool2":"3"}' expected = { ok: false } @@ -305,12 +327,36 @@ def app() expect(pool_has_ready_vm?('pool1', '2abcdefghijklmnop')).to eq(true) end + it 'returns the second VM when the first fails to respond' do + create_ready_vm 'pool1', vmname + create_ready_vm 'pool1', "2#{vmname}" + + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).with(vmname, nil).and_raise('mockerror') + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).with("2#{vmname}", nil).and_return(socket) + + post "#{prefix}/vm", '{"pool1":"1"}' + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: "2#{vmname}" + } + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + + expect(pool_has_ready_vm?('pool1', vmname)).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 create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"1"}', { 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' } @@ -335,6 +381,8 @@ def app() create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"1"}', { 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' } @@ -356,6 +404,8 @@ def app() app.settings.set :config, auth: true create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm", '{"pool1":"1"}' expect_json(ok = true, http = 200) diff --git a/spec/integration/api/v1/vm_template_spec.rb b/spec/integration/api/v1/vm_template_spec.rb index 73549a592..d94b537af 100644 --- a/spec/integration/api/v1/vm_template_spec.rb +++ b/spec/integration/api/v1/vm_template_spec.rb @@ -28,6 +28,7 @@ def app() } let(:current_time) { Time.now } + let(:socket) { double('socket') } before(:each) do app.settings.set :config, config @@ -41,6 +42,8 @@ def app() it 'returns a single VM' do create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1", '' expect_json(ok = true, http = 200) @@ -57,6 +60,8 @@ def app() it 'returns a single VM for an alias' do create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/poolone", '' expected = { @@ -104,6 +109,8 @@ def app() create_ready_vm 'pool1', 'abcdefghijklmnop' create_ready_vm 'pool2', 'qrstuvwxyz012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1+pool2", '' expect_json(ok = true, http = 200) @@ -128,6 +135,8 @@ def app() create_ready_vm 'pool2', '2qrstuvwxyz012345' create_ready_vm 'pool2', '3qrstuvwxyz012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1+pool1+pool2+pool2+pool2", '' expected = { @@ -161,6 +170,8 @@ def app() it 'returns any checked out vms to their pools when not all requested vms can be allocated' do create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1+pool2", '' expected = { ok: false } @@ -187,6 +198,8 @@ def app() create_ready_vm 'pool1', 'abcdefghijklmnop' create_ready_vm 'pool1', '0123456789012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1+pool1+pool2", '' expected = { ok: false } @@ -214,6 +227,8 @@ def app() create_ready_vm 'pool1', 'abcdefghijklmnop' create_ready_vm 'pool2', '0123456789012345' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1+pool1+pool2+pool2+pool2", '' expected = { ok: false } @@ -231,6 +246,8 @@ def app() create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1", '', { 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' } @@ -255,6 +272,8 @@ def app() create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1", '', { 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' } @@ -276,6 +295,8 @@ def app() app.settings.set :config, auth: true create_ready_vm 'pool1', 'abcdefghijklmnop' + allow_any_instance_of(Vmpooler::API::Helpers).to receive(:open_socket).and_return(socket) + post "#{prefix}/vm/pool1", '' expected = {