[PUP-526]  `name` attribute is not always included in a Puppet::Transaction::Event Created: 2013/10/04  Updated: 2015/01/23  Resolved: 2015/01/23

Status: Closed
Project: Puppet
Component/s: Docs
Affects Version/s: None
Fix Version/s: PUP 3.4.0

Type: Bug Priority: Normal
Reporter: Ruth Linehan Assignee: Ethan Brown
Resolution: Done Votes: 0
Labels: 3.0, PE, PE_3.1, PuppetCore, Reference_Manual
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Puppet 3.3.1 (PE 3.1.0-rc1) on Centos 6 x32.


Issue Links:
Relates
relates to PUP-639 If insync? on a property causes a fai... Resolved
relates to PUP-640 Report format 4 docs should be update... Closed
Template:
Epic Link: PE 3.2 Requests
Story Points: 1
Sprint: Week 2013-10-30 to 2013-11-06, Week 2013-11-06 to 2013-11-13

 Description   

If a whole resource fails - i.e. there is a resource failure that is not specific to a property - the failure event (Puppet::Transaction::Event) will not have a `name` attribute. In PE 3.1.0 release candidates this was causing reports to fail to upload (because we introduced report sanitizing which checked for the `name` attribute and failed if it was not present).

This can be reproduced by changing the owner or group of an existing file resource to an owner/group that does not exist.

Our docs (http://docs.puppetlabs.com/puppet/3/reference/format_report.html#puppettransactionevent-1) say that `name` will be absent in inspect reports, but this is not an inspect report. The docs should be updated, but the fact that `name` is absent completely, rather than empty or nil, seems like a bug.



 Comments   
Comment by Michelle Johansen (Inactive) [ 2013/10/30 ]

Waiting on PE 3.2 priority decision

Comment by Andrew Parker [ 2013/11/06 ]

The proposed fix involved setting the name attribute to always default to an empty string. This is unwanted, because a name is supposed to be a symbol, which caused the problem of there not being any clear symbol to use for the "not known" case.

We could set this field to nil. Besides the questionable nature of a field that is documented to be a symbol and present, this causes the problem that the field is documented to not be present for inspect reports.

In the end we have a couple choices:

  • Choose a name to use when we fail for an unknown part of the catalog.
  • Update the documentation to simply say that the name is an optional field.
  • Make the field nil by default and call out in the documentation that the field is present but may be nil.

This is also only an issue when reports are serialized as YAML, which is a deprecated format.

Comment by Andrew Parker [ 2013/11/06 ]

The new PR just patches failed_because to provide a name.

Overall this just all makes me sad, but to conform to the current documentation we end up needing to do something like this.

Comment by Ethan Brown [ 2013/11/07 ]

Reviewed / discussed on IRC, then proposal rejected

Comment by Ethan Brown [ 2013/11/07 ]

Reverted to old master, verified no name with

bundle exec puppet apply -e 'file { "/tmp/foo" : owner => "nothere" }'

In log

    File[/tmp/foo]: !ruby/object:Puppet::Resource::Status
      resource: File[/tmp/foo]
      file: 
      line: 1
      evaluation_time: 0.001061
      change_count: 0
      out_of_sync_count: 1
      tags: 
        - file
        - class
      time: 2013-11-07 11:56:07.451392 -05:00
      events: 
        - !ruby/object:Puppet::Transaction::Event
          audited: false
          message: "Could not find user nothere"
          status: failure
          time: 2013-11-07 11:56:07.452447 -05:00
      out_of_sync: true
      changed: false
      resource_type: File
      title: /tmp/foo
      skipped: false
      failed: true
      containment_path: 
        - Stage[main]
        - ""
        - File[/tmp/foo]

Cherry-picked in commit from PR

Got

    File[/tmp/foo]: !ruby/object:Puppet::Resource::Status
      resource: File[/tmp/foo]
      file: 
      line: 1
      evaluation_time: 0.001141
      change_count: 0
      out_of_sync_count: 1
      tags: 
        - file
        - class
      time: 2013-11-07 11:57:55.776140 -05:00
      events: 
        - !ruby/object:Puppet::Transaction::Event
          audited: false
          message: "Could not find user nothere"
          name: !ruby/sym resource_error
          status: failure
          time: 2013-11-07 11:57:55.777273 -05:00
      out_of_sync: true
      changed: false
      resource_type: File
      title: /tmp/foo
      skipped: false
      failed: true
      containment_path: 
        - Stage[main]
        - ""
        - File[/tmp/foo]

Comment by Joshua Partlow [ 2014/05/28 ]

Based on the presence of the changes of the merged PR in master, I believe this went out with 3.4.0.

Generated at Sat Sep 19 12:12:35 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.