Skip to content

Commit 1156d02

Browse files
fmoehlerfmoehler
committed
export ips_cidr in different field
1 parent 802b5ba commit 1156d02

File tree

9 files changed

+158
-51
lines changed

9 files changed

+158
-51
lines changed

‎src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -618,18 +618,15 @@ def create_instances_response_with_vm_expected(instances)
618618
end
619619

620620
def create_vm_response(instance, vm)
621-
cidr_ips = vm&.ips || []
622-
ips = cidr_ips.map{ | cidr_ip | cidr_ip.split('/')[0] }
623-
624621
{
625622
'agent_id' => vm&.agent_id,
626623
'cid' => vm&.cid,
627624
'job' => instance.job,
628625
'index' => instance.index,
629626
'id' => instance.uuid,
630627
'az' => instance.availability_zone,
631-
'ips' => ips,
632-
'ips_cidr' => cidr_ips,
628+
'ips' => vm&.ips || [],
629+
'ips_cidr' => vm&.ips_cidr || [],
633630
'vm_created_at' => vm&.created_at&.utc&.iso8601,
634631
}
635632
end

‎src/bosh-director/lib/bosh/director/deployment_plan/compilation_instance_pool.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ def create_instance_plan(stemcell)
189189

190190
compilation_network = @deployment_plan.network(@deployment_plan.compilation.network_name)
191191
reservation = DesiredNetworkReservation.new_dynamic(instance.model, compilation_network)
192+
@logger.debug("Creating new dynamic reservation #{reservation.inspect} for instance '#{instance}' and compile instance group '#{compile_instance_group}'")
192193
desired_instance = DeploymentPlan::DesiredInstance.new(compile_instance_group)
193194
instance_plan = DeploymentPlan::InstancePlan.new(
194195
existing_instance: instance.model,

‎src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -98,27 +98,16 @@ def try_to_allocate_dynamic_ip(reservation, subnet)
9898

9999
prefix = subnet.prefix
100100

101-
ip_address = find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix)
101+
ip_address_cidr = find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix)
102102

103-
unless subnet.range == ip_address || subnet.range.include?(ip_address)
104-
raise NoMoreIPsAvailableAndStopRetrying
103+
if !(subnet.range == ip_address_cidr || subnet.range.include?(ip_address_cidr)) ||
104+
subnet.range.to_range.last.to_i < (ip_address_cidr.to_i + ip_address_cidr.count)
105+
raise NoMoreIPsAvailableAndStopRetrying
105106
end
106107

107-
unless prefix
108-
if ip_address.ipv6?
109-
host_bits = 128 - prefix
110-
else
111-
host_bits = 32 - prefix
112-
end
113-
no_of_addresses = 2**host_bits
114-
if subnet.range.last < (ip_address.to_i + no_of_addresses)
115-
raise NoMoreIPsAvailableAndStopRetrying
116-
end
117-
end
108+
save_ip(ip_address_cidr.to_cidr_s, reservation, false)
118109

119-
save_ip(ip_address.to_cidr_s, reservation, false)
120-
121-
ip_address
110+
ip_address_cidr
122111
end
123112

124113
def find_next_available_ip(ip_entries, first_range_address, prefix)
@@ -131,8 +120,13 @@ def find_next_available_ip(ip_entries, first_range_address, prefix)
131120
current_prefix = Bosh::Director::IpAddrOrCidr.new("#{current_ip}/#{prefix}")
132121

133122
if filtered_ips.any? { |ip| current_prefix.include?(ip) }
134-
current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + current_prefix.count)
135-
filtered_ips.reject{ |ip| ip.to_i < current_ip.to_i }
123+
filtered_ips.reject! { |ip| ip.to_i < current_prefix.to_i }
124+
actual_ip_prefix = filtered_ips.first.count
125+
if actual_ip_prefix > current_prefix.count
126+
current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + actual_ip_prefix)
127+
else
128+
current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + current_prefix.count)
129+
end
136130
else
137131
found_cidr = current_prefix
138132
found = true

‎src/bosh-director/lib/bosh/director/jobs/vm_state.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ def process_vm_for_instance(instance, vm)
5858
disk_cid: instance.managed_persistent_disk_cid,
5959
disk_cids: instance.active_persistent_disks.collection.map { |d| d.model.disk_cid },
6060
ips: vm&.ips || [],
61+
ips_cidr: vm&.ips_cidr || [],
6162
agent_id: vm&.agent_id,
6263
job_name: instance.job,
6364
index: instance.index,

‎src/bosh-director/lib/bosh/director/models/vm.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ def network_spec=(spec)
1919
end
2020

2121
def ips
22+
ips_cidr.map{ | cidr_ip | cidr_ip.split('/')[0] }
23+
end
24+
25+
def ips_cidr
2226
manual_or_vip_ips.concat(dynamic_ips).uniq
2327
end
2428

‎src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1396,7 +1396,8 @@ def manifest_with_errand(deployment_name='errand')
13961396
'id' => "instance-#{instance_idx}",
13971397
'active' => vm_is_active,
13981398
'az' => {0 => 'az0', 1 => 'az1', nil => nil}[instance_idx],
1399-
'ips' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}/32"],
1399+
'ips' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}"],
1400+
'ips_cidr' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}/32"],
14001401
'vm_created_at' => time.utc.iso8601,
14011402
'permanent_nats_credentials' => false,
14021403
)
@@ -1478,7 +1479,71 @@ def manifest_with_errand(deployment_name='errand')
14781479
'id' => "instance-#{instance_idx}",
14791480
'active' => vm_is_active,
14801481
'az' => { 0 => 'az0', 1 => 'az1', nil => nil }[instance_idx],
1481-
'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}/32"],
1482+
'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}"],
1483+
'ips_cidr' => ["1.2.#{instance_idx}.#{vm_by_instance}/32"],
1484+
'vm_created_at' => time.utc.iso8601,
1485+
'permanent_nats_credentials' => false,
1486+
)
1487+
end
1488+
end
1489+
1490+
it 'returns ip addresses with prefix as ips_cidr for a vm' do
1491+
9.times do |i|
1492+
instance_params = {
1493+
'deployment_id' => deployment.id,
1494+
'job' => "job-#{i}",
1495+
'index' => i,
1496+
'state' => 'started',
1497+
'uuid' => "instance-#{i}",
1498+
'variable_set_id' => (Models::VariableSet.create(deployment: deployment).id)
1499+
}
1500+
1501+
instance_params['availability_zone'] = 'az0' if i == 0
1502+
instance_params['availability_zone'] = 'az1' if i == 1
1503+
instance = Models::Instance.create(instance_params)
1504+
1505+
vm_params = {
1506+
'agent_id' => "agent-#{i}",
1507+
'cid' => "cid-#{i}",
1508+
'instance_id' => instance.id,
1509+
'created_at' => time,
1510+
}
1511+
1512+
vm = Models::Vm.create(vm_params)
1513+
1514+
if j == 0
1515+
instance.active_vm = vm
1516+
end
1517+
1518+
ip_addresses_params = {
1519+
'instance_id' => instance.id,
1520+
'task_id' => i.to_s,
1521+
'address_str' => ("1.2.#{i}.#{j}/32").to_s,
1522+
'vm_id' => vm.id,
1523+
}
1524+
Models::IpAddress.create(ip_addresses_params)
1525+
end
1526+
1527+
get '/test_deployment/vms'
1528+
1529+
expect(last_response.status).to eq(200)
1530+
body = JSON.parse(last_response.body)
1531+
expect(body.size).to eq(18)
1532+
1533+
body.sort_by { |instance| instance['agent_id'] }.each_with_index do |instance_with_vm, i|
1534+
instance_idx = i / 2
1535+
vm_by_instance = i % 2
1536+
vm_is_active = vm_by_instance == 0
1537+
expect(instance_with_vm).to eq(
1538+
'agent_id' => "agent-#{instance_idx}-#{vm_by_instance}",
1539+
'job' => "job-#{instance_idx}",
1540+
'index' => instance_idx,
1541+
'cid' => "cid-#{instance_idx}-#{vm_by_instance}",
1542+
'id' => "instance-#{instance_idx}",
1543+
'active' => vm_is_active,
1544+
'az' => { 0 => 'az0', 1 => 'az1', nil => nil }[instance_idx],
1545+
'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}"],
1546+
'ips_cidr' => ["1.2.#{instance_idx}.#{vm_by_instance}/32"],
14821547
'vm_created_at' => time.utc.iso8601,
14831548
'permanent_nats_credentials' => false,
14841549
)
@@ -1529,7 +1594,8 @@ def manifest_with_errand(deployment_name='errand')
15291594
'id' => "instance-#{i}",
15301595
'active' => vm_is_active,
15311596
'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i],
1532-
'ips' => ["1.2.3.#{i}/32"],
1597+
'ips' => ["1.2.3.#{i}"],
1598+
'ips_cidr' => ["1.2.3.#{i}/32"],
15331599
'vm_created_at' => time.utc.iso8601,
15341600
'permanent_nats_credentials' => false,
15351601
)
@@ -1587,7 +1653,8 @@ def manifest_with_errand(deployment_name='errand')
15871653
'cid' => 'cid',
15881654
'id' => 'instance-id',
15891655
'index' => 0,
1590-
'ips' => ["#{vip}/32", "#{network_spec_ip}/32"],
1656+
'ips' => ["#{vip}", "#{network_spec_ip}"],
1657+
'ips_cidr' => ["#{vip}/32", "#{network_spec_ip}/32"],
15911658
'job' => 'job',
15921659
'vm_created_at' => time.utc.iso8601,
15931660
'permanent_nats_credentials' => false,
@@ -1666,6 +1733,7 @@ def manifest_with_errand(deployment_name='errand')
16661733
'id' => "instance-#{i}",
16671734
'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i],
16681735
'ips' => [],
1736+
'ips_cidr' => [],
16691737
'vm_created_at' => time.utc.iso8601,
16701738
'expects_vm' => true
16711739
)
@@ -1678,6 +1746,7 @@ def manifest_with_errand(deployment_name='errand')
16781746
'id' => "instance-#{i}",
16791747
'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i],
16801748
'ips' => [],
1749+
'ips_cidr' => [],
16811750
'vm_created_at' => nil,
16821751
'expects_vm' => true
16831752
)
@@ -1725,6 +1794,7 @@ def manifest_with_errand(deployment_name='errand')
17251794
'id' => "instance-#{i}",
17261795
'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i],
17271796
'ips' => [],
1797+
'ips_cidr' => [],
17281798
'vm_created_at' => nil,
17291799
'expects_vm' => true
17301800
)
@@ -1763,6 +1833,7 @@ def manifest_with_errand(deployment_name='errand')
17631833
'id' => "instance-#{i}",
17641834
'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i],
17651835
'ips' => [],
1836+
'ips_cidr' => [],
17661837
'vm_created_at' => nil,
17671838
'expects_vm' => true
17681839
)
@@ -1806,6 +1877,7 @@ def manifest_with_errand(deployment_name='errand')
18061877
'id' => 'instance-1',
18071878
'az' => nil,
18081879
'ips' => [],
1880+
'ips_cidr' => [],
18091881
'vm_created_at' => nil,
18101882
'expects_vm' => true
18111883
)
@@ -1830,6 +1902,7 @@ def manifest_with_errand(deployment_name='errand')
18301902
'id' => 'instance-1',
18311903
'az' => nil,
18321904
'ips' => [],
1905+
'ips_cidr' => [],
18331906
'vm_created_at' => nil,
18341907
'expects_vm' => false
18351908
)
@@ -1856,6 +1929,7 @@ def manifest_with_errand(deployment_name='errand')
18561929
'id' => 'instance-1',
18571930
'az' => nil,
18581931
'ips' => [],
1932+
'ips_cidr' => [],
18591933
'vm_created_at' => nil,
18601934
'expects_vm' => false
18611935
)

‎src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ module Bosh::Director::DeploymentPlan
123123
reservation.resolve_ip('192.168.1.3/32')
124124
reservation
125125
end
126-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
126+
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az'], '32') }
127127
let(:network_plans) do
128128
[
129129
NetworkPlanner::Plan.new(reservation: reservation, existing: true),
@@ -205,8 +205,7 @@ module Bosh::Director::DeploymentPlan
205205

206206
describe 'networks_changed?' do
207207
context 'when the instance plan has desired network plans' do
208-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
209-
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) }
208+
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) }
210209
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
211210
let(:network_plans) do
212211
[
@@ -335,8 +334,7 @@ module Bosh::Director::DeploymentPlan
335334

336335
describe 'network_settings_changed?' do
337336
context 'when the instance plan has desired network plans' do
338-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
339-
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) }
337+
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) }
340338
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
341339
let(:network_plans) do
342340
[
@@ -401,7 +399,7 @@ module Bosh::Director::DeploymentPlan
401399
},
402400
}
403401
end
404-
let(:subnet) { DynamicNetworkSubnet.new('8.8.8.8', subnet_cloud_properties, ['foo-az']) }
402+
let(:subnet) { DynamicNetworkSubnet.new('8.8.8.8', subnet_cloud_properties, ['foo-az'], '32') }
405403
let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] }
406404
let(:subnet_cloud_properties) do
407405
{}
@@ -518,8 +516,8 @@ module Bosh::Director::DeploymentPlan
518516
end
519517

520518
context 'when the vm type name has changed' do
521-
let(:subnet) { DynamicNetworkSubnet.new(['10.0.0.1'], {}, ['foo-az']) }
522-
let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) }
519+
let(:subnet) { DynamicNetworkSubnet.new(['10.0.0.1'], {}, ['foo-az'], '32') }
520+
let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) }
523521
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
524522

525523
let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] }
@@ -561,8 +559,7 @@ module Bosh::Director::DeploymentPlan
561559
context 'when the network has changed' do
562560
# everything else should be the same
563561
let(:availability_zone) { AvailabilityZone.new('foo-az', 'old' => 'value') }
564-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
565-
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) }
562+
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) }
566563
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
567564

568565
let(:network_plans) do
@@ -892,8 +889,7 @@ module Bosh::Director::DeploymentPlan
892889
end
893890

894891
context 'when the vm type name has changed' do
895-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
896-
let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) }
892+
let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) }
897893
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
898894

899895
let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] }
@@ -935,8 +931,7 @@ module Bosh::Director::DeploymentPlan
935931
context 'when the network has changed' do
936932
# everything else should be the same
937933
let(:availability_zone) { AvailabilityZone.new('foo-az', 'old' => 'value') }
938-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
939-
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) }
934+
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) }
940935
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
941936

942937
let(:network_plans) do
@@ -1040,8 +1035,7 @@ module Bosh::Director::DeploymentPlan
10401035
end
10411036

10421037
context 'when the vm type name has changed' do
1043-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
1044-
let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) }
1038+
let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) }
10451039
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
10461040

10471041
let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] }
@@ -1124,8 +1118,7 @@ module Bosh::Director::DeploymentPlan
11241118
context 'when the network has changed' do
11251119
# everything else should be the same
11261120
let(:availability_zone) { AvailabilityZone.new('foo-az', 'old' => 'value') }
1127-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
1128-
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) }
1121+
let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) }
11291122
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
11301123

11311124
let(:network_plans) do
@@ -1278,8 +1271,8 @@ module Bosh::Director::DeploymentPlan
12781271
end
12791272

12801273
describe '#persist_current_spec' do
1281-
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) }
1282-
let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) }
1274+
let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az'], '32') }
1275+
let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) }
12831276
let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) }
12841277

12851278
let(:network_plans) do

0 commit comments

Comments
 (0)