Don't create invalid floating IP ranges

Since floating IP ranges is an array then it's impossible to
override it via Hiera with deep_merge. It's possible to add new
IP ranges only, but not delete/override existing ones.
So if plugin developer wants to override floating IPs parameters
then puppet will fail to create neutron floating subnets, since IP
range from astute.yaml is still present in array after deep merge.

We should not try to create floating IP ranges that does not match
with floating CIDR. We can skip those ranges and put Puppet warning
about it.

Change-Id: I4902933c4f04132ec41f0a844cca852dddefa02a
Closes-bug: #1533676

deployment/puppet/osnailyfacter/lib/puppet/parser/functions/format_allocation_pools.rb

@@ -1,23 +1,41 @@
+require 'ipaddr'
+
 Puppet::Parser::Functions::newfunction(:format_allocation_pools, :type => :rvalue, :doc => <<-EOS
 This function gets floating ranges and format allocation_pools attribute value for neutron subnet resource.
 EOS
 ) do |args|
 
-   raise ArgumentError, ("format_allocation_pools(): wrong number of arguments (#{args.length}; must be 1)") if args.length < 1
-   raise ArgumentError, ("format_allocation_pools(): wrong number of arguments (#{args.length}; must be 1)") if args.length > 1
+   raise ArgumentError, ("format_allocation_pools(): wrong number of arguments (#{args.length}; must be 1 or 2)") if (args.length > 2 or args.length < 1)
 
    floating_ranges = args[0]
+   floating_cidr = args[1]
+
+   raise ArgumentError, 'format_allocation_pools(): floating_cidr is not string!' if floating_cidr and !floating_cidr.is_a?(String)
    raise ArgumentError, 'format_allocation_pools(): floating_ranges is not array!' if !(floating_ranges.is_a?(Array) or floating_ranges.is_a?(String))
 
-   debug "Formating allocation_pools for #{floating_ranges}"
+   debug "Formating allocation_pools for #{floating_ranges} subnet #{floating_cidr}"
    allocation_pools = []
-   if floating_ranges.is_a?(Array) # Is a temporary solution while network_data['L3']['floating'] is not array
-     floating_ranges.each do | range |
-       allocation_pools << "start=#{range.split(':')[0]},end=#{range.split(':')[1]}"
+   #TODO: Is a temporary solution while network_data['L3']['floating'] is not array
+   floating_ranges = [floating_ranges] unless floating_ranges.is_a?(Array)
+
+   floating_ranges.each do | range |
+
+     range_start, range_end = range.split(':')
+
+     unless floating_cidr
+       allocation_pools << "start=#{range_start},end=#{range_end}"
+       next
      end
-   else # TODO: remove else part after python part is merged
-     allocation_pools << "start=#{floating_ranges.split(':')[0]},end=#{floating_ranges.split(':')[1]}"
+
+     floating_range = IPAddr.new(floating_cidr)
+     if floating_range.include?(range_start) and floating_range.include?(range_end)
+       allocation_pools << "start=#{range_start},end=#{range_end}"
+     else
+       warning("Skipping #{range} floating IP range because it does not match #{floating_cidr}.")
+     end
+
    end
+
    debug("Format is done, value is: #{allocation_pools}")
    allocation_pools
 end

deployment/puppet/osnailyfacter/modular/openstack-network/networks.pp

@@ -36,7 +36,8 @@ if hiera('use_neutron', false) {
   $floating_net_shared          = try_get_value($nets, "${floating_net}/shared", false)
 
   if !empty($floating_net_floating_range) {
-    $floating_net_allocation_pool = format_allocation_pools($floating_net_floating_range)
+    $floating_cidr = try_get_value($nets, "${floating_net}/L3/subnet")
+    $floating_net_allocation_pool = format_allocation_pools($floating_net_floating_range, $floating_cidr)
   }
 
   $tenant_name         = try_get_value($access_hash, 'tenant', 'admin')

deployment/puppet/osnailyfacter/spec/functions/format_allocation_pools__spec.rb

@@ -27,23 +27,35 @@ describe 'function for formating allocation pools for neutron subnet resource' d
     end
 
     it 'error if no arguments' do
-      lambda { @scope.function_format_allocation_pools([]) }.should raise_error(ArgumentError, 'format_allocation_pools(): wrong number of arguments (0; must be 1)')
+      lambda { @scope.function_format_allocation_pools([]) }.should raise_error(ArgumentError, 'format_allocation_pools(): wrong number of arguments (0; must be 1 or 2)')
     end
 
-    it 'should require one argument' do
-      lambda { @scope.function_format_allocation_pools(['foo', 'wee']) }.should raise_error(ArgumentError, 'format_allocation_pools(): wrong number of arguments (2; must be 1)')
+    it 'should fail with wrong number of arguments' do
+      lambda { @scope.function_format_allocation_pools(['foo', 'wee', 'bla']) }.should raise_error(ArgumentError, 'format_allocation_pools(): wrong number of arguments (3; must be 1 or 2)')
     end
 
-    it 'should require flating ranges is Array' do
-      lambda { @scope.function_format_allocation_pools([{:fff => true}]) }.should raise_error(ArgumentError, 'format_allocation_pools(): floating_ranges is not array!')
+    it 'should require floating ranges is Array' do
+      lambda { @scope.function_format_allocation_pools([{:fff => true}, 'cidr']) }.should raise_error(ArgumentError, 'format_allocation_pools(): floating_ranges is not array!')
     end
 
-    it 'should be able to format allocation pool string' do
+    it 'should require floating cidr is String' do
+      lambda { @scope.function_format_allocation_pools([['foo', 'wee'], ['cidr']]) }.should raise_error(ArgumentError, 'format_allocation_pools(): floating_cidr is not string!')
+    end
+
+    it 'should be able to format allocation pool string with optional CIDR parameter' do
+      expect(@scope.function_format_allocation_pools([["10.109.1.151:10.109.1.254", "10.109.1.130:10.109.1.150"], "10.109.1.0/24"])).to eq(["start=10.109.1.151,end=10.109.1.254", "start=10.109.1.130,end=10.109.1.150"])
+    end
+
+    it 'should be able to format allocation pool string without optional CIDR parameter' do
       expect(@scope.function_format_allocation_pools([["10.109.1.151:10.109.1.254", "10.109.1.130:10.109.1.150"]])).to eq(["start=10.109.1.151,end=10.109.1.254", "start=10.109.1.130,end=10.109.1.150"])
     end
 
     it 'should be able to format allocation pool string for old structure' do
-      expect(@scope.function_format_allocation_pools(["10.109.1.133:10.109.1.169"])).to eq(["start=10.109.1.133,end=10.109.1.169"])
+      expect(@scope.function_format_allocation_pools(["10.109.1.133:10.109.1.169", "10.109.1.0/24"])).to eq(["start=10.109.1.133,end=10.109.1.169"])
+    end
+
+    it 'should be able to format allocation pool string and skip range that does not match CIDR' do
+      expect(@scope.function_format_allocation_pools([["10.109.1.151:10.109.1.254", "10.110.1.130:10.110.1.150"], "10.109.1.0/24"])).to eq(["start=10.109.1.151,end=10.109.1.254"])
     end
   end
 end