Bug #1054

Reports should be sent for runs that fail entirely

Added by Martha Greenberg over 4 years ago. Updated over 2 years ago.

Status:Closed Start date:
Priority:High Due date:
Assignee:James Turnbull % Done:

0%
Category:testing
Target version:2.6.0
Affected Puppet version:0.24.4 Branch:luke/tickets/master/1054
Keywords:
Votes: 1

Description

If the configuration cannot be instantiated or the client run otherwise fails, no report is created, which means the server never gets notified when, say, a syntax error causes the run to fail entirely.

Related issues

related to Puppet - Refactor #1405: Puppet::Network::Client::Master should be entirely refact... Closed 07/08/2008
related to Puppet - Bug #7127: prerun_command don't stop puppet on error Closed 04/15/2011
related to Puppet - Bug #2944: configurer.rb calls generate_report for --apply puppet runs Closed 12/17/2009
related to Puppet - Bug #2973: puppetd fails at '4226e018d7c8e2d52ef59edf37d3a320aa823be... Closed 12/21/2009
duplicated by Puppet - Feature #2610: Puppet should send err: emails when there is a parse error. Duplicate 09/07/2009

History

#1
Updated by Luke Kanies about 4 years ago

This requires creating the transaction reports before the transaction actually exists. Shouldn’t be too hard, especially as we refactor the client-related code for the Indirector.

#2
Updated by Redmine Admin almost 4 years ago

  • Status changed from 1 to Accepted

#3
Updated by Luke Kanies almost 4 years ago

  • Subject changed from tagmail should be sent on failed runs to Reports should be sent for runs that fail entirely
  • 3 changed from Unknown to Medium
  • Affected Puppet version set to 0.24.4

#4
Updated by James Turnbull almost 3 years ago

  • Assignee deleted (Puppet Community)

#5
Updated by James Turnbull almost 3 years ago

  • Assignee set to Luke Kanies
  • Target version set to 2.6.0

#6
Updated by Steven Jenkins over 2 years ago

Could you clarify where the new report should end up getting kicked off? Currently, the report is sent from within Puppet::Resource::Catalog.apply(). Where should it be instead?

I see the following occurrences of ‘catalog.apply’ in the source:

lib/puppet/dsl.rb:72: catalog.apply

** probably ignore that for now? (ie, put in TODO/FIXME)

lib/puppet/network/handler/resource.rb:46: transaction = catalog.apply

** probably ignore that for now? (ie, put in TODO/FIXME)

lib/puppet/configurer.rb:140: catalog.apply(options)

lib/puppet/application/puppet.rb:81: transaction = catalog.apply

This appears to be the right place to generate the report, as the catalog is created and transaction is managed from here.

Would it be reasonable to add error checking and only report if the transaction failed (i.e., let the existing transaction reporting mechanism continue to handle the success case)? That would help prevent needing to reach across abstraction layers.

lib/puppet/application/ralsh.rb:117: catalog.apply

** Needs similar refactoring.

lib/puppet/util/settings.rb:615: catalog.apply do |transaction|

** needs similar refactoring

lib/puppet/configurer/downloader.rb:32: catalog.apply do |trans|

** needs similar refactoring

Of course, all that code duplication implies additional refactoring work is needed. Does it make sense to push this into Puppet::Resource::Catalog?

#7
Updated by Luke Kanies over 2 years ago

  • Category changed from transactions to testing
  • Assignee changed from Luke Kanies to Steven Jenkins

The report should probably be sent from the Agent class, and then passed in from there to the transaction.

#8
Updated by Luke Kanies over 2 years ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Assignee changed from Steven Jenkins to James Turnbull
  • Branch set to luke/tickets/master/1054

Ok, I’ve pushed a branch (on top of the newly merged master) with a fix for this. Test away. I’ll close the other related tickets, since this fixes all of them.

#9
Updated by James Turnbull over 2 years ago

  • Status changed from In Topic Branch Pending Review to Closed

Pushed in commit:58a81ba0e074ac8b3c6b7f8cd5c59fa18eb7f58a in branch master.

Also available in: Atom PDF