Merge pull request #200 from wilson208/PE-17491

[PE-17491] Do not fail on install when a restart exit code is returned

Author: wilson208

lib/puppet/provider/sqlserver.rb

@@ -17,17 +17,19 @@ class Puppet::Provider::Sqlserver < Puppet::Provider
                  'powershell.exe'
                end
 
-  def try_execute(command, msg = nil, obfuscate_strings = nil)
-    begin
-      execute(command.compact)
-    rescue Puppet::ExecutionFailure => error
+  def try_execute(command, msg = nil, obfuscate_strings = nil, acceptable_exit_codes = [0])
+    res = execute(command.compact, failonfail: false)
+
+    unless acceptable_exit_codes.include?(res.exitstatus)
       msg = "Failure occured when trying to install SQL Server #{@resource[:name]}" if msg.nil?
-      msg += " \n #{error}"
+      msg += " \n Execution of '#{command}' returned #{res.exitstatus}: #{res.strip}"
 
       obfuscate_strings.each {|str| msg.gsub!(str, '**HIDDEN VALUE**') } unless obfuscate_strings.nil?
 
       raise Puppet::Error, msg
     end
+
+    res
   end
 
   private

lib/puppet/provider/sqlserver_features/mssql.rb

@@ -64,7 +64,10 @@ def modify_features(action, features)
       begin
         config_file = create_temp_for_install_switch unless action == 'uninstall'
         cmd_args << "/ConfigurationFile=\"#{config_file.path}\"" unless config_file.nil?
-        try_execute(cmd_args, "Unable to #{action} features (#{features.join(', ')})")
+        res = try_execute(cmd_args, "Unable to #{action} features (#{features.join(', ')})", nil, [0, 1641, 3010])
+
+        warn("#{action} of features (#{features.join(', ')} returned exit code 3010 - reboot required")  if res.exitstatus == 3010
+        warn("#{action} of features (#{features.join(', ')} returned exit code 1641 - reboot initiated") if res.exitstatus == 1641
       ensure
         if config_file
           config_file.close

lib/puppet/provider/sqlserver_instance/mssql.rb

@@ -48,7 +48,10 @@ def modify_features(features, action)
       begin
         config_file = create_temp_for_install_switch unless action == 'uninstall'
         cmd_args << "/ConfigurationFile=\"#{config_file.path}\"" unless config_file.nil?
-        try_execute(cmd_args, "Error trying to #{action} features (#{features.join(', ')}", obfuscated_strings)
+        res = try_execute(cmd_args, "Error trying to #{action} features (#{features.join(', ')}", obfuscated_strings, [0, 1641, 3010])
+
+        warn("#{action} of features (#{features.join(', ')}) returned exit code 3010 - reboot required")  if res.exitstatus == 3010
+        warn("#{action} of features (#{features.join(', ')}) returned exit code 1641 - reboot initiated") if res.exitstatus == 1641
       ensure
         if config_file
           config_file.close
@@ -161,7 +164,11 @@ def format_cmd_args_array(switch, arr, cmd_args, use_discrete = false)
 
   def destroy
     cmd_args = basic_cmd_args(current_installed_features, 'uninstall')
-    try_execute(cmd_args, "Unable to uninstall instance #{@resource[:name]}")
+    res = try_execute(cmd_args, "Unable to uninstall instance #{@resource[:name]}", nil, [0, 1641, 3010])
+
+    warn("Uninstall of instance #{@resource[:name]} returned exit code 3010 - reboot required")  if res.exitstatus == 3010
+    warn("Uninstall of instance #{@resource[:name]} returned exit code 1641 - reboot initiated") if res.exitstatus == 1641
+
     @property_hash.clear
     exists? ? (return false) : (return true)
   end

spec/unit/puppet/provider/sqlserver__instance_spec.rb

@@ -10,14 +10,16 @@
   subject { provider_class }
   let(:additional_install_switches) { [] }
 
-  def stub_uninstall(args, installed_features)
+  def stub_uninstall(args, installed_features, exit_code = 0)
     cmd_args = ["#{args[:source]}/setup.exe",
                 "/ACTION=uninstall",
                 '/Q',
                 '/IACCEPTSQLSERVERLICENSETERMS',
                 "/INSTANCENAME=#{args[:name]}",
                 "/FEATURES=#{installed_features.join(',')}",]
-    Puppet::Util::Execution.stubs(:execute).with(cmd_args.compact).returns(0)
+
+    result = Puppet::Util::Execution::ProcessOutput.new('', exit_code)
+    Puppet::Util::Execution.stubs(:execute).with(cmd_args.compact, failonfail: false).returns(result)
   end
 
   shared_examples 'run' do |args, munged_values = {}|
@@ -46,7 +48,7 @@ def stub_uninstall(args, installed_features)
       @provider.create
     }
   end
-  shared_examples 'create' do
+  shared_examples 'create' do |exit_code, warning_matcher|
     it {
       execute_args = args.merge(munged_values)
       @resource = Puppet::Type::Sqlserver_instance.new(args)
@@ -77,20 +79,27 @@ def stub_uninstall(args, installed_features)
       additional_install_switches.each do |switch|
         cmd_args << switch
       end
-      Puppet::Util::Execution.stubs(:execute).with(cmd_args.compact).returns(0)
+
+      # If warning_matcher supplied ensure warnings raised match, otherwise no warnings raised
+      @provider.stubs(:warn).with(regexp_matches(warning_matcher)).returns(nil).times(1) if warning_matcher
+      @provider.stubs(:warn).with(anything).times(0) unless warning_matcher
+
+      result = Puppet::Util::Execution::ProcessOutput.new('', exit_code || 0)
+      Puppet::Util::Execution.stubs(:execute).with(cmd_args.compact, failonfail: false).returns(result)
       @provider.create
     }
   end
 
 
-  shared_examples 'destroy' do
+  shared_examples 'destroy' do |exit_code, warning_matcher|
     it {
       @resource = Puppet::Type::Sqlserver_instance.new(args)
       @provider = provider_class.new(@resource)
 
       stub_source_which_call args[:source]
       @provider.expects(:current_installed_features).returns(installed_features)
-      stub_uninstall args, installed_features
+      stub_uninstall args, installed_features, exit_code || 0
+      @provider.stubs(:warn).with(regexp_matches(warning_matcher)).returns(nil).times(1) if warning_matcher
       @provider.destroy
     }
   end
@@ -118,6 +127,30 @@ def stub_uninstall(args, installed_features)
     end
   end
 
+  describe 'it should raise warning on install when 1641 exit code returned' do
+    it_behaves_like 'create', 1641, /reboot initiated/i do
+      args = get_basic_args
+      let(:args) { args }
+      munged = {:features => Array.new(args[:features])}
+      munged[:features].delete('SQL')
+      munged[:features] += %w(DQ FullText Replication SQLEngine)
+      munged[:features].sort!
+      let(:munged_values) { munged }
+    end
+  end
+
+  describe 'it should raise warning on install when 3010 exit code returned' do
+    it_behaves_like 'create', 3010, /reboot required/i do
+      args = get_basic_args
+      let(:args) { args }
+      munged = {:features => Array.new(args[:features])}
+      munged[:features].delete('SQL')
+      munged[:features] += %w(DQ FullText Replication SQLEngine)
+      munged[:features].sort!
+      let(:munged_values) { munged }
+    end
+  end
+
   describe 'empty array should' do
     it_behaves_like 'destroy on create' do
       let(:installed_features) { %w(SQLEngine Replication) }
@@ -140,6 +173,29 @@ def stub_uninstall(args, installed_features)
     end
 
   end
+
+  describe 'it should raise warning on uninstall when 1641 exit code returned' do
+    it_behaves_like 'destroy', 1641, /reboot initiated/i do
+      let(:args) { {
+          :name => 'MYSQLSERVER',
+          :source => 'C:\myinstallexecs',
+          :features => []
+      } }
+      let(:installed_features) { %w(SQLEngine Replication) }
+    end
+  end
+
+  describe 'it should raise warning on uninstall when 3010 exit code returned' do
+    it_behaves_like 'destroy', 3010, /reboot required/i do
+      let(:args) { {
+          :name => 'MYSQLSERVER',
+          :source => 'C:\myinstallexecs',
+          :features => []
+      } }
+      let(:installed_features) { %w(SQLEngine Replication) }
+    end
+  end
+
   describe 'installed features even if provided features' do
     it_behaves_like 'destroy' do
       let(:args) { {

spec/unit/puppet/provider/sqlserver_features_spec.rb

@@ -15,7 +15,7 @@
   let(:additional_params) { {} }
   let(:munged_args) { {} }
   let(:additional_switches) { [] }
-  shared_examples 'create' do
+  shared_examples 'create' do |exit_code, warning_matcher|
     it {
       params.merge!(additional_params)
       @resource = Puppet::Type::Sqlserver_features.new(params)
@@ -24,7 +24,8 @@
       stub_powershell_call(subject)
 
       executed_args = params.merge(munged_args)
-      stub_add_features(executed_args, executed_args[:features], additional_switches)
+      stub_add_features(executed_args, executed_args[:features], additional_switches, exit_code || 0)
+      @provider.stubs(:warn).with(regexp_matches(warning_matcher)).returns(nil).times(1) if warning_matcher
       @provider.create
     }
   end
@@ -65,20 +66,23 @@
     it_should_behave_like 'create'
   end
 
-  shared_examples 'features=' do |args|
+  shared_examples 'features=' do |args, exit_code, warning_matcher|
     it {
       @resource = Puppet::Type::Sqlserver_features.new(args)
       @provider = provider_class.new(@resource)
 
       stub_powershell_call(subject)
       stub_source_which_call args
       if !feature_remove.empty?
-        stub_remove_features(args, feature_remove)
+        stub_remove_features(args, feature_remove, exit_code || 0)
       end
       if !feature_add.empty?
-        stub_add_features(args, feature_add)
+        stub_add_features(args, feature_add, [], exit_code || 0)
       end
 
+      # If warning_matcher supplied ensure warnings raised match, otherwise no warnings raised
+      @provider.stubs(:warn).with(regexp_matches(warning_matcher)).returns(nil).times(1) if warning_matcher
+      @provider.stubs(:warn).with(anything).times(0) unless warning_matcher
       @provider.create
     }
   end
@@ -106,6 +110,20 @@
     it_should_behave_like 'features=', @feature_params
   end
 
+  context 'it should raise warning on feature install when 1641 exit code returned' do
+    include_context 'features'
+    @feature_params[:features] = %w(SSMS)
+    let(:feature_add) { %w(SSMS) }
+    it_should_behave_like 'features=', @feature_params, 1641, /reboot initiated/i
+  end
+
+  context 'it should raise warning on feature install when 3010 exit code returned' do
+    include_context 'features'
+    @feature_params[:features] = %w(SSMS)
+    let(:feature_add) { %w(SSMS) }
+    it_should_behave_like 'features=', @feature_params, 3010, /reboot required/i
+  end
+
   context 'it should install the expanded tools set' do
     include_context 'features'
     @feature_params[:features] = %w(Tools)
@@ -131,13 +149,14 @@
       @provider = provider_class.new(@resource)
       @provider.stubs(:current_installed_features).returns(%w(SSMS ADV_SSMS Conn))
       Puppet::Util.stubs(:which).with("#{feature_params[:source]}/setup.exe").returns("#{feature_params[:source]}/setup.exe")
+      result = Puppet::Util::Execution::ProcessOutput.new('', 0)
       Puppet::Util::Execution.expects(:execute).with(
         ["#{feature_params[:source]}/setup.exe",
          "/ACTION=uninstall",
          '/Q',
          '/IACCEPTSQLSERVERLICENSETERMS',
          "/FEATURES=#{%w(SSMS ADV_SSMS Conn).join(',')}",
-        ]).returns(0)
+        ], failonfail: false).returns(result)
       @provider.create
     }
   end

spec/unit/puppet/sqlserver_spec_helper.rb

@@ -7,15 +7,15 @@ def stub_powershell_call(subject)
   Puppet::Provider::Sqlserver.stubs(:run_install_dot_net).returns()
 end
 
-def stub_add_features(args, features, additional_switches = [])
-  stub_modify_features('install', args, features, additional_switches)
+def stub_add_features(args, features, additional_switches = [], exit_code = 0)
+  stub_modify_features('install', args, features, additional_switches, exit_code)
 end
 
-def stub_remove_features(args, features)
-  stub_modify_features('uninstall', args, features)
+def stub_remove_features(args, features, exit_code = 0)
+  stub_modify_features('uninstall', args, features, [], exit_code)
 end
 
-def stub_modify_features(action, args, features, additional_switches = [])
+def stub_modify_features(action, args, features, additional_switches = [], exit_code = 0)
   cmds = ["#{args[:source]}/setup.exe",
           "/ACTION=#{action}",
           '/Q',
@@ -31,5 +31,8 @@ def stub_modify_features(action, args, features, additional_switches = [])
   additional_switches.each do |switch|
     cmds << switch
   end
-  Puppet::Util::Execution.stubs(:execute).with(cmds).returns(0)
+
+  result = Puppet::Util::Execution::ProcessOutput.new('', exit_code)
+
+  Puppet::Util::Execution.stubs(:execute).with(cmds, failonfail: false).returns(result)
 end