Merge branch 'master' into hash-each-pair-return

Author: tylerjl

CHANGELOG.md

@@ -10,6 +10,7 @@ Puppet 4.10.0 is the new minimum required version of Puppet.
 * The Elasticsearch log directory is no longer recursively managed to avoid stomping on the user/mode settings that Elasticsearch prefers.
 * Package management on apt-based systems no longer encounters dependency errors when `manage_repo => false`.
 * An error related to elasticsearch_roles and `yield` errors has been fixed
+* Correctly permit instances to be set to `absent` without errors.
 
 #### Features
 

lib/puppet/provider/elasticsearch_service_file/ruby.rb

@@ -63,9 +63,18 @@ def destroy?
   end
 
   def flush
-    opt_flag, opt_flags = Puppet_X::Elastic::EsVersioning.opt_flags(
-      resource[:package_name], resource.catalog
-    )
+    begin
+      opt_flag, opt_flags = Puppet_X::Elastic::EsVersioning.opt_flags(
+        resource[:package_name], resource.catalog
+      )
+    rescue ElasticsearchPackageNotFoundError
+      # If the Elasticsearch package is not present at all, we don't know what
+      # version is present, so we just set these as empty values for the
+      # template.
+      opt_flag = ''
+      opt_flags = []
+    end
+
     # This should only be present on systemd systems.
     opt_flags.delete('--quiet') unless resource[:name].include?('systemd')
 

lib/puppet/type/elasticsearch_service_file.rb

@@ -29,6 +29,15 @@ def insync?(is)
 
       template = ERB.new(should, 0, '-')
       is == template.result(binding)
+    rescue ElasticsearchPackageNotFoundError
+      # This behavior is extremely confusing because of the fact that while
+      # someone should be able to indicate that an instance should be absent,
+      # if there is no service file to query via Puppet providers, it can't
+      # determine this fact. If no package exists and thus `absent` has been
+      # instructed, indicate that the template contents are correct, because
+      # we don't really care what's in there anyway - the service file is for
+      # an absent instance of Elasticsearch anyway.
+      return true
     end
 
     # Represent as a checksum, not the whole file

lib/puppet_x/elastic/es_versioning.rb

@@ -1,3 +1,5 @@
+class ElasticsearchPackageNotFoundError < StandardError; end
+
 module Puppet_X
   module Elastic
     # Assists with discerning the locally installed version of Elasticsearch.
@@ -50,11 +52,16 @@ def self.min_version(ver, package_name, catalog)
 
       # Fetch the package version for a locally installed package.
       def self.version(package_name, catalog)
-        if (es_pkg = catalog.resource("Package[#{package_name}]"))
-          es_pkg.provider.properties[:version] || es_pkg.provider.properties[:ensure]
-        else
-          raise Puppet::Error, "could not find `Package[#{package_name}]` resource"
+        es_pkg = catalog.resource("Package[#{package_name}]")
+        raise Puppet::Error, "could not find `Package[#{package_name}]` resource" unless es_pkg
+        [
+          es_pkg.provider.properties[:version],
+          es_pkg.provider.properties[:ensure]
+        ].each do |property|
+          return property if property.is_a? String
         end
+        Puppet.warning("could not find valid version for `Package[#{package_name}]` resource")
+        raise ElasticsearchPackageNotFoundError
       end
     end
   end

spec/helpers/acceptance/tests/removal_shared_examples.rb

@@ -17,6 +17,10 @@ class { 'elasticsearch': ensure => 'absent', oss => #{v[:oss]} }
       apply_manifest manifest, :catch_failures => true
     end
 
+    it 'is idempotent' do
+      apply_manifest manifest, :catch_changes => true
+    end
+
     instances.each do |instance|
       describe file("/etc/elasticsearch/#{instance}") do
         it { should_not be_directory }