Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(POOLER-140) Ensure a VM is alive at checkout #329

Merged
merged 1 commit into from
Jul 17, 2019
Merged

(POOLER-140) Ensure a VM is alive at checkout #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docker/Dockerfile_local
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not find a way to use allow to stub the return value of the open_socket method without using allow_any_instance_of. I suspect there's something I don't understand with where sinatra is loading the helpers, and that if I were to understand that better there is likely another way to get at this. I'm all for a cleaner solution, but after several days of banging my head against this I did not find one.


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
Open in desktop
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