From c7f2eddc18d458e785524f9d23a69611a60c1d3d Mon Sep 17 00:00:00 2001 From: Aleksandr Didenko Date: Wed, 13 Jan 2016 14:44:31 +0100 Subject: [PATCH] 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 --- .../functions/format_allocation_pools.rb | 34 ++++++++++++++----- .../modular/openstack-network/networks.pp | 3 +- .../format_allocation_pools__spec.rb | 26 ++++++++++---- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/deployment/puppet/osnailyfacter/lib/puppet/parser/functions/format_allocation_pools.rb b/deployment/puppet/osnailyfacter/lib/puppet/parser/functions/format_allocation_pools.rb index 5680722b28..153e2655c9 100644 --- a/deployment/puppet/osnailyfacter/lib/puppet/parser/functions/format_allocation_pools.rb +++ b/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 diff --git a/deployment/puppet/osnailyfacter/modular/openstack-network/networks.pp b/deployment/puppet/osnailyfacter/modular/openstack-network/networks.pp index 8f3af8739e..3f9a1ceb98 100644 --- a/deployment/puppet/osnailyfacter/modular/openstack-network/networks.pp +++ b/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') diff --git a/deployment/puppet/osnailyfacter/spec/functions/format_allocation_pools__spec.rb b/deployment/puppet/osnailyfacter/spec/functions/format_allocation_pools__spec.rb index cd355af7b0..d025592a88 100644 --- a/deployment/puppet/osnailyfacter/spec/functions/format_allocation_pools__spec.rb +++ b/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