Please review pull request #176: Split lsb facts due to nasty recursion problem opened by (stschulte)
Description:
If a fact is stored in a file that does not follow the filename convention$factname.rb we may encounter ordering and recursion issues as seen in
bugreport #11511. The concrete example was
- flush clears all facts
- load_all is triggered to reload facts
- inside an .rb file we query the operatingsystem fact directly (say outside a Facter.add block)
- the operatingsystem fact has a suitable resolver for linux which wants to query the lsbdistid fact, which is (apperently) not yet loaded (this might not even be predictable)
- the loader doesnt find a lsbdistid.rb file so it triggers load_all (remember: we are still trying to get a value for operatingsystem)
- the load_all does load other files (like processor.rb) that want to query the architecture fact directly (outside a Facter.add block)
- the architecture fact is dependent on the operatingsystem fact, we are currently trying to resolve -> boom: recursion
This commit implements one possible fix: Split the lsb facts into
differnet files so the loader finds them. We therefore dont have to run
load_all in the middle of a fact resolution.
- Opened: Thu Feb 23 22:25:26 UTC 2012
- Based on: puppetlabs:1.6.x (97d9a1e785152356b04e385f5b650f170782aab5)
- Requested merge: stschulte:ticket/1.6.x/11511 (515fd653572eb1296afc63810dd3e551d985dcd2)
Diff follows:
diff --git a/lib/facter/lsb.rb b/lib/facter/lsb.rb
deleted file mode 100644
index 7cefb5c..0000000
--- a/lib/facter/lsb.rb
+++ /dev/null
@@ -1,39 +0,0 @@
-# Fact: lsb
-#
-# Purpose: Return Linux Standard Base information for the host.
-#
-# Resolution:
-# Uses the lsb_release system command and parses the output with a series of
-# regular expressions.
-#
-# Caveats:
-# Only works on Linux (and the kfreebsd derivative) systems.
-# Requires the lsb_release program, which may not be installed by default.
-# Also is as only as accurate as that program outputs.
-
-## lsb.rb
-## Facts related to Linux Standard Base (LSB)
-
-{ "LSBRelease" => %r{^LSB Version:\t(.*)$},
- "LSBDistId" => %r{^Distributor ID:\t(.*)$},
- "LSBDistRelease" => %r{^Release:\t(.*)$},
- "LSBDistDescription" => %r{^Description:\t(.*)$},
- "LSBDistCodeName" => %r{^Codename:\t(.*)$}
-}.each do |fact, pattern|
- Facter.add(fact) do
- confine :kernel => [ :linux, :"gnu/kfreebsd" ]
- setcode do
- unless defined?(lsbdata) and defined?(lsbtime) and (Time.now.to_i - lsbtime.to_i < 5)
- type = nil
- lsbtime = Time.now
- lsbdata = Facter::Util::Resolution.exec('lsb_release -a 2>/dev/null')
- end
-
- if pattern.match(lsbdata)
- $1
- else
- nil
- end
- end
- end
-end
diff --git a/lib/facter/lsbdistcodename.rb b/lib/facter/lsbdistcodename.rb
new file mode 100644
index 0000000..00b514a
--- /dev/null
+++ b/lib/facter/lsbdistcodename.rb
@@ -0,0 +1,18 @@
+# Fact: lsbdistcodename
+#
+# Purpose: Return Linux Standard Base information for the host.
+#
+# Resolution:
+# Uses the lsb_release system command
+#
+# Caveats:
+# Only works on Linux (and the kfreebsd derivative) systems.
+# Requires the lsb_release program, which may not be installed by default.
+# Also is as only as accurate as that program outputs.
+
+Facter.add(:lsbdistcodename) do
+ confine :kernel => [ :linux, :"gnu/kfreebsd" ]
+ setcode do
+ Facter::Util::Resolution.exec('lsb_release -c -s')
+ end
+end
diff --git a/lib/facter/lsbdistdescription.rb b/lib/facter/lsbdistdescription.rb
new file mode 100644
index 0000000..fb3d760
--- /dev/null
+++ b/lib/facter/lsbdistdescription.rb
@@ -0,0 +1,21 @@
+# Fact: lsbdistdescription
+#
+# Purpose: Return Linux Standard Base information for the host.
+#
+# Resolution:
+# Uses the lsb_release system command
+#
+# Caveats:
+# Only works on Linux (and the kfreebsd derivative) systems.
+# Requires the lsb_release program, which may not be installed by default.
+# Also is as only as accurate as that program outputs.
+
+Facter.add(:lsbdistdescription) do
+ confine :kernel => [ :linux, :"gnu/kfreebsd" ]
+ setcode do
+ if output = Facter::Util::Resolution.exec('lsb_release -d -s')
+ # the output may be quoted (at least it is on gentoo)
+ output.sub(/^"(.*)"$/,'\1')
+ end
+ end
+end
diff --git a/lib/facter/lsbdistid.rb b/lib/facter/lsbdistid.rb
new file mode 100644
index 0000000..61d3ac2
--- /dev/null
+++ b/lib/facter/lsbdistid.rb
@@ -0,0 +1,18 @@
+# Fact: lsbdistid
+#
+# Purpose: Return Linux Standard Base information for the host.
+#
+# Resolution:
+# Uses the lsb_release system command
+#
+# Caveats:
+# Only works on Linux (and the kfreebsd derivative) systems.
+# Requires the lsb_release program, which may not be installed by default.
+# Also is as only as accurate as that program outputs.
+
+Facter.add(:lsbdistid) do
+ confine :kernel => [ :linux, :"gnu/kfreebsd" ]
+ setcode do
+ Facter::Util::Resolution.exec('lsb_release -i -s')
+ end
+end
diff --git a/lib/facter/lsbdistrelease.rb b/lib/facter/lsbdistrelease.rb
new file mode 100644
index 0000000..4c8c736
--- /dev/null
+++ b/lib/facter/lsbdistrelease.rb
@@ -0,0 +1,18 @@
+# Fact: lsbdistrelease
+#
+# Purpose: Return Linux Standard Base information for the host.
+#
+# Resolution:
+# Uses the lsb_release system command
+#
+# Caveats:
+# Only works on Linux (and the kfreebsd derivative) systems.
+# Requires the lsb_release program, which may not be installed by default.
+# Also is as only as accurate as that program outputs.
+
+Facter.add(:lsbdistrelease) do
+ confine :kernel => [ :linux, :"gnu/kfreebsd" ]
+ setcode do
+ Facter::Util::Resolution.exec('lsb_release -r -s')
+ end
+end
diff --git a/lib/facter/lsbrelease.rb b/lib/facter/lsbrelease.rb
new file mode 100644
index 0000000..9b1d546
--- /dev/null
+++ b/lib/facter/lsbrelease.rb
@@ -0,0 +1,18 @@
+# Fact: lsbrelease
+#
+# Purpose: Return Linux Standard Base information for the host.
+#
+# Resolution:
+# Uses the lsb_release system command
+#
+# Caveats:
+# Only works on Linux (and the kfreebsd derivative) systems.
+# Requires the lsb_release program, which may not be installed by default.
+# Also is as only as accurate as that program outputs.
+
+Facter.add(:lsbrelease) do
+ confine :kernel => [ :linux, :"gnu/kfreebsd" ]
+ setcode do
+ Facter::Util::Resolution.exec('lsb_release -v -s')
+ end
+end
diff --git a/lib/facter/operatingsystem.rb b/lib/facter/operatingsystem.rb
index be4243e..550075f 100644
--- a/lib/facter/operatingsystem.rb
+++ b/lib/facter/operatingsystem.rb
@@ -10,7 +10,6 @@
#
# Caveats:
#
-require 'facter/lsb'
Facter.add(:operatingsystem) do
confine :kernel => :sunos
diff --git a/spec/unit/lsbdistcodename_spec.rb b/spec/unit/lsbdistcodename_spec.rb
new file mode 100755
index 0000000..fa012a8
--- /dev/null
+++ b/spec/unit/lsbdistcodename_spec.rb
@@ -0,0 +1,25 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+
+describe "lsbdistcodename fact" do
+
+ [ "Linux", "GNU/kFreeBSD"].each do |kernel|
+ describe "on #{kernel}" do
+ before :each do
+ Facter.fact(:kernel).stubs(:value).returns kernel
+ end
+
+ it "should return the codename through lsb_release -c -s" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -c -s').returns 'n/a'
+ Facter.fact(:lsbdistcodename).value.should == 'n/a'
+ end
+
+ it "should return nil if lsb_release is not installed" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -c -s').returns nil
+ Facter.fact(:lsbdistcodename).value.should be_nil
+ end
+ end
+ end
+
+end
diff --git a/spec/unit/lsbdistdescription_spec.rb b/spec/unit/lsbdistdescription_spec.rb
new file mode 100755
index 0000000..5202e88
--- /dev/null
+++ b/spec/unit/lsbdistdescription_spec.rb
@@ -0,0 +1,25 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+
+describe "lsbdistdescription fact" do
+
+ [ "Linux", "GNU/kFreeBSD"].each do |kernel|
+ describe "on #{kernel}" do
+ before :each do
+ Facter.fact(:kernel).stubs(:value).returns kernel
+ end
+
+ it "should return the description through lsb_release -d -s" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -d -s').returns '"Gentoo Base System release 2.1"'
+ Facter.fact(:lsbdistdescription).value.should == 'Gentoo Base System release 2.1'
+ end
+
+ it "should return nil if lsb_release is not installed" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -d -s').returns nil
+ Facter.fact(:lsbdistdescription).value.should be_nil
+ end
+ end
+ end
+
+end
diff --git a/spec/unit/lsbdistid_spec.rb b/spec/unit/lsbdistid_spec.rb
new file mode 100755
index 0000000..ae208a1
--- /dev/null
+++ b/spec/unit/lsbdistid_spec.rb
@@ -0,0 +1,25 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+
+describe "lsbdistid fact" do
+
+ [ "Linux", "GNU/kFreeBSD"].each do |kernel|
+ describe "on #{kernel}" do
+ before :each do
+ Facter.fact(:kernel).stubs(:value).returns kernel
+ end
+
+ it "should return the id through lsb_release -i -s" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -i -s').returns 'Gentoo'
+ Facter.fact(:lsbdistid).value.should == 'Gentoo'
+ end
+
+ it "should return nil if lsb_release is not installed" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -i -s').returns nil
+ Facter.fact(:lsbdistid).value.should be_nil
+ end
+ end
+ end
+
+end
diff --git a/spec/unit/lsbdistrelease_spec.rb b/spec/unit/lsbdistrelease_spec.rb
new file mode 100755
index 0000000..5a9803d
--- /dev/null
+++ b/spec/unit/lsbdistrelease_spec.rb
@@ -0,0 +1,25 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+
+describe "lsbdistrelease fact" do
+
+ [ "Linux", "GNU/kFreeBSD"].each do |kernel|
+ describe "on #{kernel}" do
+ before :each do
+ Facter.fact(:kernel).stubs(:value).returns kernel
+ end
+
+ it "should return the release through lsb_release -r -s" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -r -s').returns '2.1'
+ Facter.fact(:lsbdistrelease).value.should == '2.1'
+ end
+
+ it "should return nil if lsb_release is not installed" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -r -s').returns nil
+ Facter.fact(:lsbdistrelease).value.should be_nil
+ end
+ end
+ end
+
+end
diff --git a/spec/unit/lsbrelease.rb b/spec/unit/lsbrelease.rb
new file mode 100755
index 0000000..1406da6
--- /dev/null
+++ b/spec/unit/lsbrelease.rb
@@ -0,0 +1,25 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+
+describe "lsbrelease fact" do
+
+ [ "Linux", "GNU/kFreeBSD"].each do |kernel|
+ describe "on #{kernel}" do
+ before :each do
+ Facter.fact(:kernel).stubs(:value).returns kernel
+ end
+
+ it "should return the release through lsb_release -v -s" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -v -s').returns 'n/a'
+ Facter.fact(:lsbrelease).value.should == 'n/a'
+ end
+
+ it "should return nil if lsb_release is not installed" do
+ Facter::Util::Resolution.stubs(:exec).with('lsb_release -v -s').returns nil
+ Facter.fact(:lsbrelease).value.should be_nil
+ end
+ end
+ end
+
+end
-- 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.
