diff options
author | Claire McQuin <claire@getchef.com> | 2015-05-08 17:31:42 -0700 |
---|---|---|
committer | Bryan McLellan <btm@loftninjas.org> | 2015-05-27 14:16:22 -0400 |
commit | 67b9fdc307fd0a7480cc4f11c18540763ec43b09 (patch) | |
tree | a08e020a19158bef3fe6ae2166d932ed2865eb87 | |
parent | 49d25cdb67ebef05aa62df3662c3c79466abace7 (diff) | |
download | chef-67b9fdc307fd0a7480cc4f11c18540763ec43b09.tar.gz |
Mark run failed on failed audits unless configured to audit as warning.
Remove unnecessary newline from RunFailedWrappingError backtrace
-rw-r--r-- | lib/chef/client.rb | 28 | ||||
-rw-r--r-- | lib/chef/config.rb | 1 | ||||
-rw-r--r-- | lib/chef/exceptions.rb | 2 | ||||
-rw-r--r-- | spec/unit/client_spec.rb | 300 | ||||
-rw-r--r-- | spec/unit/exceptions_spec.rb | 4 |
5 files changed, 203 insertions, 132 deletions
diff --git a/lib/chef/client.rb b/lib/chef/client.rb index 0764d3f3ba..bf99f003fb 100644 --- a/lib/chef/client.rb +++ b/lib/chef/client.rb @@ -336,7 +336,7 @@ class Chef @events.converge_start(run_context) Chef::Log.debug("Converging node #{node_name}") @runner = Chef::Runner.new(run_context) - runner.converge + @runner.converge @events.converge_complete rescue Exception => e @events.converge_failed(e) @@ -370,9 +370,11 @@ class Chef auditor = Chef::Audit::Runner.new(run_context) auditor.run if auditor.failed? - raise Chef::Exceptions::AuditsFailed.new(auditor.num_failed, auditor.num_total) + audit_exception = Chef::Exceptions::AuditsFailed.new(auditor.num_failed, auditor.num_total) + @events.audit_phase_failed(audit_exception) + else + @events.audit_phase_complete end - @events.audit_phase_complete rescue Exception => e Chef::Log.error("Audit phase failed with error message: #{e.message}") @events.audit_phase_failed(e) @@ -464,10 +466,9 @@ class Chef audit_error = run_audits(run_context) end - if converge_error || audit_error - e = Chef::Exceptions::RunFailedWrappingError.new(converge_error, audit_error) - e.fill_backtrace - raise e + if err = wrap_exceptions(converge_error, audit_error) + err.fill_backtrace + raise err end run_status.stop_clock @@ -537,6 +538,19 @@ class Chef Chef::ReservedNames::Win32::Security.has_admin_privileges? end + def wrap_exceptions(converge_error, audit_error) + if audit_error && !(audit_error.is_a?(Chef::Exceptions::AuditsFailed) && Chef::Config[:audit_as_warning]) + if converge_error + return Chef::Exceptions::RunFailedWrappingError.new(converge_error, audit_error) + else + return Chef::Exceptions::RunFailedWrappingError.new(audit_error) + end + elsif converge_error + return Chef::Exceptions::RunFailedWrappingError.new(converge_error) + end + nil + end + end end diff --git a/lib/chef/config.rb b/lib/chef/config.rb index 629553c8ab..9beb18b53e 100644 --- a/lib/chef/config.rb +++ b/lib/chef/config.rb @@ -51,4 +51,3 @@ class Chef end end - diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index da562e70f4..c0f4158db4 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -435,7 +435,7 @@ class Chef wrapped_errors.each_with_index do |e,i| backtrace << "#{i+1}) #{e.class} - #{e.message}" backtrace += e.backtrace if e.backtrace - backtrace << "" + backtrace << "" unless i == wrapped_errors.length - 1 end set_backtrace(backtrace) end diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb index d8c4ede796..2f03b23e32 100644 --- a/spec/unit/client_spec.rb +++ b/spec/unit/client_spec.rb @@ -407,13 +407,18 @@ describe Chef::Client do end end - describe "when converge fails" do + context "when converge errors" do include_context "a client run" do - let(:e) { Exception.new } + + let(:converge_error) do + err = Chef::Exceptions::UnsupportedAction.new("Action unsupported") + err.set_backtrace([ "/path/recipe.rb:15", "/path/recipe.rb:12" ]) + err + end + def stub_for_converge expect(Chef::Runner).to receive(:new).and_return(runner) - expect(runner).to receive(:converge).and_raise(e) - expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(Chef::Exceptions::RunFailedWrappingError) + expect(runner).to receive(:converge).and_raise(converge_error) end def stub_for_node_save @@ -432,179 +437,232 @@ describe Chef::Client do expect_any_instance_of(Chef::ResourceReporter).to receive(:run_failed) expect_any_instance_of(Chef::Audit::AuditReporter).to receive(:run_failed) end - end - it "runs the audits and raises the error" do - expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| - expect(error.wrapped_errors.size).to eq(1) - expect(error.wrapped_errors[0]).to eq(e) + before do + expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(Chef::Exceptions::RunFailedWrappingError) end - end - end - - describe "when the audit phase fails" do - context "with an exception" do - context "when audit mode is enabled" do - include_context "a client run" do - let(:e) { Exception.new } - def stub_for_audit - expect(Chef::Audit::Runner).to receive(:new).and_return(audit_runner) - expect(audit_runner).to receive(:run).and_raise(e) - expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(Chef::Exceptions::RunFailedWrappingError) - end - - def stub_for_run - expect_any_instance_of(Chef::RunLock).to receive(:acquire) - expect_any_instance_of(Chef::RunLock).to receive(:save_pid) - expect_any_instance_of(Chef::RunLock).to receive(:release) - - # Post conditions: check that node has been filled in correctly - expect(client).to receive(:run_started) - expect(client).to receive(:run_failed) - expect_any_instance_of(Chef::ResourceReporter).to receive(:run_failed) - expect_any_instance_of(Chef::Audit::AuditReporter).to receive(:run_failed) - end - end - - it "should save the node after converge and raise exception" do + context "without audit phase errors" do + it "skips node save and raises the converge error in a wrapping error" do expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| expect(error.wrapped_errors.size).to eq(1) - expect(error.wrapped_errors[0]).to eq(e) + expect(error.wrapped_errors).to include(converge_error) + expect(error.backtrace).to include(*converge_error.backtrace) end end end - context "when audit mode is disabled" do - include_context "a client run" do - before do - Chef::Config[:audit_mode] = :disabled + context "when audit phase errors" do + context "with failed audits" do + + let(:audit_error) do + err = Chef::Exceptions::AuditsFailed.new(num_failed, num_total) + err.set_backtrace([ "/path/recipe.rb:57", "/path/recipe.rb:55" ]) + err end - let(:e) { FooError.new } + let(:num_failed) { 1 } + + let(:num_total) { 5 } def stub_for_audit - expect(Chef::Audit::Runner).to_not receive(:new) + expect(Chef::Audit::Runner).to receive(:new).and_return(audit_runner) + expect(audit_runner).to receive(:run) + expect(audit_runner).to receive(:failed?).and_return(true) + expect(Chef::Exceptions::AuditsFailed).to receive(:new).with(num_failed, num_total).and_return(audit_error) + allow(audit_runner).to receive(:num_failed).and_return(num_failed) + allow(audit_runner).to receive(:num_total).and_return(num_total) end - def stub_for_converge - expect(Chef::Runner).to receive(:new).and_return(runner) - expect(runner).to receive(:converge).and_raise(e) - expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(FooError) + context "with audit_as_warning true" do + before do + Chef::Config[:audit_as_warning] = true + end + + it "skips node save and raises the converge error in a wrapping error" do + expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| + expect(error.wrapped_errors.size).to eq(1) + expect(error.wrapped_errors[0]).to eq(converge_error) + end + end end - def stub_for_node_save - expect(client).to_not receive(:save_updated_node) + context "with audit_as_warning false" do + + before do + Chef::Config[:audit_as_warning] = false + end + + it "skips node save and raises converge and audit errors in a wrapping error" do + expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| + expect(error.wrapped_errors.size).to eq(2) + expect(error.wrapped_errors).to include(converge_error) + expect(error.wrapped_errors).to include(audit_error) + expect(error.backtrace).to include(*converge_error.backtrace) + expect(error.backtrace).to include(*audit_error.backtrace) + end + end end + end - def stub_for_run - expect_any_instance_of(Chef::RunLock).to receive(:acquire) - expect_any_instance_of(Chef::RunLock).to receive(:save_pid) - expect_any_instance_of(Chef::RunLock).to receive(:release) - + context "with unexpected error" do - # Post conditions: check that node has been filled in correctly - expect(client).to receive(:run_started) - expect(client).to receive(:run_failed) + let(:audit_error) do + err = RuntimeError.new("Unexpected audit error") + err.set_backtrace([ "/path/recipe.rb:57", "/path/recipe.rb:55" ]) + err + end - expect_any_instance_of(Chef::ResourceReporter).to receive(:run_failed) + def stub_for_audit + expect(Chef::Audit::Runner).to receive(:new).and_return(audit_runner) + expect(audit_runner).to receive(:run).and_raise(audit_error) + end + context "with audit_as_warning true" do + it "skips node save and raises converge and audit errors in a wrapping error" do + expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| + expect(error.wrapped_errors.size).to eq(2) + expect(error.wrapped_errors).to include(converge_error) + expect(error.wrapped_errors).to include(audit_error) + expect(error.backtrace).to include(*converge_error.backtrace) + expect(error.backtrace).to include(*audit_error.backtrace) + end + end end - it "re-raises an unwrapped exception" do - expect { client.run }.to raise_error(FooError) + context "with audit_as_warning false" do + it "skips node save and raises converge and audit errors in a wrapping error" do + expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| + expect(error.wrapped_errors.size).to eq(2) + expect(error.wrapped_errors).to include(converge_error) + expect(error.wrapped_errors).to include(audit_error) + expect(error.backtrace).to include(*converge_error.backtrace) + expect(error.backtrace).to include(*audit_error.backtrace) + end + end end end end + end + end + context "when audit phase errors" do + include_context "a client run" do - end + def stub_for_run + expect_any_instance_of(Chef::RunLock).to receive(:acquire) + expect_any_instance_of(Chef::RunLock).to receive(:save_pid) + expect_any_instance_of(Chef::RunLock).to receive(:release) + + # Post conditions: check that node has been filled in correctly + expect(client).to receive(:run_started) + end + + context "with failed audits" do - context "with failed audits" do - include_context "a client run" do - let(:audit_runner) do - instance_double("Chef::Audit::Runner", :run => true, :failed? => true, :num_failed => 1, :num_total => 1) + let(:audit_error) do + err = Chef::Exceptions::AuditsFailed.new(num_failed, num_total) + err.set_backtrace([ "/path/recipe.rb:57", "/path/recipe.rb:55" ]) + err end + let(:num_failed) { 1 } + + let(:num_total) { 5 } + def stub_for_audit expect(Chef::Audit::Runner).to receive(:new).and_return(audit_runner) - expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(Chef::Exceptions::RunFailedWrappingError) + expect(audit_runner).to receive(:run) + expect(audit_runner).to receive(:failed?).and_return(true) + expect(Chef::Exceptions::AuditsFailed).to receive(:new).with(num_failed, num_total).and_return(audit_error) + allow(audit_runner).to receive(:num_failed).and_return(num_failed) + allow(audit_runner).to receive(:num_total).and_return(num_total) end - def stub_for_run - expect_any_instance_of(Chef::RunLock).to receive(:acquire) - expect_any_instance_of(Chef::RunLock).to receive(:save_pid) - expect_any_instance_of(Chef::RunLock).to receive(:release) - - # Post conditions: check that node has been filled in correctly - expect(client).to receive(:run_started) - expect(client).to receive(:run_failed) + context "with audit_as_warning true" do + before do + Chef::Config[:audit_as_warning] = true + end - expect_any_instance_of(Chef::ResourceReporter).to receive(:run_failed) - expect_any_instance_of(Chef::Audit::AuditReporter).to receive(:run_failed) + it "saves the node and completes the run successfully" do + expect(client).to receive(:run_completed_successfully) + expect_any_instance_of(Chef::ResourceReporter).to receive(:run_completed) + expect_any_instance_of(Chef::Audit::AuditReporter).to receive(:run_completed) + client.run + end end - end - it "should save the node after converge and raise exception" do - expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| - expect(error.wrapped_errors.size).to eq(1) - expect(error.wrapped_errors[0]).to be_instance_of(Chef::Exceptions::AuditsFailed) - end - end - end - end + context "with audit_as_warning false" do - describe "when why_run mode is enabled" do - include_context "a client run" do + before do + Chef::Config[:audit_as_warning] = false + expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(Chef::Exceptions::RunFailedWrappingError) + end - before do - Chef::Config[:why_run] = true - end + it "saves the node and raises audit error in a wrapping error" do + expect(client).to receive(:run_failed) + expect_any_instance_of(Chef::ResourceReporter).to receive(:run_failed) + expect_any_instance_of(Chef::Audit::AuditReporter).to receive(:run_failed) - def stub_for_audit - expect(Chef::Audit::Runner).to_not receive(:new) + expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| + expect(error.wrapped_errors.size).to eq(1) + expect(error.wrapped_errors).to include(audit_error) + expect(error.backtrace).to include(*audit_error.backtrace) + end + end + end end - def stub_for_node_save - # This is how we should be mocking external calls - not letting it fall all the way through to the - # REST call - expect(node).to receive(:save) - end + context "with unexpected error" do - it "runs successfully without enabling the audit runner" do - client.run + let(:audit_error) do + err = RuntimeError.new("Unexpected audit error") + err.set_backtrace([ "/path/recipe.rb:57", "/path/recipe.rb:55" ]) + err + end - # fork is stubbed, so we can see the outcome of the run - expect(node.automatic_attrs[:platform]).to eq("example-platform") - expect(node.automatic_attrs[:platform_version]).to eq("example-platform-1.0") - end - end - end + def stub_for_audit + expect(Chef::Audit::Runner).to receive(:new).and_return(audit_runner) + expect(audit_runner).to receive(:run).and_raise(audit_error) + end - describe "when audits are disabled" do - include_context "a client run" do + before do + expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(Chef::Exceptions::RunFailedWrappingError) + end - before do - Chef::Config[:audit_mode] = :disabled - end + context "with audit_as_warning true" do + it "saves the node and raises audit error in a wrapping error" do + expect(client).to receive(:run_failed) + expect_any_instance_of(Chef::ResourceReporter).to receive(:run_failed) + expect_any_instance_of(Chef::Audit::AuditReporter).to receive(:run_failed) - def stub_for_audit - expect(Chef::Audit::Runner).to_not receive(:new) - end + expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| + expect(error.wrapped_errors.size).to eq(1) + expect(error.wrapped_errors).to include(audit_error) + expect(error.backtrace).to include(*audit_error.backtrace) + end + end + end - it "runs successfully without enabling the audit runner" do - client.run + context "with audit_as_warning false" do + it "skips node save and raises converge and audit errors in a wrapping error" do + expect(client).to receive(:run_failed) + expect_any_instance_of(Chef::ResourceReporter).to receive(:run_failed) + expect_any_instance_of(Chef::Audit::AuditReporter).to receive(:run_failed) - # fork is stubbed, so we can see the outcome of the run - expect(node.automatic_attrs[:platform]).to eq("example-platform") - expect(node.automatic_attrs[:platform_version]).to eq("example-platform-1.0") + expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error| + expect(error.wrapped_errors.size).to eq(1) + expect(error.wrapped_errors).to include(audit_error) + expect(error.backtrace).to include(*audit_error.backtrace) + end + end + end end end - end + end end - describe "when handling run failures" do it "should remove the run_lock on failure of #load_node" do diff --git a/spec/unit/exceptions_spec.rb b/spec/unit/exceptions_spec.rb index d35ecc8ec8..fd90aeab71 100644 --- a/spec/unit/exceptions_spec.rb +++ b/spec/unit/exceptions_spec.rb @@ -113,7 +113,7 @@ describe Chef::Exceptions do context "initialized with 1 error and nil" do let(:e) { Chef::Exceptions::RunFailedWrappingError.new(RuntimeError.new("foo"), nil) } let(:num_errors) { 1 } - let(:backtrace) { ["1) RuntimeError - foo", ""] } + let(:backtrace) { ["1) RuntimeError - foo"] } include_examples "RunFailedWrappingError expectations" end @@ -121,7 +121,7 @@ describe Chef::Exceptions do context "initialized with 2 errors" do let(:e) { Chef::Exceptions::RunFailedWrappingError.new(RuntimeError.new("foo"), RuntimeError.new("bar")) } let(:num_errors) { 2 } - let(:backtrace) { ["1) RuntimeError - foo", "", "2) RuntimeError - bar", ""] } + let(:backtrace) { ["1) RuntimeError - foo", "", "2) RuntimeError - bar"] } include_examples "RunFailedWrappingError expectations" end |