Make rabbitmq providers to retry the commands

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 <[email protected]>,
  Colleen Murphy <[email protected]>

Closes-bug: #MODULES-1452

Signed-off-by: Bogdan Dobrelya <[email protected]>

lib/puppet/provider/rabbitmq_exchange/rabbitmqadmin.rb

@@ -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'
@@ -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)

lib/puppet/provider/rabbitmq_plugin/rabbitmqplugins.rb

@@ -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'
@@ -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
@@ -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

lib/puppet/provider/rabbitmq_policy/rabbitmqctl.rb

@@ -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

lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb

@@ -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'
@@ -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
@@ -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

lib/puppet/provider/rabbitmq_user_permissions/rabbitmqctl.rb

@@ -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'
@@ -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] =

lib/puppet/provider/rabbitmq_vhost/rabbitmqctl.rb

@@ -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'
@@ -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
@@ -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

lib/puppet/provider/rabbitmqctl.rb

@@ -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

spec/unit/puppet/provider/rabbitmq_plugin/rabbitmqctl_spec.rb

@@ -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

spec/unit/puppet/provider/rabbitmq_user/rabbitmqctl_spec.rb

@@ -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
@@ -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
 

spec/unit/puppet/provider/rabbitmq_user_permissions/rabbitmqctl_spec.rb

@@ -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