diff options
author | John Keiser <john@johnkeiser.com> | 2015-07-31 12:42:39 -0600 |
---|---|---|
committer | John Keiser <john@johnkeiser.com> | 2015-07-31 12:42:39 -0600 |
commit | b769876511717fb7b622183eeb5013ac7e1cd644 (patch) | |
tree | 46fbc04b7e9aeb445848843ca1b8f04ba10d3465 | |
parent | 221e63a2de6414ae1d4422753589dccf7d35c351 (diff) | |
parent | 2a600705e6f7740fca4212b2f7e34508d44babad (diff) | |
download | chef-b769876511717fb7b622183eeb5013ac7e1cd644.tar.gz |
Merge branch 'jk/converge_if_changed'
-rw-r--r-- | DOC_CHANGES.md | 78 | ||||
-rw-r--r-- | Gemfile | 2 | ||||
-rw-r--r-- | lib/chef/dsl/recipe.rb | 6 | ||||
-rw-r--r-- | lib/chef/exceptions.rb | 3 | ||||
-rw-r--r-- | lib/chef/provider.rb | 56 | ||||
-rw-r--r-- | lib/chef/resource.rb | 41 | ||||
-rw-r--r-- | lib/chef/resource/action_provider.rb | 69 | ||||
-rw-r--r-- | spec/integration/recipes/resource_action_spec.rb | 10 | ||||
-rw-r--r-- | spec/integration/recipes/resource_converge_if_changed_spec.rb | 423 |
9 files changed, 648 insertions, 40 deletions
diff --git a/DOC_CHANGES.md b/DOC_CHANGES.md index 2073b4c089..b8eb61d12c 100644 --- a/DOC_CHANGES.md +++ b/DOC_CHANGES.md @@ -288,5 +288,79 @@ end Now, the above recipe knows what the current homepage is, and will not change it! This capability is also used for several other things, including reporting (to -describe what changed) and deeper custom resources (ones that don't use recipes, -which we'll cover later). +describe what changed) and pure Ruby actions. + +#### Pure Ruby Actions + +Some resources need to talk directly to Ruby to do their dirty work, rather than using other resources. In those cases, you need to: + +- Make the updates only if the user specified properties that *need* to change. +- Make sure and call updates if the resource does not exist (need to be created). +- Print useful green text if the update is happening. +- Not actually make any changes in why-run mode! + +`converge_if_changed` handles all of the above by comparing the user's desired +property values against the *current* value as loaded by `load_current_value`. +Simply wrap the part of your recipe that does a set in `converge_if_changed`. +As an example, here is a basic `my_file` resource that creates a file with the +given content: + +```ruby +# resources/my_file.rb +property :path, String, name_property: true +property :content, String + +load_current_value do + if File.exist?(path) + content IO.read(path) + end +end + +action :create do + converge_if_changed do + IO.write(path, content) + end +end +``` + +The above code will only call `IO.write` if the file does not exist, or if the +user specified content that is different from what is on disk. It will print out +something like this, showing the changes: + +```ruby +Recipe: basic_chef_client::block + * my_file[blah] action create + - update my_file[blah] + - set content to "hola mundo" (was "hello world") + ``` + +##### Handling Multiple Operations + +If you have two separate, expensive operations to handle converge, `converge_if_changed` +can be called multiple times with multiple properties. Adding `mode` to `my_file` +demonstrates this: + +```ruby +# resources/my_file.rb +property :path, String, name_property: true +property :content, String +property :mode, String + +load_current_value do + if File.exist?(path) + content IO.read(path) + mode File.stat(path).mode + end +end + +action :create do + # Only change content here + converge_if_changed :content do + IO.write(path, content) + end + # Only change mode here + converge_if_changed :mode do + File.chmod(mode, path) + end +end +``` @@ -13,7 +13,7 @@ end group(:development, :test) do gem "simplecov" gem 'rack', "~> 1.5.1" - gem 'cheffish', "~> 1.2" + gem 'cheffish', "~> 1.3" gem 'ruby-shadow', :platforms => :ruby unless RUBY_PLATFORM.downcase.match(/(aix|cygwin)/) end diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb index d00d0df247..29cfcd478c 100644 --- a/lib/chef/dsl/recipe.rb +++ b/lib/chef/dsl/recipe.rb @@ -19,12 +19,10 @@ require 'chef/mixin/convert_to_class_name' require 'chef/exceptions' -require 'chef/resource_builder' require 'chef/mixin/shell_out' require 'chef/mixin/powershell_out' require 'chef/dsl/resources' require 'chef/dsl/definitions' -require 'chef/resource' class Chef module DSL @@ -198,6 +196,10 @@ class Chef end end +# Avoid circular references for things that are only used in instance methods +require 'chef/resource_builder' +require 'chef/resource' + # **DEPRECATED** # This used to be part of chef/mixin/recipe_definition_dsl_core. Load the file to activate the deprecation code. require 'chef/mixin/recipe_definition_dsl_core' diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index e2e36e8162..16f985acaa 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -105,6 +105,9 @@ class Chef class VerificationNotFound < RuntimeError; end class InvalidEventType < ArgumentError; end class MultipleIdentityError < RuntimeError; end + # Used in Resource::ActionProvider#load_current_resource to denote that + # the resource doesn't actually exist (for example, the file does not exist) + class CurrentValueDoesNotExist < RuntimeError; end # Can't find a Resource of this type that is valid on this platform. class NoSuchResourceType < NameError diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb index 089c28be50..f2a493c3e6 100644 --- a/lib/chef/provider.rb +++ b/lib/chef/provider.rb @@ -175,6 +175,62 @@ class Chef converge_actions.add_action(descriptions, &block) end + # + # Handle patchy convergence safely. + # + # - Does *not* call the block if the current_resource's properties match + # the properties the user specified on the resource. + # - Calls the block if current_resource does not exist + # - Calls the block if the user has specified any properties in the resource + # whose values are *different* from current_resource. + # - Does *not* call the block if why-run is enabled (just prints out text). + # - Prints out automatic green text saying what properties have changed. + # + # @param properties An optional list of property names (symbols). If not + # specified, `new_resource.class.state_properties` will be used. + # @param converge_block The block to do the converging in. + # + # @return [Boolean] whether the block was executed. + # + def converge_if_changed(*properties, &converge_block) + if !converge_block + raise ArgumentError, "converge_if_changed must be passed a block!" + end + + properties = new_resource.class.state_properties.map { |p| p.name } if properties.empty? + properties = properties.map { |p| p.to_sym } + if current_resource + # Collect the list of modified properties + specified_properties = properties.select { |property| new_resource.property_is_set?(property) } + modified = specified_properties.select { |p| new_resource.send(p) != current_resource.send(p) } + if modified.empty? + Chef::Log.debug("Skipping update of #{new_resource.to_s}: has not changed any of the specified properties #{specified_properties.map { |p| "#{p}=#{new_resource.send(p).inspect}" }.join(", ")}.") + return false + end + + # Print the pretty green text and run the block + property_size = modified.map { |p| p.size }.max + modified = modified.map { |p| " set #{p.to_s.ljust(property_size)} to #{new_resource.send(p).inspect} (was #{current_resource.send(p).inspect})" } + converge_by([ "update #{current_resource.identity}" ] + modified, &converge_block) + + else + # The resource doesn't exist. Mark that we are *creating* this, and + # write down any properties we are setting. + property_size = properties.map { |p| p.size }.max + created = [] + properties.each do |property| + if new_resource.property_is_set?(property) + created << " set #{property.to_s.ljust(property_size)} to #{new_resource.send(property).inspect}" + else + created << " set #{property.to_s.ljust(property_size)} to #{new_resource.send(property).inspect} (default value)" + end + end + + converge_by([ "create #{new_resource.identity}" ] + created, &converge_block) + end + true + end + def self.provides(short_name, opts={}, &block) Chef.provider_handler_map.set(short_name, self, opts, &block) end diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index bb5c6050fe..b9c074117f 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -1,7 +1,8 @@ # # Author:: Adam Jacob (<adam@opscode.com>) # Author:: Christopher Walters (<cw@opscode.com>) -# Copyright:: Copyright (c) 2008 Opscode, Inc. +# Author:: John Keiser (<jkeiser@chef.io) +# Copyright:: Copyright (c) 2008-2015 Chef, Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,6 +18,7 @@ # limitations under the License. # +require 'chef/exceptions' require 'chef/mixin/params_validate' require 'chef/dsl/platform_introspection' require 'chef/dsl/data_query' @@ -27,6 +29,7 @@ require 'chef/mixin/convert_to_class_name' require 'chef/guard_interpreter/resource_guard_interpreter' require 'chef/resource/conditional' require 'chef/resource/conditional_action_not_nothing' +require 'chef/resource/action_provider' require 'chef/resource_collection' require 'chef/node_map' require 'chef/node' @@ -1386,6 +1389,16 @@ class Chef end # + # Call this in `load_current_value` to indicate that the value does not + # exist and that `current_resource` should therefore be `nil`. + # + # @raise Chef::Exceptions::CurrentValueDoesNotExist + # + def current_value_does_not_exist! + raise Chef::Exceptions::CurrentValueDoesNotExist + end + + # # Get the current actual value of this resource. # # This does not cache--a new value will be returned each time. @@ -1439,35 +1452,13 @@ class Chef resource_class = self @action_provider_class = Class.new(base_provider) do - use_inline_resources - include_resource_dsl true + include ActionProvider define_singleton_method(:to_s) { "#{resource_class} action provider" } def self.inspect to_s end - def load_current_resource - if new_resource.respond_to?(:load_current_value!) - # dup the resource and then reset desired-state properties. - current_resource = new_resource.dup - - # We clear desired state in the copy, because it is supposed to be actual state. - # We keep identity properties and non-desired-state, which are assumed to be - # "control" values like `recurse: true` - current_resource.class.properties.each do |name,property| - if property.desired_state? && !property.identity? && !property.name_property? - property.reset(current_resource) - end - end - - if current_resource.method(:load_current_value!).arity > 0 - current_resource.load_current_value!(new_resource) - else - current_resource.load_current_value! - end - end - @current_resource = current_resource - end end + @action_provider_class end # diff --git a/lib/chef/resource/action_provider.rb b/lib/chef/resource/action_provider.rb new file mode 100644 index 0000000000..d71b54ef4d --- /dev/null +++ b/lib/chef/resource/action_provider.rb @@ -0,0 +1,69 @@ +# +# Author:: John Keiser (<jkeiser@chef.io) +# Copyright:: Copyright (c) 2015 Opscode, 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. +# + +require 'chef/exceptions' + +class Chef + class Resource + module ActionProvider + # + # If load_current_value! is defined on the resource, use that. + # + def load_current_resource + if new_resource.respond_to?(:load_current_value!) + # dup the resource and then reset desired-state properties. + current_resource = new_resource.dup + + # We clear desired state in the copy, because it is supposed to be actual state. + # We keep identity properties and non-desired-state, which are assumed to be + # "control" values like `recurse: true` + current_resource.class.properties.each do |name,property| + if property.desired_state? && !property.identity? && !property.name_property? + property.reset(current_resource) + end + end + + # Call the actual load_current_value! method. If it raises + # CurrentValueDoesNotExist, set current_resource to `nil`. + begin + # If the user specifies load_current_value do |desired_resource|, we + # pass in the desired resource as well as the current one. + if current_resource.method(:load_current_value!).arity > 0 + current_resource.load_current_value!(new_resource) + else + current_resource.load_current_value! + end + rescue Chef::Exceptions::CurrentValueDoesNotExist + current_resource = nil + end + end + + @current_resource = current_resource + end + + def self.included(other) + other.extend(ClassMethods) + other.use_inline_resources + other.include_resource_dsl true + end + + module ClassMethods + end + end + end +end diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb index 1881ab0d03..53611c144f 100644 --- a/spec/integration/recipes/resource_action_spec.rb +++ b/spec/integration/recipes/resource_action_spec.rb @@ -3,16 +3,6 @@ require 'support/shared/integration/integration_helper' describe "Resource.action" do include IntegrationSupport - def converge(str=nil, file=nil, line=nil, &block) - if block - super(&block) - else - super() do - eval(str, nil, file, line) - end - end - end - shared_context "ActionJackson" do it "The default action is the first declared action" do converge <<-EOM, __FILE__, __LINE__+1 diff --git a/spec/integration/recipes/resource_converge_if_changed_spec.rb b/spec/integration/recipes/resource_converge_if_changed_spec.rb new file mode 100644 index 0000000000..d00252a717 --- /dev/null +++ b/spec/integration/recipes/resource_converge_if_changed_spec.rb @@ -0,0 +1,423 @@ +require 'support/shared/integration/integration_helper' + +describe "Resource::ActionProvider#converge_if_changed" do + include IntegrationSupport + + module Namer + extend self + attr_accessor :current_index + def incrementing_value + @incrementing_value += 1 + @incrementing_value + end + attr_writer :incrementing_value + end + + before(:all) { Namer.current_index = 1 } + before { Namer.current_index += 1 } + before { Namer.incrementing_value = 0 } + + context "when the resource has identity, state and control properties" do + let(:resource_name) { :"converge_if_changed_dsl#{Namer.current_index}" } + let(:resource_class) { + result = Class.new(Chef::Resource) do + def self.to_s; resource_name; end + def self.inspect; resource_name.inspect; end + property :identity1, identity: true, default: 'default_identity1' + property :control1, desired_state: false, default: 'default_control1' + property :state1, default: 'default_state1' + property :state2, default: 'default_state2' + attr_accessor :converged + def initialize(*args) + super + @converged = 0 + end + end + result.resource_name resource_name + result + } + let(:converged_recipe) { converge(converge_recipe) } + let(:resource) { converged_recipe.resources.first } + + context "and converge_if_changed with no parameters" do + before :each do + resource_class.action :create do + converge_if_changed do + new_resource.converged += 1 + end + end + end + + context "and current_resource with state1=current, state2=current" do + before :each do + resource_class.load_current_value do + state1 'current_state1' + state2 'current_state2' + end + end + + context "and nothing is set" do + let(:converge_recipe) { "#{resource_name} 'blah'" } + + it "the resource updates nothing" do + expect(resource.converged).to eq 0 + expect(resource.updated?).to be_falsey + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create (up to date) + EOM + end + end + + context "and state1 is set to a new value" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'new_state1' + end + EOM + } + + it "the resource updates state1" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - update default_identity1 + - set state1 to "new_state1" (was "current_state1") + EOM + end + end + + context "and state1 and state2 are set to new values" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'new_state1' + state2 'new_state2' + end + EOM + } + + it "the resource updates state1 and state2" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - update default_identity1 + - set state1 to "new_state1" (was "current_state1") + - set state2 to "new_state2" (was "current_state2") +EOM + end + end + + context "and state1 is set to its current value but state2 is set to a new value" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'current_state1' + state2 'new_state2' + end + EOM + } + + it "the resource updates state2" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - update default_identity1 + - set state2 to "new_state2" (was "current_state2") +EOM + end + end + + context "and state1 and state2 are set to their current values" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'current_state1' + state2 'current_state2' + end + EOM + } + + it "the resource updates nothing" do + expect(resource.converged).to eq 0 + expect(resource.updated?).to be_falsey + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create (up to date) +EOM + end + end + + context "and identity1 and control1 are set to new values" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + identity1 'new_identity1' + control1 'new_control1' + end + EOM + } + + # Because the identity value is copied over to the new resource, by + # default they do not register as "changed" + it "the resource updates nothing" do + expect(resource.converged).to eq 0 + expect(resource.updated?).to be_falsey + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create (up to date) +EOM + end + end + end + + context "and current_resource with identity1=current, control1=current" do + before :each do + resource_class.load_current_value do + identity1 'current_identity1' + control1 'current_control1' + end + end + + context "and identity1 and control1 are set to new values" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + identity1 'new_identity1' + control1 'new_control1' + end + EOM + } + + # Control values are not desired state and are therefore not considered + # a reason for converging. + it "the resource updates identity1" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - update current_identity1 + - set identity1 to "new_identity1" (was "current_identity1") + EOM + end + end + end + + context "and has no current_resource" do + before :each do + resource_class.load_current_value do + current_value_does_not_exist! + end + end + + context "and nothing is set" do + let(:converge_recipe) { "#{resource_name} 'blah'" } + + it "the resource is created" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - create default_identity1 + - set identity1 to "default_identity1" (default value) + - set state1 to "default_state1" (default value) + - set state2 to "default_state2" (default value) +EOM + end + end + + context "and state1 and state2 are set" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'new_state1' + state2 'new_state2' + end + EOM + } + + it "the resource is created" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - create default_identity1 + - set identity1 to "default_identity1" (default value) + - set state1 to "new_state1" + - set state2 to "new_state2" +EOM + end + end + end + end + + context "and separate converge_if_changed :state1 and converge_if_changed :state2" do + before :each do + resource_class.action :create do + converge_if_changed :state1 do + new_resource.converged += 1 + end + converge_if_changed :state2 do + new_resource.converged += 1 + end + end + end + + context "and current_resource with state1=current, state2=current" do + before :each do + resource_class.load_current_value do + state1 'current_state1' + state2 'current_state2' + end + end + + context "and nothing is set" do + let(:converge_recipe) { "#{resource_name} 'blah'" } + + it "the resource updates nothing" do + expect(resource.converged).to eq 0 + expect(resource.updated?).to be_falsey + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create (up to date) +EOM + end + end + + context "and state1 is set to a new value" do + + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'new_state1' + end + EOM + } + + it "the resource updates state1" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - update default_identity1 + - set state1 to "new_state1" (was "current_state1") +EOM + end + end + + context "and state1 and state2 are set to new values" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'new_state1' + state2 'new_state2' + end + EOM + } + + it "the resource updates state1 and state2" do + expect(resource.converged).to eq 2 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - update default_identity1 + - set state1 to "new_state1" (was "current_state1") + - update default_identity1 + - set state2 to "new_state2" (was "current_state2") +EOM + end + end + + context "and state1 is set to its current value but state2 is set to a new value" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'current_state1' + state2 'new_state2' + end + EOM + } + + it "the resource updates state2" do + expect(resource.converged).to eq 1 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - update default_identity1 + - set state2 to "new_state2" (was "current_state2") +EOM + end + end + + context "and state1 and state2 are set to their current values" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'current_state1' + state2 'current_state2' + end + EOM + } + + it "the resource updates nothing" do + expect(resource.converged).to eq 0 + expect(resource.updated?).to be_falsey + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create (up to date) +EOM + end + end + end + + context "and no current_resource" do + before :each do + resource_class.load_current_value do + current_value_does_not_exist! + end + end + + context "and nothing is set" do + let(:converge_recipe) { + "#{resource_name} 'blah'" + } + + it "the resource is created" do + expect(resource.converged).to eq 2 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - create default_identity1 + - set state1 to "default_state1" (default value) + - create default_identity1 + - set state2 to "default_state2" (default value) +EOM + end + end + + context "and state1 and state2 are set to new values" do + let(:converge_recipe) { + <<-EOM + #{resource_name} 'blah' do + state1 'new_state1' + state2 'new_state2' + end + EOM + } + + it "the resource is created" do + expect(resource.converged).to eq 2 + expect(resource.updated?).to be_truthy + expect(converged_recipe.stdout).to eq <<-EOM +* #{resource_name}[blah] action create + - create default_identity1 + - set state1 to "new_state1" + - create default_identity1 + - set state2 to "new_state2" +EOM + end + end + end + end + + end +end |