Merge "Don't create invalid floating IP ranges"

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