Skip to content

Commit 2e880ab

Browse files
fmoehlerfmoehler
authored
Merge pull request #2641 from cloudfoundry/add-prefix-to-network-settings-for-dynamic-and-vip-network
add prefix to network settings for dynamic and vip network
2 parents db97335 + c3cdf66 commit 2e880ab

11 files changed

Lines changed: 64 additions & 32 deletions

File tree

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ def self.parse(network_spec, availability_zones, logger)
99
Canonicalizer.canonicalize(name)
1010
logger = TaggedLogger.new(logger, 'network-configuration')
1111
name_server_parser = NetworkParser::NameServersParser.new
12+
prefix = Network::IPV4_DEFAULT_PREFIX_SIZE
1213

1314
validate_network_has_no_key('az', name, network_spec)
1415
validate_network_has_no_key('azs', name, network_spec)
@@ -22,15 +23,14 @@ def self.parse(network_spec, availability_zones, logger)
2223
subnets = network_spec['subnets'].map do |subnet_properties|
2324
name_servers = name_server_parser.parse(subnet_properties['name'], subnet_properties)
2425
cloud_properties = safe_property(subnet_properties, 'cloud_properties', class: Hash, default: {})
25-
prefix = safe_property(subnet_properties, 'prefix', class: Integer, default: Network::IPV4_DEFAULT_PREFIX_SIZE)
26-
raise NetworkInvalidProperty, "Prefix property is not supported for dynamic networks." unless prefix == Network::IPV4_DEFAULT_PREFIX_SIZE
26+
subnet_prefix = safe_property(subnet_properties, 'prefix', class: Integer, default: Network::IPV4_DEFAULT_PREFIX_SIZE)
27+
raise NetworkInvalidProperty, "Prefix property is not supported for dynamic networks." unless subnet_prefix == Network::IPV4_DEFAULT_PREFIX_SIZE
2728
subnet_availability_zones = parse_availability_zones(subnet_properties, availability_zones, name)
2829
DynamicNetworkSubnet.new(name_servers, cloud_properties, subnet_availability_zones, prefix)
2930
end
3031
else
3132
cloud_properties = safe_property(network_spec, 'cloud_properties', class: Hash, default: {})
3233
# We need to set the IPv4 default value (dynamic networks only support IPv4)
33-
prefix = Network::IPV4_DEFAULT_PREFIX_SIZE
3434

3535
name_servers = name_server_parser.parse(network_spec['name'], network_spec)
3636
subnets = [DynamicNetworkSubnet.new(name_servers, cloud_properties, nil, prefix)]
@@ -116,7 +116,8 @@ def network_settings(reservation, default_properties = Network::REQUIRED_DEFAULT
116116

117117
config = {
118118
"type" => "dynamic",
119-
"cloud_properties" => subnet.cloud_properties
119+
"cloud_properties" => subnet.cloud_properties,
120+
"prefix" => @prefix.to_s,
120121
}
121122
config["dns"] = subnet.dns if subnet.dns
122123

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ def network_settings(reservation, default_properties = REQUIRED_DEFAULTS, _avail
5454
'type' => 'vip',
5555
'ip' => to_ipaddr(reservation.ip).base_addr,
5656
'cloud_properties' => @cloud_properties,
57+
'prefix' => @prefix
5758
}
5859
end
5960

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ def manual_or_vip_ips
3939
end
4040

4141
def dynamic_ips
42-
network_spec.map { |_, network| "#{network['ip']}/#{network['prefix']}" }
42+
network_spec.map do |_, network|
43+
network['prefix'].nil? || network['prefix'].empty? ? network['ip'] : "#{network['ip']}/#{network['prefix']}"
44+
end
4345
end
4446
end
4547
end

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@
363363
expect(@network.network_settings(reservation,[])).to eq({
364364
'type' => 'dynamic',
365365
'cloud_properties' => {'foz' => 'baz'},
366-
'default' => []
366+
'default' => [],
367+
'prefix' => '32',
367368
})
368369
end
369370

@@ -373,7 +374,8 @@
373374
expect(@network.network_settings(reservation)).to eq({
374375
'type' => 'dynamic',
375376
'cloud_properties' => {'foz' => 'baz'},
376-
'default' => ['dns', 'gateway']
377+
'default' => ['dns', 'gateway'],
378+
'prefix' => '32'
377379
})
378380
end
379381

@@ -424,13 +426,15 @@
424426
expect(network.network_settings(reservation, [], az1)).to eq({
425427
'type' => 'dynamic',
426428
'cloud_properties' => {'subnet_key' => 'subnet_value'},
427-
'default' => []
429+
'default' => [],
430+
'prefix' => '32'
428431
})
429432

430433
expect(network.network_settings(reservation, [], az2)).to eq({
431434
'type' => 'dynamic',
432435
'cloud_properties' => {'subnet_key' => 'subnet_value'},
433-
'default' => []
436+
'default' => [],
437+
'prefix' => '32'
434438
})
435439
end
436440

@@ -464,7 +468,8 @@
464468
expect(network.network_settings(reservation, [])).to eq({
465469
'type' => 'dynamic',
466470
'cloud_properties' => {'subnet_key' => 'subnet_value'},
467-
'default' => []
471+
'default' => [],
472+
'prefix' => '32'
468473
})
469474
end
470475
end
@@ -491,7 +496,8 @@
491496
expect(network.network_settings(reservation, [], az2)).to eq({
492497
'type' => 'dynamic',
493498
'cloud_properties' => {'subnet_key' => 'subnet_value2'},
494-
'default' => []
499+
'default' => [],
500+
'prefix' => '32'
495501
})
496502
end
497503

@@ -501,7 +507,8 @@
501507
expect(network.network_settings(reservation, [])).to eq({
502508
'type' => 'dynamic',
503509
'cloud_properties' => {'subnet_key' => 'subnet_value'},
504-
'default' => []
510+
'default' => [],
511+
'prefix' => '32'
505512
})
506513
end
507514

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ module Bosh::Director::DeploymentPlan
155155
plan
156156
end
157157
let(:network_settings) do
158-
{ 'a' => { 'type' => 'dynamic', 'cloud_properties' => {}, 'dns' => ['10.0.0.1'], 'default' => %w[dns gateway] } }
158+
{ 'a' => { 'type' => 'dynamic', 'cloud_properties' => {}, 'dns' => ['10.0.0.1'], 'default' => %w[dns gateway], 'prefix' => '32' } }
159159
end
160160

161161
before do
@@ -370,11 +370,13 @@ module Bosh::Director::DeploymentPlan
370370
'type' => 'dynamic',
371371
'cloud_properties' => {},
372372
'dns' => '10.0.0.1',
373+
'prefix' => '32',
373374
},
374375
'obsolete-network' => {
375376
'type' => 'dynamic',
376377
'cloud_properties' => {},
377378
'dns' => '10.0.0.1',
379+
'prefix' => '32',
378380
},
379381
}
380382
end
@@ -388,6 +390,7 @@ module Bosh::Director::DeploymentPlan
388390
'existing-network' => {
389391
'type' => 'dynamic',
390392
'cloud_properties' => {},
393+
'prefix' => '32',
391394
'dns' => '10.0.0.1',
392395
},
393396
'a' => {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ module Bosh::Director::DeploymentPlan
143143
'type' => 'dynamic',
144144
'cloud_properties' => network_spec['subnets'].first['cloud_properties'],
145145
'default' => ['gateway'],
146+
'prefix' => '32',
146147
)
147148

148149
expect(spec['packages']).to eq(packages)
@@ -182,6 +183,7 @@ module Bosh::Director::DeploymentPlan
182183
'type' => 'dynamic',
183184
'cloud_properties' => network_spec['subnets'].first['cloud_properties'],
184185
'default' => ['gateway'],
186+
'prefix' => '32',
185187
)
186188

187189
expect(spec['packages']).to eq(packages)
@@ -470,6 +472,7 @@ module Bosh::Director::DeploymentPlan
470472
'type' => 'dynamic',
471473
'cloud_properties' => { 'foo' => 'bar' },
472474
'default' => ['gateway'],
475+
'prefix' => '32',
473476
},
474477
},
475478
'vm_type' => {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ module Bosh::Director::DeploymentPlan
7979
'default' => ['gateway'],
8080
'ip' => '10.0.0.6',
8181
'netmask' => '255.255.255.0',
82-
'gateway' => '10.0.0.1'}
82+
'gateway' => '10.0.0.1',
83+
'prefix' => '32'
84+
}
8385
})
8486
end
8587
end

‎src/bosh-director/spec/unit/bosh/director/deployment_plan/steps/create_vm_step_spec.rb‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -762,6 +762,7 @@ module Steps
762762
'cloud_properties' => network_cloud_properties,
763763
'dns' => 'dns',
764764
'default' => ['gateway'],
765+
'prefix' => '32'
765766
},
766767
}
767768
end
@@ -773,6 +774,7 @@ module Steps
773774
'cloud_properties' => resolved_network_cloud_properties,
774775
'dns' => 'dns',
775776
'default' => ['gateway'],
777+
'prefix' => '32'
776778
},
777779
}
778780
end

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
'cloud_properties' => {
5555
'foz' => 'baz',
5656
},
57+
'prefix' => '32',
5758
)
5859
end
5960

‎src/bosh-director/spec/unit/bosh/director/models/vm_spec.rb‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,16 @@ module Models
5252
it 'returns ips without prefix for single ips and with prefix for prefix ips order by size of the integer representation' do
5353
expect(vm.ips).to match_array(['1.1.1.1', '1.1.1.2', '1.1.1.16/28', '1.1.1.4', '2001:db8::1/64'])
5454
end
55+
56+
context 'with old VMs that do not have a prefix' do
57+
before do
58+
vm.network_spec = { 'some' => { 'ip' => '2001:db8::1' }, 'some_other' => { 'ip' => '1.1.1.4' } }
59+
end
60+
61+
it 'returns ips without prefix for single ips and with prefix for prefix ips order by size of the integer representation' do
62+
expect(vm.ips).to match_array(['1.1.1.1', '1.1.1.2', '1.1.1.16/28', '1.1.1.4', '2001:db8::1'])
63+
end
64+
end
5565
end
5666
end
5767
end

0 commit comments

Comments
 (0)