diff options
author | tyler-ball <tyleraball@gmail.com> | 2014-12-11 15:14:58 -0800 |
---|---|---|
committer | tyler-ball <tyleraball@gmail.com> | 2014-12-16 17:59:33 -0800 |
commit | f0e75a337daa62608b9e816ef8bdc2335bf2b033 (patch) | |
tree | de2b7df214b45d6fe05987f8da5923018e2a739b /lib/chef | |
parent | 1d5b9bf57df6812b6f6a2961d0995aa6c2d8f695 (diff) | |
download | chef-f0e75a337daa62608b9e816ef8bdc2335bf2b033.tar.gz |
First pass at fixing broken notifications
Fixing some code I changed unecessarily
Removing TODOs which are not necessary
Diffstat (limited to 'lib/chef')
-rw-r--r-- | lib/chef/dsl/recipe.rb | 3 | ||||
-rw-r--r-- | lib/chef/provider/package/homebrew.rb | 2 | ||||
-rw-r--r-- | lib/chef/resource.rb | 109 | ||||
-rw-r--r-- | lib/chef/resource/homebrew_package.rb | 3 | ||||
-rw-r--r-- | lib/chef/resource_collection/resource_set.rb | 16 | ||||
-rw-r--r-- | lib/chef/resource_notification.rb | 91 | ||||
-rw-r--r-- | lib/chef/run_context.rb | 8 |
7 files changed, 127 insertions, 105 deletions
diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb index 120497d56e..d70266c14c 100644 --- a/lib/chef/dsl/recipe.rb +++ b/lib/chef/dsl/recipe.rb @@ -81,7 +81,7 @@ class Chef resource = build_resource(type, name, created_at, &resource_attrs_block) - run_context.resource_collection.insert(resource, resource_type:type, instance_name:name) + run_context.resource_collection.insert(resource, resource_type: type, instance_name: name) resource end @@ -101,6 +101,7 @@ class Chef resource = resource_class.new(name, run_context) resource.source_line = created_at + resource.declared_type = type # If we have a resource like this one, we want to steal its state # This behavior is very counter-intuitive and should be removed. # See CHEF-3694, https://tickets.opscode.com/browse/CHEF-3694 diff --git a/lib/chef/provider/package/homebrew.rb b/lib/chef/provider/package/homebrew.rb index 1b407a1901..342c3fd952 100644 --- a/lib/chef/provider/package/homebrew.rb +++ b/lib/chef/provider/package/homebrew.rb @@ -26,7 +26,7 @@ class Chef class Package class Homebrew < Chef::Provider::Package - provides :homebrew_package, os: "mac_os_x" + provides :homebrew_package, os: ["mac_os_x", "darwin"] include Chef::Mixin::HomebrewUser diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index d2718c859e..2e2bd2c84e 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -30,97 +30,13 @@ require 'chef/resource_collection' require 'chef/node_map' require 'chef/node' require 'chef/platform' +require 'chef/resource_notification' require 'chef/mixin/deprecation' require 'chef/mixin/descendants_tracker' class Chef class Resource - class Notification < Struct.new(:resource, :action, :notifying_resource) - - def duplicates?(other_notification) - unless other_notification.respond_to?(:resource) && other_notification.respond_to?(:action) - msg = "only duck-types of Chef::Resource::Notification can be checked for duplication "\ - "you gave #{other_notification.inspect}" - raise ArgumentError, msg - end - other_notification.resource == resource && other_notification.action == action - end - - # If resource and/or notifying_resource is not a resource object, this will look them up in the resource collection - # and fix the references from strings to actual Resource objects. - def resolve_resource_reference(resource_collection) - return resource if resource.kind_of?(Chef::Resource) && notifying_resource.kind_of?(Chef::Resource) - - if not(resource.kind_of?(Chef::Resource)) - fix_resource_reference(resource_collection) - end - - if not(notifying_resource.kind_of?(Chef::Resource)) - fix_notifier_reference(resource_collection) - end - end - - # This will look up the resource if it is not a Resource Object. It will complain if it finds multiple - # resources, can't find a resource, or gets invalid syntax. - def fix_resource_reference(resource_collection) - matching_resource = resource_collection.find(resource) - if Array(matching_resource).size > 1 - msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple resources, "\ - "but can only notify one resource. Notifying resource was defined on #{notifying_resource.source_line}" - raise Chef::Exceptions::InvalidResourceReference, msg - end - self.resource = matching_resource - - rescue Chef::Exceptions::ResourceNotFound => e - err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL) -resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \ -but #{resource} cannot be found in the resource collection. #{notifying_resource} is defined in \ -#{notifying_resource.source_line} -FAIL - err.set_backtrace(e.backtrace) - raise err - rescue Chef::Exceptions::InvalidResourceSpecification => e - err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F) -Resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \ -but #{resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \ -is defined near #{notifying_resource.source_line} -F - err.set_backtrace(e.backtrace) - raise err - end - - # This will look up the notifying_resource if it is not a Resource Object. It will complain if it finds multiple - # resources, can't find a resource, or gets invalid syntax. - def fix_notifier_reference(resource_collection) - matching_notifier = resource_collection.find(notifying_resource) - if Array(matching_notifier).size > 1 - msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple notifying "\ - "resources, but can only originate from one resource. Destination resource was defined "\ - "on #{resource.source_line}" - raise Chef::Exceptions::InvalidResourceReference, msg - end - self.notifying_resource = matching_notifier - - rescue Chef::Exceptions::ResourceNotFound => e - err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL) -Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \ -but #{notifying_resource} cannot be found in the resource collection. #{resource} is defined in \ -#{resource.source_line} -FAIL - err.set_backtrace(e.backtrace) - raise err - rescue Chef::Exceptions::InvalidResourceSpecification => e - err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F) -Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \ -but #{notifying_resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \ -is defined near #{resource.source_line} -F - err.set_backtrace(e.backtrace) - raise err - end - - end FORBIDDEN_IVARS = [:@run_context, :@not_if, :@only_if, :@enclosing_provider] HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider] @@ -211,6 +127,7 @@ F attr_accessor :source_line attr_accessor :retries attr_accessor :retry_delay + attr_accessor :declared_type attr_reader :updated @@ -298,7 +215,7 @@ F def load_prior_resource(resource_type, instance_name) begin - key = ::Chef::ResourceCollection::ResourceSet.create_key(resource_type, instance_name) + key = "#{resource_type}[#{instance_name}]" prior_resource = run_context.resource_collection.lookup(key) # if we get here, there is a prior resource (otherwise we'd have jumped # to the rescue clause). @@ -425,11 +342,11 @@ F def notifies(action, resource_spec, timing=:delayed) # when using old-style resources(:template => "/foo.txt") style, you # could end up with multiple resources. + validate_resource_spec!(resource_spec) + resources = [ resource_spec ].flatten resources.each do |resource| - validate_resource_spec!(resource_spec) - case timing.to_s when 'delayed' notifies_delayed(action, resource) @@ -448,8 +365,12 @@ F # resolve_resource_reference on each in turn, causing them to # resolve lazy/forward references. def resolve_notification_references - run_context.immediate_notifications(self).each { |n| n.resolve_resource_reference(run_context.resource_collection) } - run_context.delayed_notifications(self).each {|n| n.resolve_resource_reference(run_context.resource_collection) } + run_context.immediate_notifications(self).each { |n| + n.resolve_resource_reference(run_context.resource_collection) + } + run_context.delayed_notifications(self).each {|n| + n.resolve_resource_reference(run_context.resource_collection) + } end def notifies_immediately(action, resource_spec) @@ -498,6 +419,14 @@ F end end + # We usually want to store and reference resources by their declared type and not the actual type that + # was looked up by the Resolver (IE, "package" becomes YumPackage class). If we have not been provided + # the declared key we want to fall back on the old to_s key. + def declared_key + return to_s if declared_type.nil? + "#{declared_type}[#{@name}]" + end + def to_s "#{@resource_name}[#{@name}]" end diff --git a/lib/chef/resource/homebrew_package.rb b/lib/chef/resource/homebrew_package.rb index 952552e3a8..3bd5dc16dc 100644 --- a/lib/chef/resource/homebrew_package.rb +++ b/lib/chef/resource/homebrew_package.rb @@ -25,7 +25,8 @@ class Chef class Resource class HomebrewPackage < Chef::Resource::Package - provides :homebrew_package, os: "mac_os_x" + provides :homebrew_package + provides :package, os: ["mac_os_x", "darwin"] def initialize(name, run_context=nil) super diff --git a/lib/chef/resource_collection/resource_set.rb b/lib/chef/resource_collection/resource_set.rb index 6425c2ab08..1b39298cb4 100644 --- a/lib/chef/resource_collection/resource_set.rb +++ b/lib/chef/resource_collection/resource_set.rb @@ -44,7 +44,7 @@ class Chef is_chef_resource!(resource) resource_type ||= resource.resource_name instance_name ||= resource.name - key = ResourceSet.create_key(resource_type, instance_name) + key = create_key(resource_type, instance_name) @resources_by_key[key] = resource end @@ -53,7 +53,7 @@ class Chef when key.kind_of?(String) lookup_by = key when key.kind_of?(Chef::Resource) - lookup_by = ResourceSet.create_key(key.resource_name, key.name) + lookup_by = create_key(key.resource_name, key.name) else raise ArgumentError, "Must pass a Chef::Resource or String to lookup" end @@ -128,18 +128,18 @@ class Chef end end - def self.create_key(resource_type, instance_name) + private + + def create_key(resource_type, instance_name) "#{resource_type}[#{instance_name}]" end - private - def find_resource_by_hash(arg) results = Array.new arg.each do |resource_type, name_list| instance_names = name_list.kind_of?(Array) ? name_list : [ name_list ] instance_names.each do |instance_name| - results << lookup(ResourceSet.create_key(resource_type, instance_name)) + results << lookup(create_key(resource_type, instance_name)) end end return results @@ -153,12 +153,12 @@ class Chef arg =~ /^.+\[(.+)\]$/ resource_list = $1 resource_list.split(",").each do |instance_name| - results << lookup(ResourceSet.create_key(resource_type, instance_name)) + results << lookup(create_key(resource_type, instance_name)) end when SINGLE_RESOURCE_MATCH resource_type = $1 name = $2 - results << lookup(ResourceSet.create_key(resource_type, name)) + results << lookup(create_key(resource_type, name)) else raise ArgumentError, "Bad string format #{arg}, you must have a string like resource_type[name]!" end diff --git a/lib/chef/resource_notification.rb b/lib/chef/resource_notification.rb new file mode 100644 index 0000000000..c02c268709 --- /dev/null +++ b/lib/chef/resource_notification.rb @@ -0,0 +1,91 @@ +require 'chef/resource' + +class Chef + class Resource + class Notification < Struct.new(:resource, :action, :notifying_resource) + + def duplicates?(other_notification) + unless other_notification.respond_to?(:resource) && other_notification.respond_to?(:action) + msg = "only duck-types of Chef::Resource::Notification can be checked for duplication "\ + "you gave #{other_notification.inspect}" + raise ArgumentError, msg + end + other_notification.resource == resource && other_notification.action == action + end + + # If resource and/or notifying_resource is not a resource object, this will look them up in the resource collection + # and fix the references from strings to actual Resource objects. + def resolve_resource_reference(resource_collection) + return resource if resource.kind_of?(Chef::Resource) && notifying_resource.kind_of?(Chef::Resource) + + if not(resource.kind_of?(Chef::Resource)) + fix_resource_reference(resource_collection) + end + + if not(notifying_resource.kind_of?(Chef::Resource)) + fix_notifier_reference(resource_collection) + end + end + + # This will look up the resource if it is not a Resource Object. It will complain if it finds multiple + # resources, can't find a resource, or gets invalid syntax. + def fix_resource_reference(resource_collection) + matching_resource = resource_collection.find(resource) + if Array(matching_resource).size > 1 + msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple resources, "\ + "but can only notify one resource. Notifying resource was defined on #{notifying_resource.source_line}" + raise Chef::Exceptions::InvalidResourceReference, msg + end + self.resource = matching_resource + + rescue Chef::Exceptions::ResourceNotFound => e + err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL) +resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \ +but #{resource} cannot be found in the resource collection. #{notifying_resource} is defined in \ +#{notifying_resource.source_line} + FAIL + err.set_backtrace(e.backtrace) + raise err + rescue Chef::Exceptions::InvalidResourceSpecification => e + err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F) +Resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \ +but #{resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \ +is defined near #{notifying_resource.source_line} + F + err.set_backtrace(e.backtrace) + raise err + end + + # This will look up the notifying_resource if it is not a Resource Object. It will complain if it finds multiple + # resources, can't find a resource, or gets invalid syntax. + def fix_notifier_reference(resource_collection) + matching_notifier = resource_collection.find(notifying_resource) + if Array(matching_notifier).size > 1 + msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple notifying "\ + "resources, but can only originate from one resource. Destination resource was defined "\ + "on #{resource.source_line}" + raise Chef::Exceptions::InvalidResourceReference, msg + end + self.notifying_resource = matching_notifier + + rescue Chef::Exceptions::ResourceNotFound => e + err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL) +Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \ +but #{notifying_resource} cannot be found in the resource collection. #{resource} is defined in \ +#{resource.source_line} + FAIL + err.set_backtrace(e.backtrace) + raise err + rescue Chef::Exceptions::InvalidResourceSpecification => e + err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F) +Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \ +but #{notifying_resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \ +is defined near #{resource.source_line} + F + err.set_backtrace(e.backtrace) + raise err + end + + end + end +end diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb index 1a2d7ba3a3..22679822a4 100644 --- a/lib/chef/run_context.rb +++ b/lib/chef/run_context.rb @@ -100,7 +100,7 @@ class Chef if nr.instance_of?(Chef::Resource) @immediate_notification_collection[nr.name] << notification else - @immediate_notification_collection[nr.to_s] << notification + @immediate_notification_collection[nr.declared_key] << notification end end @@ -111,7 +111,7 @@ class Chef if nr.instance_of?(Chef::Resource) @delayed_notification_collection[nr.name] << notification else - @delayed_notification_collection[nr.to_s] << notification + @delayed_notification_collection[nr.declared_key] << notification end end @@ -119,7 +119,7 @@ class Chef if resource.instance_of?(Chef::Resource) return @immediate_notification_collection[resource.name] else - return @immediate_notification_collection[resource.to_s] + return @immediate_notification_collection[resource.declared_key] end end @@ -127,7 +127,7 @@ class Chef if resource.instance_of?(Chef::Resource) return @delayed_notification_collection[resource.name] else - return @delayed_notification_collection[resource.to_s] + return @delayed_notification_collection[resource.declared_key] end end |