Deal with LCM failing when a resource does not support WhatIf

7 files changed, 79 insertions, 37 deletions

diff --git a/lib/chef/provider/dsc_script.rb b/lib/chef/provider/dsc_script.rb
index a1bb9c63a7..4e488557e6 100644
--- a/lib/chef/provider/dsc_script.rb
+++ b/lib/chef/provider/dsc_script.rb
@@ -131,11 +131,11 @@ class Chef
       private
 
       def generate_description
-        ["DSC resource script for configuration '#{configuration_friendly_name}'"] + 
+        ["converge DSC configuration '#{configuration_friendly_name}'"] + 
           @dsc_resources_info.map do |resource|
             # We ignore the last log message because it only contains the time it took, which looks weird
             cleaned_messages = resource.change_log[0..-2].map { |c| c.sub(/^#{Regexp.escape(resource.name)}/, '').strip }
-            cleaned_messages.find_all{ |c| c != ''}.join("\n")
+            "converge DSC resource #{resource.name} by #{cleaned_messages.find_all{ |c| c != ''}.join("\n")}"
           end
       end
     end
diff --git a/lib/chef/util/dsc/configuration_generator.rb b/lib/chef/util/dsc/configuration_generator.rb
index 98b4a8bbf4..9f8c0e7309 100644
--- a/lib/chef/util/dsc/configuration_generator.rb
+++ b/lib/chef/util/dsc/configuration_generator.rb
@@ -44,7 +44,7 @@ class Chef::Util::DSC
 
       merged_configuration_flags = get_merged_configuration_flags!(configuration_flags, configuration_name)
 
-      document_generation_cmdlet.run(merged_configuration_flags, shellout_flags)
+      document_generation_cmdlet.run!(merged_configuration_flags, shellout_flags)
       configuration_document_location = find_configuration_document(configuration_name)
 
       if ! configuration_document_location
diff --git a/lib/chef/util/dsc/lcm_output_parser.rb b/lib/chef/util/dsc/lcm_output_parser.rb
index 420901bcfa..6532d79d6f 100644
--- a/lib/chef/util/dsc/lcm_output_parser.rb
+++ b/lib/chef/util/dsc/lcm_output_parser.rb
@@ -16,6 +16,7 @@
 # limitations under the License.
 #
 
+require 'chef/log'
 require 'chef/util/dsc/resource_info'
 
 class Chef
@@ -110,6 +111,10 @@ class Chef
                 when :test
                   stack[-1].add_test(new_op)
                 when :resource
+                  while stack[-1].op_type != :set
+                    Chef::Log.warn("Can't add resource to set...popping until it is allowed.")
+                    popped_op = stack.pop
+                  end
                   stack[-1].add_resource(new_op)
                 else
                   Chef::Log.warn("Unknown op_action #{op_action}: Read line #{line}")
@@ -118,8 +123,9 @@ class Chef
               when :end
                 popped_op = stack.pop
                 popped_op.add_info(info)
-                if popped_op.op_type != op_type
-                  raise LCMOutputParseException, "Unmatching end for op_type. Expected op_type=#{op_type}, found op_type=#{popped_op.op_type}"
+                while popped_op.op_type != op_type
+                  Chef::Log::warn("Unmatching end for op_type. Expected op_type=#{op_type}, found op_type=#{popped_op.op_type}. From output:\n#{lcm_output}")
+                  popped_op = stack.pop
                 end
               when :skip
                 # We don't really have anything to do here
diff --git a/lib/chef/util/dsc/local_configuration_manager.rb b/lib/chef/util/dsc/local_configuration_manager.rb
index d7d532a686..79cfcd2996 100644
--- a/lib/chef/util/dsc/local_configuration_manager.rb
+++ b/lib/chef/util/dsc/local_configuration_manager.rb
@@ -26,12 +26,20 @@ class Chef::Util::DSC
       @configuration_path = configuration_path
       clear_execution_time
     end
-    
+
     def test_configuration(configuration_document)
       status = run_configuration_cmdlet(configuration_document)
+      unless status.succeeded?
+        # LCM returns an error if any of the resources do not support the opptional What-If
+        if status.stderr.gsub(/\s+/, ' ') =~ /A parameter cannot be found that matches parameter name 'Whatif'/
+          Chef::Log::warn("Received error while testing configuration due to resource not supporting 'WhatIf'")
+        else
+          raise Chef::Exceptions::PowershellCmdletException, "Powershell Cmdlet failed: #{status.stderr.gsub(/\s+/, ' ')}"
+        end
+      end
       configuration_update_required?(status.return_value)
     end
-    
+
     def set_configuration(configuration_document)
       run_configuration_cmdlet(configuration_document, true)
     end
@@ -55,7 +63,11 @@ class Chef::Util::DSC
       begin
         save_configuration_document(configuration_document)
         cmdlet = ::Chef::Util::Powershell::Cmdlet.new(@node, "#{command_code}")
-        status = cmdlet.run
+        if apply_configuration
+          status = cmdlet.run!
+        else
+          status = cmdlet.run
+        end
       ensure
         end_operation_timing
         remove_configuration_document
@@ -69,8 +81,12 @@ class Chef::Util::DSC
 
     def configuration_update_required?(what_if_output)
       Chef::Log.debug("DSC: DSC returned the following '-whatif' output from test operation:\n#{what_if_output}")
-      #parse_what_if_output(what_if_output)
-      Parser::parse(what_if_output)
+      begin
+        Parser::parse(what_if_output)
+      rescue Chef::Util::DSC::LocalConfigurationManager::Parser => e
+        Chef::Log::warn("Could not parse parse LCM output: #{e}")
+        [Chef::Util::DSC::ResourceInfo.new('Unknown DSC Resources', true, ['Unknown changes because LCM output was not parsable.'])]
+      end
     end
 
     def save_configuration_document(configuration_document)
diff --git a/lib/chef/util/powershell/cmdlet.rb b/lib/chef/util/powershell/cmdlet.rb
index 71bd876e40..f99812dc24 100644
--- a/lib/chef/util/powershell/cmdlet.rb
+++ b/lib/chef/util/powershell/cmdlet.rb
@@ -25,7 +25,7 @@ class Chef::Util::Powershell
     def initialize(node, cmdlet, output_format=nil, output_format_options={})
       @output_format = output_format
       @node = node
-      
+
       case output_format
       when nil
         @json_format = false
@@ -55,7 +55,7 @@ class Chef::Util::Powershell
       if @json_format && @output_format_options.has_key?(:depth)
         json_depth = @output_format_options[:depth]
       end
-      
+
       json_command = @json_format ? " | convertto-json -compress -depth #{json_depth}" : ""
       command_string = "powershell.exe -executionpolicy bypass -noprofile -noninteractive -command \"trap [Exception] {write-error -exception ($_.Exception.Message);exit 1};#{@cmdlet} #{switches_string} #{arguments_string}#{json_command}\";if ( ! $? ) { exit 1 }"
 
@@ -65,13 +65,17 @@ class Chef::Util::Powershell
       os_architecture = "#{ENV['PROCESSOR_ARCHITEW6432']}" == 'AMD64' ? :x86_64 : :i386
 
       status = nil
-      
+
       with_os_architecture(@node) do
-        status = command.run_command      
+        status = command.run_command
       end
-      
-      result = CmdletResult.new(status, @output_format)
-      
+
+      CmdletResult.new(status, @output_format)
+    end
+
+    def run!(switches={}, execution_options={}, *arguments)
+      result = run(switches, execution_options, arguments)
+
       if ! result.succeeded?
         raise Chef::Exceptions::PowershellCmdletException, "Powershell Cmdlet failed: #{result.stderr}"
       end
diff --git a/spec/functional/util/powershell/cmdlet_spec.rb b/spec/functional/util/powershell/cmdlet_spec.rb
index 57a1117b51..c392a2ee66 100644
--- a/spec/functional/util/powershell/cmdlet_spec.rb
+++ b/spec/functional/util/powershell/cmdlet_spec.rb
@@ -40,8 +40,12 @@ describe Chef::Util::Powershell::Cmdlet, :windows_only do
     expect(result.succeeded?).to eq(true)

   end

 

-  it "returns a PowershellCmdletException exception if the command cannot be executed" do

-    expect {invalid_cmdlet.run}.to raise_error(Chef::Exceptions::PowershellCmdletException)

+  it "#run does not raise a PowershellCmdletException exception if the command cannot be executed" do

+    expect {invalid_cmdlet.run}.not_to raise_error

+  end

+

+  it "#run! raises a PowershellCmdletException exception if the command cannot be executed" do

+    expect {invalid_cmdlet.run!}.to raise_error(Chef::Exceptions::PowershellCmdletException)

   end

 

   it "executes a 64-bit command on a 64-bit OS, 32-bit otherwise" do

diff --git a/spec/unit/util/dsc/lcm_output_parser_spec.rb b/spec/unit/util/dsc/lcm_output_parser_spec.rb
index 8ed305827c..802c0ea429 100644
--- a/spec/unit/util/dsc/lcm_output_parser_spec.rb
+++ b/spec/unit/util/dsc/lcm_output_parser_spec.rb
@@ -133,34 +133,46 @@ EOF
         Chef::Util::DSC::LocalConfigurationManager::Parser::ParseException))
     end
 
-    it 'should raise an exception if a resource is found inside a test' do
-      str = <<EOF
+    it 'should allow missing a [End Resource] when its the last one and still find all the resource' do
+      str = <<-EOF
 logtype: [machinename]: LCM:  [ Start  Set      ]
-logtype: [machinename]: LCM:  [ Start  Resource ] [name]
+logtype: [machinename]: LCM:  [ Start  Resource ]  [name]
 logtype: [machinename]: LCM:  [ Start  Test     ]
-logtype: [machinename]: LCM:  [ Start  Resource ]
-logtype: [machinename]: LCM:  [ End    Resource  ]
 logtype: [machinename]: LCM:  [ End    Test     ]
-logtype: [machinename]: LCM:  [ End    Resource ] [name]
+logtype: [machinename]: LCM:  [ Skip   Set      ]
+logtype: [machinename]: LCM:  [ End    Resource ]
+logtype: [machinename]: LCM:  [ Start  Resource ]  [name2]
+logtype: [machinename]: LCM:  [ Start  Test     ]
+logtype: [machinename]: LCM:  [ End    Test     ]
+logtype: [machinename]: LCM:  [ Start  Set      ]
+logtype: [machinename]: LCM:  [ End    Set      ]
 logtype: [machinename]: LCM:  [ End    Set      ]
 EOF
-      expect { Chef::Util::DSC::LocalConfigurationManager::Parser::parse(str) }.to(raise_error(
-        Chef::Util::DSC::LocalConfigurationManager::Parser::ParseException))
+
+      resources = Chef::Util::DSC::LocalConfigurationManager::Parser::parse(str)
+      resources[0].changes_state?.should be_false
+      resources[1].changes_state?.should be_true
     end
 
-    it 'should raise an exception if a resource is found inside a resource' do
-      str = <<EOF
+    it 'should allow missing a [End Resource] when its the first one and still find all the resource' do
+      str = <<-EOF
 logtype: [machinename]: LCM:  [ Start  Set      ]
-logtype: [machinename]: LCM:  [ Start  Resource ] [name]
-logtype: [machinename]: LCM:  [ Start  Resource ]
-logtype: [machinename]: LCM:  [ End    Resource  ]
-logtype: [machinename]: LCM:  [ End    Resource ] [name]
+logtype: [machinename]: LCM:  [ Start  Resource ]  [name]
+logtype: [machinename]: LCM:  [ Start  Test     ]
+logtype: [machinename]: LCM:  [ End    Test     ]
+logtype: [machinename]: LCM:  [ Skip   Set      ]
+logtype: [machinename]: LCM:  [ Start  Resource ]  [name2]
+logtype: [machinename]: LCM:  [ Start  Test     ]
+logtype: [machinename]: LCM:  [ End    Test     ]
+logtype: [machinename]: LCM:  [ Start  Set      ]
+logtype: [machinename]: LCM:  [ End    Set      ]
+logtype: [machinename]: LCM:  [ End    Resource ]
 logtype: [machinename]: LCM:  [ End    Set      ]
 EOF
-      expect { Chef::Util::DSC::LocalConfigurationManager::Parser::parse(str) }.to(raise_error(
-        Chef::Util::DSC::LocalConfigurationManager::Parser::ParseException))
-    end
 
+      resources = Chef::Util::DSC::LocalConfigurationManager::Parser::parse(str)
+      resources[0].changes_state?.should be_false
+      resources[1].changes_state?.should be_true
+    end
   end
-
 end