Merge pull request #6001 from chef/smurawski/exit_status_default

Make Standardized Exit Codes The Default Behavior

10 files changed, 35 insertions, 294 deletions

diff --git a/lib/chef/application/exit_code.rb b/lib/chef/application/exit_code.rb
index 610a356a7c..917aa16e62 100644
--- a/lib/chef/application/exit_code.rb
+++ b/lib/chef/application/exit_code.rb
@@ -45,47 +45,17 @@ class Chef
       class << self
 
         def normalize_exit_code(exit_code = nil)
-          if normalization_not_configured?
-            normalize_legacy_exit_code_with_warning(exit_code)
-          elsif normalization_disabled?
-            normalize_legacy_exit_code(exit_code)
+          normalized_exit_code = normalize_legacy_exit_code(exit_code)
+          if valid_exit_codes.include? normalized_exit_code
+            normalized_exit_code
           else
-            normalize_exit_code_to_rfc(exit_code)
+            Chef::Log.warn(non_standard_exit_code_warning(normalized_exit_code))
+            VALID_RFC_062_EXIT_CODES[:GENERIC_FAILURE]
           end
         end
 
-        def enforce_rfc_062_exit_codes?
-          !normalization_disabled? && !normalization_not_configured?
-        end
-
-        def notify_reboot_exit_code_deprecation
-          return if normalization_disabled?
-          notify_on_deprecation(reboot_deprecation_warning)
-        end
-
-        def notify_deprecated_exit_code
-          return if normalization_disabled?
-          notify_on_deprecation(deprecation_warning)
-        end
-
         private
 
-        def normalization_disabled?
-          Chef::Config[:exit_status] == :disabled
-        end
-
-        def normalization_not_configured?
-          Chef::Config[:exit_status].nil?
-        end
-
-        def normalize_legacy_exit_code_with_warning(exit_code)
-          normalized_exit_code = normalize_legacy_exit_code(exit_code)
-          unless valid_exit_codes.include? normalized_exit_code
-            notify_on_deprecation(deprecation_warning)
-          end
-          normalized_exit_code
-        end
-
         def normalize_legacy_exit_code(exit_code)
           case exit_code
           when Integer
@@ -93,15 +63,6 @@ class Chef
           when Exception
             lookup_exit_code_by_exception(exit_code)
           else
-            default_exit_code
-          end
-        end
-
-        def normalize_exit_code_to_rfc(exit_code)
-          normalized_exit_code = normalize_legacy_exit_code_with_warning(exit_code)
-          if valid_exit_codes.include? normalized_exit_code
-            normalized_exit_code
-          else
             VALID_RFC_062_EXIT_CODES[:GENERIC_FAILURE]
           end
         end
@@ -111,15 +72,6 @@ class Chef
             VALID_RFC_062_EXIT_CODES[:SIGINT_RECEIVED]
           elsif sigterm_received?(exception)
             VALID_RFC_062_EXIT_CODES[:SIGTERM_RECEIVED]
-          elsif normalization_disabled? || normalization_not_configured?
-            if legacy_exit_code?(exception)
-              # We have lots of "Chef::Application.fatal!('', 2)
-              # This maintains that behavior at initial introduction
-              # and when the RFC exit_status compliance is disabled.
-              VALID_RFC_062_EXIT_CODES[:SIGINT_RECEIVED]
-            else
-              VALID_RFC_062_EXIT_CODES[:GENERIC_FAILURE]
-            end
           elsif reboot_scheduled?(exception)
             VALID_RFC_062_EXIT_CODES[:REBOOT_SCHEDULED]
           elsif reboot_needed?(exception)
@@ -135,12 +87,6 @@ class Chef
           end
         end
 
-        def legacy_exit_code?(exception)
-          resolve_exception_array(exception).any? do |e|
-            e.is_a? Chef::Exceptions::DeprecatedExitCode
-          end
-        end
-
         def reboot_scheduled?(exception)
           resolve_exception_array(exception).any? do |e|
             e.is_a? Chef::Exceptions::Reboot
@@ -204,26 +150,11 @@ class Chef
             # the current exit code assignment.
         end
 
-        def deprecation_warning
-          "Chef RFC 062 (https://github.com/chef/chef-rfc/blob/master/rfc062-exit-status.md) defines the" \
+        def non_standard_exit_code_warning(exit_code)
+          "Chef attempted to exit with a non-standard exit code of #{exit_code}." \
+          " Chef RFC 062 (https://github.com/chef/chef-rfc/blob/master/rfc062-exit-status.md) defines the" \
           " exit codes that should be used with Chef.  Chef::Application::ExitCode defines valid exit codes"  \
-          " In a future release, non-standard exit codes will be redefined as" \
-          " GENERIC_FAILURE unless `exit_status` is set to `:disabled` in your client.rb."
-        end
-
-        def reboot_deprecation_warning
-          "Per RFC 062 (https://github.com/chef/chef-rfc/blob/master/rfc062-exit-status.md)" \
-          ", when a reboot is requested Chef Client will exit with an exit code of 35, REBOOT_SCHEDULED." \
-          " To maintain the current behavior (an exit code of 0), you will need to set `exit_status` to" \
-          " `:disabled` in your client.rb"
-        end
-
-        def default_exit_code
-          if normalization_disabled? || normalization_not_configured?
-            DEPRECATED_RFC_062_EXIT_CODES[:DEPRECATED_FAILURE]
-          else
-            VALID_RFC_062_EXIT_CODES[:GENERIC_FAILURE]
-          end
+          " Non-standard exit codes are redefined as GENERIC_FAILURE."
         end
 
       end
diff --git a/lib/chef/application/windows_service.rb b/lib/chef/application/windows_service.rb
index 7bc68a586d..c30f5d1fe8 100644
--- a/lib/chef/application/windows_service.rb
+++ b/lib/chef/application/windows_service.rb
@@ -318,11 +318,11 @@ class Chef
 
           Chef::Config.merge!(config)
         rescue SocketError
-          Chef::Application.fatal!("Error getting config file #{Chef::Config[:config_file]}", Chef::Exceptions::DeprecatedExitCode.new)
+          Chef::Application.fatal!("Error getting config file #{Chef::Config[:config_file]}")
         rescue Chef::Exceptions::ConfigurationError => error
-          Chef::Application.fatal!("Error processing config file #{Chef::Config[:config_file]} with error #{error.message}", Chef::Exceptions::DeprecatedExitCode.new)
+          Chef::Application.fatal!("Error processing config file #{Chef::Config[:config_file]} with error #{error.message}")
         rescue Exception => error
-          Chef::Application.fatal!("Unknown error processing config file #{Chef::Config[:config_file]} with error #{error.message}", Chef::Exceptions::DeprecatedExitCode.new)
+          Chef::Application.fatal!("Unknown error processing config file #{Chef::Config[:config_file]} with error #{error.message}")
         end
       end
 
diff --git a/lib/chef/config_fetcher.rb b/lib/chef/config_fetcher.rb
index ee1b64956a..e14428157c 100644
--- a/lib/chef/config_fetcher.rb
+++ b/lib/chef/config_fetcher.rb
@@ -25,7 +25,7 @@ class Chef
       begin
         Chef::JSONCompat.from_json(config_data)
       rescue Chef::Exceptions::JSON::ParseError => error
-        Chef::Application.fatal!("Could not parse the provided JSON file (#{config_location}): " + error.message, Chef::Exceptions::DeprecatedExitCode.new)
+        Chef::Application.fatal!("Could not parse the provided JSON file (#{config_location}): " + error.message)
       end
     end
 
@@ -40,15 +40,15 @@ class Chef
     def fetch_remote_config
       http.get("")
     rescue SocketError, SystemCallError, Net::HTTPServerException => error
-      Chef::Application.fatal!("Cannot fetch config '#{config_location}': '#{error.class}: #{error.message}", Chef::Exceptions::DeprecatedExitCode.new)
+      Chef::Application.fatal!("Cannot fetch config '#{config_location}': '#{error.class}: #{error.message}")
     end
 
     def read_local_config
       ::File.read(config_location)
     rescue Errno::ENOENT
-      Chef::Application.fatal!("Cannot load configuration from #{config_location}", Chef::Exceptions::DeprecatedExitCode.new)
+      Chef::Application.fatal!("Cannot load configuration from #{config_location}")
     rescue Errno::EACCES
-      Chef::Application.fatal!("Permissions are incorrect on #{config_location}. Please chmod a+r #{config_location}", Chef::Exceptions::DeprecatedExitCode.new)
+      Chef::Application.fatal!("Permissions are incorrect on #{config_location}. Please chmod a+r #{config_location}")
     end
 
     def config_missing?
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 78bdf0cf4a..a09a3a062c 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -59,14 +59,6 @@ class Chef
     class UnsupportedAction < RuntimeError; end
     class MissingLibrary < RuntimeError; end
 
-    class DeprecatedExitCode < RuntimeError
-      def initalize
-        super "Exiting with a non RFC 062 Exit Code."
-        require "chef/application/exit_code"
-        Chef::Application::ExitCode.notify_deprecated_exit_code
-      end
-    end
-
     class CannotDetermineNodeName < RuntimeError
       def initialize
         super "Unable to determine node name: configure node_name or configure the system's hostname and fqdn"
diff --git a/lib/chef/platform/rebooter.rb b/lib/chef/platform/rebooter.rb
index 6829b66539..33a6e24be2 100644
--- a/lib/chef/platform/rebooter.rb
+++ b/lib/chef/platform/rebooter.rb
@@ -51,8 +51,7 @@ class Chef
             raise Chef::Exceptions::RebootFailed.new(e.message)
           end
 
-          raise Chef::Exceptions::Reboot.new(msg) if Chef::Application::ExitCode.enforce_rfc_062_exit_codes?
-          Chef::Application::ExitCode.notify_reboot_exit_code_deprecation
+          raise Chef::Exceptions::Reboot.new(msg)
         end
 
         # this is a wrapper function so Chef::Client only needs a single line of code.
diff --git a/spec/functional/rebooter_spec.rb b/spec/functional/rebooter_spec.rb
index 1b6e95b39c..a28491cc0b 100644
--- a/spec/functional/rebooter_spec.rb
+++ b/spec/functional/rebooter_spec.rb
@@ -72,6 +72,7 @@ describe Chef::Platform::Rebooter do
         def test_rebooter_method(method_sym, is_windows, expected_reboot_str)
           allow(ChefConfig).to receive(:windows?).and_return(is_windows)
           expect(rebooter).to receive(:shell_out!).once.with(expected_reboot_str)
+          expect(rebooter).to receive(:raise).with(Chef::Exceptions::Reboot)
           expect(rebooter).to receive(method_sym).once.and_call_original
           rebooter.send(method_sym, run_context.node)
         end
diff --git a/spec/integration/client/exit_code_spec.rb b/spec/integration/client/exit_code_spec.rb
index 30020f6a3f..4397426723 100644
--- a/spec/integration/client/exit_code_spec.rb
+++ b/spec/integration/client/exit_code_spec.rb
@@ -25,7 +25,7 @@ describe "chef-client" do
 
   let(:critical_env_vars) { %w{PATH RUBYOPT BUNDLE_GEMFILE GEM_PATH}.map { |o| "#{o}=#{ENV[o]}" } .join(" ") }
 
-  when_the_repository "does not have exit_status configured" do
+  when_the_repository "uses RFC 062 defined exit codes" do
 
     def setup_client_rb
       file "config/client.rb", <<EOM
@@ -43,110 +43,6 @@ EOM
     end
 
     def run_chef_client_and_expect_exit_code(exit_code)
-      shell_out!(
-        "#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'x::default'",
-        :cwd => chef_dir,
-        :returns => [exit_code])
-    end
-
-    context "has a cookbook" do
-      context "with a library" do
-        context "which cannot be loaded" do
-          before do
-            file "cookbooks/x/recipes/default.rb", ""
-            file "cookbooks/x/libraries/error.rb", "require 'does/not/exist'"
-          end
-
-          it "exits with GENERIC_FAILURE, 1" do
-            setup_client_rb
-            run_chef_client_and_expect_exit_code 1
-          end
-        end
-      end
-
-      context "with an audit recipe" do
-        context "which fails" do
-          before do
-            file "cookbooks/x/recipes/default.rb", <<-RECIPE
-control_group "control group without top level control" do
-  it "should fail" do
-    expect(2 - 2).to eq(1)
-  end
-end
-RECIPE
-          end
-
-          it "exits with GENERIC_FAILURE, 1" do
-            setup_client_rb_with_audit_mode
-            run_chef_client_and_expect_exit_code 1
-          end
-        end
-      end
-
-      context "with a recipe" do
-        context "which throws an error" do
-          before { file "cookbooks/x/recipes/default.rb", "raise 'BOOM'" }
-
-          it "exits with GENERIC_FAILURE, 1" do
-            setup_client_rb
-            run_chef_client_and_expect_exit_code 1
-          end
-        end
-
-        context "with a recipe which calls Chef::Application.fatal with a non-RFC exit code" do
-          before { file "cookbooks/x/recipes/default.rb", "Chef::Application.fatal!('BOOM', 123)" }
-
-          it "exits with the specified exit code" do
-            setup_client_rb
-            run_chef_client_and_expect_exit_code 123
-          end
-        end
-
-        context "with a recipe which calls Chef::Application.exit with a non-RFC exit code" do
-          before { file "cookbooks/x/recipes/default.rb", "Chef::Application.exit!('BOOM', 231)" }
-
-          it "exits with the specified exit code" do
-            setup_client_rb
-            run_chef_client_and_expect_exit_code 231
-          end
-        end
-      end
-
-      context "when an attempt to reboot fails (like from the reboot resource)" do
-        before do
-          file "cookbooks/x/recipes/default.rb", <<EOM
-raise Chef::Exceptions::RebootFailed.new
-EOM
-        end
-
-        it "exits with GENERIC_FAILURE, 1" do
-          setup_client_rb
-          run_chef_client_and_expect_exit_code 1
-        end
-      end
-    end
-  end
-
-  when_the_repository "does has exit_status enabled" do
-
-    def setup_client_rb
-      file "config/client.rb", <<EOM
-local_mode true
-cookbook_path "#{path_to('cookbooks')}"
-exit_status :enabled
-EOM
-    end
-
-    def setup_client_rb_with_audit_mode
-      file "config/client.rb", <<EOM
-local_mode true
-cookbook_path "#{path_to('cookbooks')}"
-exit_status :enabled
-audit_mode :audit_only
-EOM
-    end
-
-    def run_chef_client_and_expect_exit_code(exit_code)
       shell_out!("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'x::default'",
         :cwd => chef_dir,
         :returns => [exit_code])
diff --git a/spec/unit/application/exit_code_spec.rb b/spec/unit/application/exit_code_spec.rb
index 5abf19fc02..7783cf3ed7 100644
--- a/spec/unit/application/exit_code_spec.rb
+++ b/spec/unit/application/exit_code_spec.rb
@@ -70,92 +70,11 @@ describe Chef::Application::ExitCode do
     end
   end
 
-  context "when Chef::Config :exit_status is not configured" do
-    before do
-      allow(Chef::Config).to receive(:[]).with(:exit_status).and_return(nil)
-      allow(Chef::Config).to receive(:[]).with(:treat_deprecation_warnings_as_errors).and_return(false)
-    end
-
-    it "writes a deprecation warning" do
-      expect(Chef).to receive(:deprecated).with(:exit_code, /^Chef RFC 062/)
-      expect(exit_codes.normalize_exit_code(151)).to eq(151)
-    end
-
-    it "does not modify non-RFC exit codes" do
-      expect(exit_codes.normalize_exit_code(151)).to eq(151)
-    end
-
-    it "returns DEPRECATED_FAILURE when no exit code is specified" do
-      expect(exit_codes.normalize_exit_code()).to eq(-1)
-    end
-
-    it "returns SIGINT_RECEIVED when a SIGINT is received" do
-      expect(exit_codes.normalize_exit_code(Chef::Exceptions::SigInt.new("BOOM"))).to eq(2)
-    end
-
-    it "returns SIGTERM_RECEIVED when a SIGTERM is received" do
-      expect(exit_codes.normalize_exit_code(Chef::Exceptions::SigTerm.new("BOOM"))).to eq(3)
-    end
-
-    it "returns SIGINT_RECEIVED when a deprecated exit code error is received" do
-      expect(exit_codes.normalize_exit_code(Chef::Exceptions::DeprecatedExitCode.new("BOOM"))).to eq(2)
-    end
-
-    it "returns GENERIC_FAILURE when an exception is specified" do
-      expect(exit_codes.normalize_exit_code(Exception.new("BOOM"))).to eq(1)
-    end
-
-  end
-
-  context "when Chef::Config :exit_status is configured to not validate exit codes" do
-    before do
-      allow(Chef::Config).to receive(:[]).with(:exit_status).and_return(:disabled)
-      allow(Chef::Config).to receive(:[]).with(:treat_deprecation_warnings_as_errors).and_return(false)
-    end
-
-    it "does not write a deprecation warning" do
-      expect(Chef).not_to receive(:deprecated).with(:exit_code, /^Chef RFC 062/)
-      expect(exit_codes.normalize_exit_code(151)).to eq(151)
-    end
-
-    it "does not modify non-RFC exit codes" do
-      expect(exit_codes.normalize_exit_code(151)).to eq(151)
-    end
-
-    it "returns DEPRECATED_FAILURE when no exit code is specified" do
-      expect(exit_codes.normalize_exit_code()).to eq(-1)
-    end
-
-    it "returns GENERIC_FAILURE when an exception is specified" do
-      expect(exit_codes.normalize_exit_code(Exception.new("BOOM"))).to eq(1)
-    end
-
-    it "returns SUCCESS when a reboot is pending" do
-      allow(Chef::DSL::RebootPending).to receive(:reboot_pending?).and_return(true)
-      expect(exit_codes.normalize_exit_code(0)).to eq(0)
-    end
-
-    it "returns SIGINT_RECEIVED when a SIGINT is received" do
-      expect(exit_codes.normalize_exit_code(Chef::Exceptions::SigInt.new("BOOM"))).to eq(2)
-    end
-
-    it "returns SIGTERM_RECEIVED when a SIGTERM is received" do
-      expect(exit_codes.normalize_exit_code(Chef::Exceptions::SigTerm.new("BOOM"))).to eq(3)
-    end
-
-    it "returns SIGINT_RECEIVED when a deprecated exit code error is received" do
-      expect(exit_codes.normalize_exit_code(Chef::Exceptions::DeprecatedExitCode.new("BOOM"))).to eq(2)
-    end
-  end
+  context "when Chef validates exit codes" do
 
-  context "when Chef::Config :exit_status is configured to validate exit codes" do
-    before do
-      allow(Chef::Config).to receive(:[]).with(:exit_status).and_return(:enabled)
-      allow(Chef::Config).to receive(:[]).with(:treat_deprecation_warnings_as_errors).and_return(false)
-    end
-
-    it "does write a deprecation warning" do
-      expect(Chef).to receive(:deprecated).with(:exit_code, /^Chef RFC 062/)
+    it "does write a warning on non-standard exit codes" do
+      expect(Chef::Log).to receive(:warn).with(
+        /^Chef attempted to exit with a non-standard exit code of 151/)
       expect(exit_codes.normalize_exit_code(151)).to eq(1)
     end
 
@@ -175,10 +94,6 @@ describe Chef::Application::ExitCode do
       expect(exit_codes.normalize_exit_code(Chef::Exceptions::SigTerm.new("BOOM"))).to eq(3)
     end
 
-    it "returns GENERIC_FAILURE when a deprecated exit code error is received" do
-      expect(exit_codes.normalize_exit_code(Chef::Exceptions::DeprecatedExitCode.new("BOOM"))).to eq(1)
-    end
-
     it "returns GENERIC_FAILURE when an exception is specified" do
       expect(exit_codes.normalize_exit_code(Exception.new("BOOM"))).to eq(1)
     end
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index 867cd3f9c2..a12935fa78 100644
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -299,16 +299,23 @@ describe Chef::Application do
       Chef::Application.fatal! "blah"
     end
 
-    describe "when an exit code is supplied" do
+    describe "when a standard exit code is supplied" do
       it "should exit with the given exit code" do
-        expect(Process).to receive(:exit).with(-100).and_return(true)
+        expect(Process).to receive(:exit).with(42).and_return(true)
+        Chef::Application.fatal! "blah", 42
+      end
+    end
+
+    describe "when a non-standard exit code is supplied" do
+      it "should exit with the default exit code" do
+        expect(Process).to receive(:exit).with(1).and_return(true)
         Chef::Application.fatal! "blah", -100
       end
     end
 
     describe "when an exit code is not supplied" do
       it "should exit with the default exit code" do
-        expect(Process).to receive(:exit).with(-1).and_return(true)
+        expect(Process).to receive(:exit).with(1).and_return(true)
         Chef::Application.fatal! "blah"
       end
     end
diff --git a/spec/unit/config_fetcher_spec.rb b/spec/unit/config_fetcher_spec.rb
index 6847ee5fd3..a674d4de33 100644
--- a/spec/unit/config_fetcher_spec.rb
+++ b/spec/unit/config_fetcher_spec.rb
@@ -58,7 +58,7 @@ describe Chef::ConfigFetcher do
             and_return(invalid_json)
 
           expect(Chef::Application).to receive(:fatal!).
-            with(invalid_json_error_regex, Chef::Exceptions::DeprecatedExitCode.new)
+            with(invalid_json_error_regex)
           fetcher.fetch_json
         end
       end
@@ -104,7 +104,7 @@ describe Chef::ConfigFetcher do
               with("").and_return(invalid_json)
 
             expect(Chef::Application).to receive(:fatal!).
-              with(invalid_json_error_regex, Chef::Exceptions::DeprecatedExitCode.new)
+              with(invalid_json_error_regex)
             fetcher.fetch_json
           end
         end