Bug #3703

facter may not report the correct macaddress on multi port Macs

Added by Allan Marcus about 2 years ago. Updated about 1 year ago.

Status:Closed Start date:04/29/2010
Priority:High Due date:
Assignee:Rein Henrichs % Done:

0%
Category:library
Target version:1.5.9
Keywords: Affected Facter version:
Branch:ticket/master/3703-macaddress-on-mac
Votes: 0

Description

The macaddress.rb code for facter under darwin doesn’t take into account multi port macs. the ipaddress.rb code does. I copied the code from ipaddress.rb to macaddress.rb and it works.

I also propose changing the check for 10baseT to determine if there is an enternet address to “ether”.

I’m not set up to submit code changes, so I will include the proposed code here.

I appreciate your help.

This is proposed code for facter/macaddress.rb for the darwin section.

Facter.add(:macaddress) do

confine :kernel => :darwin
setcode do
    ether = nil

    ip = nil
    iface = ""
    output = %x{/usr/sbin/netstat -rn}
    if output =~ /^default\s*\S*\s*\S*\s*\S*\s*\S*\s*(\S*).*/
      iface = $1
    else
      warn "Could not find a default route. Using first non-loopback interface"
    end
    if(iface != "")
      output = %x{/sbin/ifconfig #{iface}}
    else
      output = %x{/sbin/ifconfig}
    end

    output.split(/^\S/).each do |str|
        if str =~ /ether/ # we have an ethernet address on this interface
            str =~ /ether (\w\w:\w\w:\w\w:\w\w:\w\w:\w\w)/
            ether = $1
        end
    end

    ether
end

end

History

#1
Updated by James Turnbull about 2 years ago

  • Category set to library
  • Status changed from Unreviewed to Investigating
  • Assignee set to James Turnbull

#2
Updated by Rein Henrichs almost 2 years ago

  • Priority changed from Normal to High

This is currently causing the only failing spec (on my machine) on master. I’m bumping the priority to high since having a green build is a high priority. James, are you still investigating?

#3
Updated by Paul Nasrat almost 2 years ago

Having multiple ifconfig parsing is dumb. ipaddress.rb and macaddress.rb should possibly use Facter::Util::IP routines with a concept of primary interface which is based on default route.

This concept of a primary ipaddress is kinda complex though, also related Issue #3713

#4
Updated by Paul Nasrat almost 2 years ago

Note this isn’t to imply the original patch or poster is dumb, just having duplication which we already have in these facts is less than ideal.

#5
Updated by Rein Henrichs almost 2 years ago

  • Status changed from Investigating to Merged - Pending Release
  • Assignee changed from James Turnbull to Rein Henrichs
  • Branch set to ticket/master/3703-macaddress-on-mac

Fix tested for Darwins 9.8.0, 10.3.0 and 10.6.4 (and presumably working for all the rest) available in my ticket/master/3703-macaddress-on-mac branch and in reductivelabs/testing. Please confirm.

#6
Updated by Rein Henrichs almost 2 years ago

Paul Nasrat wrote:

Having multiple ifconfig parsing is dumb. ipaddress.rb and macaddress.rb should possibly use Facter::Util::IP routines with a concept of primary interface which is based on default route.

This concept of a primary ipaddress is kinda complex though, also related Issue #3713

Paul, I have a “default_interface” method on my Facter::Util::Macaddress::Darwin utility module. Perhaps it can/should be moved into the IP module instead.

I suspect that if we have a way of determining the default/primary interface across BSDen and Linuxen, we should be able to parse ifconfig for all of those systems in one go using:

(?:ether|HWaddr|lladdr)\s+(\w\w:\w\w:\w\w:\w\w:\w\w:\w\w)/

#7
Updated by Paul Nasrat almost 2 years ago

We probably want to put off refactoring this to 1.6.0. If the patch gets it working that’s what we want for 1.5.8

Adding the more idiomatic fixtures dir is great. Note that you seem to be adding .DS_Store around the place please can you fix that.

#8
Updated by Rein Henrichs almost 2 years ago

  • Target version set to 1.5.8

Updated my branch to remove stupid .DS_Store, also merged into testing again. Stupid, stupid OS X.

Full ack to get the fix in 1.5.8 and save refactoring for 1.6.0. Updating target version for this patch to 1.5.8.

#9
Updated by Rein Henrichs almost 2 years ago

  • Status changed from Merged - Pending Release to Ready For Checkin
  • Target version changed from 1.5.8 to 1.6.0

Merged into next

#10
Updated by Rein Henrichs over 1 year ago

  • Status changed from Ready For Checkin to Closed

#11
Updated by James Turnbull about 1 year ago

  • Target version changed from 1.6.0 to 1.5.9

Also available in: Atom PDF