From 760f4c773157500af3fff879e363755e93207b2a Mon Sep 17 00:00:00 2001 From: Bogdan Dobrelya Date: Wed, 12 Aug 2015 09:35:06 +0200 Subject: [PATCH] Sync nova_admin_tenant_id_setter from upstream Remove custom provider change due to implemented cutom retry logic is faily. The solution is to sync upstream provider. Source: 6.0.0 c83277d21c42da040c8d8d34d2346da6ac319950 https://github.com/openstack/puppet-neutron Closes-bug: #1483648 Change-Id: I9d5f36ab528864c3cd5b7c993e8051109e054e0c Signed-off-by: Bogdan Dobrelya --- .../ini_setting.rb | 289 +++++++++--------- deployment/puppet/neutron/spec/spec_helper.rb | 6 + .../neutron_spec.rb | 73 +++-- 3 files changed, 206 insertions(+), 162 deletions(-) diff --git a/deployment/puppet/neutron/lib/puppet/provider/nova_admin_tenant_id_setter/ini_setting.rb b/deployment/puppet/neutron/lib/puppet/provider/nova_admin_tenant_id_setter/ini_setting.rb index 83bce8b0cc..63366d8afa 100644 --- a/deployment/puppet/neutron/lib/puppet/provider/nova_admin_tenant_id_setter/ini_setting.rb +++ b/deployment/puppet/neutron/lib/puppet/provider/nova_admin_tenant_id_setter/ini_setting.rb @@ -23,67 +23,86 @@ end class KeystoneAPIError < KeystoneError end -RETRY_COUNT = 10 -RETRY_SLEEP = 3 +Puppet::Type.type(:nova_admin_tenant_id_setter).provide(:ruby) do + @tenant_id = nil -# Provides common request handling semantics to the other methods in -# this module. -# -# +req+:: -# An HTTPRequest object -# +url+:: -# A parsed URL (returned from URI.parse) -def neutron_handle_request(req, url) + def authenticate + keystone_v2_authenticate( + @resource[:auth_url], + @resource[:auth_username], + @resource[:auth_password], + nil, + @resource[:auth_tenant_name]) + end + + def find_tenant_by_name (token) + tenants = keystone_v2_tenants( + @resource[:auth_url], + token) + + tenants.select { |tenant| tenant['name'] == @resource[:tenant_name] } + end + + # Provides common request handling semantics to the other methods in + # this module. + # + # +req+:: + # An HTTPRequest object + # +url+:: + # A parsed URL (returned from URI.parse) + def neutron_handle_request(req, url) begin - # There is issue with ipv6 where address has to be in brackets, this causes the - # underlying ruby TCPSocket to fail. Net::HTTP.new will fail without brackets on - # joining the ipv6 address with :port or passing brackets to TCPSocket. It was - # found that if we use Net::HTTP.start with url.hostname the incriminated code - # won't be hit. - use_ssl = url.scheme == "https" ? true : false - http = Net::HTTP.start(url.hostname, url.port, {:use_ssl => use_ssl}) - res = http.request(req) + # There is issue with ipv6 where address has to be in brackets, this causes the + # underlying ruby TCPSocket to fail. Net::HTTP.new will fail without brackets on + # joining the ipv6 address with :port or passing brackets to TCPSocket. It was + # found that if we use Net::HTTP.start with url.hostname the incriminated code + # won't be hit. + use_ssl = url.scheme == "https" ? true : false + http = Net::HTTP.start(url.hostname, url.port, {:use_ssl => use_ssl}) + res = http.request(req) - if res.code != '200' - raise KeystoneAPIError, "Received error response from Keystone server at #{url}: #{res.message}" - end + if res.code != '200' + raise KeystoneAPIError, "Received error response from Keystone server at #{url}: #{res.message}" + end rescue Errno::ECONNREFUSED => detail - raise KeystoneConnectionError, "Failed to connect to Keystone server at #{url}: #{detail}" + raise KeystoneConnectionError, "Failed to connect to Keystone server at #{url}: #{detail}" rescue SocketError => detail - raise KeystoneConnectionError, "Failed to connect to Keystone server at #{url}: #{detail}" + raise KeystoneConnectionError, "Failed to connect to Keystone server at #{url}: #{detail}" end res -end + end -# Authenticates to a Keystone server and obtains an authentication token. -# It returns a 2-element +[token, authinfo]+, where +token+ is a token -# suitable for passing to openstack apis in the +X-Auth-Token+ header, and -# +authinfo+ is the complete response from Keystone, including the service -# catalog (if available). -# -# +auth_url+:: -# Keystone endpoint URL. This function assumes API version -# 2.0 and an administrative endpoint, so this will typically look like -# +http://somehost:35357/v2.0+. -# -# +username+:: -# Username for authentication. -# -# +password+:: -# Password for authentication -# -# +tenantID+:: -# Tenant UUID -# -# +tenantName+:: -# Tenant name -# -def keystone_v2_authenticate(auth_url, - username, - password, - tenantId=nil, - tenantName=nil) + # Authenticates to a Keystone server and obtains an authentication token. + # It returns a 2-element +[token, authinfo]+, where +token+ is a token + # suitable for passing to openstack apis in the +X-Auth-Token+ header, and + # +authinfo+ is the complete response from Keystone, including the service + # catalog (if available). + # + # +auth_url+:: + # Keystone endpoint URL. This function assumes API version + # 2.0 and an administrative endpoint, so this will typically look like + # +http://somehost:35357/v2.0+. + # + # +username+:: + # Username for authentication. + # + # +password+:: + # Password for authentication + # + # +tenantID+:: + # Tenant UUID + # + # +tenantName+:: + # Tenant name + # + + def keystone_v2_authenticate(auth_url, + username, + password, + tenantId=nil, + tenantName=nil) + debug 'Call: keystone_v2_authenticate' post_args = { 'auth' => { @@ -94,11 +113,11 @@ def keystone_v2_authenticate(auth_url, }} if tenantId - post_args['auth']['tenantId'] = tenantId + post_args['auth']['tenantId'] = tenantId end if tenantName - post_args['auth']['tenantName'] = tenantName + post_args['auth']['tenantName'] = tenantName end url = URI.parse("#{auth_url}/tokens") @@ -109,21 +128,21 @@ def keystone_v2_authenticate(auth_url, res = neutron_handle_request(req, url) data = JSON.parse res.body return data['access']['token']['id'] -end + end -# Queries a Keystone server to a list of all tenants. -# -# +auth_url+:: -# Keystone endpoint. See the notes for +auth_url+ in -# +keystone_v2_authenticate+. -# -# +token+:: -# A Keystone token that will be passed in requests as the value of the -# +X-Auth-Token+ header. -# -def keystone_v2_tenants(auth_url, - token) + # Queries a Keystone server to a list of all tenants. + # + # +auth_url+:: + # Keystone endpoint. See the notes for +auth_url+ in + # +keystone_v2_authenticate+. + # + # +token+:: + # A Keystone token that will be passed in requests as the value of the + # +X-Auth-Token+ header. + # + def keystone_v2_tenants(auth_url, token) + debug 'Call: keystone_v2_tenants' url = URI.parse("#{auth_url}/tenants") req = Net::HTTP::Get.new url.path req['content-type'] = 'application/json' @@ -132,82 +151,76 @@ def keystone_v2_tenants(auth_url, res = neutron_handle_request(req, url) data = JSON.parse res.body data['tenants'] -end + end -Puppet::Type.type(:nova_admin_tenant_id_setter).provide(:ruby) do - @tenant_id = nil - - def authenticate - keystone_v2_authenticate( - @resource[:auth_url], - @resource[:auth_username], - @resource[:auth_password], - nil, - @resource[:auth_tenant_name]) - end - - def find_tenant_by_name (token) - tenants = keystone_v2_tenants( - @resource[:auth_url], - token) - - tenants.select{|tenant| tenant['name'] == @resource[:tenant_name]} - end - - def exists? - ini_file = Puppet::Util::IniConfig::File.new - ini_file.read("/etc/neutron/neutron.conf") - ini_file['DEFAULT'] && ini_file['DEFAULT']['nova_admin_tenant_id'] && ini_file['DEFAULT']['nova_admin_tenant_id'] == tenant_id - end - - def create - config - end - - def tenant_id - @tenant_id ||= get_tenant_id - end - - # This looks for the tenant specified by the 'tenant_name' parameter to - # the resource and returns the corresponding UUID if there is a single - # match. - # - # Raises a KeystoneAPIError if: - # - # - There are multiple matches, or - # - There are zero matches - def get_tenant_id - token = authenticate - RETRY_COUNT.times do |n| - begin - tenants = find_tenant_by_name(token) - rescue => e - debug "Request failed: '#{e.message}' Retry: '#{n}'" - sleep RETRY_SLEEP - next - end - if tenants.length == 1 - return tenants[0]['id'] - elsif tenants.length > 1 - name = tenants[0]['name'] - raise KeystoneAPIError, "Found multiple matches for domain name: '#{name}'" - else - if n == RETRY_COUNT - 1 - raise KeystoneAPIError, 'Unable to find matching tenant' - else - debug "Tenant '#{@resource[:tenant_name]}' not found! Retry: '#{n}'" - sleep RETRY_SLEEP - next - end - end + # This looks for the tenant specified by the 'tenant_name' parameter to + # the resource and returns the corresponding UUID if there is a single + # match. + # + # Raises a KeystoneAPIError if: + # + # - There are multiple matches, or + # - There are zero matches + def get_tenant_id + debug 'Call: get_tenant_id' + exception = nil + retry_count.times do + begin + return get_tenant_id_request + rescue => exception + debug exception.message + break if exception.message.include? 'multiple matches' + sleep retry_sleep + next end end + raise exception if exception + end - def config - Puppet::Type.type(:neutron_config).new( - {:name => 'DEFAULT/nova_admin_tenant_id', :value => "#{tenant_id}"} - ).create + def get_tenant_id_request + debug 'Call: get_tenant_id_request' + token = authenticate + tenants = find_tenant_by_name(token) + + if tenants.length == 1 + return tenants.first['id'] + elsif tenants.length > 1 + raise KeystoneAPIError, "Found multiple matches for domain name: '#{tenants.first['name']}' while geting the id of the tenant: '#{@resource[:tenant_name]}'!" + else + raise KeystoneAPIError, "Unable to find matching tenant: '#{@resource[:tenant_name]}'!" end + end + + ################################### + + def retry_count + 10 + end + + def retry_sleep + 3 + end + + def exists? + debug 'Call: exists?' + ini_file = Puppet::Util::IniConfig::File.new + ini_file.read("/etc/neutron/neutron.conf") + ini_file['DEFAULT'] && ini_file['DEFAULT']['nova_admin_tenant_id'] && ini_file['DEFAULT']['nova_admin_tenant_id'] == tenant_id + end + + def create + config + end + + def tenant_id + @tenant_id ||= get_tenant_id + end + + def config + Puppet::Type.type(:neutron_config).new( + {:name => 'DEFAULT/nova_admin_tenant_id', :value => "#{tenant_id}"} + ).create + end end diff --git a/deployment/puppet/neutron/spec/spec_helper.rb b/deployment/puppet/neutron/spec/spec_helper.rb index 3df4cede10..bce6a2a42c 100644 --- a/deployment/puppet/neutron/spec/spec_helper.rb +++ b/deployment/puppet/neutron/spec/spec_helper.rb @@ -8,3 +8,9 @@ RSpec.configure do |c| end at_exit { RSpec::Puppet::Coverage.report! } + +def puppet_debug_override + return unless ENV['SPEC_PUPPET_DEBUG'] + Puppet::Util::Log.level = :debug + Puppet::Util::Log.newdestination(:console) +end diff --git a/deployment/puppet/neutron/spec/unit/provider/nova_admin_tenant_id_setter/neutron_spec.rb b/deployment/puppet/neutron/spec/unit/provider/nova_admin_tenant_id_setter/neutron_spec.rb index b7cac34db5..5bf8afdada 100644 --- a/deployment/puppet/neutron/spec/unit/provider/nova_admin_tenant_id_setter/neutron_spec.rb +++ b/deployment/puppet/neutron/spec/unit/provider/nova_admin_tenant_id_setter/neutron_spec.rb @@ -54,11 +54,27 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do } end + let(:resource) do + Puppet::Type::Nova_admin_tenant_id_setter.new(params) + end + + let(:provider) do + provider = provider_class.new(resource) + provider.stubs(:retry_count).returns(3) + provider.stubs(:retry_sleep).returns(0) + provider + end + it 'should have a non-nil provider' do - expect(provider_class).not_to be_nil + expect(provider_class).not_to be_nil + end + + before(:each) do + puppet_debug_override end context 'when url is correct' do + before :each do stub_request(:post, "http://127.0.0.1:35357/v2.0/tokens"). to_return(:status => 200, @@ -72,8 +88,6 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do end it 'should create a resource' do - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) expect(provider.exists?).to be_falsey expect(provider.create).to be_nil end @@ -84,8 +98,6 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do Puppet::Util::IniConfig::File.expects(:new).returns(mock) mock.expects(:read).with('/etc/neutron/neutron.conf') - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) expect(provider.exists?).to be_truthy expect(provider.create).to be_nil end @@ -108,12 +120,15 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do params.merge!(:tenant_name => 'bad_tenant_name') end - it 'should receive an api error' do - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) + it 'should receive an "Unable to find matching tenant" api error' do expect(provider.exists?).to be_falsey expect { provider.create }.to raise_error KeystoneAPIError, /Unable to find matching tenant/ end + + it 'should retry get_tenant_id defined number of times' do + provider.expects(:get_tenant_id_request).times(3).raises KeystoneAPIError, 'Unable to find matching tenant' + expect { provider.create }.to raise_error KeystoneAPIError, /Unable to find matching tenant/ + end end # What happens if we ask for a tenant name that results in multiple @@ -133,11 +148,14 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do params.merge!(:tenant_name => 'multiple_matches_tenant') end - it 'should receive an api error' do - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) + it 'should receive an "Found multiple matches" api error' do expect(provider.exists?).to be_falsey - expect { provider.create }.to raise_error KeystoneAPIError, /Found multiple matches for domain name: 'multiple_matches_tenant'/ + expect { provider.create }.to raise_error KeystoneAPIError, /Found multiple matches/ + end + + it 'should not retry get_tenant_id on this error' do + provider.expects(:get_tenant_id_request).once.raises KeystoneAPIError, 'Found multiple matches' + expect { provider.create }.to raise_error KeystoneAPIError, /Found multiple matches/ end end @@ -151,10 +169,13 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do end it 'should receive an authentication error' do - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) expect(provider.exists?).to be_falsey - expect { provider.create }.to raise_error KeystoneAPIError + expect { provider.create }.to raise_error KeystoneAPIError, /Received error response from Keystone server/ + end + + it 'should retry get_tenant_id defined number of times' do + provider.expects(:get_tenant_id_request).times(3).raises KeystoneAPIError, 'Received error response from Keystone server' + expect { provider.create }.to raise_error KeystoneAPIError, /Received error response from Keystone server/ end end @@ -165,24 +186,30 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do end it 'should receive a connection error' do - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) expect(provider.exists?).to be_falsey - expect { provider.create }.to raise_error KeystoneConnectionError + expect { provider.create }.to raise_error KeystoneConnectionError, /Connection refused/ + end + + it 'should retry get_tenant_id defined number of times' do + provider.expects(:get_tenant_id_request).times(3).raises KeystoneConnectionError, 'Connection refused' + expect { provider.create }.to raise_error KeystoneConnectionError, /Connection refused/ end end # What happens if we mistype the hostname? context 'when keystone server is unknown' do before :each do - stub_request(:post, "http://127.0.0.1:35357/v2.0/tokens").to_raise SocketError, 'getaddrinfo: Name or service not known' + stub_request(:post, "http://127.0.0.1:35357/v2.0/tokens").to_raise SocketError.new 'getaddrinfo: Name or service not known' end it 'should receive a connection error' do - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) expect(provider.exists?).to be_falsey - expect { provider.create }.to raise_error KeystoneConnectionError + expect { provider.create }.to raise_error KeystoneConnectionError, /Name or service not known/ + end + + it 'should retry get_tenant_id defined number of times' do + provider.expects(:get_tenant_id_request).times(3).raises KeystoneAPIError, 'Name or service not known' + expect { provider.create }.to raise_error KeystoneAPIError, /Name or service not known/ end end @@ -201,8 +228,6 @@ describe 'Puppet::Type.type(:nova_admin_tenant_id_setter)' do end it 'should create a resource' do - resource = Puppet::Type::Nova_admin_tenant_id_setter.new(params) - provider = provider_class.new(resource) expect(provider.exists?).to be_falsey expect(provider.create).to be_nil end