From 43e8d8b751293bf1eb03b69e6dc3c2c621154ed4 Mon Sep 17 00:00:00 2001 From: tyler-ball Date: Mon, 13 Oct 2014 10:50:24 -0500 Subject: Finishing all code changes to split resource collection into 2 data containers --- lib/chef/formatters/doc.rb | 2 +- lib/chef/formatters/minimal.rb | 2 +- lib/chef/provider/breakpoint.rb | 4 +- lib/chef/provider/deploy.rb | 3 +- lib/chef/provider/lwrp_base.rb | 2 +- lib/chef/provider/route.rb | 2 +- lib/chef/recipe.rb | 2 +- lib/chef/resource.rb | 23 ++-- lib/chef/resource_collection.rb | 148 ++++----------------- .../resource_collection_serialization.rb | 53 ++++++++ lib/chef/resource_list.rb | 16 +-- lib/chef/resource_set.rb | 28 ++-- lib/chef/run_context.rb | 14 +- lib/chef/run_status.rb | 4 +- lib/chef/runner.rb | 4 +- lib/chef/shell/ext.rb | 4 +- lib/chef/shell/shell_session.rb | 4 +- 17 files changed, 131 insertions(+), 184 deletions(-) create mode 100644 lib/chef/resource_collection/resource_collection_serialization.rb (limited to 'lib/chef') diff --git a/lib/chef/formatters/doc.rb b/lib/chef/formatters/doc.rb index 4a08b9d095..2e102a82c6 100644 --- a/lib/chef/formatters/doc.rb +++ b/lib/chef/formatters/doc.rb @@ -143,7 +143,7 @@ class Chef # Called before convergence starts def converge_start(run_context) - puts_line "Converging #{run_context.resource_collection.all_resources.size} resources" + puts_line "Converging #{run_context.resource_list.all_resources.size} resources" end # Called when the converge phase is finished. diff --git a/lib/chef/formatters/minimal.rb b/lib/chef/formatters/minimal.rb index a189cc67eb..eaf7e8f40f 100644 --- a/lib/chef/formatters/minimal.rb +++ b/lib/chef/formatters/minimal.rb @@ -144,7 +144,7 @@ class Chef # Called before convergence starts def converge_start(run_context) - puts "Converging #{run_context.resource_collection.all_resources.size} resources" + puts "Converging #{run_context.resource_list.all_resources.size} resources" end # Called when the converge phase is finished. diff --git a/lib/chef/provider/breakpoint.rb b/lib/chef/provider/breakpoint.rb index 224e2758eb..caa8bd5231 100644 --- a/lib/chef/provider/breakpoint.rb +++ b/lib/chef/provider/breakpoint.rb @@ -25,9 +25,9 @@ class Chef def action_break if defined?(Shell) && Shell.running? - run_context.resource_collection.iterator.pause + run_context.resource_list.iterator.pause @new_resource.updated_by_last_action(true) - run_context.resource_collection.iterator + run_context.resource_list.iterator end end diff --git a/lib/chef/provider/deploy.rb b/lib/chef/provider/deploy.rb index db147278c2..d55dc35f94 100644 --- a/lib/chef/provider/deploy.rb +++ b/lib/chef/provider/deploy.rb @@ -375,7 +375,8 @@ class Chef def gem_resource_collection_runner gems_collection = Chef::ResourceCollection.new - gem_packages.each { |rbgem| gems_collection << rbgem } + # We don't need to populated the resource_set because converge simply runs through each resource and applies them + gem_packages.each { |rbgem| gems_collection.resource_list << rbgem } gems_run_context = run_context.dup gems_run_context.resource_collection = gems_collection Chef::Runner.new(gems_run_context) diff --git a/lib/chef/provider/lwrp_base.rb b/lib/chef/provider/lwrp_base.rb index 135a3f6b7c..ce55b00729 100644 --- a/lib/chef/provider/lwrp_base.rb +++ b/lib/chef/provider/lwrp_base.rb @@ -62,7 +62,7 @@ class Chef return_value ensure @run_context = saved_run_context - if temp_run_context.resource_collection.any? {|r| r.updated? } + if temp_run_context.resource_list.any? {|r| r.updated? } new_resource.updated_by_last_action(true) end end diff --git a/lib/chef/provider/route.rb b/lib/chef/provider/route.rb index 208a4f4139..d430860f04 100644 --- a/lib/chef/provider/route.rb +++ b/lib/chef/provider/route.rb @@ -162,7 +162,7 @@ class Chef::Provider::Route < Chef::Provider case node[:platform] when "centos", "redhat", "fedora" # walk the collection - run_context.resource_collection.each do |resource| + run_context.resource_list.each do |resource| if resource.is_a? Chef::Resource::Route # default to eth0 if resource.device diff --git a/lib/chef/recipe.rb b/lib/chef/recipe.rb index 32578da5ab..599c451ce4 100644 --- a/lib/chef/recipe.rb +++ b/lib/chef/recipe.rb @@ -79,7 +79,7 @@ class Chef # Used by the DSL to look up resources when executing in the context of a # recipe. def resources(*args) - run_context.resource_collection.find(*args) + run_context.resource_set.find(*args) end # This was moved to Chef::Node#tag, redirecting here for compatability diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index 52c0b95591..377a0c4aef 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -49,22 +49,22 @@ class Chef # 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) + def resolve_resource_reference(resource_set) 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) + fix_resource_reference(resource_set) end if not(notifying_resource.kind_of?(Chef::Resource)) - fix_notifier_reference(resource_collection) + fix_notifier_reference(resource_set) 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) + def fix_resource_reference(resource_set) + matching_resource = resource_set.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}" @@ -92,8 +92,8 @@ F # 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) + def fix_notifier_reference(resource_set) + matching_notifier = resource_set.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 "\ @@ -298,10 +298,9 @@ F end end - # TODO can we perform this without having the resource know its declared_type? def load_prior_resource(resource_type, instance_name) begin - prior_resource = run_context.resource_collection.lookup(resource_type, instance_name) + prior_resource = run_context.resource_set.lookup(resource_type, instance_name) # if we get here, there is a prior resource (otherwise we'd have jumped # to the rescue clause). Chef::Log.warn("Cloning resource attributes for #{::Chef::ResourceSet.create_key(resource_type, resource_name)} from prior resource (CHEF-3694)") @@ -446,8 +445,8 @@ 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_set) } + run_context.delayed_notifications(self).each {|n| n.resolve_resource_reference(run_context.resource_set) } end def notifies_immediately(action, resource_spec) @@ -467,7 +466,7 @@ F end def resources(*args) - run_context.resource_collection.find(*args) + run_context.resource_set.find(*args) end def subscribes(action, resources, timing=:delayed) diff --git a/lib/chef/resource_collection.rb b/lib/chef/resource_collection.rb index 2cac42703e..f62245686a 100644 --- a/lib/chef/resource_collection.rb +++ b/lib/chef/resource_collection.rb @@ -19,148 +19,52 @@ require 'chef/resource_set' require 'chef/resource_list' +require 'chef/resource_collection/resource_collection_serialization' +## +# TODO add class documentation class Chef class ResourceCollection + include ResourceCollectionSerialization + + attr_reader :resource_set, :resource_list def initialize @resource_set = ResourceSet.new @resource_list = ResourceList.new end - # TODO proxy all calls with - # http://simonecarletti.com/blog/2010/05/understanding-ruby-and-rails-proxy-patter-delegation-and-basicobject/ - # TODO fundamentally we want to write objects into 2 different data containers. We can proxy reads, but it is # much harder to proxy writes through 1 object. - def all_resources - @resource_list.all_resources - end - - def [](index) - @resource_list[index] - end - - def []=(index, arg) - @resource_list[index] = arg - end - - def <<(*args) - @resource_list.send(:<<, *args) - end - - # 'push' is an alias method to << - alias_method :push, :<< - - def insert(resource) - @resource_list.insert(resource) - end - - def insert_at(insert_at_index, *resources) - @resource_list.insert_at(insert_at_index, *resources) - end - - def insert_as(resource_type, instance_name, resource) - # TODO how does this compete with the above 2 methods? How do I combine them? - @resource_set.insert_as(resource_type, instance_name, resource) - end - - def each - @resource_list.each - end - - def execute_each_resource(&resource_exec_block) - @resource_list.execute_each_resource(&resource_exec_block) - end - - def each_index - @resource_list.each_index - end - - def empty? - @resources_list.empty? - end - - def lookup(resource_type, instance_name) - @resource_set.lookup(resource_type, instance_name) - end - - def find(*args) - @resource_list.find(*args) - end + # TODO insert calls we might need? + # :insert, :insert_at, :[]=, :<<, :push + # :insert_as - # resources is a poorly named, but we have to maintain it for back - # compat. - alias_method :resources, :find + # TODO when there were 2 resources with the same key in resource_set, how do we handle notifications since they get copied? + # Did the old class only keep the last seen reference? - private + # TODO do we need to implement a dup method? Run_context was shallowly copying resource_collection before - def find_resource_by_hash(arg) - results = Array.new - arg.each do |resource_name, name_list| - names = name_list.kind_of?(Array) ? name_list : [ name_list ] - names.each do |name| - res_name = "#{resource_name.to_s}[#{name}]" - results << lookup(res_name) - end - end - return results - end + RESOURCE_LIST_METHODS = Enumerable.instance_methods + + [:all_resources, :[], :each, :execute_each_resource, :each_index, :empty?] + RESOURCE_SET_METHODS = [:lookup, :find, :resources] - def find_resource_by_string(arg) - results = Array.new - case arg - when MULTIPLE_RESOURCE_MATCH - resource_type = $1 - arg =~ /^.+\[(.+)\]$/ - resource_list = $1 - resource_list.split(",").each do |name| - resource_name = "#{resource_type}[#{name}]" - results << lookup(resource_name) - end - when SINGLE_RESOURCE_MATCH - resource_type = $1 - name = $2 - resource_name = "#{resource_type}[#{name}]" - results << lookup(resource_name) - else - raise ArgumentError, "Bad string format #{arg}, you must have a string like resource_type[name]!" - end - return results + def method_missing(name, *args, &block) + if RESOURCE_LIST_METHODS.include?(name) + proxy = @resource_list + elsif RESOURCE_SET_METHODS.include?(name) + proxy = @resource_set + else + raise NoMethodError.new("ResourceCollection does not proxy `#{name}`", name, args) end - - def is_chef_resource(arg) - unless arg.kind_of?(Chef::Resource) - raise ArgumentError, "Cannot insert a #{arg.class} into a resource collection: must be a subclass of Chef::Resource" - end - true + if block + proxy.send(name, *args, &block) + else + proxy.send(name, *args) end - end - module ResourceCollectionSerialization - # Serialize this object as a hash - def to_hash - instance_vars = Hash.new - self.instance_variables.each do |iv| - instance_vars[iv] = self.instance_variable_get(iv) - end - { - 'json_class' => self.class.name, - 'instance_vars' => instance_vars - } end - def to_json(*a) - Chef::JSONCompat.to_json(to_hash, *a) - end - - def self.json_create(o) - collection = self.new() - o["instance_vars"].each do |k,v| - collection.instance_variable_set(k.to_sym, v) - end - collection - end end end diff --git a/lib/chef/resource_collection/resource_collection_serialization.rb b/lib/chef/resource_collection/resource_collection_serialization.rb new file mode 100644 index 0000000000..708e72d848 --- /dev/null +++ b/lib/chef/resource_collection/resource_collection_serialization.rb @@ -0,0 +1,53 @@ +# +# Author:: Tyler Ball () +# Copyright:: Copyright (c) 2014 Chef Software, Inc. +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +class Chef + class ResourceCollection + module ResourceCollectionSerialization + # Serialize this object as a hash + def to_hash + instance_vars = Hash.new + self.instance_variables.each do |iv| + instance_vars[iv] = self.instance_variable_get(iv) + end + { + 'json_class' => self.class.name, + 'instance_vars' => instance_vars + } + end + + def to_json(*a) + Chef::JSONCompat.to_json(to_hash, *a) + end + + def self.json_create(o) + collection = self.new() + o["instance_vars"].each do |k,v| + collection.instance_variable_set(k.to_sym, v) + end + collection + end + + def is_chef_resource(arg) + unless arg.kind_of?(Chef::Resource) + raise ArgumentError, "Cannot insert a #{arg.class} into a resource collection: must be a subclass of Chef::Resource" + end + true + end + end + end +end diff --git a/lib/chef/resource_list.rb b/lib/chef/resource_list.rb index 883906d01a..1392c6b557 100644 --- a/lib/chef/resource_list.rb +++ b/lib/chef/resource_list.rb @@ -18,9 +18,11 @@ require 'chef/resource' require 'chef/resource_collection/stepable_iterator' +require 'chef/resource_collection/resource_collection_serialization' class Chef class ResourceList + include ResourceCollection::ResourceCollectionSerialization include Enumerable attr_reader :iterator @@ -81,7 +83,7 @@ class Chef end def execute_each_resource(&resource_exec_block) - @iterator = StepableIterator.for_collection(@resources) + @iterator = ResourceCollection::StepableIterator.for_collection(@resources) @iterator.each_with_index do |resource, idx| @insert_after_idx = idx yield resource @@ -98,17 +100,5 @@ class Chef @resources.empty? end - # TODO the json serialization/unserialization could be put into a module - - private - - # TODO put into common module - def is_chef_resource(arg) - unless arg.kind_of?(Chef::Resource) - raise ArgumentError, "Cannot insert a #{arg.class} into a resource collection: must be a subclass of Chef::Resource" - end - true - end - end end diff --git a/lib/chef/resource_set.rb b/lib/chef/resource_set.rb index 45e69918cf..e0e2038f78 100644 --- a/lib/chef/resource_set.rb +++ b/lib/chef/resource_set.rb @@ -17,9 +17,11 @@ # require 'chef/resource' +require 'chef/resource_collection/resource_collection_serialization' class Chef class ResourceSet + include ResourceCollection::ResourceCollectionSerialization # Matches a multiple resource lookup specification, # e.g., "service[nginx,unicorn]" @@ -33,15 +35,22 @@ class Chef @resources_by_key = Hash.new end + def keys + @resources_by_key.keys + end + def insert_as(resource_type, instance_name, resource) - assert_chef_resource(resource) - @resources_by_key[create_key(resource_type, instance_name)] = resource + is_chef_resource(resource) + key = ResourceSet.create_key(resource_type, instance_name) + @resources_by_key[key] = resource end # TODO when do we need to lookup a resource by resource? That smells like we should still have the resource say how its indexed, or we shouldn't be doing that. def lookup(resource_type, instance_name) - raise ArgumentError, "Must pass a String to lookup" unless key.kind_of?(String) - res = @resources_by_key[create_key(resource_type, instance_name)] + raise ArgumentError, "Must pass resource_type as a String or Symbol and instance_name as a String" unless + (resource_type.kind_of?(String) || resource_type.kind_of?(Symbol)) && instance_name.kind_of?(String) + key = ResourceSet.create_key(resource_type, instance_name) + res = @resources_by_key[key] unless res raise Chef::Exceptions::ResourceNotFound, "Cannot find a resource matching #{key} (did you define it first?)" end @@ -79,7 +88,6 @@ class Chef # resources is a poorly named, but we have to maintain it for back # compat. - # TODO do we anymore? alias_method :resources, :find # Returns true if +query_object+ is a valid string for looking up a @@ -111,8 +119,6 @@ class Chef end end - # TODO the json serialization/unserialization could be put into a module - def self.create_key(resource_type, instance_name) "#{resource_type}[#{instance_name}]" end @@ -150,13 +156,5 @@ class Chef return results end - # TODO put into common module - def assert_chef_resource(arg) - unless arg.kind_of?(Chef::Resource) - raise ArgumentError, "Cannot insert a #{arg.class} into a resource collection: must be a subclass of Chef::Resource" - end - true - end - end end diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb index 39773f2ceb..b77a2af9b0 100644 --- a/lib/chef/run_context.rb +++ b/lib/chef/run_context.rb @@ -18,8 +18,6 @@ # limitations under the License. require 'chef/resource_collection' -require 'chef/resource_set' -require 'chef/resource_list' require 'chef/cookbook_version' require 'chef/node' require 'chef/role' @@ -51,10 +49,14 @@ class Chef # The Chef::ResourceCollection for this run. Populated by evaluating # recipes, which is triggered by #load. (See also: CookbookCompiler) attr_accessor :resource_collection - # TODO do I even want to expose these, or run everything through the facade? Writers don't really care about - # populating both, I mostly separated them - attr_accessor :resource_set - attr_accessor :resource_list + + def resource_set + resource_collection.resource_set + end + + def resource_list + resource_collection.resource_list + end # A Hash containing the immediate notifications triggered by resources # during the converge phase of the chef run. diff --git a/lib/chef/run_status.rb b/lib/chef/run_status.rb index 0f181426b0..d47a35459c 100644 --- a/lib/chef/run_status.rb +++ b/lib/chef/run_status.rb @@ -70,13 +70,13 @@ class Chef::RunStatus # The list of all resources in the current run context's +resource_collection+ def all_resources - @run_context && @run_context.resource_collection.all_resources + @run_context && @run_context.resource_list.all_resources end # The list of all resources in the current run context's +resource_collection+ # that are marked as updated def updated_resources - @run_context && @run_context.resource_collection.select { |r| r.updated } + @run_context && @run_context.resource_list.select { |r| r.updated } end # The backtrace from +exception+, if any diff --git a/lib/chef/runner.rb b/lib/chef/runner.rb index 6125fe59e1..e27e250b21 100644 --- a/lib/chef/runner.rb +++ b/lib/chef/runner.rb @@ -72,12 +72,12 @@ class Chef # +run_action+ for each resource in turn. def converge # Resolve all lazy/forward references in notifications - run_context.resource_collection.each do |resource| + run_context.resource_list.each do |resource| resource.resolve_notification_references end # Execute each resource. - run_context.resource_collection.execute_each_resource do |resource| + run_context.resource_list.execute_each_resource do |resource| Array(resource.action).each {|action| run_action(resource, action)} end diff --git a/lib/chef/shell/ext.rb b/lib/chef/shell/ext.rb index bc4e955169..ea11d9d1a8 100644 --- a/lib/chef/shell/ext.rb +++ b/lib/chef/shell/ext.rb @@ -244,7 +244,7 @@ E :skip_back => "move back in the run list", :skip_forward => "move forward in the run list" def chef_run - Shell.session.resource_collection.iterator + Shell.session.resource_list.iterator end desc "resets the current recipe" @@ -547,7 +547,7 @@ E desc "list all the resources on the current recipe" def resources(*args) if args.empty? - pp run_context.resource_collection.instance_variable_get(:@resources_by_name).keys + pp run_context.resource_set.keys else pp resources = original_resources(*args) resources diff --git a/lib/chef/shell/shell_session.rb b/lib/chef/shell/shell_session.rb index 73e6c34ebb..6486a5ebeb 100644 --- a/lib/chef/shell/shell_session.rb +++ b/lib/chef/shell/shell_session.rb @@ -69,8 +69,8 @@ module Shell @node.consume_attributes(@node_attributes) end - def resource_collection - run_context.resource_collection + def resource_list + run_context.resource_list end def run_context -- cgit v1.2.1