Add retry to keystone_user.exists?

Put back exists? method in keystone_user in line with the usual
openstacklib mechanism.  This is done by adding the possibility for
request call to pass regexp messages that shouldn't be retried.

Now we can safely call fetch_user without worrying about having the call
retried by opentacklib.

Fetch_project has the same behavior, so I added it to the mix.  It may
be a performance killer somewhere.

Change-Id: I368cf6a06d21d018337af3e6d09cdabee839a563
Closes-Bug: 1597357
This commit is contained in:
Sofer Athlan-Guyot 2016-06-29 19:25:47 +02:00
parent 85ee4416ea
commit 07cee48dfc
5 changed files with 23 additions and 14 deletions

View File

@ -153,14 +153,18 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
def self.fetch_project(name, domain) def self.fetch_project(name, domain)
domain ||= default_domain domain ||= default_domain
request('project', 'show', [name, '--domain', domain]) request('project', 'show',
[name, '--domain', domain],
{:no_retry_exception_msgs => /No project with a name or ID/})
rescue Puppet::ExecutionFailure => e rescue Puppet::ExecutionFailure => e
raise e unless e.message =~ /No project with a name or ID/ raise e unless e.message =~ /No project with a name or ID/
end end
def self.fetch_user(name, domain) def self.fetch_user(name, domain)
domain ||= default_domain domain ||= default_domain
request('user', 'show', [name, '--domain', domain]) request('user', 'show',
[name, '--domain', domain],
{:no_retry_exception_msgs => /No user with a name or ID/})
rescue Puppet::ExecutionFailure => e rescue Puppet::ExecutionFailure => e
raise e unless e.message =~ /No user with a name or ID/ raise e unless e.message =~ /No user with a name or ID/
end end
@ -226,18 +230,18 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
end end
end end
def self.request(service, action, properties=nil) def self.request(service, action, properties=nil, options={})
super super
rescue Puppet::Error::OpenstackAuthInputError, Puppet::Error::OpenstackUnauthorizedError => error rescue Puppet::Error::OpenstackAuthInputError, Puppet::Error::OpenstackUnauthorizedError => error
request_by_service_token(service, action, error, properties) request_by_service_token(service, action, error, properties, options=options)
end end
def self.request_by_service_token(service, action, error, properties=nil) def self.request_by_service_token(service, action, error, properties=nil, options={})
properties ||= [] properties ||= []
@credentials.token = admin_token @credentials.token = admin_token
@credentials.url = service_url @credentials.url = service_url
raise error unless @credentials.service_token_set? raise error unless @credentials.service_token_set?
Puppet::Provider::Openstack.request(service, action, properties, @credentials) Puppet::Provider::Openstack.request(service, action, properties, @credentials, options)
end end
def self.service_url def self.service_url

View File

@ -78,11 +78,9 @@ Puppet::Type.type(:keystone_user).provide(
def exists? def exists?
return true if @property_hash[:ensure] == :present return true if @property_hash[:ensure] == :present
domain_name = self.class.domain_id_from_name(resource[:domain]) domain_name = self.class.domain_id_from_name(resource[:domain])
self.class.request_without_retry do @property_hash =
@property_hash = self.class.fetch_user(resource[:name], domain_name)
self.class.fetch_user(resource[:name], domain_name) @property_hash ||= {}
@property_hash ||= {}
end
# This can happen in bad LDAP mapping # This can happen in bad LDAP mapping
@property_hash[:enabled] = 'true' if @property_hash[:enabled].nil? @property_hash[:enabled] = 'true' if @property_hash[:enabled].nil?

View File

@ -0,0 +1,7 @@
---
fixes:
- Fixes `bug 1597357
<https://bugs.launchpad.net/puppet-keystone/+bug/1597357>`__
exits? was not using any retry as it expects an error to determine
if the user exist. This fix it and enable classic retry mechanism
for other errors.

View File

@ -109,7 +109,7 @@ id="newid"
klass.expects(:request_timeout).returns(0) klass.expects(:request_timeout).returns(0)
klass.expects(:openstack) klass.expects(:openstack)
.with('project', 'show', '--format', 'shell', ['no_project', '--domain', 'Default']) .with('project', 'show', '--format', 'shell', ['no_project', '--domain', 'Default'])
.times(2) .once
.raises(Puppet::ExecutionFailure, "Execution of '/usr/bin/openstack project show --format shell no_project' returned 1: No project with a name or ID of 'no_project' exists.") .raises(Puppet::ExecutionFailure, "Execution of '/usr/bin/openstack project show --format shell no_project' returned 1: No project with a name or ID of 'no_project' exists.")
expect(klass.fetch_project('no_project', 'Default')).to be_falsey expect(klass.fetch_project('no_project', 'Default')).to be_falsey
end end
@ -141,7 +141,7 @@ id="the_project_id"
klass.expects(:request_timeout).returns(0) klass.expects(:request_timeout).returns(0)
klass.expects(:openstack) klass.expects(:openstack)
.with('user', 'show', '--format', 'shell', ['no_user', '--domain', 'Default']) .with('user', 'show', '--format', 'shell', ['no_user', '--domain', 'Default'])
.times(2) .once
.raises(Puppet::ExecutionFailure, "Execution of '/usr/bin/openstack user show --format shell no_user' returned 1: No user with a name or ID of 'no_user' exists.") .raises(Puppet::ExecutionFailure, "Execution of '/usr/bin/openstack user show --format shell no_user' returned 1: No user with a name or ID of 'no_user' exists.")
expect(klass.fetch_user('no_user', 'Default')).to be_falsey expect(klass.fetch_user('no_user', 'Default')).to be_falsey
end end

View File

@ -79,7 +79,7 @@ username="user1"
described_class.expects(:openstack) described_class.expects(:openstack)
.with('user', 'show', '--format', 'shell', .with('user', 'show', '--format', 'shell',
['user1', '--domain', 'domain1_id']) ['user1', '--domain', 'domain1_id'])
.twice .once
.raises(Puppet::ExecutionFailure, .raises(Puppet::ExecutionFailure,
"No user with a name or ID of 'user1' exists.") "No user with a name or ID of 'user1' exists.")
expect(provider.exists?).to be_falsey expect(provider.exists?).to be_falsey