Go ahead and submit this as a pull request against facter, we'll continue the code review using that. Be sure to pay attention to the CONTRIBUTING.md for how to submit the pull request and write the commit messages.
Thanks for all of the work you've put into this! We can't support all of these platforms without people like you taking the time to submit patches against your platforms. On Thu, Sep 20, 2012 at 1:37 AM, Alex Harvey <[email protected]> wrote: > I have made the following changes and tested to confirm it fixes the bug and > tested on AIX 5.3 & 6.1, HP-UX 11.23, and Solaris 2.6 & 7. I have also > created a fairly exhaustive set of RSpec tests I think. > > I am still wondering if people will object to the fact I've removed the > kstat method for Solaris - I think the uptime method is better and generally > more reliable (I've seen kstat broken before a number of times in Solaris). > That said, it's easy to leave the kstat in if people prefer. > > Changes to RSpec tests / new tests - > > # diff -u ./spec/unit/util/uptime_spec.rb.orig > ./spec/unit/util/uptime_spec.rb > --- ./spec/unit/util/uptime_spec.rb.orig Wed Sep 19 22:26:16 2012 > +++ ./spec/unit/util/uptime_spec.rb Thu Sep 20 17:18:31 2012 > @@ -41,36 +41,50 @@ > > describe "nor is 'sysctl kern.boottime'" do > before :each do > - Facter::Util::Uptime.stubs(:uptime_sysctl_cmd).returns("cat > \"#{@nonexistent_file}\"") > + Facter::Util::Uptime.stubs(:uptime_proc_uptime).returns(false) > + Facter::Util::Uptime.stubs(:uptime_sysctl).returns(false) > + Facter.fact(:kernel).stubs(:value).returns('SunOS') > end > > - it "should use 'kstat -p unix:::boot_time'" do > - kstat_output_file = my_fixture('kstat_boot_time') # > unix:0:system_misc:boot_time 1236919980 > - Facter::Util::Uptime.stubs(:uptime_kstat_cmd).returns("cat > \"#{kstat_output_file}\"") > - Time.stubs(:now).returns Time.at(1236923580) #one hour later > - Facter::Util::Uptime.get_uptime_seconds_unix.should == 60 * 60 > - end > + describe "should use '/usr/bin/uptime'" do > + # Note about uptime variations. > + # Solaris (5.6, 5.7, 5.8, 5.9, 5.10 & 5.11) and HP-UX (11.00, > 11.11, 11.23, 11.31) have time right justified at > + # at 8 characters, and two spaces before 'up'. > + # Solaris differs from all other Unices (and Linux) in that the > plural/singular case of minutes/hours/days are > + # written min(s)/hr(s)/day(s) instead of min/mins/hr/hrs etc., > e.g. 1 min(s), 2 min(s) as opposed to > + # 1 min, 2 mins, etc. > + # AIX (4.3.3, 5.2, 5.3, 6.1) differs from other SysV Unices in > that times are padded with a leading 0 in the > + # hour column where necessary, and have AM/PM in uppercase, and > there are three spaces before 'up'. > + # Tru64 (4.0, 5.1) differs from other SysV Unices in that times > are in 24 hour format, and there are no > + # leading spaces. > + # Linux (RHEL 5) differs from the Unices in that seconds are > given in the time, time is right justified at > + # 9 characters, one space before up. > + test_cases = [ > + [' 4:42pm up 1 min(s), 0 users, load average: 0.95, 0.25, > 0.09', 1*60], > + [' 10:14pm up 3 hr(s), 0 users, load average: 0.00, 0.00, > 0.00', 3*60*60 ], > + [' 9:01pm up 1:47, 0 users, load average: 0.00, 0.00, > 0.00', 1*60*60 + 47*60], > + [' 1:56pm up 25 day(s), 2 users, load average: 0.59, 0.56, > 0.50', 25*24*60*60 ], > + [' 2:23pm up 25 day(s), 27 min(s), 2 users, load average: > 0.49, 0.45, 0.46', 25*24*60*60 + 27*60], > + [' 1:07pm up 174 day(s), 16 hr(s), 0 users, load average: > 0.05, 0.04, 0.03', 174*24*60*60 + 16*60*60 ], > + [' 8:59pm up 94 day(s), 3:17, 46 users, load average: > 0.66, 0.67, 0.70', 94*24*60*60 + 3*60*60 + 17*60], > + [' 02:42PM up 1 day, 39 mins, 0 users, load average: 1.49, > 1.74, 1.80', 1*24*60*60 + 39*60], > + [' 02:34PM up 621 days, 18 hrs, 0 users, load average: > 2.67, 2.52, 2.56', 621*24*60*60 + 18*60*60 ], > + [' 02:42PM up 41 days, 2:38, 0 users, load average: > 0.38, 0.70, 0.55', 41*24*60*60 + 2*60*60 + 38*60], > + [' 1:29pm up 485 days, 0 users, load average: 0.00, 0.01, > 0.01', 485*24*60*60 ], > + [' 3:30am up 108 days, 1 hr, 31 users, load average: 0.39, > 0.40, 0.41', 108*24*60*60 + 1*60*60 ], > + ['13:16 up 58 mins, 2 users, load average: 0.00, 0.02, > 0.05', 58*60], > + ['13:18 up 1 hr, 1 user, load average: 0.58, 0.23, 0.14', > 1*60*60 ], > + ['13:19 up 1:01, 1 user, load average: 0.10, 0.26, 0.21', > 1*60*60 + 1*60], > + ['15:56 up 152 days, 17 hrs, 0 users, load average: 0.01, > 0.06, 0.07', 152*24*60*60 + 17*60*60 ], > + [' 13:36:05 up 118 days, 1:15, 1 user, load average: 0.00, > 0.00, 0.00', 118*24*60*60 + 1*60*60 + 15*60] > + ] > > - describe "nor is 'kstat -p unix:::boot_time'" do > - before :each do > - Facter::Util::Uptime.stubs(:uptime_kstat_cmd).returns("cat > \"#{@nonexistent_file}\"") > - end > - > - it "should use 'who -b'" do > - who_b_output_file = my_fixture('who_b_boottime') # Aug 1 14:13 > - Facter::Util::Uptime.stubs(:uptime_who_cmd).returns("cat > \"#{who_b_output_file}\"") > - Time.stubs(:now).returns Time.parse("Aug 01 15:13") # one hour > later > - Facter::Util::Uptime.get_uptime_seconds_unix.should == 60 * 60 > - end > - > - describe "nor is 'who -b'" do > - before :each do > - Facter::Util::Uptime.stubs(:uptime_who_cmd).returns("cat > \"#{@nonexistent_file}\"") > + test_cases.each do |uptime_output, expected| > + it "should return #{expected} for #{uptime_output}" do > + Facter::Util::Resolution.stubs(:exec).with('/usr/bin/uptime > 2>/dev/null').returns(uptime_output) > + Facter.fact(:uptime_seconds).value.should == expected > end > - > - it "should return nil" do > - Facter::Util::Uptime.get_uptime_seconds_unix.should == nil > - end > end > end > end > > # diff -u ./lib/facter/util/uptime.rb.orig ./lib/facter/util/uptime.rb > --- ./lib/facter/util/uptime.rb.orig Wed Sep 19 22:26:11 2012 > +++ ./lib/facter/util/uptime.rb Thu Sep 20 18:28:01 2012 > @@ -4,7 +4,7 @@ > # > module Facter::Util::Uptime > def self.get_uptime_seconds_unix > - uptime_proc_uptime or uptime_sysctl or uptime_kstat or > uptime_who_dash_b > + uptime_proc_uptime or uptime_sysctl or uptime_uptime > end > > def self.get_uptime_seconds_win > @@ -31,18 +31,33 @@ > end > end > > - def self.uptime_kstat > - if output = Facter::Util::Resolution.exec("#{uptime_kstat_cmd} > 2>/dev/null") > - compute_uptime(Time.at(output.chomp.split(/\s/).last.to_i)) > + def self.uptime_uptime > + if output = Facter::Util::Resolution.exec("#{uptime_uptime_cmd} > 2>/dev/null") > + up=0 > + if output =~ /(\d+) day(?:s|\(s\))?,\s+(\d+):(\d+)/ > + # Regexp handles Solaris, AIX, HP-UX, and Tru64. > + # 'day(?:s|\(s\))?' says maybe 'day', 'days', > + # or 'day(s)', and don't set $2. > + up=86400*$1.to_i + 3600*$2.to_i + 60*$3.to_i > + elsif output =~ /(\d+) day(?:s|\(s\))?,\s+(\d+) hr(?:s|\(s\))?,/ > + up=86400*$1.to_i + 3600*$2.to_i > + elsif output =~ /(\d+) day(?:s|\(s\))?,\s+(\d+) min(?:s|\(s\))?,/ > + up=86400*$1.to_i + 60*$2.to_i > + elsif output =~ /(\d+) day(?:s|\(s\))?,/ > + up=86400*$1.to_i > + elsif output =~ /up\s+(\d+):(\d+),/ > + # must anchor to 'up' to avoid matching time of day > + # at beginning of line. > + up=3600*$1.to_i + 60*$2.to_i > + elsif output =~ /(\d+) hr(?:s|\(s\))?,/ > + up=3600*$1.to_i > + elsif output =~ /(\d+) min(?:s|\(s\))?,/ > + up=60*$1.to_i > + end > + up > end > end > > - def self.uptime_who_dash_b > - if output = Facter::Util::Resolution.exec("#{uptime_who_cmd} > 2>/dev/null") > - compute_uptime(Time.parse(output)) > - end > - end > - > def self.compute_uptime(time) > (Time.now - time).to_i > end > @@ -55,11 +70,7 @@ > 'sysctl -n kern.boottime' > end > > - def self.uptime_kstat_cmd > - 'kstat -p unix:::boot_time' > + def self.uptime_uptime_cmd > + "/usr/bin/uptime" > end > - > - def self.uptime_who_cmd > - 'who -b' > - end > end > > If there are no objections I guess I should submit the change. > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To view this discussion on the web visit > https://groups.google.com/d/msg/puppet-dev/-/Y8OHNXCw6HoJ. > > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
