Merge pull request #308 from rtib/GH-307

fix issues on nodes without lvm

README.md

@@ -347,6 +347,13 @@ Logical volume size can be extended, but not reduced -- this is for
 safety, as manual intervention is probably required for data
 migration, etc.
 
+## Deprecation Notice
+
+Some facts reported by this module are being deprecated in favor of upcomming structured facts.  The following facts are being deprecated:
+
+* `lvm_vg_*`
+* `lvm_vg_*_pvs`
+* `lvm_pv_*`
 
 # Contributors
 

lib/facter/lvm_support.rb

@@ -17,24 +17,24 @@
 Facter.add('lvm_vgs') do
   confine lvm_support: true
 
-  vgs = Facter::Core::Execution.execute('vgs -o name --noheadings 2>/dev/null', timeout: 30)
-
-  if vgs.nil?
-    setcode { 0 }
-  else
-    vg_list = vgs.split
-    setcode { vg_list.length }
+  if Facter.value(:lvm_support)
+    vgs = Facter::Core::Execution.execute('vgs -o name --noheadings 2>/dev/null', timeout: 30)
+    vg_list = vgs.split unless vgs.nil?
   end
+
+  setcode { vg_list.length }
 end
 
 # lvm_vg_[0-9]+
 #   VG name by index
 vg_list.each_with_index do |vg, i|
-  Facter.add("lvm_vg_#{i}") { setcode { vg } }
+  Facter.add("lvm_vg_#{i}") do
+    setcode { vg }
+  end
   Facter.add("lvm_vg_#{vg}_pvs") do
     setcode do
-      pvs = Facter::Core::Execution.execute("vgs -o pv_name #{vg} 2>/dev/null", timeout: 30)
       res = nil
+      pvs = Facter::Core::Execution.execute("vgs -o pv_name #{vg} 2>/dev/null", timeout: 30)
       res = pvs.split("\n").grep(%r{^\s+/}).map(&:strip).sort.join(',') unless pvs.nil?
       res
     end
@@ -47,17 +47,18 @@
 Facter.add('lvm_pvs') do
   confine lvm_support: true
 
-  pvs = Facter::Core::Execution.execute('pvs -o name --noheadings 2>/dev/null', timeout: 30)
-  if pvs.nil?
-    setcode { 0 }
-  else
-    pv_list = pvs.split
-    setcode { pv_list.length }
+  if Facter.value(:lvm_support)
+    pvs = Facter::Core::Execution.execute('pvs -o name --noheadings 2>/dev/null', timeout: 30)
+    pv_list = pvs.split unless pvs.nil?
   end
+
+  setcode { pv_list.length }
 end
 
 # lvm_pv_[0-9]+
 #   PV name by index
 pv_list.each_with_index do |pv, i|
-  Facter.add("lvm_pv_#{i}") { setcode { pv } }
+  Facter.add("lvm_pv_#{i}") do
+    setcode { pv }
+  end
 end

spec/unit/facter/lvm_support_spec.rb

@@ -54,25 +54,10 @@
   context 'when there is lvm support' do
     context 'when there are no vgs' do
       it 'is set to 0' do
-        Facter::Core::Execution.stubs(:execute) # All other calls
-        Facter::Core::Execution.expects(:execute).at_least(1).with('vgs -o name --noheadings 2>/dev/null', timeout: 30).returns(nil)
         Facter.fact(:lvm_support).expects(:value).at_least(1).returns(true)
-        Facter.value(:lvm_vgs).should == 0
-      end
-    end
-
-    context 'when there are vgs' do
-      it 'lists vgs' do
         Facter::Core::Execution.stubs(:execute) # All other calls
-        Facter::Core::Execution.expects(:execute).at_least(1).with('vgs -o name --noheadings 2>/dev/null', timeout: 30).returns("  vg0\n  vg1")
-        Facter::Core::Execution.expects(:execute).at_least(1).with('vgs -o pv_name vg0 2>/dev/null', timeout: 30).returns("  PV\n  /dev/pv3\n  /dev/pv2")
-        Facter::Core::Execution.expects(:execute).at_least(1).with('vgs -o pv_name vg1 2>/dev/null', timeout: 30).returns("  PV\n  /dev/pv0")
-        Facter.fact(:lvm_support).expects(:value).at_least(1).returns(true)
-        Facter.value(:lvm_vgs).should == 2
-        Facter.value(:lvm_vg_0).should == 'vg0'
-        Facter.value(:lvm_vg_1).should == 'vg1'
-        Facter.value(:lvm_vg_vg0_pvs).should == '/dev/pv2,/dev/pv3'
-        Facter.value(:lvm_vg_vg1_pvs).should == '/dev/pv0'
+        Facter::Core::Execution.expects(:execute).at_least(0).with('vgs -o name --noheadings 2>/dev/null', timeout: 30).returns(nil)
+        Facter.value(:lvm_vgs).should == 0
       end
     end
   end
@@ -95,21 +80,10 @@
     context 'when there are no pvs' do
       it 'is set to 0' do
         Facter::Core::Execution.stubs('execute') # All other calls
-        Facter::Core::Execution.expects('execute').at_least(1).with('pvs -o name --noheadings 2>/dev/null', timeout: 30).returns(nil)
         Facter.fact(:lvm_support).expects(:value).at_least(1).returns(true)
+        Facter::Core::Execution.expects('execute').at_least(0).with('pvs -o name --noheadings 2>/dev/null', timeout: 30).returns(nil)
         Facter.value(:lvm_pvs).should == 0
       end
     end
-
-    context 'when there are pvs' do
-      it 'lists pvs' do
-        Facter::Core::Execution.stubs('execute') # All other calls
-        Facter::Core::Execution.expects('execute').at_least(1).with('pvs -o name --noheadings 2>/dev/null', timeout: 30).returns("  pv0\n  pv1")
-        Facter.fact(:lvm_support).expects(:value).at_least(1).returns(true)
-        Facter.value(:lvm_pvs).should == 2
-        Facter.value(:lvm_pv_0).should == 'pv0'
-        Facter.value(:lvm_pv_1).should == 'pv1'
-      end
-    end
   end
 end