diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index 5740fe7a22d..04d864129d8 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -197,7 +197,9 @@ def stop_processing? # Are there any failed resources in this transaction? def any_failed? - report.resource_statuses.values.detect { |status| status.failed? } + report.resource_statuses.values.detect { |status| + status.failed? || status.failed_to_restart? + } end # Find all of the changed resources. @@ -245,7 +247,10 @@ def prefetch_if_necessary(resource) def apply(resource, ancestor = nil) status = resource_harness.evaluate(resource) add_resource_status(status) - event_manager.queue_events(ancestor || resource, status.events) unless status.failed? + ancestor ||= resource + if !(status.failed? || status.failed_to_restart?) + event_manager.queue_events(ancestor, status.events) + end rescue => detail resource.err _("Could not evaluate: %{detail}") % { detail: detail } end @@ -297,7 +302,7 @@ def propagate_failure(resource) relationship_graph.direct_dependencies_of(resource).each do |dep| if (s = resource_status(dep)) failed.merge(s.failed_dependencies) if s.dependency_failed? - if s.failed? + if s.failed? || s.failed_to_restart? failed.add(dep) end end diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb index b261ad10e7f..89f1c7e20b2 100644 --- a/lib/puppet/transaction/report.rb +++ b/lib/puppet/transaction/report.rb @@ -170,7 +170,9 @@ def add_resource_status(status) # @api private def compute_status(resource_metrics, change_metric) - if resources_failed_to_generate || (resource_metrics["failed"] || 0) > 0 + if resources_failed_to_generate || + (resource_metrics["failed"] || 0) > 0 || + (resource_metrics["failed_to_restart"] || 0) > 0 'failed' elsif change_metric > 0 'changed' diff --git a/spec/integration/transaction_spec.rb b/spec/integration/transaction_spec.rb index 9c8c27ff31e..fb61a8ae539 100644 --- a/spec/integration/transaction_spec.rb +++ b/spec/integration/transaction_spec.rb @@ -233,7 +233,7 @@ def touch(path) expect(Puppet::FileSystem.exist?(path)).not_to be_truthy end - it "should not let one failed refresh result in other refreshes failing" do + it "one failed refresh should propagate its failure to dependent refreshes" do path = tmpfile("path") newfile = tmpfile("file") file = Puppet::Type.type(:file).new( @@ -263,7 +263,7 @@ def touch(path) catalog = mk_catalog(file, exec1, exec2) catalog.apply - expect(Puppet::FileSystem.exist?(newfile)).to be_truthy + expect(Puppet::FileSystem.exist?(newfile)).to be_falsey end # Ensure when resources have been generated with eval_generate that event diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb index 873adfb5768..c4dad642a0c 100644 --- a/spec/unit/transaction/report_spec.rb +++ b/spec/unit/transaction/report_spec.rb @@ -292,6 +292,12 @@ def add_statuses(count, type = :file) expect(@report.status).to eq('failed') end + it "should mark the report as 'failed' if resources failed to restart" do + add_statuses(1) { |status| status.failed_to_restart = true } + @report.finalize_report + expect(@report.status).to eq('failed') + end + it "should mark the report as 'failed' if resources_failed_to_generate" do @report.resources_failed_to_generate = true @report.finalize_report diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb index fad6f9ba0b8..0b1d94251ca 100644 --- a/spec/unit/transaction_spec.rb +++ b/spec/unit/transaction_spec.rb @@ -50,7 +50,7 @@ def transaction_with_resource(resource) expect(transaction.report.resource_statuses[resource.to_s]).to equal(status) end - it "should not consider there to be failed resources if no statuses are marked failed" do + it "should not consider there to be failed or failed_to_restart resources if no statuses are marked failed" do resource = Puppet::Type.type(:notify).new :title => "foobar" transaction = transaction_with_resource(resource) transaction.evaluate @@ -174,6 +174,13 @@ def transaction_with_resource(resource) expect(@transaction).to be_any_failed end + + it "should report any_failed if any resources failed to restart" do + @transaction.evaluate + @transaction.report.resource_statuses[@resource.to_s].failed_to_restart = true + + expect(@transaction).to be_any_failed + end end describe "#unblock" do