Skip to content

Commit

Permalink
(POOLER-140) Ensure a VM is alive at checkout
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@mattkirby
mattkirby committed Jul 17, 2019
1 parent a755d8d commit d6e948d
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 17 deletions.
2 changes: 1 addition & 1 deletion docker/Dockerfile_local
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
23 changes: 23 additions & 0 deletions lib/vmpooler/api/helpers.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 11 additions & 4 deletions lib/vmpooler/api/v1.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
74 changes: 62 additions & 12 deletions spec/integration/api/v1/vm_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def app()
{
config: {
'site_name' => 'test pooler',
'vm_lifetime_auth' => 2,
'vm_lifetime_auth' => 2
},
pools: [
{'name' => 'pool1', 'size' => 5},
Expand All @@ -28,6 +28,7 @@ def app()
}
}
let(:current_time) { Time.now }
let(:vmname) { 'abcdefghijkl' }

before(:each) do
app.settings.set :config, config
Expand All @@ -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)
Expand All @@ -56,32 +57,37 @@ 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)

expected = {
ok: true,
pool1: {
hostname: 'abcdefghijklmnop'
hostname: vmname
}
}

expect(last_response.body).to eq(JSON.pretty_generate(expected))
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)

expected = {
ok: true,
poolone: {
hostname: 'abcdefghijklmnop'
hostname: vmname
}
}

Expand All @@ -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"

Expand All @@ -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"

Expand All @@ -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'
Expand All @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand Down Expand Up @@ -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 }
Expand All @@ -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 }
Expand All @@ -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 }
Expand All @@ -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'
}
Expand All @@ -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'
}
Expand All @@ -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)

Expand Down
21 changes: 21 additions & 0 deletions spec/integration/api/v1/vm_template_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def app()
}

let(:current_time) { Time.now }
let(:socket) { double('socket') }

before(:each) do
app.settings.set :config, config
Expand All @@ -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)

Expand All @@ -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 = {
Expand Down Expand Up @@ -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)

Expand All @@ -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 = {
Expand Down Expand Up @@ -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 }
Expand All @@ -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 }
Expand Down Expand Up @@ -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 }
Expand All @@ -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'
}
Expand All @@ -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'
}
Expand All @@ -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 = {
Expand Down

0 comments on commit d6e948d

Please sign in to comment.