diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a53289a..fc902a9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## 2017-03-07 - Supported Release 1.1.6 + +### Summary + +- Minor release with a small bug fix. + +#### Bug Fixes + +- Fix issue where error was raised when adding or removing features if setup.exe returned 1641 (Reboot initiated) or 3010 (Reboot required) exit codes, only a warning is raised now ([MODULES-4468](https://tickets.puppetlabs.com/browse/MODULES-4468)). + ## 2017-02-15 - Supported Release 1.1.5 ### Summary diff --git a/lib/puppet/provider/sqlserver.rb b/lib/puppet/provider/sqlserver.rb index 2d4127bb..2ac2682c 100644 --- a/lib/puppet/provider/sqlserver.rb +++ b/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 diff --git a/lib/puppet/provider/sqlserver_features/mssql.rb b/lib/puppet/provider/sqlserver_features/mssql.rb index 13b26c96..e5c7ad79 100644 --- a/lib/puppet/provider/sqlserver_features/mssql.rb +++ b/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 diff --git a/lib/puppet/provider/sqlserver_instance/mssql.rb b/lib/puppet/provider/sqlserver_instance/mssql.rb index 23a81dc8..34b1c5bb 100644 --- a/lib/puppet/provider/sqlserver_instance/mssql.rb +++ b/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 diff --git a/metadata.json b/metadata.json index f6ae33f3..c865ba3a 100644 --- a/metadata.json +++ b/metadata.json @@ -1,6 +1,6 @@ { "name": "puppetlabs-sqlserver", - "version": "1.1.5", + "version": "1.1.6", "author": "Puppet Inc", "summary": "The `sqlserver` module installs and manages MS SQL Server 2012 and 2014 on Windows systems.", "license": "proprietary", diff --git a/spec/unit/puppet/provider/sqlserver__instance_spec.rb b/spec/unit/puppet/provider/sqlserver__instance_spec.rb index 7cbc704f..7cb995ec 100644 --- a/spec/unit/puppet/provider/sqlserver__instance_spec.rb +++ b/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) { { diff --git a/spec/unit/puppet/provider/sqlserver_features_spec.rb b/spec/unit/puppet/provider/sqlserver_features_spec.rb index 69455359..2d33646a 100644 --- a/spec/unit/puppet/provider/sqlserver_features_spec.rb +++ b/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,7 +66,7 @@ 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) @@ -73,12 +74,15 @@ 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 diff --git a/spec/unit/puppet/sqlserver_spec_helper.rb b/spec/unit/puppet/sqlserver_spec_helper.rb index 141a4259..fe10aa28 100644 --- a/spec/unit/puppet/sqlserver_spec_helper.rb +++ b/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