The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #7138

Facter use exec + cat ( or exec = hostname ) when not needed

Added by Michael Scherer about 3 years ago. Updated about 1 year ago.

Status:Merged - Pending ReleaseStart date:04/18/2011
Priority:LowDue date:
Assignee:-% Done:

0%

Category:interface
Target version:2.0.0
Keywords: Affected Facter version:
Branch:https://github.com/puppetlabs/facter/pull/388

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

In various place in the code, facter use Facter::Util::Resolution.exec while this could be avoided, most notably when reading file with cat. I have patches that I can split if needed to use either ruby functions ( for hostname ), or File::open.


Related issues

Related to Facter - Bug #10909: Shell out when reading from /proc Rejected 11/16/2011

History

#1 Updated by Michael Scherer about 3 years ago

  • Target version set to 1.5.9

As I didn’t find how to change the branch field ( maybe I cannot until the issue is reviewed ), I have pushed my 2 changes on https://github.com/mscherer/facter/tree/ticket/1.5.x/7138

#2 Updated by Ben Hughes about 3 years ago

  • Status changed from Unreviewed to Accepted

Thank you for the patches!

The ones in /proc are deliberate I’m afraid due to a linux/ruby bug http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/155744

From https://projects.puppetlabs.com/issues/4466

But I don’t believe the others to be, so thanks.

#3 Updated by Michael Scherer almost 3 years ago

The url speak of a 5 years old bug, and it seems to be fixed by using a newer kernel ( 2.6.13, according to the mail in the next mail of the thread ). I cannot reproduce the problem on my laptop, so there is maybe another reason ( or maybe it was the wrong reference ? ).

#4 Updated by James Turnbull almost 3 years ago

  • Category set to interface
  • Status changed from Accepted to In Topic Branch Pending Review
  • Branch set to https://github.com/mscherer/facter/tree/ticket/1.5.x/7138

#5 Updated by James Turnbull almost 3 years ago

  • Target version changed from 1.5.9 to 1.6.0

#6 Updated by Michael Stahnke almost 3 years ago

  • Target version changed from 1.6.0 to 144

#7 Updated by Josh Cooper over 2 years ago

Calling IO.readlines or IO.read on files in /proc should be avoided. For example, we’re seeing an issue with buggy linux kernels which is triggered by facter trying to read from /proc/cpuinfo https://support.puppetlabs.com/tickets/482

#8 Updated by Ken Barber over 2 years ago

  • Status changed from In Topic Branch Pending Review to Code Insufficient

As Josh says – the use of cat for proc in virtual.rb has to stay. The other fixes are good – cheers. If you can remove that virtual.rb change and submit a pull request from your branch that would be great.

If you have some time – we would really like some test coverage for these cases as well :–). As currently we have no coverage which is not so great:

https://github.com/puppetlabs/facter/blob/1.6.x/spec/unit/operatingsystemrelease_spec.rb

#9 Updated by Jeff Weiss almost 2 years ago

  • Target version deleted (144)

#10 Updated by Andrew Parker over 1 year ago

  • Status changed from Code Insufficient to In Topic Branch Pending Review
  • Target version set to 2.0.0
  • Branch changed from https://github.com/mscherer/facter/tree/ticket/1.5.x/7138 to https://github.com/puppetlabs/facter/pull/266

This has been picked up again as part of performance work. We’ll put this in the 2.x codeline, which should help us get onto newer kernels and avoid the problems mentioned.

#11 Updated by Jeff McCune about 1 year ago

  • Branch changed from https://github.com/puppetlabs/facter/pull/266 to https://github.com/puppetlabs/facter/pull/388

#12 Updated by Jeff McCune about 1 year ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

Merged into master as 8ee42bb.

This should be released in 2.0.0.

Thanks again for the contribution!

-Jeff

Also available in: Atom PDF