Add Resource.action.converged_if_changed

3 files changed, 585 insertions, 25 deletions

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index bb5c6050fe..b6355dab55 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 Opscode, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -27,6 +28,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'
@@ -1439,35 +1441,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..544f7c1285
--- /dev/null
+++ b/lib/chef/resource/action_provider.rb
@@ -0,0 +1,111 @@
+#
+# 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.
+#
+
+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
+
+          if current_resource.method(:load_current_value!).arity > 0
+            current_resource.load_current_value!(new_resource)
+          else
+            current_resource.load_current_value!
+          end
+        elsif superclass.public_instance_method?(:load_current_resource)
+          super
+        end
+        @current_resource = current_resource
+      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)
+        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 #{new_resource.to_s}" ] + modified, &converge_block)
+
+        else
+          # The resource doesn't exist. Mark that we are *creating* this, and
+          # write down any properties we are setting.
+          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 << "  default #{property.to_s.ljust(property_size)} to #{new_resource.send(property).inspect}"
+            end
+          end
+
+          converge_by([ "create #{new_resource.to_s}" ] + created, &converge_block)
+        end
+        true
+      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_converge_if_changed_spec.rb b/spec/integration/recipes/resource_converge_if_changed_spec.rb
new file mode 100644
index 0000000000..ef768a693e
--- /dev/null
+++ b/spec/integration/recipes/resource_converge_if_changed_spec.rb
@@ -0,0 +1,469 @@
+require 'support/shared/integration/integration_helper'
+
+describe "Resource::ActionProvider#converge_if_changed" 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
+
+  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_identity1'
+        attr_accessor :converged
+        def initialize(*args)
+          super
+          @converged = 0
+        end
+      end
+      result.resource_name resource_name
+      result
+    }
+    let(:resource) {
+      converge_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 = self.resource_name
+            converge {
+              public_send(resource_name, 'blah')
+            }
+          }
+
+          it "the resource updates nothing" do
+            expect(resource.converged).to eq 0
+            expect(resource.updated?).to  be_falsey
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create (up to date)
+            EOM
+          end
+        end
+
+        context "and state1 is set to a new value" do
+          let(:converge_recipe) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'new_state1'
+              end
+            }
+          }
+
+          it "the resource updates state1" do
+            expect(resource.converged).to eq 1
+            expect(resource.updated?).to  be_truthy
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create
+    - update #{resource_name}[blah]
+    -   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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'new_state1'
+                state2 'new_state2'
+              end
+            }
+          }
+
+          it "the resource updates state1 and state2" do
+            expect(resource.converged).to eq 1
+            expect(resource.updated?).to  be_truthy
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create
+    - update #{resource_name}[blah]
+    -   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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'current_state1'
+                state2 'new_state2'
+              end
+            }
+          }
+
+          it "the resource updates state2" do
+            expect(resource.converged).to eq 1
+            expect(resource.updated?).to  be_truthy
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create
+    - update #{resource_name}[blah]
+    -   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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'current_state1'
+                state2 'current_state2'
+              end
+            }
+          }
+
+          it "the resource updates nothing" do
+            expect(resource.converged).to eq 0
+            expect(resource.updated?).to  be_falsey
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                identity1 'new_identity1'
+                control1 'new_control1'
+              end
+            }
+          }
+
+          # 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(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                identity1 'new_identity1'
+                control1 'new_control1'
+              end
+            }
+          }
+
+          # 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(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create
+    - update #{resource_name}[blah]
+    -   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
+#             value_does_not_exist!
+#           end
+#         end
+#
+#         context "and nothing is set" do
+#           let(:converge_recipe) {
+#             resource_name = self.resource_name
+#             converge {
+#               public_send(resource_name, 'blah')
+#             }
+#           }
+#
+#           it "the resource is created" do
+#             expect(resource.converged).to eq 1
+#             expect(resource.updated?).to  be_truthy
+#             expect(converge_recipe.stdout).to eq <<-EOM
+# Recipe: basic_chef_client::block
+#   * #{resource_name}[blah] action create
+#     - create #{resource_name}[blah]
+#     -   default state1 to "default_state1"
+#     -   default state2 to "default_state2"
+# EOM
+#           end
+#         end
+#
+#         context "and state1 and state2 are set" do
+#           let(:converge_recipe) {
+#             resource_name = self.resource_name
+#             converge {
+#               public_send(resource_name, 'blah') do
+#                 state1 'new_state1'
+#                 state2 'new_state2'
+#               end
+#             }
+#           }
+#
+#           it "the resource is created" do
+#             expect(resource.converged).to eq 1
+#             expect(resource.updated?).to  be_truthy
+#             expect(converge_recipe.stdout).to eq <<-EOM
+# Recipe: basic_chef_client::block
+#   * #{resource_name}[blah] action create
+#     - create #{resource_name}[blah]
+#     -       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 = self.resource_name
+            converge {
+              public_send(resource_name, 'blah')
+            }
+          }
+
+          it "the resource updates nothing" do
+            expect(resource.converged).to eq 0
+            expect(resource.updated?).to  be_falsey
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create (up to date)
+EOM
+          end
+        end
+
+        context "and state1 is set to a new value" do
+          let(:converge_recipe) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'new_state1'
+              end
+            }
+          }
+
+          it "the resource updates state1" do
+            expect(resource.converged).to eq 1
+            expect(resource.updated?).to  be_truthy
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create
+    - update #{resource_name}[blah]
+    -   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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'new_state1'
+                state2 'new_state2'
+              end
+            }
+          }
+
+          it "the resource updates state1 and state2" do
+            expect(resource.converged).to eq 2
+            expect(resource.updated?).to  be_truthy
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create
+    - update #{resource_name}[blah]
+    -   set state1 to "new_state1" (was "current_state1")
+    - update #{resource_name}[blah]
+    -   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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'current_state1'
+                state2 'new_state2'
+              end
+            }
+          }
+
+          it "the resource updates state2" do
+            expect(resource.converged).to eq 1
+            expect(resource.updated?).to  be_truthy
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{resource_name}[blah] action create
+    - update #{resource_name}[blah]
+    -   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) {
+            resource_name = self.resource_name
+            converge {
+              public_send(resource_name, 'blah') do
+                state1 'current_state1'
+                state2 'current_state2'
+              end
+            }
+          }
+
+          it "the resource updates nothing" do
+            expect(resource.converged).to eq 0
+            expect(resource.updated?).to  be_falsey
+            expect(converge_recipe.stdout).to eq <<-EOM
+Recipe: basic_chef_client::block
+  * #{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
+#             resource_does_not_exist!
+#           end
+#         end
+#
+#         context "and nothing is set" do
+#           let(:converge_recipe) {
+#             resource_name = self.resource_name
+#             converge {
+#               public_send(resource_name, 'blah')
+#             }
+#           }
+#
+#           it "the resource is created" do
+#             expect(resource.converged).to eq 2
+#             expect(resource.updated?).to  be_truthy
+#             expect(converge_recipe.stdout).to eq <<-EOM
+# Recipe: basic_chef_client::block
+# * #{resource_name}[blah] action create (up to date)
+# EOM
+#           end
+#         end
+#
+#         context "and state1 and state2 are set to their current values" do
+#           let(:converge_recipe) {
+#             resource_name = self.resource_name
+#             converge {
+#               public_send(resource_name, 'blah') do
+#                 state1 'current_state1'
+#                 state2 'current_state2'
+#               end
+#             }
+#           }
+#
+#           it "the resource is created" do
+#             expect(resource.converged).to eq 2
+#             expect(resource.updated?).to  be_truthy
+#             expect(converge_recipe.stdout).to eq <<-EOM
+# Recipe: basic_chef_client::block
+# * #{resource_name}[blah] action create (up to date)
+# EOM
+#           end
+#         end
+#       end
+    end
+
+  end
+end