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

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #7138

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

Added by Michael Scherer over 4 years ago. Updated over 2 years 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


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 over 4 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 over 4 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 over 4 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 over 4 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 over 4 years ago

  • Target version changed from 1.5.9 to 1.6.0

#6 Updated by Michael Stahnke about 4 years ago

  • Target version changed from 1.6.0 to 144

#7 Updated by Josh Cooper almost 4 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 almost 4 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 over 3 years ago

  • Target version deleted (144)

#10 Updated by Anonymous about 3 years 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 over 2 years ago

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

#12 Updated by Jeff McCune over 2 years 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