Merge pull request #4167 from chef/smurawski/new_lcm_settings

reboot_action for dsc_resource and WMF 5 and Win 10 Threshold 2 Allow dsc_resource with the LCM enabled.

6 files changed, 152 insertions, 49 deletions

diff --git a/DOC_CHANGES.md b/DOC_CHANGES.md
index b2e2c3d117..381cb9b047 100644
--- a/DOC_CHANGES.md
+++ b/DOC_CHANGES.md
@@ -56,3 +56,10 @@ Assuming both of those make 12.6, placeholder pages:
 chocolatey_package: https://docs.chef.io/release/12-6/resource_chocolatey_package.html
 
 ksh: https://docs.chef.io/release/12-6/resource_ksh.html
+
+### `dsc_resource` resource
+
+Added 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.
diff --git a/lib/chef/platform/query_helpers.rb b/lib/chef/platform/query_helpers.rb
index dfb99ed750..699e98737c 100644
--- a/lib/chef/platform/query_helpers.rb
+++ b/lib/chef/platform/query_helpers.rb
@@ -85,10 +85,12 @@ class Chef
       end
 
       def supports_dsc_invoke_resource?(node)
-        require 'rubygems'
         supports_dsc?(node) &&
-          Gem::Version.new(node[:languages][:powershell][:version]) >=
-            Gem::Version.new("5.0.10018.0")
+          supported_powershell_version?(node, "5.0.10018.0")
+      end
+
+      def supports_refresh_mode_enabled?(node)
+        supported_powershell_version?(node, "5.0.10586.0")
       end
 
       def dsc_refresh_mode_disabled?(node)
@@ -97,6 +99,15 @@ class Chef
         metadata = cmdlet.run!.return_value
         metadata['RefreshMode'] == 'Disabled'
       end
+      
+
+      def supported_powershell_version?(node, version_string)
+        return false unless node[:languages] && node[:languages][:powershell]
+        require 'rubygems'
+        Gem::Version.new(node[:languages][:powershell][:version]) >=
+          Gem::Version.new(version_string)
+      end
+
     end
   end
 end
diff --git a/lib/chef/provider/dsc_resource.rb b/lib/chef/provider/dsc_resource.rb
index 65830131ab..212e3b0e09 100644
--- a/lib/chef/provider/dsc_resource.rb
+++ b/lib/chef/provider/dsc_resource.rb
@@ -15,7 +15,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-
 require 'chef/util/powershell/cmdlet'
 require 'chef/util/dsc/local_configuration_manager'
 require 'chef/mixin/powershell_type_coercions'
@@ -25,19 +24,19 @@ class Chef
   class Provider
     class DscResource < Chef::Provider
       include Chef::Mixin::PowershellTypeCoercions
-
       provides :dsc_resource, os: "windows"
-
       def initialize(new_resource, run_context)
         super
         @new_resource = new_resource
         @module_name = new_resource.module_name
+        @reboot_resource = nil
       end
 
       def action_run
         if ! test_resource
           converge_by(generate_description) do
             result = set_resource
+            reboot_if_required
           end
         end
       end
@@ -59,8 +58,9 @@ class Chef
           a.block_action!
         end
         requirements.assert(:run) do |a|
-          a.assertion { dsc_refresh_mode_disabled? }
-          err = ["The LCM must have its RefreshMode set to Disabled. "]
+          a.assertion { supports_refresh_mode_enabled? || dsc_refresh_mode_disabled? }
+          err = ["The LCM must have its RefreshMode set to Disabled for" \
+                 " PowerShell versions before 5.0.10586.0."]
           a.failure_message Chef::Exceptions::ProviderNotFound, err.join(' ')
           a.whyrun err + ["Assuming a previous resource sets the RefreshMode."]
           a.block_action!
@@ -83,11 +83,15 @@ class Chef
       def supports_dsc_invoke_resource?
         run_context && Chef::Platform.supports_dsc_invoke_resource?(node)
       end
-      
+
       def dsc_refresh_mode_disabled?
         Chef::Platform.dsc_refresh_mode_disabled?(node)
       end
 
+      def supports_refresh_mode_enabled?
+        Chef::Platform.supports_refresh_mode_enabled?(node)
+      end
+
       def generate_description
         @converge_description
       end
@@ -99,7 +103,6 @@ class Chef
       def module_name
         @module_name ||= begin
           found = resource_store.find(dsc_resource_name)
-
           r = case found.length
               when 0
                 raise Chef::Exceptions::ResourceNotFound,
@@ -123,18 +126,14 @@ class Chef
         # however Invoke-DscResource is not correctly writing to that
         # stream and instead just dumping to stdout
         @converge_description = result.stdout
-
-        if result.return_value.is_a?(Array)
-          # WMF Feb 2015 Preview
-          result.return_value[0]["InDesiredState"]
-        else
-          # WMF April 2015 Preview
-          result.return_value["InDesiredState"]
-        end
+        return_dsc_resource_result(result, "InDesiredState")
       end
 
       def set_resource
         result = invoke_resource(:set)
+        if return_dsc_resource_result(result, 'RebootRequired')
+          create_reboot_resource
+        end
         result.return_value
       end
 
@@ -142,11 +141,9 @@ class Chef
         properties = translate_type(@new_resource.properties)
         switches = "-Method #{method.to_s} -Name #{@new_resource.resource}"\
                    " -Property #{properties} -Verbose"
-
         if module_name != :none
           switches += " -Module #{module_name}"
         end
-
         cmdlet = Chef::Util::Powershell::Cmdlet.new(
           node,
           "Invoke-DscResource #{switches}",
@@ -155,6 +152,38 @@ class Chef
         cmdlet.run!
       end
 
+      def return_dsc_resource_result(result, property_name)
+        if result.return_value.is_a?(Array)
+          # WMF Feb 2015 Preview
+          result.return_value[0][property_name]
+        else
+          # WMF April 2015 Preview
+          result.return_value[property_name]
+        end
+      end
+
+      def create_reboot_resource
+        @reboot_resource = Chef::Resource::Reboot.new(
+          "Reboot for #{@new_resource.name}",
+          run_context
+        ).tap do |r|
+          r.reason("Reboot for #{@new_resource.resource}.")
+        end
+      end
+
+      def reboot_if_required
+        reboot_action = @new_resource.reboot_action
+        unless @reboot_resource.nil?
+          case reboot_action
+          when :nothing
+            Chef::Log.debug("A reboot was requested by the DSC resource, but reboot_action is :nothing.")
+            Chef::Log.debug("This dsc_resource will not reboot the node.")
+          else
+            Chef::Log.debug("Requesting node reboot with #{reboot_action}.")
+            @reboot_resource.run_action(reboot_action)
+          end
+        end
+      end
     end
   end
-end
+end
\ No newline at end of file
diff --git a/lib/chef/resource/dsc_resource.rb b/lib/chef/resource/dsc_resource.rb
index 5db00f49ca..b6167e76d0 100644
--- a/lib/chef/resource/dsc_resource.rb
+++ b/lib/chef/resource/dsc_resource.rb
@@ -31,6 +31,7 @@ class Chef
         super

         @properties = {}

         @resource = nil

+        @reboot_action = :nothing

       end

 

       def resource(value=nil)

@@ -68,6 +69,17 @@ class Chef
         end

       end

 

+      # This property takes the action message for the reboot resource

+      # If the set method of the DSC resource indicate that a reboot

+      # is necessary, reboot_action provides the mechanism for a reboot to 

+      # be requested.

+      def reboot_action(value=nil)

+        if value

+          @reboot_action = value

+        else

+          @reboot_action

+        end

+      end

       private

 

       def value_of(value)

diff --git a/spec/unit/provider/dsc_resource_spec.rb b/spec/unit/provider/dsc_resource_spec.rb
index 9946ab8410..46cdbd93a6 100644
--- a/spec/unit/provider/dsc_resource_spec.rb
+++ b/spec/unit/provider/dsc_resource_spec.rb
@@ -16,7 +16,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-
 require 'chef'
 require 'spec_helper'
 
@@ -34,7 +33,6 @@ describe Chef::Provider::DscResource do
       node.automatic[:languages][:powershell][:version] = '4.0'
       node
     }
-
     it 'raises a ProviderNotFound exception' do
       expect(provider).not_to receive(:meta_configuration)
       expect{provider.run_action(:run)}.to raise_error(
@@ -43,35 +41,76 @@ describe Chef::Provider::DscResource do
   end
 
   context 'when Powershell supports Invoke-DscResource' do
+
+    context 'when RefreshMode is not set to Disabled' do
+      context 'and the WMF 5 is a preview release' do
+        let (:node) {
+          node = Chef::Node.new
+          node.automatic[:languages][:powershell][:version] = '5.0.10018.0'
+          node
+        }
+        it 'raises an exception' do
+          expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(false)
+          expect { provider.run_action(:run) }.to raise_error(
+            Chef::Exceptions::ProviderNotFound, /Disabled/)
+        end
+      end
+      context 'and the WMF is 5 RTM or newer' do
+        let (:node) {
+          node = Chef::Node.new
+          node.automatic[:languages][:powershell][:version] = '5.0.10586.0'
+          node
+        }
+        it 'does not raises an exception' do
+          expect(provider).to receive(:test_resource)
+          expect(provider).to receive(:set_resource)
+          expect(provider).to receive(:reboot_if_required)
+          expect { provider.run_action(:run) }.to_not raise_error
+        end
+      end
+    end
+  end
+  
+  context 'when the LCM supports Invoke-DscResource' do
     let (:node) {
       node = Chef::Node.new
       node.automatic[:languages][:powershell][:version] = '5.0.10018.0'
       node
     }
 
-    context 'when RefreshMode is not set to Disabled' do
-      it 'raises an exception' do
-        expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(false)
-        expect { provider.run_action(:run) }.to raise_error(
-          Chef::Exceptions::ProviderNotFound, /Disabled/)
-      end
+    it 'does not update the resource if it is up to date' do
+      expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(true)
+      expect(provider).to receive(:test_resource).and_return(true)
+      provider.run_action(:run)
+      expect(resource).not_to be_updated
     end
 
-    context 'when RefreshMode is set to Disabled' do
-      it 'does not update the resource if it is up to date' do
-        expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(true)
-        expect(provider).to receive(:test_resource).and_return(true)
-        provider.run_action(:run)
-        expect(resource).not_to be_updated
-      end
-
-      it 'converges the resource if it is not up to date' do
-        expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(true)
-        expect(provider).to receive(:test_resource).and_return(false)
-        expect(provider).to receive(:set_resource)
-        provider.run_action(:run)
-        expect(resource).to be_updated
-      end
+    it 'converges the resource if it is not up to date' do
+      expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(true)
+      expect(provider).to receive(:test_resource).and_return(false)
+      expect(provider).to receive(:set_resource)
+      provider.run_action(:run)
+      expect(resource).to be_updated
+    end
+    
+    it 'flags the resource as reboot required when required' do
+      expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(true)
+      expect(provider).to receive(:test_resource).and_return(false)
+      expect(provider).to receive(:invoke_resource).
+        and_return(double(:stdout => '', :return_value =>nil))
+      expect(provider).to receive(:return_dsc_resource_result).and_return(true)
+      expect(provider).to receive(:create_reboot_resource)
+      provider.run_action(:run)
+    end
+    
+    it 'does not flag the resource as reboot required when not required' do
+      expect(provider).to receive(:dsc_refresh_mode_disabled?).and_return(true)
+      expect(provider).to receive(:test_resource).and_return(false)
+      expect(provider).to receive(:invoke_resource).
+        and_return(double(:stdout => '', :return_value =>nil))
+      expect(provider).to receive(:return_dsc_resource_result).and_return(false)
+      expect(provider).to_not receive(:create_reboot_resource)
+      provider.run_action(:run)
     end
   end
-end
+end
\ No newline at end of file
diff --git a/spec/unit/resource/dsc_resource_spec.rb b/spec/unit/resource/dsc_resource_spec.rb
index 06769d86ce..6fa74a139f 100644
--- a/spec/unit/resource/dsc_resource_spec.rb
+++ b/spec/unit/resource/dsc_resource_spec.rb
@@ -15,13 +15,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-
 require 'spec_helper'
-
 describe Chef::Resource::DscResource do
   let(:dsc_test_resource_name) { 'DSCTest' }
   let(:dsc_test_property_name) { :DSCTestProperty }
   let(:dsc_test_property_value) { 'DSCTestValue' }
+  let(:dsc_test_reboot_action) { :reboot_now }
 
   context 'when Powershell supports Dsc' do
     let(:dsc_test_run_context) {
@@ -30,6 +29,7 @@ describe Chef::Resource::DscResource do
       empty_events = Chef::EventDispatch::Dispatcher.new
       Chef::RunContext.new(node, {}, empty_events)
     }
+
     let(:dsc_test_resource) {
       Chef::Resource::DscResource.new(dsc_test_resource_name, dsc_test_run_context)
     }
@@ -38,7 +38,7 @@ describe Chef::Resource::DscResource do
       expect(dsc_test_resource.action).to eq([:run])
     end
 
-    it "has an allowed_actions attribute with only the `:run` and `:nothing` attributes" do
+    it "has an ed_actions attribute with only the `:run` and `:nothing` attributes" do
       expect(dsc_test_resource.allowed_actions.to_set).to eq([:run,:nothing].to_set)
     end
 
@@ -52,6 +52,11 @@ describe Chef::Resource::DscResource do
       expect(dsc_test_resource.module_name).to eq(dsc_test_resource_name)
     end
 
+    it "allows the reboot_action attribute to be set" do
+      dsc_test_resource.reboot_action(dsc_test_reboot_action)
+      expect(dsc_test_resource.reboot_action).to eq(dsc_test_reboot_action)
+    end
+
     context "when setting a dsc property" do
       it "allows setting a dsc property with a property name of type Symbol" do
         dsc_test_resource.property(dsc_test_property_name, dsc_test_property_value)
@@ -82,4 +87,4 @@ describe Chef::Resource::DscResource do
       end
     end
   end
-end
+end
\ No newline at end of file