RabbitMQ 3.7.9+ list compatibility and provider cleanup (voxpupuli#759)

* RabbitMQ 3.7.9+ list compatibility and provider cleanup (voxpupuli#759)

lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb

@@ -2,14 +2,8 @@
 require 'puppet'
 require 'digest'
 
-Puppet::Type.type(:rabbitmq_binding).provide(:rabbitmqadmin) do
-  has_command(:rabbitmqctl, 'rabbitmqctl') do
-    environment HOME: '/tmp'
-  end
-  has_command(:rabbitmqadmin, '/usr/local/bin/rabbitmqadmin') do
-    environment HOME: '/tmp'
-  end
-
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_binding).provide(:rabbitmqadmin, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
   # Without this, the composite namevar stuff doesn't work properly.
@@ -25,14 +19,14 @@ def should_vhost
 
   def self.all_vhosts
     vhosts = []
-    rabbitmqctl('list_vhosts', '-q').split(%r{\n}).map do |vhost|
+    rabbitmqctl_list('vhosts').split(%r{\n}).map do |vhost|
       vhosts.push(vhost)
     end
     vhosts
   end
 
   def self.all_bindings(vhost)
-    rabbitmqctl('list_bindings', '-q', '-p', vhost, 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments').split(%r{\n})
+    rabbitmqctl_list('bindings', '-p', vhost, 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments').split(%r{\n})
   end
 
   def self.instances

lib/puppet/provider/rabbitmq_cli.rb

@@ -0,0 +1,70 @@
+class Puppet::Provider::RabbitmqCli < Puppet::Provider
+  initvars
+
+  def self.rabbitmq_command(name, binary)
+    path = Puppet::Util.which(binary) || "/usr/lib/rabbitmq/bin/#{binary}"
+    home_tmp_command name, path
+  end
+
+  def self.home_tmp_command(name, path)
+    has_command name, path do
+      environment HOME: '/tmp'
+    end
+  end
+
+  rabbitmq_command :rabbitmqctl, 'rabbitmqctl'
+  rabbitmq_command :rabbitmqplugins, 'rabbitmq-plugins'
+
+  home_tmp_command :rabbitmqadmin, '/usr/local/bin/rabbitmqadmin'
+
+  def self.rabbitmq_version
+    return @rabbitmq_version if defined? @rabbitmq_version
+
+    output = rabbitmqctl('-q', 'status')
+    version = output.match(%r{\{rabbit,"RabbitMQ","([\d\.]+)"\}})
+    @rabbitmq_version = version[1] if version
+  end
+
+  def self.rabbitmqctl_list(resource, *opts)
+    list_opts =
+      if Puppet::Util::Package.versioncmp(rabbitmq_version, '3.7.9') >= 0
+        ['-q', '--no-table-headers']
+      else
+        ['-q']
+      end
+    rabbitmqctl("list_#{resource}", *list_opts, *opts)
+  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::Error
+        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
+
+  def self.define_instance_method(name)
+    return if method_defined?(name)
+    define_method(name) do |*args, &block|
+      self.class.send(name, *args, &block)
+    end
+  end
+  private_class_method :define_instance_method
+
+  define_instance_method :rabbitmqctl_list
+  define_instance_method :run_with_retries
+end

lib/puppet/provider/rabbitmq_exchange/rabbitmqadmin.rb

@@ -1,17 +1,6 @@
 require 'puppet'
-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'
-    commands rabbitmqadmin: '/usr/local/bin/rabbitmqadmin'
-  else
-    has_command(:rabbitmqctl, 'rabbitmqctl') do
-      environment HOME: '/tmp'
-    end
-    has_command(:rabbitmqadmin, '/usr/local/bin/rabbitmqadmin') do
-      environment HOME: '/tmp'
-    end
-  end
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_exchange).provide(:rabbitmqadmin, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
   def should_vhost
@@ -23,12 +12,12 @@ def should_vhost
   end
 
   def self.all_vhosts
-    run_with_retries { rabbitmqctl('-q', 'list_vhosts') }.split(%r{\n})
+    run_with_retries { rabbitmqctl_list('vhosts') }.split(%r{\n})
   end
 
   def self.all_exchanges(vhost)
     exchange_list = run_with_retries do
-      rabbitmqctl('-q', 'list_exchanges', '-p', vhost, 'name', 'type', 'internal', 'durable', 'auto_delete', 'arguments')
+      rabbitmqctl_list('exchanges', '-p', vhost, 'name', 'type', 'internal', 'durable', 'auto_delete', 'arguments')
     end
     exchange_list.split(%r{\n}).reject { |exchange| exchange =~ %r{^federation:} }
   end

lib/puppet/provider/rabbitmq_parameter/rabbitmqctl.rb

@@ -1,8 +1,8 @@
 require 'json'
 require 'puppet/util/package'
 
-require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
-Puppet::Type.type(:rabbitmq_parameter).provide(:rabbitmqctl, parent: Puppet::Provider::Rabbitmqctl) do
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_parameter).provide(:rabbitmqctl, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
   # cache parameters
@@ -11,7 +11,7 @@ def self.parameters(name, vhost)
     unless @parameters[vhost]
       @parameters[vhost] = {}
       parameter_list = run_with_retries do
-        rabbitmqctl('list_parameters', '-q', '-p', vhost)
+        rabbitmqctl_list('parameters', '-p', vhost)
       end
       parameter_list.split(%r{\n}).each do |line|
         raise Puppet::Error, "cannot parse line from list_parameter:#{line}" unless line =~ %r{^(\S+)\s+(\S+)\s+(\S+)$}

lib/puppet/provider/rabbitmq_plugin/rabbitmqplugins.rb

@@ -1,16 +1,5 @@
-require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
-Puppet::Type.type(:rabbitmq_plugin).provide(:rabbitmqplugins, parent: Puppet::Provider::Rabbitmqctl) do
-  # Prefer rabbitmq-plugins if it's in $PATH, but fall back to /usr/lib/rabbitmq/bin
-  if Puppet::Util.which('rabbitmq-plugins')
-    has_command(:rabbitmqplugins, 'rabbitmq-plugins') do
-      environment HOME: '/tmp'
-    end
-  else
-    has_command(:rabbitmqplugins, '/usr/lib/rabbitmq/bin/rabbitmq-plugins') do
-      environment HOME: '/tmp'
-    end
-  end
-
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_plugin).provide(:rabbitmqplugins, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
   def self.instances
@@ -37,6 +26,6 @@ def destroy
   end
 
   def exists?
-    self.class.run_with_retries { rabbitmqplugins('list', '-E', '-m') }.split(%r{\n}).include? resource[:name]
+    run_with_retries { rabbitmqplugins('list', '-E', '-m') }.split(%r{\n}).include? resource[:name]
   end
 end

lib/puppet/provider/rabbitmq_policy/rabbitmqctl.rb

@@ -1,8 +1,8 @@
 require 'json'
 require 'puppet/util/package'
 
-require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
-Puppet::Type.type(:rabbitmq_policy).provide(:rabbitmqctl, parent: Puppet::Provider::Rabbitmqctl) do
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_policy).provide(:rabbitmqctl, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
   # cache policies
@@ -11,7 +11,7 @@ def self.policies(vhost, name)
     unless @policies[vhost]
       @policies[vhost] = {}
       policy_list = run_with_retries do
-        rabbitmqctl('list_policies', '-q', '-p', vhost)
+        rabbitmqctl_list('policies', '-p', vhost)
       end
 
       # rabbitmq<3.2 does not support the applyto field

lib/puppet/provider/rabbitmq_queue/rabbitmqadmin.rb

@@ -1,17 +1,8 @@
 require 'json'
 require 'puppet'
-Puppet::Type.type(:rabbitmq_queue).provide(:rabbitmqadmin) do
-  if Puppet::PUPPETVERSION.to_f < 3
-    commands rabbitmqctl: 'rabbitmqctl'
-    commands rabbitmqadmin: '/usr/local/bin/rabbitmqadmin'
-  else
-    has_command(:rabbitmqctl, 'rabbitmqctl') do
-      environment HOME: '/tmp'
-    end
-    has_command(:rabbitmqadmin, '/usr/local/bin/rabbitmqadmin') do
-      environment HOME: '/tmp'
-    end
-  end
+
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_queue).provide(:rabbitmqadmin, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
   def should_vhost
@@ -23,11 +14,11 @@ def should_vhost
   end
 
   def self.all_vhosts
-    rabbitmqctl('list_vhosts', '-q').split(%r{\n})
+    rabbitmqctl_list('vhosts').split(%r{\n})
   end
 
   def self.all_queues(vhost)
-    rabbitmqctl('list_queues', '-q', '-p', vhost, 'name', 'durable', 'auto_delete', 'arguments').split(%r{\n})
+    rabbitmqctl_list('queues', '-p', vhost, 'name', 'durable', 'auto_delete', 'arguments').split(%r{\n})
   end
 
   def self.instances

lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb

@@ -1,12 +1,8 @@
-require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
 Puppet::Type.type(:rabbitmq_user).provide(
   :rabbitmqctl,
-  parent: Puppet::Provider::Rabbitmqctl
+  parent: Puppet::Provider::RabbitmqCli
 ) do
-  has_command(:rabbitmqctl, 'rabbitmqctl') do
-    environment HOME: '/tmp'
-  end
-
   confine feature: :posix
 
   def initialize(value = {})
@@ -16,7 +12,7 @@ def initialize(value = {})
 
   def self.instances
     user_list = run_with_retries do
-      rabbitmqctl('-q', 'list_users')
+      rabbitmqctl_list('users')
     end
 
     user_list.split(%r{\n}).map do |line|

lib/puppet/provider/rabbitmq_user_permissions/rabbitmqctl.rb

@@ -1,13 +1,5 @@
-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'
-  else
-    has_command(:rabbitmqctl, 'rabbitmqctl') do
-      environment HOME: '/tmp'
-    end
-  end
-
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_user_permissions).provide(:rabbitmqctl, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
   # cache users permissions
@@ -16,7 +8,7 @@ def self.users(name, vhost)
     unless @users[name]
       @users[name] = {}
       user_permission_list = run_with_retries do
-        rabbitmqctl('-q', 'list_user_permissions', name)
+        rabbitmqctl_list('user_permissions', name)
       end
       user_permission_list.split(%r{\n}).each do |line|
         line = strip_backslashes(line)

lib/puppet/provider/rabbitmq_vhost/rabbitmqctl.rb

@@ -1,16 +1,10 @@
-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'
-  else
-    has_command(:rabbitmqctl, 'rabbitmqctl') do
-      environment HOME: '/tmp'
-    end
-  end
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmq_cli'))
+Puppet::Type.type(:rabbitmq_vhost).provide(:rabbitmqctl, parent: Puppet::Provider::RabbitmqCli) do
+  confine feature: :posix
 
   def self.instances
     vhost_list = run_with_retries do
-      rabbitmqctl('-q', 'list_vhosts')
+      rabbitmqctl_list('vhosts')
     end
 
     vhost_list.split(%r{\n}).map do |line|
@@ -28,6 +22,6 @@ def destroy
   end
 
   def exists?
-    self.class.run_with_retries { rabbitmqctl('-q', 'list_vhosts') }.split(%r{\n}).include? resource[:name]
+    run_with_retries { rabbitmqctl_list('vhosts') }.split(%r{\n}).include? resource[:name]
   end
 end

lib/puppet/type/rabbitmq_binding.rb

@@ -178,11 +178,11 @@ def validate_argument(argument)
   # Validate that we have both source and destination now that these are not
   # necessarily only coming from the resource title.
   validate do
-    if !self[:source] && !defined? provider.source
+    if !self[:source] && provider.source == :absent
       raise ArgumentError, '`source` must be defined'
     end
 
-    if !self[:destination] && !defined? provider.destination
+    if !self[:destination] && provider.destination == :absent
       raise ArgumentError, '`destination` must be defined'
     end
   end