Permalink
Browse files

Merge pull request #5166 from nicklewis/PUP-6497-better-error-for-con…

…flicting-capabilities

(PUP-6497) Improve error message with conflicting capability resources
2 parents 4ac92d1 + ccf5377 commit f45665e8c4b10770d91b39ff0317160668bc075e @joshcooper joshcooper committed on GitHub Jul 18, 2017
@@ -270,7 +270,7 @@ def add_parameters_from_consume
t = Puppet::Pops::Evaluator::Runtime3ResourceSupport.find_resource_type(scope, t) unless t == 'class' || t == 'node'
cap = catalog.resource(t, ref.title)
if cap.nil?
- raise _("Resource %{ref} could not be found; it might not have been produced yet") % { ref: ref }
+ raise Puppet::Error, _("Resource %{ref} could not be found; it might not have been produced yet") % { ref: ref }
end
# Ensure that the found resource is a capability resource
@@ -19,7 +19,7 @@ module Puppet::Resource::CapabilityFinder
#
# @param environment [String] environment name
# @param code_id [String,nil] code_id of the catalog
- # @param cap [Puppet::Type] the capability resource type instance
+ # @param cap [Puppet::Resource] the capability resource type instance
# @return [Puppet::Resource,nil] The found capability resource or `nil` if it could not be found
def self.find(environment, code_id, cap)
unless Puppet::Util.const_defined?('Puppetdb')
@@ -33,24 +33,22 @@ def self.find(environment, code_id, cap)
resources = resources.select { |r| r['tags'].any? { |t| t == "producer:#{environment}" } }
end
- if resources.size > 1 && code_id
- Puppet.debug "Found multiple resources when looking up capability #{cap}, filtering by code id #{code_id}"
- resources = search(environment, code_id, cap)
- end
-
- if resources.size > 1
+ if resources.empty?
+ Puppet.debug "Could not find capability resource #{cap} in PuppetDB"
+ elsif resources.size == 1
+ resource_hash = resources.first
+ elsif code_id_resource = disambiguate_by_code_id(environment, code_id, cap)
+ resource_hash = code_id_resource
+ else
raise Puppet::DevError,
"Unexpected response from PuppetDB when looking up #{cap}:\n" \
"expected exactly one resource but got #{resources.size};\n" \
"returned data is:\n#{resources.inspect}"
end
- if resource_hash = resources.first
+ if resource_hash
resource_hash['type'] = cap.resource_type
instantiate_resource(resource_hash)
- else
- Puppet.debug "Could not find capability resource #{cap} in PuppetDB"
- nil
end
end
@@ -111,6 +109,27 @@ def self.query_puppetdb(query)
private
+ # Find a distinct copy of the given capability resource by searching for only
+ # resources matching the given code_id. Returns `nil` if no code_id is
+ # supplied or if there isn't exactly one matching resource.
+ #
+ # @param environment [String] environment name
+ # @param code_id [String,nil] code_id of the catalog
+ # @param cap [Puppet::Resource] the capability resource type instance
+ def self.disambiguate_by_code_id(environment, code_id, cap)
+ if code_id
+ Puppet.debug "Found multiple resources when looking up capability #{cap}, filtering by code id #{code_id}"
+ resources = search(environment, code_id, cap)
+
+ if resources.size > 1
+ Puppet.debug "Found multiple resources matching code id #{code_id} when looking up #{cap}"
+ nil
+ else
+ resources.first
+ end
+ end
+ end
+
def self.instantiate_resource(resource_hash)
real_type = resource_hash['type']
resource = Puppet::Resource.new(real_type, resource_hash['title'])
@@ -124,22 +124,21 @@ def make_cap_type
expect(result['host']).to eq('chost')
end
- it 'should return nil if no resource matches code_id' do
+ it 'should fail if no resource matches code_id' do
Puppet::Resource::CapabilityFinder.stubs(:search).with('production', code_id, capability).returns []
- result = Puppet::Resource::CapabilityFinder.find('production', code_id, capability)
- expect(result).to be_nil
+ expect { Puppet::Resource::CapabilityFinder.find('production', code_id, capability) }.to raise_error(Puppet::Error, /expected exactly one resource but got 2/)
end
it 'should fail if multiple resources match code_id' do
Puppet::Resource::CapabilityFinder.stubs(:search).with('production', code_id, capability).returns resources
- expect { Puppet::Resource::CapabilityFinder.find('production', code_id, capability) }.to raise_error(Puppet::DevError, /expected exactly one resource/)
+ expect { Puppet::Resource::CapabilityFinder.find('production', code_id, capability) }.to raise_error(Puppet::DevError, /expected exactly one resource but got 2/)
end
it 'should fail if no code_id was specified' do
Puppet::Resource::CapabilityFinder.stubs(:search).with('production', nil, capability).returns resources
- expect { Puppet::Resource::CapabilityFinder.find('production', nil, capability) }.to raise_error(Puppet::DevError, /expected exactly one resource/)
+ expect { Puppet::Resource::CapabilityFinder.find('production', nil, capability) }.to raise_error(Puppet::DevError, /expected exactly one resource but got 2/)
end
end
end

0 comments on commit f45665e

Please sign in to comment.