| Yes, there is confusion about title vs resource identity (as determined by its uniqueness_keys). As a result the resources type, doesn't realize the resource is already in the catalog, and tries to add an ensure => absent resource to purge the package. But that conflicts with already present package in the catalog. The problem starts in the resources type
reject { |r| catalog.resource_refs.include? r.ref }.
|
First, resource_refs is an Array, so we're doing a linear scan of all resources in the catalog. And we repeat that for each resource_type.instances. So we've got a quadratic there. Second, resource_refs contains strings with the type and title. But the title is taken from the manifest, so it's something like "Package[waste-of-time]". As a result, the installed package sl never matches. Third, it's possible to call catalog.resource(r.class, r.title) and try to lookup the resource. However, the method generates an intermediate Puppet::Resource object, and loses track of other namevars (like provider for the package type). As a result the generated uniqueness key contains:
instead of
It would be better to add a method to the catalog that checks if a resource is already present, and call that from resources. It avoids creating intermediate Puppet::Resource objects and ensures all of the uniqueness keys are present. Something like:
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb |
index d4bde7cc78..92b11f85b2 100644 |
--- a/lib/puppet/resource/catalog.rb |
+++ b/lib/puppet/resource/catalog.rb |
@@ -403,6 +403,12 @@ class Puppet::Resource::Catalog < Puppet::Graph::SimpleGraph |
result |
end |
|
+ def contains?(resource) |
+ raise ArgumentError unless resource.is_a?(Puppet::Type) |
+ type_name = Puppet::Resource.new(resource).type |
+ @resource_table.has_key?([type_name, resource.uniqueness_key].flatten) |
+ end |
+ |
def resource_refs |
resource_keys.collect{ |type, name| name.is_a?( String ) ? "#{type}[#{name}]" : nil}.compact |
end |
diff --git a/lib/puppet/type/resources.rb b/lib/puppet/type/resources.rb |
index 51c102c5f4..83ae3af465 100644 |
--- a/lib/puppet/type/resources.rb |
+++ b/lib/puppet/type/resources.rb |
@@ -115,7 +115,7 @@ Puppet::Type.newtype(:resources) do |
def generate |
return [] unless self.purge? |
resource_type.instances. |
- reject { |r| catalog.resource_refs.include? r.ref }. |
+ reject { |r| catalog.contains?(r) }. |
select { |r| check(r) }. |
select { |r| r.class.validproperty?(:ensure) }. |
select { |r| able_to_ensure_absent?(r) }.
|
|
