Skip to content

Commit

Permalink
add ipv6 support manual networking
Browse files Browse the repository at this point in the history
- switches IpAddress model to store ip addresses as strings
- renamed column to address_str to avoid confusion that address returns int string

Signed-off-by: dmitriy kalinin <dkalinin@pivotal.io>
  • Loading branch information
Brian Cunnie authored and cunnie committed Sep 2, 2017
1 parent be30c4c commit 4a35c4b
Show file tree
Hide file tree
Showing 19 changed files with 568 additions and 49 deletions.
@@ -0,0 +1,18 @@
Sequel.migration do
change do
alter_table :ip_addresses do
add_column(:address_temp, String, unique: true)
end

self[:ip_addresses].each do |ip_address|
self[:ip_addresses].filter(:id => ip_address[:id]).update(address_temp: ip_address[:address].to_s)
end

alter_table :ip_addresses do
drop_column :address
rename_column :address_temp, :address_str
add_index [:address_str], unique: true, address_str: 'unique_address_str'
set_column_not_null :address_str
end
end
end
1 change: 1 addition & 0 deletions src/bosh-director/db/migrations/migration_digests.json
Expand Up @@ -120,5 +120,6 @@
"20170804191205_add_deployment_and_errand_name_to_errand_runs": "4429f1e811e064c236ee4c32b3964992ec7004e5", "20170804191205_add_deployment_and_errand_name_to_errand_runs": "4429f1e811e064c236ee4c32b3964992ec7004e5",
"20170815175515_change_variable_ids_to_bigint": "0488ede3a8d8611f8414e6a9c4f54ec9828a3bff", "20170815175515_change_variable_ids_to_bigint": "0488ede3a8d8611f8414e6a9c4f54ec9828a3bff",
"20170821141953_remove_unused_credentials_json_columns": "0daf4d1363029304fc84d0df7b7b69fa04713e37", "20170821141953_remove_unused_credentials_json_columns": "0daf4d1363029304fc84d0df7b7b69fa04713e37",
"20170825141953_change_address_to_be_string_for_ipv6": "bfc9534474853bb9f729b8725675bd030c8fb4a4",
"20170828174622_add_spec_json_to_templates": "ecd7869cd86c451b0ff134aff944b6e2c22f9b31" "20170828174622_add_spec_json_to_templates": "ecd7869cd86c451b0ff134aff944b6e2c22f9b31"
} }
Expand Up @@ -11,7 +11,7 @@ def initialize(logger)
def delete(ip, _) def delete(ip, _)
cidr_ip = CIDRIP.new(ip) cidr_ip = CIDRIP.new(ip)


ip_address = Bosh::Director::Models::IpAddress.first(address: cidr_ip.to_i) ip_address = Bosh::Director::Models::IpAddress.first(address_str: cidr_ip.to_i.to_s)


if ip_address if ip_address
@logger.debug("Releasing ip '#{cidr_ip}'") @logger.debug("Releasing ip '#{cidr_ip}'")
Expand Down Expand Up @@ -73,7 +73,11 @@ def try_to_allocate_dynamic_ip(reservation, subnet)
.reject {|a| a < first_range_address } .reject {|a| a < first_range_address }
.sort .sort
.find { |a| !addresses_we_cant_allocate.include?(a+1) } .find { |a| !addresses_we_cant_allocate.include?(a+1) }
ip_address = NetAddr::CIDRv4.new(addr+1) if subnet.range.version == 6
ip_address = NetAddr::CIDRv6.new(addr+1)
else
ip_address = NetAddr::CIDRv4.new(addr+1)
end


unless subnet.range == ip_address || subnet.range.contains?(ip_address) unless subnet.range == ip_address || subnet.range.contains?(ip_address)
raise NoMoreIPsAvailableAndStopRetrying raise NoMoreIPsAvailableAndStopRetrying
Expand All @@ -85,14 +89,14 @@ def try_to_allocate_dynamic_ip(reservation, subnet)
end end


def all_ip_addresses def all_ip_addresses
Bosh::Director::Models::IpAddress.select(:address).all.map { |a| a.address } Bosh::Director::Models::IpAddress.select(:address_str).all.map { |a| a.address_str.to_i }
end end


def reserve_with_instance_validation(instance_model, ip, reservation, is_static) def reserve_with_instance_validation(instance_model, ip, reservation, is_static)
# try to save IP first before validating its instance to prevent race conditions # try to save IP first before validating its instance to prevent race conditions
save_ip(ip, reservation, is_static) save_ip(ip, reservation, is_static)
rescue IpFoundInDatabaseAndCanBeRetried rescue IpFoundInDatabaseAndCanBeRetried
ip_address = Bosh::Director::Models::IpAddress.first(address: ip.to_i) ip_address = Bosh::Director::Models::IpAddress.first(address_str: ip.to_i.to_s)


retry unless ip_address retry unless ip_address


Expand All @@ -119,7 +123,7 @@ def validate_instance_and_update_reservation_type(instance_model, ip, ip_address


def save_ip(ip, reservation, is_static) def save_ip(ip, reservation, is_static)
reservation.instance_model.add_ip_address( reservation.instance_model.add_ip_address(
address: ip.to_i, address_str: ip.to_i.to_s,
network_name: reservation.network.name, network_name: reservation.network.name,
task_id: Bosh::Director::Config.current_job.task_id, task_id: Bosh::Director::Config.current_job.task_id,
static: is_static static: is_static
Expand Down
Expand Up @@ -22,7 +22,7 @@ def self.parse(network_name, subnet_spec, availability_zones, legacy_reserved_ra


netmask = range.wildcard_mask netmask = range.wildcard_mask
network_id = range.network(:Objectify => true) network_id = range.network(:Objectify => true)
broadcast = range.broadcast(:Objectify => true) broadcast = range.version == 6 ? range.last(:Objectify => true) : range.broadcast(:Objectify => true)


ignore_missing_gateway = Bosh::Director::Config.ignore_missing_gateway ignore_missing_gateway = Bosh::Director::Config.ignore_missing_gateway
gateway_property = safe_property(subnet_spec, "gateway", class: String, optional: ignore_missing_gateway) gateway_property = safe_property(subnet_spec, "gateway", class: String, optional: ignore_missing_gateway)
Expand Down
13 changes: 11 additions & 2 deletions src/bosh-director/lib/bosh/director/models/ip_address.rb
Expand Up @@ -5,8 +5,9 @@ class IpAddress < Sequel::Model(Bosh::Director::Config.db)
def validate def validate
validates_presence :instance_id validates_presence :instance_id
validates_presence :task_id validates_presence :task_id
validates_presence :address validates_presence :address_str
validates_unique :address validates_unique :address_str
raise "Invalid type for address_str column" unless address_str.is_a?(String)
end end


def before_create def before_create
Expand All @@ -15,6 +16,7 @@ def before_create


def info def info
instance_info = "#{self.instance.deployment.name}.#{self.instance.job}/#{self.instance.index}" instance_info = "#{self.instance.deployment.name}.#{self.instance.job}/#{self.instance.index}"
formatted_ip = NetAddr::CIDR.create(address_str.to_i).ip
"#{instance_info} - #{self.network_name} - #{formatted_ip} (#{type})" "#{instance_info} - #{self.network_name} - #{formatted_ip} (#{type})"
end end


Expand All @@ -26,6 +28,13 @@ def type
self.static ? 'static' : 'dynamic' self.static ? 'static' : 'dynamic'
end end


def address
unless address_str.to_s =~ /\A\d+\z/
raise "Unexpected address '#{address_str}' (#{info rescue "missing info"})"
end
address_str.to_i
end

def to_s def to_s
info info
end end
Expand Down
2 changes: 1 addition & 1 deletion src/bosh-director/spec/blueprints.rb
Expand Up @@ -102,7 +102,7 @@ module Bosh::Director::Models
end end


IpAddress.blueprint do IpAddress.blueprint do
address { NetAddr::CIDR.create(Sham.ip) } address_str { NetAddr::CIDR.create(Sham.ip).to_i.to_s }
instance { Instance.make } instance { Instance.make }
static { false } static { false }
network_name { Sham.name } network_name { Sham.name }
Expand Down
Expand Up @@ -4,6 +4,7 @@
module Bosh::Director module Bosh::Director
module Api module Api
describe Controllers::DeploymentsController do describe Controllers::DeploymentsController do
include IpUtil
include Rack::Test::Methods include Rack::Test::Methods


subject(:app) { described_class.new(config) } subject(:app) { described_class.new(config) }
Expand Down Expand Up @@ -759,7 +760,7 @@ def manifest_with_errand(deployment_name='errand')
ip_addresses_params = { ip_addresses_params = {
'instance_id' => instance.id, 'instance_id' => instance.id,
'task_id' => "#{i}", 'task_id' => "#{i}",
'address' => NetAddr::CIDR.create("1.2.3.#{i}"), 'address_str' => ip_to_i("1.2.3.#{i}").to_s,
} }
Models::IpAddress.create(ip_addresses_params) Models::IpAddress.create(ip_addresses_params)
end end
Expand Down Expand Up @@ -943,7 +944,7 @@ def manifest_with_errand(deployment_name='errand')
ip_addresses_params = { ip_addresses_params = {
'instance_id' => instance.id, 'instance_id' => instance.id,
'task_id' => "#{i}", 'task_id' => "#{i}",
'address' => NetAddr::CIDR.create("1.2.3.#{i}"), 'address_str' => ip_to_i("1.2.3.#{i}").to_s,
} }
Models::IpAddress.create(ip_addresses_params) Models::IpAddress.create(ip_addresses_params)
end end
Expand Down
@@ -0,0 +1,27 @@
require_relative '../../../../db_spec_helper'
require 'netaddr'

module Bosh::Director
describe 'change address column in IP address to be a string to record IPv6 addresses' do
let(:db) { DBSpecHelper.db }
let(:migration_file) { '20170825141953_change_address_to_be_string_for_ipv6.rb' }

before { DBSpecHelper.migrate_all_before(migration_file) }

it 'allows instance_id to be null' do
db[:ip_addresses] << {id: 1, instance_id: nil, address: NetAddr::CIDR.create("192.168.50.6").to_i}

DBSpecHelper.migrate(migration_file)

expect(db[:ip_addresses].first[:address_str]).to eq(NetAddr::CIDR.create("192.168.50.6").to_i.to_s)

expect {
db[:ip_addresses] << {id: 2, instance_id: nil, address_str: NetAddr::CIDR.create("192.168.50.6").to_i.to_s}
}.to raise_error(Sequel::UniqueConstraintViolation, /ip_addresses.address/)

expect {
db[:ip_addresses] << {id: 3, instance_id: nil, address_str: nil}
}.to raise_error(Sequel::NotNullConstraintViolation, /ip_addresses.address/)
end
end
end
Expand Up @@ -30,9 +30,9 @@ module Bosh::Director
let(:ip2) { NetAddr::CIDR.create('192.168.0.2').to_i } let(:ip2) { NetAddr::CIDR.create('192.168.0.2').to_i }


before do before do
ip_model1 = Models::IpAddress.make(address: ip1, network_name: 'fake-network') ip_model1 = Models::IpAddress.make(address_str: ip1.to_s, network_name: 'fake-network')
instance_model.add_ip_address(ip_model1) instance_model.add_ip_address(ip_model1)
ip_model2 = Models::IpAddress.make(address: ip2, network_name: 'fake-network') ip_model2 = Models::IpAddress.make(address_str: ip2.to_s, network_name: 'fake-network')
instance_model.add_ip_address(ip_model2) instance_model.add_ip_address(ip_model2)
end end


Expand Down
Expand Up @@ -343,7 +343,7 @@ def make_instance_with_existing_model(existing_instance_model)
let(:existing_instance_model) { BD::Models::Instance.make(job: 'foo-instance_group', index: 0, bootstrap: true, availability_zone: az.name) } let(:existing_instance_model) { BD::Models::Instance.make(job: 'foo-instance_group', index: 0, bootstrap: true, availability_zone: az.name) }


before do before do
BD::Models::IpAddress.make(address: ip_to_i('192.168.1.5'), network_name: 'fake-network', instance: existing_instance_model) BD::Models::IpAddress.make(address_str: ip_to_i('192.168.1.5').to_s, network_name: 'fake-network', instance: existing_instance_model)


allow(deployment).to receive(:network).with('fake-network') { manual_network } allow(deployment).to receive(:network).with('fake-network') { manual_network }


Expand Down
Expand Up @@ -62,7 +62,7 @@
describe 'binding existing reservations' do describe 'binding existing reservations' do
context 'when instance has reservations in db' do context 'when instance has reservations in db' do
before do before do
existing_instance.add_ip_address(BD::Models::IpAddress.make(address: 123)) existing_instance.add_ip_address(BD::Models::IpAddress.make(address_str: "123"))
end end


it 'is using reservation from database' do it 'is using reservation from database' do
Expand Down Expand Up @@ -160,7 +160,7 @@
describe 'binding existing reservations' do describe 'binding existing reservations' do
context 'when instance has reservations in db' do context 'when instance has reservations in db' do
before do before do
existing_instance.add_ip_address(BD::Models::IpAddress.make(address: 123)) existing_instance.add_ip_address(BD::Models::IpAddress.make(address_str: "123"))
end end


it 'is using reservation from database' do it 'is using reservation from database' do
Expand Down

0 comments on commit 4a35c4b

Please sign in to comment.