diff options
| -rw-r--r-- | .travis.yml | 6 | ||||
| -rw-r--r-- | lib/chef/resource_builder.rb | 2 | ||||
| -rw-r--r-- | lib/chef/resource_collection.rb | 50 | ||||
| -rw-r--r-- | lib/chef/run_context.rb | 26 | ||||
| -rw-r--r-- | lib/chef/runner.rb | 40 | ||||
| -rw-r--r-- | spec/integration/recipes/notifies_spec.rb | 334 | ||||
| -rw-r--r-- | spec/unit/resource_collection_spec.rb | 70 |
7 files changed, 500 insertions, 28 deletions
diff --git a/.travis.yml b/.travis.yml index b3321813a6..e93d6ce246 100644 --- a/.travis.yml +++ b/.travis.yml @@ -149,11 +149,11 @@ matrix: - sudo cat /var/log/squid3/access.log allow_failures: + - rvm: 2.2 + env: "GEMFILE_MOD=\"gem 'poise', github: 'poise/poise'\"" + script: bundle exec rake poise_spec - rvm: 2.3.0 - rvm: rbx - - rvm: 2.2 - env: "GEMFILE_MOD=\"gem 'halite', github: 'poise/halite'\"" - script: bundle exec rake halite_spec notifications: on_change: true diff --git a/lib/chef/resource_builder.rb b/lib/chef/resource_builder.rb index f3ca2e95ad..138e401d5c 100644 --- a/lib/chef/resource_builder.rb +++ b/lib/chef/resource_builder.rb @@ -137,7 +137,7 @@ class Chef @prior_resource ||= begin key = "#{type}[#{name}]" - run_context.resource_collection.lookup(key) + run_context.resource_collection.lookup_local(key) rescue Chef::Exceptions::ResourceNotFound nil end diff --git a/lib/chef/resource_collection.rb b/lib/chef/resource_collection.rb index 6c5b4289d8..1429c25d7f 100644 --- a/lib/chef/resource_collection.rb +++ b/lib/chef/resource_collection.rb @@ -33,9 +33,12 @@ class Chef extend Forwardable attr_reader :resource_set, :resource_list - private :resource_set, :resource_list + attr_accessor :run_context - def initialize + protected :resource_set, :resource_list + + def initialize(run_context = nil) + @run_context = run_context @resource_set = ResourceSet.new @resource_list = ResourceList.new end @@ -79,11 +82,50 @@ class Chef resource_list_methods = Enumerable.instance_methods + [:iterator, :all_resources, :[], :each, :execute_each_resource, :each_index, :empty?] - - [:find] # find needs to run on the set - resource_set_methods = [:lookup, :find, :resources, :keys, :validate_lookup_spec!] + [:find] # find overridden below + resource_set_methods = [:resources, :keys, :validate_lookup_spec!] def_delegators :resource_list, *resource_list_methods def_delegators :resource_set, *resource_set_methods + def lookup_local(key) + resource_set.lookup(key) + end + + def find_local(*args) + resource_set.find(*args) + end + + def lookup(key) + if run_context.nil? + lookup_local(key) + else + lookup_recursive(run_context, key) + end + end + + def find(*args) + if run_context.nil? + find_local(*args) + else + find_recursive(run_context, *args) + end + end + + private + + def lookup_recursive(rc, key) + rc.resource_collection.resource_set.lookup(key) + rescue Chef::Exceptions::ResourceNotFound + raise if rc.parent_run_context.nil? + lookup_recursive(rc.parent_run_context, key) + end + + def find_recursive(rc, *args) + rc.resource_collection.resource_set.find(*args) + rescue Chef::Exceptions::ResourceNotFound + raise if rc.parent_run_context.nil? + find_recursive(rc.parent_run_context, *args) + end end end diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb index cb3338d3de..29c936a932 100644 --- a/lib/chef/run_context.rb +++ b/lib/chef/run_context.rb @@ -130,6 +130,14 @@ class Chef # attr_reader :delayed_notification_collection + # + # An Array containing the delayed (end of run) notifications triggered by + # resources during the converge phase of the chef run. + # + # @return [Array[Chef::Resource::Notification]] An array of notification objects + # + attr_reader :delayed_actions + # Creates a new Chef::RunContext object and populates its fields. This object gets # used by the Chef Server to generate a fully compiled recipe list for a node. # @@ -152,6 +160,7 @@ class Chef @loaded_attributes_hash = {} @reboot_info = {} @cookbook_compiler = nil + @delayed_actions = [] initialize_child_state end @@ -172,10 +181,11 @@ class Chef # def initialize_child_state @audits = {} - @resource_collection = Chef::ResourceCollection.new + @resource_collection = Chef::ResourceCollection.new(self) @before_notification_collection = Hash.new { |h, k| h[k] = [] } @immediate_notification_collection = Hash.new { |h, k| h[k] = [] } @delayed_notification_collection = Hash.new { |h, k| h[k] = [] } + @delayed_actions = [] end # @@ -221,6 +231,18 @@ class Chef end # + # Adds a delayed action to the +delayed_actions+. + # + def add_delayed_action(notification) + if delayed_actions.any? { |existing_notification| existing_notification.duplicates?(notification) } + Chef::Log.info( "#{notification.notifying_resource} not queuing delayed action #{notification.action} on #{notification.resource}"\ + " (delayed), as it's already been queued") + else + delayed_actions << notification + end + end + + # # Get the list of before notifications sent by the given resource. # # TODO seriously, this is actually wrong. resource.name is not unique, @@ -640,6 +662,8 @@ ERROR_MESSAGE audits audits= create_child + add_delayed_action + delayed_actions delayed_notification_collection delayed_notification_collection= delayed_notifications diff --git a/lib/chef/runner.rb b/lib/chef/runner.rb index ce128203f2..cd5615bcec 100644 --- a/lib/chef/runner.rb +++ b/lib/chef/runner.rb @@ -30,13 +30,14 @@ class Chef attr_reader :run_context - attr_reader :delayed_actions - include Chef::Mixin::ParamsValidate def initialize(run_context) - @run_context = run_context - @delayed_actions = [] + @run_context = run_context + end + + def delayed_actions + @run_context.delayed_actions end def events @@ -48,16 +49,11 @@ class Chef def run_action(resource, action, notification_type = nil, notifying_resource = nil) # If there are any before notifications, why-run the resource # and notify anyone who needs notifying - # TODO cheffish has a bug where it passes itself instead of the run_context to us, so doesn't have before_notifications. Fix there, update dependency requirement, and remove this if statement. - before_notifications = run_context.before_notifications(resource) if run_context.respond_to?(:before_notifications) - if before_notifications && !before_notifications.empty? - whyrun_before = Chef::Config[:why_run] - begin - Chef::Config[:why_run] = true + before_notifications = run_context.before_notifications(resource) || [] + unless before_notifications.empty? + forced_why_run do Chef::Log.info("#{resource} running why-run #{action} action to support before action") resource.run_action(action, notification_type, notifying_resource) - ensure - Chef::Config[:why_run] = whyrun_before end if resource.updated_by_last_action? @@ -65,8 +61,8 @@ class Chef Chef::Log.info("#{resource} sending #{notification.action} action to #{notification.resource} (before)") run_action(notification.resource, notification.action, :before, resource) end + resource.updated_by_last_action(false) end - end # Actually run the action for realsies @@ -82,12 +78,8 @@ class Chef end run_context.delayed_notifications(resource).each do |notification| - if delayed_actions.any? { |existing_notification| existing_notification.duplicates?(notification) } - Chef::Log.info( "#{resource} not queuing delayed action #{notification.action} on #{notification.resource}"\ - " (delayed), as it's already been queued") - else - delayed_actions << notification - end + # send the notification to the run_context of the receiving resource + notification.resource.run_context.add_delayed_action(notification) end end end @@ -137,5 +129,15 @@ class Chef rescue Exception => e e end + + # helper to run a block of code with why_run forced to true and then restore it correctly + def forced_why_run + saved = Chef::Config[:why_run] + Chef::Config[:why_run] = true + yield + ensure + Chef::Config[:why_run] = saved + end + end end diff --git a/spec/integration/recipes/notifies_spec.rb b/spec/integration/recipes/notifies_spec.rb new file mode 100644 index 0000000000..000f5e37bf --- /dev/null +++ b/spec/integration/recipes/notifies_spec.rb @@ -0,0 +1,334 @@ +require "support/shared/integration/integration_helper" +require "chef/mixin/shell_out" + +describe "notifications" do + include IntegrationSupport + include Chef::Mixin::ShellOut + + let(:chef_dir) { File.expand_path("../../../../bin", __FILE__) } + let(:chef_client) { "ruby '#{chef_dir}/chef-client' --minimal-ohai" } + + when_the_repository "notifies delayed one" do + before do + directory "cookbooks/x" do + + file "resources/notifying_test.rb", <<EOM +default_action :run +provides :notifying_test +resource_name :notifying_test + +action :run do + log "bar" do + notifies :write, 'log[foo]', :delayed + end +end +EOM + + file "recipes/default.rb", <<EOM +log "foo" do + action :nothing +end +notifying_test "whatever" +log "baz" +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + # our delayed notification should run at the end of the parent run_context after the baz resource + expect(result.stdout).to match(/\* log\[bar\] action write\s+\* log\[baz\] action write\s+\* log\[foo\] action write/) + result.error! + end + end + + when_the_repository "notifies delayed two" do + before do + directory "cookbooks/x" do + + file "resources/notifying_test.rb", <<EOM +default_action :run +provides :notifying_test +resource_name :notifying_test + +action :run do + log "bar" do + notifies :write, 'log[foo]', :delayed + end +end +EOM + + file "recipes/default.rb", <<EOM +log "foo" do + action :nothing +end +notifying_test "whatever" +log "baz" do + notifies :write, 'log[foo]', :delayed +end +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + # our delayed notification should run at the end of the parent run_context after the baz resource + expect(result.stdout).to match(/\* log\[bar\] action write\s+\* log\[baz\] action write\s+\* log\[foo\] action write/) + # and only run once + expect(result.stdout).not_to match(/\* log\[foo\] action write.*\* log\[foo\] action write/) + result.error! + end + end + + when_the_repository "notifies delayed three" do + before do + directory "cookbooks/x" do + + file "resources/notifying_test.rb", <<EOM +default_action :run +provides :notifying_test +resource_name :notifying_test + +action :run do + log "bar" do + notifies :write, 'log[foo]', :delayed + end +end +EOM + + file "recipes/default.rb", <<EOM +log "foo" do + action :nothing +end +log "quux" do + notifies :write, 'log[foo]', :delayed + notifies :write, 'log[baz]', :delayed +end +notifying_test "whatever" +log "baz" +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + # the delayed notification from the sub-resource is de-duplicated by the notification already in the parent run_context + expect(result.stdout).to match(/\* log\[quux\] action write\s+\* notifying_test\[whatever\] action run\s+\* log\[bar\] action write\s+\* log\[baz\] action write\s+\* log\[foo\] action write\s+\* log\[baz\] action write/) + # and only run once + expect(result.stdout).not_to match(/\* log\[foo\] action write.*\* log\[foo\] action write/) + result.error! + end + end + + when_the_repository "notifies delayed four" do + before do + directory "cookbooks/x" do + file "recipes/default.rb", <<EOM +log "foo" do + action :nothing +end +log "bar" do + notifies :write, 'log[foo]', :delayed +end +log "baz" do + notifies :write, 'log[foo]', :delayed +end +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + # the delayed notification from the sub-resource is de-duplicated by the notification already in the parent run_context + expect(result.stdout).to match(/\* log\[bar\] action write\s+\* log\[baz\] action write\s+\* log\[foo\] action write/) + # and only run once + expect(result.stdout).not_to match(/\* log\[foo\] action write.*\* log\[foo\] action write/) + result.error! + end + end + + when_the_repository "notifies immediately" do + before do + directory "cookbooks/x" do + + file "resources/notifying_test.rb", <<EOM +default_action :run +provides :notifying_test +resource_name :notifying_test + +action :run do + log "bar" do + notifies :write, 'log[foo]', :immediately + end +end +EOM + + file "recipes/default.rb", <<EOM +log "foo" do + action :nothing +end +notifying_test "whatever" +log "baz" +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + expect(result.stdout).to match(/\* log\[bar\] action write\s+\* log\[foo\] action write\s+\* log\[baz\] action write/) + result.error! + end + end + + when_the_repository "uses old notifies syntax" do + before do + directory "cookbooks/x" do + + file "resources/notifying_test.rb", <<EOM +default_action :run +provides :notifying_test +resource_name :notifying_test + +action :run do + log "bar" do + notifies :write, resources(log: "foo"), :immediately + end +end +EOM + + file "recipes/default.rb", <<EOM +log "foo" do + action :nothing +end +notifying_test "whatever" +log "baz" +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + expect(result.stdout).to match(/\* log\[bar\] action write\s+\* log\[foo\] action write\s+\* log\[baz\] action write/) + result.error! + end + end + + when_the_repository "does not have a matching resource" do + before do + directory "cookbooks/x" do + + file "resources/notifying_test.rb", <<EOM +default_action :run +provides :notifying_test +resource_name :notifying_test + +action :run do + log "bar" do + notifies :write, "log[foo]" + end +end +EOM + + file "recipes/default.rb", <<EOM +notifying_test "whatever" +log "baz" +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + expect(result.stdout).to match(/Chef::Exceptions::ResourceNotFound/) + expect(result.exitstatus).not_to eql(0) + end + end + + when_the_repository "encounters identical resources in parent and child resource collections" do + before do + directory "cookbooks/x" do + + file "resources/cloning_test.rb", <<EOM +default_action :run +provides :cloning_test +resource_name :cloning_test + +action :run do + log "bar" do + level :info + end +end +EOM + + file "recipes/default.rb", <<EOM +log "bar" do + level :warn +end + +cloning_test "whatever" +EOM + + end + end + + it "should complete with success" do + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +log_level :warn +EOM + + result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir) + expect(result.stdout).not_to match(/CHEF-3694/) + result.error! + end + end +end diff --git a/spec/unit/resource_collection_spec.rb b/spec/unit/resource_collection_spec.rb index a6ad895c51..256ae090e0 100644 --- a/spec/unit/resource_collection_spec.rb +++ b/spec/unit/resource_collection_spec.rb @@ -283,6 +283,76 @@ describe Chef::ResourceCollection do end end + describe "multiple run_contexts" do + let(:node) { Chef::Node.new } + let(:parent_run_context) { Chef::RunContext.new(node, {}, nil) } + let(:parent_resource_collection) { parent_run_context.resource_collection } + let(:child_run_context) { parent_run_context.create_child } + let(:child_resource_collection) { child_run_context.resource_collection } + + it "should find resources in the parent run_context with lookup" do + zmr = Chef::Resource::ZenMaster.new("dog") + parent_resource_collection << zmr + expect(child_resource_collection.lookup(zmr.to_s)).to eql(zmr) + end + + it "should not find resources in the parent run_context with lookup_local" do + zmr = Chef::Resource::ZenMaster.new("dog") + parent_resource_collection << zmr + expect { child_resource_collection.lookup_local(zmr.to_s) }.to raise_error(Chef::Exceptions::ResourceNotFound) + end + + it "should find resources in the child run_context with lookup_local" do + zmr = Chef::Resource::ZenMaster.new("dog") + child_resource_collection << zmr + expect(child_resource_collection.lookup_local(zmr.to_s)).to eql(zmr) + end + + it "should find resources in the parent run_context with find" do + zmr = Chef::Resource::ZenMaster.new("dog") + parent_resource_collection << zmr + expect(child_resource_collection.find(zmr.to_s)).to eql(zmr) + end + + it "should not find resources in the parent run_context with find_local" do + zmr = Chef::Resource::ZenMaster.new("dog") + parent_resource_collection << zmr + expect { child_resource_collection.find_local(zmr.to_s) }.to raise_error(Chef::Exceptions::ResourceNotFound) + end + + it "should find resources in the child run_context with find_local" do + zmr = Chef::Resource::ZenMaster.new("dog") + child_resource_collection << zmr + expect(child_resource_collection.find_local(zmr.to_s)).to eql(zmr) + end + + it "should not find resources in the child run_context in any way from the parent" do + zmr = Chef::Resource::ZenMaster.new("dog") + child_resource_collection << zmr + expect { parent_resource_collection.find_local(zmr.to_s) }.to raise_error(Chef::Exceptions::ResourceNotFound) + expect { parent_resource_collection.find(zmr.to_s) }.to raise_error(Chef::Exceptions::ResourceNotFound) + expect { parent_resource_collection.lookup_local(zmr.to_s) }.to raise_error(Chef::Exceptions::ResourceNotFound) + expect { parent_resource_collection.lookup(zmr.to_s) }.to raise_error(Chef::Exceptions::ResourceNotFound) + end + + it "should behave correctly when there is an identically named resource in the child and parent" do + a = Chef::Resource::File.new("something") + a.content("foo") + parent_resource_collection << a + b = Chef::Resource::File.new("something") + b.content("bar") + child_resource_collection << b + expect(child_resource_collection.find_local("file[something]").content).to eql("bar") + expect(child_resource_collection.find("file[something]").content).to eql("bar") + expect(child_resource_collection.lookup_local("file[something]").content).to eql("bar") + expect(child_resource_collection.lookup("file[something]").content).to eql("bar") + expect(parent_resource_collection.find_local("file[something]").content).to eql("foo") + expect(parent_resource_collection.find("file[something]").content).to eql("foo") + expect(parent_resource_collection.lookup_local("file[something]").content).to eql("foo") + expect(parent_resource_collection.lookup("file[something]").content).to eql("foo") + end + end + def check_by_names(results, *names) names.each do |res_name| expect(results.detect { |res| res.name == res_name }).not_to eql(nil) |