From ea02defe18d4d2bb23eaae5c0e81547133965e45 Mon Sep 17 00:00:00 2001 From: Michael Hashizume Date: Mon, 5 Aug 2024 11:23:26 -0700 Subject: [PATCH 1/3] Use Hash#compact Ruby 2.4 introduced the compact method to drop nil values from a Hash. Since Facter recommends Ruby 2.5 and above, we are able to replace more lengthy logic with Hash#compact instead. --- lib/facter/resolvers/partitions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/facter/resolvers/partitions.rb b/lib/facter/resolvers/partitions.rb index 7faaf14656..83db5dbf7c 100644 --- a/lib/facter/resolvers/partitions.rb +++ b/lib/facter/resolvers/partitions.rb @@ -69,7 +69,7 @@ def populate_partitions(partition_name, block_path, blkid_and_lsblk, backing_fil size: Facter::Util::Facts::UnitConverter.bytes_to_human_readable(size_bytes), backing_file: backing_file } info_hash.merge!(populate_from_syscalls(partition_name, blkid_and_lsblk)) - @fact_list[:partitions][partition_name] = info_hash.reject { |_key, value| value.nil? } + @fact_list[:partitions][partition_name] = info_hash.compact end def populate_from_syscalls(partition_name, blkid_and_lsblk) From d6d5bd92d44a5027fedb541a6646ec7da78ca2f8 Mon Sep 17 00:00:00 2001 From: Michael Hashizume Date: Mon, 5 Aug 2024 10:09:22 -0700 Subject: [PATCH 2/3] Prefer lsblk over blkid Prior to this commit, the partitions resolver only used lsblk as a fallback if blkid did not return any information. However, there are many advantages to lsblk. lsblk does not necessarily require root (while blkid does), it returns more information (like partition type GUIDs), and blkid specifically recommends using lsblk in its manpage. This commit updates the partitions resolver to prefer lsblk and use blkid as a fallback. --- lib/facter/resolvers/partitions.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/facter/resolvers/partitions.rb b/lib/facter/resolvers/partitions.rb index 83db5dbf7c..fc70a14485 100644 --- a/lib/facter/resolvers/partitions.rb +++ b/lib/facter/resolvers/partitions.rb @@ -73,9 +73,10 @@ def populate_partitions(partition_name, block_path, blkid_and_lsblk, backing_fil end def populate_from_syscalls(partition_name, blkid_and_lsblk) - part_info = populate_from_blkid(partition_name, blkid_and_lsblk) + # Prefer lsblk over blkid since lsblk does not require root, returns more information, and is recommended by blkid + part_info = populate_from_lsblk(partition_name, blkid_and_lsblk) - return populate_from_lsblk(partition_name, blkid_and_lsblk) if part_info.empty? + return populate_from_blkid(partition_name, blkid_and_lsblk) if part_info.empty? part_info end From 82b980e080ab22e04874ecbdeecaf028e6dc4f77 Mon Sep 17 00:00:00 2001 From: Pat Riehecky Date: Fri, 5 Aug 2022 11:30:13 -0500 Subject: [PATCH 3/3] (FACT-3140) Add partition type GUID This commit adds a new fact to the partitions fact hash, partition type GUID. Co-authored-by: Michael Hashizume --- lib/facter/resolvers/partitions.rb | 46 +++++++++++++++++------- lib/schema/facter.yaml | 3 ++ spec/facter/resolvers/partitions_spec.rb | 46 ++++++++++++++++++++++-- spec/fixtures/lsblk_output | 7 ---- spec/fixtures/lsblk_output_new | 3 ++ spec/fixtures/lsblk_output_old | 3 ++ 6 files changed, 85 insertions(+), 23 deletions(-) delete mode 100644 spec/fixtures/lsblk_output create mode 100644 spec/fixtures/lsblk_output_new create mode 100644 spec/fixtures/lsblk_output_old diff --git a/lib/facter/resolvers/partitions.rb b/lib/facter/resolvers/partitions.rb index fc70a14485..cca2ca365b 100644 --- a/lib/facter/resolvers/partitions.rb +++ b/lib/facter/resolvers/partitions.rb @@ -122,26 +122,46 @@ def execute_and_extract_blkid_info def populate_from_lsblk(partition_name, blkid_and_lsblk) return {} unless available?('lsblk', blkid_and_lsblk) - blkid_and_lsblk[:lsblk] ||= Facter::Core::Execution.execute('lsblk -fp', logger: log) + lsblk_version_raw = Facter::Core::Execution.execute('lsblk --version 2>&1', logger: log) + lsblk_version = lsblk_version_raw.match(/ \d\.\d+/)[0].to_f - part_info = blkid_and_lsblk[:lsblk].match(/#{partition_name}.*/).to_s.split(' ') - return {} if part_info.empty? + # The -p/--paths option was added in lsblk 2.23, return early and fall back to blkid with earlier versions + return {} if lsblk_version < 2.23 - parse_part_info(part_info) - end + blkid_and_lsblk[:lsblk] ||= execute_and_extract_lsblk_info(lsblk_version) - def parse_part_info(part_info) - result = { filesystem: part_info[1] } + partition_data = blkid_and_lsblk[:lsblk][partition_name] + return {} unless partition_data - if part_info.count.eql?(5) - result[:label] = part_info[2] - result[:uuid] = part_info[3] - else - result[:uuid] = part_info[2] - end + filesys = partition_data['FSTYPE'] + uuid = partition_data['UUID'] + label = partition_data['LABEL'] + part_uuid = partition_data['PARTUUID'] + part_label = partition_data['PARTLABEL'] + part_type = partition_data['PARTTYPE'] + + result = { filesystem: filesys, uuid: uuid, label: label, partuuid: part_uuid, partlabel: part_label } + result[:parttype] = part_type if part_type result end + + def execute_and_extract_lsblk_info(lsblk_version) + # lsblk 2.25 added support for GPT partition type GUIDs + stdout = if lsblk_version >= 2.25 + Facter::Core::Execution.execute('lsblk -p -P -o NAME,FSTYPE,UUID,LABEL,PARTUUID,PARTLABEL,PARTTYPE', logger: log) + else + Facter::Core::Execution.execute('lsblk -p -P -o NAME,FSTYPE,LABEL,UUID,PARTUUID,PARTLABEL', logger: log) + end + + output_hash = Hash[*stdout.split(/^(NAME=\S+)/)[1..-1]] + output_hash.transform_keys! { |key| key.delete('NAME=')[1..-2] } + output_hash.each do |key, value| + output_hash[key] = Hash[*value.chomp.rstrip.split(/ ([^= ]+)=/)[1..-1].each { |x| x.delete!('"') }] + end + output_hash.each_value { |value_hash| value_hash.delete_if { |_k, v| v.empty? } } + output_hash.delete_if { |_k, v| v.empty? } + end end end end diff --git a/lib/schema/facter.yaml b/lib/schema/facter.yaml index e8bbf63e93..0850dae370 100644 --- a/lib/schema/facter.yaml +++ b/lib/schema/facter.yaml @@ -1335,6 +1335,9 @@ partitions: backing_file: type: string description: The path to the file backing the partition. + parttype: + type: string + description: The partition type GUID. path: type: string diff --git a/spec/facter/resolvers/partitions_spec.rb b/spec/facter/resolvers/partitions_spec.rb index d97bc22229..6b3209eabf 100644 --- a/spec/facter/resolvers/partitions_spec.rb +++ b/spec/facter/resolvers/partitions_spec.rb @@ -42,7 +42,9 @@ allow(Facter::Core::Execution).to receive(:which) .with('lsblk').and_return('/usr/bin/lsblk') allow(Facter::Core::Execution).to receive(:execute) - .with('lsblk -fp', logger: an_instance_of(Facter::Log)).and_return(load_fixture('lsblk_output').read) + .with('lsblk --version 2>&1', logger: an_instance_of(Facter::Log)).and_return('lsblk from util-linux 2.24') + allow(Facter::Core::Execution).to receive(:execute) + .with('lsblk -p -P -o NAME,FSTYPE,LABEL,UUID,PARTUUID,PARTLABEL', logger: an_instance_of(Facter::Log)).and_return(load_fixture('lsblk_output_old').read) end context 'when block has a device subdir' do @@ -68,12 +70,12 @@ context 'when there is more than one partition' do it 'checks for blkid only once' do resolver.resolve(:partitions) - expect(Facter::Core::Execution).to have_received(:which).with('blkid').once + expect(Facter::Core::Execution).to have_received(:which).with('blkid').at_most(:once) end it 'checks for lsblk only once' do resolver.resolve(:partitions) - expect(Facter::Core::Execution).to have_received(:which).with('lsblk').once + expect(Facter::Core::Execution).to have_received(:which).with('lsblk').at_most(:once) end end @@ -189,5 +191,43 @@ end end end + + context 'when lsblk can read partition types' do + let(:partitions) do + { '/dev/sda1' => { filesystem: 'ext3', label: '/boot', parttype: '21686148-6449-6E6F-744E-656564454649', size: '117.00 KiB', + size_bytes: 119_808, uuid: '88077904-4fd4-476f-9af2-0f7a806ca25e', + partuuid: '00061fe0-01' }, + '/dev/sda2' => { filesystem: 'LVM2_member', parttype: '0fc63daf-8483-4772-8e79-3d69d8477de4', size: '98.25 MiB', + size_bytes: 103_021_056, uuid: 'edi7s0-2WVa-ZBan' } } + end + + let(:sda_subdirs) do + ['/sys/block/sda/queue', + '/sys/block/sda/sda2', + '/sys/block/sda/sda2/stat', + '/sys/block/sda/sda2/dev', + '/sys/block/sda/sda2/uevent', + '/sys/block/sda/sda1'] + end + + before do + allow(File).to receive(:directory?).with("#{sys_block_path}/sda/device").and_return(true) + allow(Dir).to receive(:[]).with("#{sys_block_path}/sda/**/*").and_return(sda_subdirs) + sda_subdirs.each { |subdir| allow(File).to receive(:directory?).with(subdir).and_return(true) } + allow(Facter::Util::FileHelper).to receive(:safe_read) + .with("#{sys_block_path}/sda/sda2/size", '0').and_return('201213') + allow(Facter::Util::FileHelper).to receive(:safe_read) + .with("#{sys_block_path}/sda/sda1/size", '0').and_return('234') + end + + it 'return partitions fact with part_type' do + allow(Facter::Core::Execution).to receive(:execute) + .with('lsblk --version 2>&1', logger: an_instance_of(Facter::Log)).and_return('lsblk from util-linux 2.25') + allow(Facter::Core::Execution).to receive(:execute) + .with('lsblk -p -P -o NAME,FSTYPE,UUID,LABEL,PARTUUID,PARTLABEL,PARTTYPE', logger: an_instance_of(Facter::Log)).and_return(load_fixture('lsblk_output_new').read) + + expect(resolver.resolve(:partitions)).to eq(partitions) + end + end end end diff --git a/spec/fixtures/lsblk_output b/spec/fixtures/lsblk_output deleted file mode 100644 index e8852caea6..0000000000 --- a/spec/fixtures/lsblk_output +++ /dev/null @@ -1,7 +0,0 @@ -NAME FSTYPE LABEL UUID MOUNTPOINT -/dev/sda -|-/dev/sda1 xfs 5ba36204-050f-4ab6 /boot -`-/dev/sda2 LVM2_member edi7s0-2WVa-ZBan - |-/dev/mapper/rhel-root xfs cb455c09-da6b-44e6 / - `-/dev/mapper/rhel-swap swap 0fd5997d-eb82-493d [SWAP] -/dev/sr0 \ No newline at end of file diff --git a/spec/fixtures/lsblk_output_new b/spec/fixtures/lsblk_output_new new file mode 100644 index 0000000000..9aa5587a5c --- /dev/null +++ b/spec/fixtures/lsblk_output_new @@ -0,0 +1,3 @@ +NAME="/dev/sda" FSTYPE="" UUID="" LABEL="" PARTUUID="" PARTLABEL="" PARTTYPE="" +NAME="/dev/sda1" FSTYPE="ext3" UUID="88077904-4fd4-476f-9af2-0f7a806ca25e" LABEL="/boot" PARTUUID="00061fe0-01" PARTLABEL="" PARTTYPE="21686148-6449-6E6F-744E-656564454649" +NAME="/dev/sda2" FSTYPE="LVM2_member" UUID="edi7s0-2WVa-ZBan" LABEL="" PARTUUID="" PARTLABEL="" PARTTYPE="0fc63daf-8483-4772-8e79-3d69d8477de4" \ No newline at end of file diff --git a/spec/fixtures/lsblk_output_old b/spec/fixtures/lsblk_output_old new file mode 100644 index 0000000000..2736dcf2ec --- /dev/null +++ b/spec/fixtures/lsblk_output_old @@ -0,0 +1,3 @@ +NAME="/dev/sda" FSTYPE="" UUID="" LABEL="" PARTUUID="" PARTLABEL="" PARTTYPE="" +NAME="/dev/sda1" FSTYPE="ext3" UUID="88077904-4fd4-476f-9af2-0f7a806ca25e" LABEL="/boot" PARTUUID="00061fe0-01" PARTLABEL="" +NAME="/dev/sda2" FSTYPE="LVM2_member" UUID="edi7s0-2WVa-ZBan" LABEL="" PARTUUID="" PARTLABEL="" \ No newline at end of file