-
Notifications
You must be signed in to change notification settings - Fork 78
Enhancement: IP/NIC selection #122
Changes from all commits
6c2141a
8713856
00ef312
000c04c
87790be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,3 +18,4 @@ tags | |
|
||
.vagrant | ||
.vagrant_dns.json | ||
.idea |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,20 @@ | ||
require 'rubygems' | ||
require 'bundler/setup' | ||
require 'bundler/gem_tasks' | ||
require 'rake/testtask' | ||
require 'rubocop/rake_task' | ||
|
||
# Immediately sync all stdout | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you need these changes to the Rakefile? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah sorry, that was me comparing the Rakefile with the one I cooked up for the landrush-ip repository because it wouldn't work with my OS X' Rake. These changes aren't necessary at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make sense, thanks |
||
$stdout.sync = true | ||
$stderr.sync = true | ||
|
||
# Change to the directory of this file | ||
Dir.chdir(File.expand_path('../', __FILE__)) | ||
|
||
# Rubocop | ||
RuboCop::RakeTask.new | ||
|
||
# Tests | ||
Rake::TestTask.new do |t| | ||
t.pattern = 'test/**/*_test.rb' | ||
t.libs << 'test' | ||
|
@@ -15,5 +28,3 @@ task default: [ | |
task :generate_diagrams do | ||
sh 'cd doc; seqdiag --fontmap=support/seqdiag.fontmap -Tsvg vagrant_dns_without_landrush.diag' | ||
end | ||
|
||
RuboCop::RakeTask.new |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# -*- mode: ruby -*- | ||
# vi: set ft=ruby : | ||
|
||
Vagrant.configure('2') do |config| | ||
config.vm.define 'landrush-test-debian' do |machine| | ||
machine.vm.box = 'debian/jessie64' | ||
|
||
# Add a DHCP network so we don't know its IP :P | ||
machine.vm.network 'private_network', type: 'dhcp' | ||
|
||
machine.vm.provider :virtualbox do |provider, _| | ||
provider.memory = 512 | ||
provider.cpus = 2 | ||
end | ||
|
||
machine.landrush_ip.override = true | ||
|
||
machine.vm.hostname = 'landrush-dev' | ||
machine.vm.network 'private_network', type: 'dhcp' | ||
|
||
# Landrush (DNS) | ||
machine.landrush.enabled = true | ||
machine.landrush.tld = 'landrush' | ||
machine.landrush.interface = 'eth1' | ||
machine.landrush.exclude = [/lo[0-9]*/, /docker[0-9]+/, /tun[0-9]+/] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you specify the interface explicitly, do the excludes matter at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, they are still used as a fallback. Basically the mechanic works as follows:
See the tests, I added a test for that too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I guess I just did not see it. Need to take a closer look, once I have this locally and also can run the tests |
||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
module Landrush | ||
module Cap | ||
module All | ||
module ReadHostVisibleIpAddress | ||
def self.filter_addresses(addresses) | ||
unless @machine.config.landrush.exclude.nil? | ||
re = Regexp.union(@machine.config.landrush.exclude) | ||
|
||
addresses = addresses.select do |addr| | ||
!addr['name'].match(re) | ||
end | ||
end | ||
|
||
addresses | ||
end | ||
|
||
def self.read_host_visible_ip_address(machine) | ||
@machine = machine | ||
|
||
addr = nil | ||
addresses = machine.guest.capability(:landrush_ip_get) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just so that I understand correctly, this is the part which "requests" the landrush-ip capability to which we have now a dependency in the Gemfile and which you release from https://github.com/Werelds/landrush-ip, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. And because the cap will be supported on all platforms (at least, I don't think Vagrant has a guest that Go doesn't support), this code will apply to all platforms. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice |
||
|
||
# Short circuit this one first: if an explicit interface is defined, look for it and return it if found. | ||
# Technically, we could do a single loop, but execution time is not vital here. | ||
# This allows us to be more accurate, especially with logging what's going on. | ||
unless machine.config.landrush.interface.nil? | ||
addr = addresses.detect { |a| a['name'] == machine.config.landrush.interface } | ||
|
||
machine.env.ui.warn "[landrush] Unable to find interface #{machine.config.landrush.interface}" if addr.nil? | ||
end | ||
|
||
if addr.nil? | ||
addresses = filter_addresses addresses | ||
|
||
raise 'No addresses found' if addresses.empty? | ||
|
||
addr = addresses.last | ||
end | ||
|
||
ip = IPAddr.new(addr['ipv4']) | ||
|
||
machine.env.ui.info "[landrush] Using #{addr['name']} (#{addr['ipv4']})" | ||
|
||
ip.to_s | ||
end | ||
end | ||
end | ||
end | ||
end |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
require 'test_helper' | ||
|
||
module Landrush | ||
module Cap | ||
module All | ||
describe ReadHostVisibleIpAddress do | ||
let(:machine) { fake_machine } | ||
let(:addresses) { fake_addresses } | ||
|
||
def call_cap(machine) | ||
Landrush::Cap::All::ReadHostVisibleIpAddress.read_host_visible_ip_address(machine) | ||
end | ||
|
||
before do | ||
# TODO: Is there a way to only unstub it for read_host_visible_ip_address | ||
machine.guest.unstub(:capability) | ||
machine.guest.stubs(:capability).with(:landrush_ip_get).returns(fake_addresses) | ||
end | ||
|
||
describe 'read_host_visible_ip_address' do | ||
# First, test with an empty response (no addresses) | ||
it 'should throw an error when there are no addresses' do | ||
machine.guest.stubs(:capability).with(:landrush_ip_get).returns([]) | ||
|
||
lambda do | ||
call_cap(machine) | ||
end.must_raise(RuntimeError, 'No addresses found') | ||
end | ||
|
||
# Step 1: nothing excluded, nothing explicitly selected | ||
it 'should return the last address' do | ||
machine.config.landrush.interface = nil | ||
machine.config.landrush.exclude = [] | ||
|
||
expected = addresses.last['ipv4'] | ||
|
||
# call_cap(machine).must_equal expected | ||
call_cap(machine).must_equal expected | ||
end | ||
|
||
# Test exclusion mechanics; it should select the las | ||
it 'should ignore interfaces that are excluded and select the last not excluded interface' do | ||
machine.config.landrush.interface = nil | ||
machine.config.landrush.exclude = [/exclude[0-9]+/] | ||
|
||
expected = addresses.detect { |a| a['name'] == 'include3' } | ||
expected = expected['ipv4'] | ||
|
||
call_cap(machine).must_equal expected | ||
end | ||
|
||
# Explicitly select one; this supersedes the exclusion mechanic | ||
it 'should select the desired interface' do | ||
machine.config.landrush.interface = 'include1' | ||
machine.config.landrush.exclude = [/exclude[0-9]+/] | ||
|
||
expected = addresses.detect { |a| a['name'] == 'include1' } | ||
expected = expected['ipv4'] | ||
|
||
call_cap(machine).must_equal expected | ||
end | ||
|
||
# Now make sure it returns the last not excluded interface when the desired interface does not exist | ||
it 'should return the last not excluded interface if the desired interface does not exist' do | ||
machine.config.landrush.interface = 'dummy' | ||
machine.config.landrush.exclude = [/exclude[0-9]+/] | ||
|
||
expected = addresses.detect { |a| a['name'] == 'include3' } | ||
expected = expected['ipv4'] | ||
|
||
call_cap(machine).must_equal expected | ||
end | ||
|
||
# Now make sure it returns the last interface overall when nothing is excluded | ||
it 'should return the last interface if the desired interface does not exist' do | ||
machine.config.landrush.interface = 'dummy' | ||
machine.config.landrush.exclude = [] | ||
|
||
expected = addresses.last['ipv4'] | ||
|
||
call_cap(machine).must_equal expected | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 good idea