openstack/puppet-keystone
Code Issues Proposed changes

Fix region association for endpoints.

Browse Source 
The code matching the existing endpoints did not take the region into
account.  This was giving random results and messing up the catalog
badly.

This code fix it this and add associated rspec checks.

Closes-Bug: #1535939

Change-Id: If1cdf30c37194b3a7b08bf85860cf7fb7266f6e1
This commit is contained in:
Matt Fischer 2016-01-19 15:56:49 -07:00 committed by Sofer Athlan-Guyot
parent cfa106045f
commit 3e50ac09ee
5 changed files with 132 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
lib/puppet/provider/keystone_endpoint/openstack.rb
View File

@ -98,7 +98,8 @@ Puppet::Type.type(:keystone_endpoint).provide(
endpoints.each do |ep_osc|
if (ep_osc[:id] != current[:id]) &&
(ep_osc[:service_name] == current[:service_name]) &&
(ep_osc[:service_type] == current[:service_type])
(ep_osc[:service_type] == current[:service_type]) &&
(ep_osc[:region] == current[:region])
endpoint.merge!(ep_osc[:interface].to_sym => ep_osc)
end
end

12
spec/classes/keystone_endpoint_spec.rb
View File

@ -12,7 +12,8 @@ describe 'keystone::endpoint' do
:ensure => 'present',
:public_url => 'http://127.0.0.1:5000/v2.0',
:admin_url => 'http://127.0.0.1:35357/v2.0',
:internal_url => 'http://127.0.0.1:5000/v2.0'
:internal_url => 'http://127.0.0.1:5000/v2.0',
:region => 'RegionOne'
)}
end
@ -22,14 +23,17 @@ describe 'keystone::endpoint' do
{ :version => 'v42.6',
:public_url => 'https://identity.some.tld/the/main/endpoint',
:admin_url => 'https://identity-int.some.tld/some/admin/endpoint',
:internal_url => 'https://identity-int.some.tld/some/internal/endpoint' }
:internal_url => 'https://identity-int.some.tld/some/internal/endpoint',
:region => 'East'
}
end
it { is_expected.to contain_keystone_endpoint('RegionOne/keystone::identity').with(
it { is_expected.to contain_keystone_endpoint('East/keystone::identity').with(
:ensure => 'present',
:public_url => 'https://identity.some.tld/the/main/endpoint/v42.6',
:admin_url => 'https://identity-int.some.tld/some/admin/endpoint/v42.6',
:internal_url => 'https://identity-int.some.tld/some/internal/endpoint/v42.6'
:internal_url => 'https://identity-int.some.tld/some/internal/endpoint/v42.6',
:region => 'East'
)}
end

17
spec/defines/keystone_resource_service_identity_spec.rb
View File

@ -57,6 +57,22 @@ describe 'keystone::resource::service_identity' do
:public_url => 'http://7.7.7.7:9696',
:internal_url => 'http://10.0.0.1:9696',
:admin_url => 'http://192.168.0.1:9696',
:region => 'RegionOne',
)}
end
context 'when explicitly setting an region' do
let :params do
required_params.merge(
:region => 'East',
)
end
it { is_expected.to contain_keystone_endpoint("East/#{title}::network").with(
:ensure => 'present',
:public_url => 'http://7.7.7.7:9696',
:internal_url => 'http://10.0.0.1:9696',
:admin_url => 'http://192.168.0.1:9696',
:region => 'East',
)}
end
@ -109,6 +125,7 @@ describe 'keystone::resource::service_identity' do
:roles => ['admin'],
)}
end
context 'with user and project domain' do
let :params do
required_params.merge({

17
spec/support/matchers/have_array_of_instances_hash.rb Normal file
View File

@ -0,0 +1,17 @@
RSpec::Matchers.define :have_array_of_instances_hash do |expected|
expected_array = expected.dup
match do |actual|
if actual.count != expected_array.count
return false
end
actual_array = actual.map { |i| i.instance_variable_get('@property_hash') }
expected_array.each do |e|
actual_array.each do |a|
if e == a
actual_array.delete(a)
end
end
end
actual_array.empty?
end
end

94
spec/unit/provider/keystone_endpoint/openstack_spec.rb
View File

@ -118,16 +118,98 @@ region="region"
end
describe '#instances' do
it 'finds every tenant' do
described_class.expects(:openstack)
.with('endpoint', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Region","Service Name","Service Type","Enabled","Interface","URL"
context 'basic' do
it 'finds every tenant' do
described_class.expects(:openstack)
.with('endpoint', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Region","Service Name","Service Type","Enabled","Interface","URL"
"endpoint1_id","RegionOne","keystone","identity",True,"admin","http://127.0.0.1:5002"
"endpoint2_id","RegionOne","keystone","identity",True,"internal","https://127.0.0.1:5001"
"endpoint3_id","RegionOne","keystone","identity",True,"public","https://127.0.0.1:5000"
')
instances = described_class.instances
expect(instances.count).to eq(1)
instances = described_class.instances
expect(instances.count).to eq(1)
end
end
context 'many different region' do
it 'should not mix up the endpoints' do
described_class.expects(:openstack)
.with('endpoint', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Region","Service Name","Service Type","Enabled","Interface","URL"
"endpoint1_id","RegionOne","keystone","identity",True,"admin","http://One-127.0.0.1:5002"
"endpoint2_id","RegionOne","keystone","identity",True,"internal","https://One-127.0.0.1:5001"
"endpoint3_id","RegionOne","keystone","identity",True,"public","https://One-127.0.0.1:5000"
"endpoint4_id","RegionTwo","keystone","identity",True,"admin","http://Two-127.0.0.1:5002"
"endpoint5_id","RegionTwo","keystone","identity",True,"internal","https://Two-127.0.0.1:5001"
"endpoint6_id","RegionTwo","keystone","identity",True,"public","https://Two-127.0.0.1:5000"
"endpoint7_id","RegionThree","keystone","identity",True,"admin","http://Three-127.0.0.1:5002"
"endpoint8_id","RegionThree","keystone","identity",True,"internal","https://Three-127.0.0.1:5001"
"endpoint9_id","RegionThree","keystone","identity",True,"public","https://Three-127.0.0.1:5000"
"endpoint10_id","RegionFour","keystone","identity",True,"admin","http://Four-127.0.0.1:5002"
"endpoint11_id","RegionFour","keystone","identity",True,"internal","https://Four-127.0.0.1:5001"
"endpoint12_id","RegionFour","keystone","identity",True,"public","https://Four-127.0.0.1:5000"
"endpoint13_id","RegionFive","keystone","identity",True,"admin","http://Five-127.0.0.1:5002"
"endpoint14_id","RegionFive","keystone","identity",True,"internal","https://Five-127.0.0.1:5001"
"endpoint15_id","RegionFive","keystone","identity",True,"public","https://Five-127.0.0.1:5000"
"endpoint16_id","RegionSix","keystone","identity",True,"admin","http://Six-127.0.0.1:5002"
"endpoint17_id","RegionSix","keystone","identity",True,"internal","https://Six-127.0.0.1:5001"
"endpoint18_id","RegionSix","keystone","identity",True,"public","https://Six-127.0.0.1:5000"
"endpoint19_id","RegionSeven","keystone","identity",True,"admin","http://Seven-127.0.0.1:5002"
"endpoint20_id","RegionSeven","keystone","identity",True,"internal","https://Seven-127.0.0.1:5001"
"endpoint21_id","RegionSeven","keystone","identity",True,"public","https://Seven-127.0.0.1:5000"
')
instances = described_class.instances
expect(instances).to have_array_of_instances_hash([
{:name=>"RegionOne/keystone::identity",
:ensure=>:present,
:id=>"endpoint1_id,endpoint2_id,endpoint3_id",
:region=>"RegionOne",
:admin_url=>"http://One-127.0.0.1:5002",
:internal_url=>"https://One-127.0.0.1:5001",
:public_url=>"https://One-127.0.0.1:5000"},
{:name=>"RegionTwo/keystone::identity",
:ensure=>:present,
:id=>"endpoint4_id,endpoint5_id,endpoint6_id",
:region=>"RegionTwo",
:admin_url=>"http://Two-127.0.0.1:5002",
:internal_url=>"https://Two-127.0.0.1:5001",
:public_url=>"https://Two-127.0.0.1:5000"},
{:name=>"RegionThree/keystone::identity",
:ensure=>:present,
:id=>"endpoint7_id,endpoint8_id,endpoint9_id",
:region=>"RegionThree",
:admin_url=>"http://Three-127.0.0.1:5002",
:internal_url=>"https://Three-127.0.0.1:5001",
:public_url=>"https://Three-127.0.0.1:5000"},
{:name=>"RegionFour/keystone::identity",
:ensure=>:present,
:id=>"endpoint10_id,endpoint11_id,endpoint12_id",
:region=>"RegionFour",
:admin_url=>"http://Four-127.0.0.1:5002",
:internal_url=>"https://Four-127.0.0.1:5001",
:public_url=>"https://Four-127.0.0.1:5000"},
{:name=>"RegionFive/keystone::identity",
:ensure=>:present,
:id=>"endpoint13_id,endpoint14_id,endpoint15_id",
:region=>"RegionFive",
:admin_url=>"http://Five-127.0.0.1:5002",
:internal_url=>"https://Five-127.0.0.1:5001",
:public_url=>"https://Five-127.0.0.1:5000"},
{:name=>"RegionSix/keystone::identity",
:ensure=>:present,
:id=>"endpoint16_id,endpoint17_id,endpoint18_id",
:region=>"RegionSix",
:admin_url=>"http://Six-127.0.0.1:5002",
:internal_url=>"https://Six-127.0.0.1:5001",
:public_url=>"https://Six-127.0.0.1:5000"},
{:name=>"RegionSeven/keystone::identity",
:ensure=>:present,
:id=>"endpoint19_id,endpoint20_id,endpoint21_id",
:region=>"RegionSeven",
:admin_url=>"http://Seven-127.0.0.1:5002",
:internal_url=>"https://Seven-127.0.0.1:5001",
:public_url=>"https://Seven-127.0.0.1:5000"}])
end
end
end