Skip to content

Commit

Permalink
Make rabbitmq providers to retry the commands
Browse files Browse the repository at this point in the history
W/o this fix, then RabbitMQ put in a cluster and managed,
for example, by Pacemaker, rabbitmqctl/rabbitmqadmin commands
could fail due to cluster is not ready yet.

The solution is:
1) Add a run_with_retries method to the existing Rabbitmqctl class
   as a retry decorator to ensure the commands will retry instead
   of fail then the cluster is not ready.
2) Derive the providers from the former one to make
   them able to retry all of the list_* commands and, as a result,
   to wait for RabbitMQ ready as well.
3) Update rspecs. Add missing rspec for rabbitmq_plugin

Co-authors:
  Dmitry Ilyin <dilyin@mirantis.com>,
  Colleen Murphy <colleen@puppetlabs.com>

Closes-bug: #MODULES-1452

Signed-off-by: Bogdan Dobrelya <bdobrelia@mirantis.com>
  • Loading branch information
Bogdan Dobrelya committed Jan 5, 2015
1 parent 1cc2e54 commit 9b23a90
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 20 deletions.
15 changes: 12 additions & 3 deletions lib/puppet/provider/rabbitmq_exchange/rabbitmqadmin.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'puppet'
Puppet::Type.type(:rabbitmq_exchange).provide(:rabbitmqadmin) do
require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
Puppet::Type.type(:rabbitmq_exchange).provide(:rabbitmqadmin, :parent => Puppet::Provider::Rabbitmqctl) do

if Puppet::PUPPETVERSION.to_f < 3
commands :rabbitmqctl => 'rabbitmqctl'
Expand All @@ -24,15 +25,23 @@ def should_vhost

def self.all_vhosts
vhosts = []
parse_command(rabbitmqctl('list_vhosts')).collect do |vhost|
parse_command(
self.run_with_retries {
rabbitmqctl('list_vhosts')
}
).collect do |vhost|
vhosts.push(vhost)
end
vhosts
end

def self.all_exchanges(vhost)
exchanges = []
parse_command(rabbitmqctl('list_exchanges', '-p', vhost, 'name', 'type'))
parse_command(
self.run_with_retries {
rabbitmqctl('list_exchanges', '-p', vhost, 'name', 'type')
}
)
end

def self.parse_command(cmd_output)
Expand Down
11 changes: 8 additions & 3 deletions lib/puppet/provider/rabbitmq_plugin/rabbitmqplugins.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Puppet::Type.type(:rabbitmq_plugin).provide(:rabbitmqplugins) do
require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
Puppet::Type.type(:rabbitmq_plugin).provide(:rabbitmqplugins, :parent => Puppet::Provider::Rabbitmqctl) do

if Puppet::PUPPETVERSION.to_f < 3
if Facter.value(:osfamily) == 'RedHat'
Expand All @@ -21,7 +22,9 @@
defaultfor :feature => :posix

def self.instances
rabbitmqplugins('list', '-E', '-m').split(/\n/).map do |line|
self.run_with_retries {
rabbitmqplugins('list', '-E', '-m')
}.split(/\n/).map do |line|
if line =~ /^(\S+)$/
new(:name => $1)
else
Expand All @@ -39,7 +42,9 @@ def destroy
end

def exists?
rabbitmqplugins('list', '-E', '-m').split(/\n/).detect do |line|
self.class.run_with_retries {
rabbitmqplugins('list', '-E', '-m')
}.split(/\n/).detect do |line|
line.match(/^#{resource[:name]}$/)
end
end
Expand Down
4 changes: 3 additions & 1 deletion lib/puppet/provider/rabbitmq_policy/rabbitmqctl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ def self.policies(name, vhost)
@policies = {} unless @policies
unless @policies[vhost]
@policies[vhost] = {}
rabbitmqctl('list_policies', '-q', '-p', vhost).split(/\n/).each do |line|
self.run_with_retries {
rabbitmqctl('list_policies', '-q', '-p', vhost)
}.split(/\n/).each do |line|
# rabbitmq<3.2 does not support the applyto field
# 1 2 3? 4 5 6
# / ha-all all .* {"ha-mode":"all","ha-sync-mode":"automatic"} 0
Expand Down
13 changes: 9 additions & 4 deletions lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'puppet'
require 'set'
Puppet::Type.type(:rabbitmq_user).provide(:rabbitmqctl) do
require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
Puppet::Type.type(:rabbitmq_user).provide(:rabbitmqctl, :parent => Puppet::Provider::Rabbitmqctl) do

if Puppet::PUPPETVERSION.to_f < 3
commands :rabbitmqctl => 'rabbitmqctl'
Expand All @@ -13,7 +14,9 @@
defaultfor :feature => :posix

def self.instances
rabbitmqctl('-q', 'list_users').split(/\n/).collect do |line|
self.run_with_retries {
rabbitmqctl('-q', 'list_users')
}.split(/\n/).collect do |line|
if line =~ /^(\S+)(\s+\[.*?\]|)$/
new(:name => $1)
else
Expand Down Expand Up @@ -55,12 +58,14 @@ def destroy
end

def exists?
rabbitmqctl('-q', 'list_users').split(/\n/).detect do |line|
self.class.run_with_retries {
rabbitmqctl('-q', 'list_users')
}.split(/\n/).detect do |line|
line.match(/^#{Regexp.escape(resource[:name])}(\s+(\[.*?\]|\S+)|)$/)
end
end


def tags
get_user_tags.entries.sort
end
Expand Down
7 changes: 5 additions & 2 deletions lib/puppet/provider/rabbitmq_user_permissions/rabbitmqctl.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Puppet::Type.type(:rabbitmq_user_permissions).provide(:rabbitmqctl) do
require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
Puppet::Type.type(:rabbitmq_user_permissions).provide(:rabbitmqctl, :parent => Puppet::Provider::Rabbitmqctl) do

if Puppet::PUPPETVERSION.to_f < 3
commands :rabbitmqctl => 'rabbitmqctl'
Expand All @@ -15,7 +16,9 @@ def self.users(name, vhost)
@users = {} unless @users
unless @users[name]
@users[name] = {}
rabbitmqctl('-q', 'list_user_permissions', name).split(/\n/).each do |line|
self.run_with_retries {
rabbitmqctl('-q', 'list_user_permissions', name)
}.split(/\n/).each do |line|
line = self::strip_backslashes(line)
if line =~ /^(\S+)\s+(\S*)\s+(\S*)\s+(\S*)$/
@users[name][$1] =
Expand Down
11 changes: 8 additions & 3 deletions lib/puppet/provider/rabbitmq_vhost/rabbitmqctl.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Puppet::Type.type(:rabbitmq_vhost).provide(:rabbitmqctl) do
require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
Puppet::Type.type(:rabbitmq_vhost).provide(:rabbitmqctl, :parent => Puppet::Provider::Rabbitmqctl) do

if Puppet::PUPPETVERSION.to_f < 3
commands :rabbitmqctl => 'rabbitmqctl'
Expand All @@ -9,7 +10,9 @@
end

def self.instances
rabbitmqctl('-q', 'list_vhosts').split(/\n/).map do |line|
self.run_with_retries {
rabbitmqctl('-q', 'list_vhosts')
}.split(/\n/).map do |line|
if line =~ /^(\S+)$/
new(:name => $1)
else
Expand All @@ -27,7 +30,9 @@ def destroy
end

def exists?
out = rabbitmqctl('-q', 'list_vhosts').split(/\n/).detect do |line|
out = self.class.run_with_retries {
rabbitmqctl('-q', 'list_vhosts')
}.split(/\n/).detect do |line|
line.match(/^#{Regexp.escape(resource[:name])}$/)
end
end
Expand Down
23 changes: 23 additions & 0 deletions lib/puppet/provider/rabbitmqctl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,27 @@ def self.rabbitmq_version
version = output.match(/\{rabbit,"RabbitMQ","([\d\.]+)"\}/)
version[1] if version
end

# Retry the given code block 'count' retries or until the
# command suceeeds. Use 'step' delay between retries.
# Limit each query time by 'timeout'.
# For example:
# users = self.class.run_with_retries { rabbitmqctl 'list_users' }
def self.run_with_retries(count=30, step=6, timeout=10)
count.times do |n|
begin
output = Timeout::timeout(timeout) do
yield
end
rescue Puppet::ExecutionFailure, Timeout
Puppet.debug 'Command failed, retrying'
sleep step
else
Puppet.debug 'Command succeeded'
return output
end
end
raise Puppet::Error, "Command is still failing after #{count * step} seconds expired!"
end

end
26 changes: 26 additions & 0 deletions spec/unit/puppet/provider/rabbitmq_plugin/rabbitmqctl_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
require 'puppet'
require 'mocha'
RSpec.configure do |config|
config.mock_with :mocha
end
provider_class = Puppet::Type.type(:rabbitmq_plugin).provider(:rabbitmqplugins)
describe provider_class do
before :each do
@resource = Puppet::Type::Rabbitmq_plugin.new(
{:name => 'foo'}
)
@provider = provider_class.new(@resource)
end
it 'should match plugins' do
@provider.expects(:rabbitmqplugins).with('list', '-E', '-m').returns("foo\n")
@provider.exists?.should == 'foo'
end
it 'should call rabbitmqplugins to enable' do
@provider.expects(:rabbitmqplugins).with('enable', 'foo')
@provider.create
end
it 'should call rabbitmqplugins to disable' do
@provider.expects(:rabbitmqplugins).with('disable', 'foo')
@provider.destroy
end
end
4 changes: 2 additions & 2 deletions spec/unit/puppet/provider/rabbitmq_user/rabbitmqctl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
kitchen []
kitchen2 [abc, def, ghi]
EOT
@provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['bar','baz', 'administrator'].sort)
@provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['bar','baz', 'administrator'].sort)
@provider.admin=:true
end
it 'should be able to unset admin value' do
Expand All @@ -118,7 +118,7 @@
kitchen []
kitchen2 [abc, def, ghi]
EOT
@provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['bar','baz'].sort)
@provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['bar','baz'].sort)
@provider.admin=:false
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@
@provider.instance_variable_set(:@should_vhost, "bar")
@provider.instance_variable_set(:@should_user, "foo")
@provider.expects(:rabbitmqctl).with('set_permissions', '-p', 'bar', 'foo', "''", "''", "''")
@provider.create
@provider.create
end
it 'should destroy permissions' do
@provider.instance_variable_set(:@should_vhost, "bar")
@provider.instance_variable_set(:@should_user, "foo")
@provider.expects(:rabbitmqctl).with('clear_permissions', '-p', 'bar', 'foo')
@provider.destroy
@provider.destroy
end
{:configure_permission => '1', :write_permission => '2', :read_permission => '3'}.each do |k,v|
it "should be able to retrieve #{k}" do
Expand Down

0 comments on commit 9b23a90

Please sign in to comment.