Skip to content

Commit

Permalink
Use open socket method for opening socket
Browse files Browse the repository at this point in the history 
This commit updates pool manager to use a method for opening a socket
instead of opening it directly from check_pending_vm. Support is added
for specifying the domain of the VM to connect to, which lays the
groundwork for doing away with the assumption of having DNS search
domains set for vmpooler to move VMs to the ready state.
  • Loading branch information
@mattkirby
mattkirby committed Nov 16, 2016
1 parent 6e2e595 commit c217d73
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions lib/vmpooler/pool_manager.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ def check_pending_vm(vm, pool, timeout, vsphere)

def open_socket(host, domain=nil, timeout=5, port=22)
Timeout.timeout(timeout) do
target_host = vm
target_host = "#{vm}.#{domain}" if domain
TCPSocket.new target_host, port
target_host = host
target_host = "#{host}.#{domain}" if domain
sock = TCPSocket.new target_host, port
sock

This comment has been minimized.

Sign in to view

Copy link
@joshcooper

joshcooper Nov 17, 2016

When are these sockets closed? Are we leaking file descriptors? (not new in this PR, just commenting on existing behavior). Since we're just testing if the other end is reachable, we probably want to do something like this:

def open_socket(host, domain=nil, timeout=5, port=22, &block)
  ...
  sock = TCPSocket.new target_host, port
  begin
    yield sock if block_given?
  ensure
    sock.close
  end
end

This comment has been minimized.

Sign in to view

Copy link
@mattkirby

mattkirby Nov 17, 2016 •
edited
Loading

Author Contributor

Good point. I have added a block like this so connections will be closed.
end
end

Expand All @@ -39,9 +40,7 @@ def _check_pending_vm(vm, pool, timeout, vsphere)

if host
begin
Timeout.timeout(5) do
TCPSocket.new vm, 22
end
open_socket vm
move_pending_vm_to_ready(vm, pool, host)
rescue
fail_pending_vm(vm, pool, timeout)
Expand Down

0 comments on commit c217d73

Please sign in to comment.