From 6e811badf0dc43b980ca3479e47a4a13eb9cad4b Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Mon, 21 Dec 2015 13:52:06 +0100 Subject: [PATCH] Manage Keystone_endpoint and Keystone_service without warnings by default A recent change (1] made Keystone_endpoint matching service by name/type. This change added a warning if endpoints were not created with a default service. The way this new feature was added is problematic because we had warnings by default, since all Puppet modules use this define, so it introduced a poor user experience. This patch makes sure a service type is configured with the new way in the keystone::resource::service_identity function, when creating endpoint. For backward compatibility, when no service type is specified, we have now a conditional that sends a warning if no service type is set but still create the endpoints. For the service management, it adds the service type with the new way, so we don't have any warning by default. So from this patch, we don't have this kind of warning by default: /Keystone_endpoint[RegionOne/keystone]/type: Support for a endpoint without the type set is deprecated in Liberty. It will be dropped in Mitaka [1] http://git.openstack.org/cgit/openstack/puppet-keystone/commit/?id=0a4e06abb0f5b3f324464ff5219d2885816311ce Closes-Bug: #1528308 Change-Id: I6e411d8f81c7ae5c768d85a236c0942d265c74dd --- manifests/resource/service_identity.pp | 33 +++++++++++++------ spec/classes/keystone_endpoint_spec.rb | 11 +++---- ...keystone_resource_service_identity_spec.rb | 18 ++++++++-- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/manifests/resource/service_identity.pp b/manifests/resource/service_identity.pp index 5ad3f9ce6..2298201b0 100644 --- a/manifests/resource/service_identity.pp +++ b/manifests/resource/service_identity.pp @@ -168,9 +168,8 @@ define keystone::resource::service_identity( if $configure_service { if $service_type { - ensure_resource('keystone_service', $service_name_real, { + ensure_resource('keystone_service', "${service_name_real}::${service_type}", { 'ensure' => 'present', - 'type' => $service_type, 'description' => $service_description, }) } else { @@ -179,15 +178,29 @@ define keystone::resource::service_identity( } if $configure_endpoint { - if $public_url and $admin_url and $internal_url { - ensure_resource('keystone_endpoint', "${region}/${service_name_real}", { - 'ensure' => 'present', - 'public_url' => $public_url, - 'admin_url' => $admin_url, - 'internal_url' => $internal_url, - }) + if $service_type { + if $public_url and $admin_url and $internal_url { + ensure_resource('keystone_endpoint', "${region}/${service_name_real}::${service_type}", { + 'ensure' => 'present', + 'public_url' => $public_url, + 'admin_url' => $admin_url, + 'internal_url' => $internal_url, + }) + } else { + fail ('When configuring an endpoint, you need to set the _url parameters.') + } } else { - fail ('When configuring an endpoint, you need to set the _url parameters.') + if $public_url and $admin_url and $internal_url { + ensure_resource('keystone_endpoint', "${region}/${service_name_real}", { + 'ensure' => 'present', + 'public_url' => $public_url, + 'admin_url' => $admin_url, + 'internal_url' => $internal_url, + }) + } else { + fail ('When configuring an endpoint, you need to set the _url parameters.') + } + warning('Defining a endpoint without the type is supported in Liberty and will be dropped in Mitaka. See https://bugs.launchpad.net/puppet-keystone/+bug/1506996') } } } diff --git a/spec/classes/keystone_endpoint_spec.rb b/spec/classes/keystone_endpoint_spec.rb index bafad575e..ec6f6751d 100644 --- a/spec/classes/keystone_endpoint_spec.rb +++ b/spec/classes/keystone_endpoint_spec.rb @@ -2,14 +2,13 @@ require 'spec_helper' describe 'keystone::endpoint' do - it { is_expected.to contain_keystone_service('keystone').with( + it { is_expected.to contain_keystone_service('keystone::identity').with( :ensure => 'present', - :type => 'identity', :description => 'OpenStack Identity Service' )} describe 'with default parameters' do - it { is_expected.to contain_keystone_endpoint('RegionOne/keystone').with( + it { is_expected.to contain_keystone_endpoint('RegionOne/keystone::identity').with( :ensure => 'present', :public_url => 'http://127.0.0.1:5000/v2.0', :admin_url => 'http://127.0.0.1:35357/v2.0', @@ -26,7 +25,7 @@ describe 'keystone::endpoint' do :internal_url => 'https://identity-int.some.tld/some/internal/endpoint' } end - it { is_expected.to contain_keystone_endpoint('RegionOne/keystone').with( + it { is_expected.to contain_keystone_endpoint('RegionOne/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', @@ -41,7 +40,7 @@ describe 'keystone::endpoint' do { :version => '' } end - it { is_expected.to contain_keystone_endpoint('RegionOne/keystone').with( + it { is_expected.to contain_keystone_endpoint('RegionOne/keystone::identity').with( :ensure => 'present', :public_url => 'http://127.0.0.1:5000', :admin_url => 'http://127.0.0.1:35357', @@ -56,7 +55,7 @@ describe 'keystone::endpoint' do end it 'internal_url should default to public_url' do - is_expected.to contain_keystone_endpoint('RegionOne/keystone').with( + is_expected.to contain_keystone_endpoint('RegionOne/keystone::identity').with( :ensure => 'present', :public_url => 'https://identity.some.tld/the/main/endpoint/v2.0', :internal_url => 'https://identity.some.tld/the/main/endpoint/v2.0' diff --git a/spec/defines/keystone_resource_service_identity_spec.rb b/spec/defines/keystone_resource_service_identity_spec.rb index b729b546f..d6769eec1 100644 --- a/spec/defines/keystone_resource_service_identity_spec.rb +++ b/spec/defines/keystone_resource_service_identity_spec.rb @@ -47,12 +47,26 @@ describe 'keystone::resource::service_identity' do :roles => ['admin'], )} - it { is_expected.to contain_keystone_service(title).with( + it { is_expected.to contain_keystone_service("#{title}::network").with( :ensure => 'present', - :type => 'network', :description => 'neutron service', )} + it { is_expected.to contain_keystone_endpoint("RegionOne/#{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', + )} + end + + context 'when trying to create an endpoint without service_type (will be dropped in Mitaka)' do + let :params do + required_params.merge( + :configure_service => false, + :service_type => false, + ) + end it { is_expected.to contain_keystone_endpoint("RegionOne/#{title}").with( :ensure => 'present', :public_url => 'http://7.7.7.7:9696',