diff options
| -rw-r--r-- | CHANGELOG | 5 | ||||
| -rw-r--r-- | lib/facter/kernelrelease.rb | 2 | ||||
| -rw-r--r-- | lib/facter/util/collection.rb | 28 | ||||
| -rw-r--r-- | lib/facter/util/fact.rb | 14 | ||||
| -rw-r--r-- | lib/facter/util/resolution.rb | 21 | ||||
| -rwxr-xr-x | spec/unit/util/collection.rb | 40 | ||||
| -rwxr-xr-x | spec/unit/util/fact.rb | 7 | ||||
| -rwxr-xr-x | spec/unit/util/resolution.rb | 16 |
8 files changed, 107 insertions, 26 deletions
@@ -1,4 +1,9 @@ ?: + Set the timeout on the AIX kernelrelease fact to 5 seconds. + + Refactored so each fact resolution can specify a separate timeout, + but the default is still 0.5. + Refactered ipmess.rb and util/ip.rb to support separate *BSD logic for *BSD aliased interfaces. diff --git a/lib/facter/kernelrelease.rb b/lib/facter/kernelrelease.rb index 8c6ac57..652fd6a 100644 --- a/lib/facter/kernelrelease.rb +++ b/lib/facter/kernelrelease.rb @@ -2,7 +2,7 @@ setcode 'uname -r' end - Facter.add(:kernelrelease) do + Facter.add(:kernelrelease, :timeout => 5) do confine :kernel => :aix setcode 'oslevel -s' end diff --git a/lib/facter/util/collection.rb b/lib/facter/util/collection.rb index edc65c7..3f8e0f8 100644 --- a/lib/facter/util/collection.rb +++ b/lib/facter/util/collection.rb @@ -17,11 +17,35 @@ class Facter::Util::Collection name = canonize(name) unless fact = @facts[name] - fact = Facter::Util::Fact.new(name, options) + fact = Facter::Util::Fact.new(name) + @facts[name] = fact end - fact.add(&block) if block + # Set any fact-appropriate options. + options.each do |opt, value| + method = opt.to_s + "=" + if fact.respond_to?(method) + fact.send(method, value) + options.delete(opt) + end + end + + if block + resolve = fact.add(&block) + # Set any resolve-appropriate options + options.each do |opt, value| + method = opt.to_s + "=" + if resolve.respond_to?(method) + resolve.send(method, value) + options.delete(opt) + end + end + end + + unless options.empty? + raise ArgumentError, "Invalid facter option(s) %s" % options.keys.collect { |k| k.to_s }.join(",") + end return fact end diff --git a/lib/facter/util/fact.rb b/lib/facter/util/fact.rb index 0b2d5b9..c96ca3b 100644 --- a/lib/facter/util/fact.rb +++ b/lib/facter/util/fact.rb @@ -1,9 +1,9 @@ require 'facter' require 'facter/util/resolution' -require 'timeout' - class Facter::Util::Fact + TIMEOUT = 5 + attr_accessor :name, :searching, :ldapname # Create a new fact, with no resolution mechanisms. @@ -82,15 +82,7 @@ class Facter::Util::Fact next unless resolve.suitable? foundsuits = true - tmp = nil - begin - Timeout.timeout(0.5) do - tmp = resolve.value - end - rescue Timeout::Error => detail - warn "Timed out seeking value for %s" % self.name - next - end + tmp = resolve.value break tmp unless tmp.nil? or tmp == "" } diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb index b6aae77..1feda3f 100644 --- a/lib/facter/util/resolution.rb +++ b/lib/facter/util/resolution.rb @@ -5,8 +5,10 @@ # suitable. require 'facter/util/confine' +require 'timeout' + class Facter::Util::Resolution - attr_accessor :interpreter, :code, :name + attr_accessor :interpreter, :code, :name, :timeout def self.have_which if @have_which.nil? @@ -60,6 +62,7 @@ class Facter::Util::Resolution @name = name @confines = [] @value = nil + @timeout = 0.5 end # Return the number of confines. @@ -95,10 +98,18 @@ class Facter::Util::Resolution # How we get a value for our resolution mechanism. def value - if @code.is_a?(Proc) - result = @code.call() - else - result = Facter::Util::Resolution.exec(@code,@interpreter) + result = nil + begin + Timeout.timeout(timeout) do + if @code.is_a?(Proc) + result = @code.call() + else + result = Facter::Util::Resolution.exec(@code,@interpreter) + end + end + rescue Timeout::Error => detail + warn "Timed out seeking value for %s" % self.name + return nil end return nil if result == "" diff --git a/spec/unit/util/collection.rb b/spec/unit/util/collection.rb index 81e5d98..7585a52 100755 --- a/spec/unit/util/collection.rb +++ b/spec/unit/util/collection.rb @@ -44,6 +44,46 @@ describe Facter::Util::Collection do @coll.add(:myname) end + it "should accept options" do + @coll.add(:myname, :ldapname => "whatever") { } + end + + it "should set any appropriate options on the fact instances" do + # Use a real fact instance, because we're using respond_to? + fact = Facter::Util::Fact.new(:myname) + fact.expects(:ldapname=).with("testing") + Facter::Util::Fact.expects(:new).with(:myname).returns fact + + @coll.add(:myname, :ldapname => "testing") + end + + it "should set appropriate options on the resolution instance" do + fact = Facter::Util::Fact.new(:myname) + Facter::Util::Fact.expects(:new).with(:myname).returns fact + + resolve = Facter::Util::Resolution.new(:myname) {} + fact.expects(:add).returns resolve + + @coll.add(:myname, :timeout => "myval") {} + end + + it "should not pass fact-specific options to resolutions" do + fact = Facter::Util::Fact.new(:myname) + Facter::Util::Fact.expects(:new).with(:myname).returns fact + + resolve = Facter::Util::Resolution.new(:myname) {} + fact.expects(:add).returns resolve + + fact.expects(:ldapname=).with("foo") + resolve.expects(:timeout=).with("myval") + + @coll.add(:myname, :timeout => "myval", :ldapname => "foo") {} + end + + it "should fail if invalid options are provided" do + lambda { @coll.add(:myname, :foo => :bar) }.should raise_error(ArgumentError) + end + describe "and a block is provided" do it "should use the block to add a resolution to the fact" do fact = mock 'fact' diff --git a/spec/unit/util/fact.rb b/spec/unit/util/fact.rb index cee70b9..1652032 100755 --- a/spec/unit/util/fact.rb +++ b/spec/unit/util/fact.rb @@ -121,13 +121,6 @@ describe Facter::Util::Fact do @fact.value.should be_nil end - - it "should timeout after 0.5 seconds" do - @fact.expects(:warn) - @fact.add { setcode { sleep 2; raise "This is a test" } } - - @fact.value.should be_nil - end end it "should have a method for flushing the cached fact" do diff --git a/spec/unit/util/resolution.rb b/spec/unit/util/resolution.rb index 493ee3a..e546713 100755 --- a/spec/unit/util/resolution.rb +++ b/spec/unit/util/resolution.rb @@ -17,6 +17,14 @@ describe Facter::Util::Resolution do Facter::Util::Resolution.new("yay").should respond_to(:setcode) end + it "should support a timeout value" do + Facter::Util::Resolution.new("yay").should respond_to(:timeout=) + end + + it "should default to a timeout of 0.5 seconds" do + Facter::Util::Resolution.new("yay").timeout.should == 0.5 + end + describe "when setting the code" do before do @resolve = Facter::Util::Resolution.new("yay") @@ -82,6 +90,14 @@ describe Facter::Util::Resolution do @resolve.setcode { "" } @resolve.value.should be_nil end + + it "should timeout after the provided timeout" do + @resolve.expects(:warn) + @resolve.timeout = 0.1 + @resolve.setcode { sleep 2; raise "This is a test" } + + @resolve.value.should be_nil + end end end |