Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issues on nodes without lvm #308

Merged
merged 5 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Contributor

@Ramesh7 Ramesh7 Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The below facts are the custom flat facts, so not sure if these are gonna deprecate as well?
AFAIK the custom facts (irrespective of flat or structured facts) these will not be deprecated and Puppet will allow them to define for long run as well. Only the legacy facts which will get deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Facter code generating fact names dynamically on the fly is problematic: violates coding rules, hard to maintain, even harder to test. That's why we should get rid of them.


* `lvm_vg_*`
* `lvm_vg_*_pvs`
* `lvm_pv_*`

# Contributors

Expand Down
33 changes: 17 additions & 16 deletions lib/facter/lvm_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
rtib marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -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)
rtib marked this conversation as resolved.
Show resolved Hide resolved
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
32 changes: 3 additions & 29 deletions spec/unit/facter/lvm_support_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Loading