diff options
-rw-r--r-- | RELEASE_NOTES.md | 20 | ||||
-rw-r--r-- | lib/chef/resource.rb | 20 | ||||
-rw-r--r-- | lib/chef/run_context.rb | 43 | ||||
-rw-r--r-- | lib/chef/runner.rb | 25 | ||||
-rw-r--r-- | spec/functional/notifications_spec.rb | 78 | ||||
-rw-r--r-- | spec/support/shared/integration/integration_helper.rb | 6 |
6 files changed, 187 insertions, 5 deletions
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 5a06af0dbb..f714590c30 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -72,6 +72,26 @@ This change is for the scenario when running chef client as a Windows service. C * Previously, Chef required the LCM Refreshmode to be set to Disabled when utilizing dsc_resource. Microsoft has relaxed this requirement in Windows Management Framework 5 (WMF5) (PowerShell 5.0.10586.0 or later). Now, we only require the RefreshMode to be disabled when running on earlier versions of PowerShell 5. * Added a reboot_action attribute to dsc_resource. If the DSC resource indicates that it requires a reboot, reboot_action can use the reboot resource to either reboot immediately (:reboot_now) or queue a reboot (:request_reboot). The default value of reboot_action is :nothing. +In addition to `:immediately` and `:delayed`, we have added the new notification timing `:before`. `:before` will trigger just before the +resource converges, but will only trigger if the resource is going to +actually cause an update. + +For example, this will stop apache if you are about to upgrade your particularly sensitive web app (which can't run while installing for +whatever reason) and start it back up afterwards. + +``` +execute 'install my app' do + only_if { i_should_install_my_app } +end + +# Only stop and start apache if i_should_install_my_app +service 'httpd' do + action :nothing + subscribes :stop, 'template[/etc/httpd.conf]', :before + subscribes :start, 'template[/etc/httpd.conf]' +end +``` + ## Other items There are a large number of other PRs in this release. Please see the CHANGELOG for the full set of changes. diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index 43fc1f997d..863d357561 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -209,6 +209,8 @@ class Chef # actions have been run. This is the default. # - `immediate`, `immediately`: Will run the action on the other resource # immediately (before any other action is run). + # - `before`: Will run the action on the other resource + # immediately *before* the action is actually run. # # @example Resource by string # file '/foo.txt' do @@ -251,9 +253,11 @@ class Chef notifies_delayed(action, resource) when 'immediate', 'immediately' notifies_immediately(action, resource) + when 'before' + notifies_before(action, resource) else raise ArgumentError, "invalid timing: #{timing} for notifies(#{action}, #{resources.inspect}, #{timing}) resource #{self} "\ - "Valid timings are: :delayed, :immediate, :immediately" + "Valid timings are: :delayed, :immediate, :immediately, :before" end end @@ -277,6 +281,8 @@ class Chef # actions have been run. This is the default. # - `immediate`, `immediately`: The action will run immediately following # the other resource being updated. + # - `before`: The action will run immediately before the + # other resource is updated. # # @example Resources by string # file '/foo.txt' do @@ -1238,6 +1244,9 @@ class Chef # resolve_resource_reference on each in turn, causing them to # resolve lazy/forward references. def resolve_notification_references + run_context.before_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) } @@ -1247,6 +1256,11 @@ class Chef end # Helper for #notifies + def notifies_before(action, resource_spec) + run_context.notifies_before(Notification.new(resource_spec, action, self)) + end + + # Helper for #notifies def notifies_immediately(action, resource_spec) run_context.notifies_immediately(Notification.new(resource_spec, action, self)) end @@ -1341,6 +1355,10 @@ class Chef "#{declared_type}[#{@name}]" end + def before_notifications + run_context.before_notifications(self) + end + def immediate_notifications run_context.immediate_notifications(self) end diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb index f7ab88f7e0..6e02f47ff1 100644 --- a/lib/chef/run_context.rb +++ b/lib/chef/run_context.rb @@ -104,6 +104,15 @@ class Chef # # + # A Hash containing the before notifications triggered by resources + # during the converge phase of the chef run. + # + # @return [Hash[String, Array[Chef::Resource::Notification]]] A hash from + # <notifying resource name> => <list of notifications it sent> + # + attr_reader :before_notification_collection + + # # A Hash containing the immediate notifications triggered by resources # during the converge phase of the chef run. # @@ -164,11 +173,26 @@ class Chef def initialize_child_state @audits = {} @resource_collection = Chef::ResourceCollection.new + @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] = []} end # + # Adds an before notification to the +before_notification_collection+. + # + # @param [Chef::Resource::Notification] The notification to add. + # + def notifies_before(notification) + nr = notification.notifying_resource + if nr.instance_of?(Chef::Resource) + before_notification_collection[nr.name] << notification + else + before_notification_collection[nr.declared_key] << notification + end + end + + # # Adds an immediate notification to the +immediate_notification_collection+. # # @param [Chef::Resource::Notification] The notification to add. @@ -197,6 +221,22 @@ class Chef end # + # Get the list of before notifications sent by the given resource. + # + # TODO seriously, this is actually wrong. resource.name is not unique, + # you need the type as well. + # + # @return [Array[Notification]] + # + def before_notifications(resource) + if resource.instance_of?(Chef::Resource) + return before_notification_collection[resource.name] + else + return before_notification_collection[resource.declared_key] + end + end + + # # Get the list of immediate notifications sent by the given resource. # # TODO seriously, this is actually wrong. resource.name is not unique, @@ -608,10 +648,13 @@ ERROR_MESSAGE immediate_notification_collection immediate_notification_collection= immediate_notifications + before_notification_collection + before_notifications include_recipe initialize_child_state load_recipe load_recipe_file + notifies_before notifies_immediately notifies_delayed parent_run_context diff --git a/lib/chef/runner.rb b/lib/chef/runner.rb index 6125fe59e1..61b68e2375 100644 --- a/lib/chef/runner.rb +++ b/lib/chef/runner.rb @@ -46,6 +46,31 @@ class Chef # Determine the appropriate provider for the given resource, then # execute it. 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 + 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? + before_notifications.each do |notification| + Chef::Log.info("#{resource} sending #{notification.action} action to #{notification.resource} (before)") + run_action(notification.resource, notification.action, :before, resource) + end + end + + end + + # Actually run the action for realsies resource.run_action(action, notification_type, notifying_resource) # Execute any immediate and queue up any delayed notifications diff --git a/spec/functional/notifications_spec.rb b/spec/functional/notifications_spec.rb index a02fdffe5e..f14f35ebba 100644 --- a/spec/functional/notifications_spec.rb +++ b/spec/functional/notifications_spec.rb @@ -74,6 +74,76 @@ describe "Notifications" do runner.converge end + it "should notify from one resource to another before" do + log_resource = recipe.declare_resource(:log, "log") do + message "This is a log message" + action :write + notifies :install, "package[vim]", :before + end + update_action(log_resource, 2) + + package_resource = recipe.declare_resource(:package, "vim") do + action :nothing + end + + actions = [] + [ log_resource, package_resource ].each do |resource| + allow(resource).to receive(:run_action).and_wrap_original do |m, action, notification_type, notifying_resource| + actions << { resource: resource.to_s, action: action } + actions[-1][:why_run] = Chef::Config[:why_run] if Chef::Config[:why_run] + actions[-1][:notification_type] = notification_type if notification_type + actions[-1][:notifying_resource] = notifying_resource.to_s if notifying_resource + m.call(action, notification_type, notifying_resource) + end + end + + runner.converge + + expect(actions).to eq [ + # First it runs why-run to check if the resource would update + { resource: log_resource.to_s, action: :write, why_run: true }, + # Then it runs the before action + { resource: package_resource.to_s, action: :install, notification_type: :before, notifying_resource: log_resource.to_s }, + # Then it runs the actual action + { resource: log_resource.to_s, action: :write }, + { resource: package_resource.to_s, action: :nothing } + ] + end + + it "should not notify from one resource to another before if the resource is not updated" do + log_resource = recipe.declare_resource(:log, "log") do + message "This is a log message" + action :write + notifies :install, "package[vim]", :before + end + + package_resource = recipe.declare_resource(:package, "vim") do + action :nothing + end + + actions = [] + [ log_resource, package_resource ].each do |resource| + allow(resource).to receive(:run_action).and_wrap_original do |m, action, notification_type, notifying_resource| + actions << { resource: resource.to_s, action: action } + actions[-1][:why_run] = Chef::Config[:why_run] if Chef::Config[:why_run] + actions[-1][:notification_type] = notification_type if notification_type + actions[-1][:notifying_resource] = notifying_resource.to_s if notifying_resource + m.call(action, notification_type, notifying_resource) + end + end + + runner.converge + + expect(actions).to eq [ + # First it runs why-run to check if the resource would update + { resource: log_resource.to_s, action: :write, why_run: true }, + # Then it does NOT run the before action + # Then it runs the actual action + { resource: log_resource.to_s, action: :write }, + { resource: package_resource.to_s, action: :nothing } + ] + end + it "should notify from one resource to another delayed" do log_resource = recipe.declare_resource(:log, "log") do message "This is a log message" @@ -94,7 +164,7 @@ describe "Notifications" do runner.converge end - + describe "when one resource is defined lazily" do it "subscribes to a resource defined in a ruby block" do @@ -158,10 +228,10 @@ describe "Notifications" do end # Mocks having the provider run successfully and update the resource - def update_action(resource) + def update_action(resource, times=1) p = Chef::Provider.new(resource, run_context) - expect(resource).to receive(:provider_for_action).and_return(p) - expect(p).to receive(:run_action) { + expect(resource).to receive(:provider_for_action).exactly(times).times.and_return(p) + expect(p).to receive(:run_action).exactly(times).times { resource.updated_by_last_action(true) } end diff --git a/spec/support/shared/integration/integration_helper.rb b/spec/support/shared/integration/integration_helper.rb index 927ff2f42b..7d62a698d8 100644 --- a/spec/support/shared/integration/integration_helper.rb +++ b/spec/support/shared/integration/integration_helper.rb @@ -27,6 +27,12 @@ require 'support/shared/integration/app_server_support' require 'cheffish/rspec/chef_run_support' require 'spec_helper' +module Cheffish + class BasicChefClient + def_delegators :@run_context, :before_notifications + end +end + module IntegrationSupport include ChefZero::RSpec |