From f19875de1a193c33d762be60144929eed45c1d0d Mon Sep 17 00:00:00 2001 From: Andrei Filipovici Date: Fri, 16 Oct 2020 17:59:42 +0300 Subject: [PATCH] (FACT-2829) Fixed partitions naming and convert root partition partuuid to path --- lib/facter/resolvers/mountpoints_resolver.rb | 71 +++++++---- lib/facter/resolvers/partitions.rb | 117 +++++++++--------- .../resolvers/mountpoints_resolver_spec.rb | 38 ++++++ spec/facter/resolvers/partitions_spec.rb | 74 ++++++----- spec/fixtures/blkid_output_root_has_partuuid | 6 + spec/fixtures/cmdline_root_device_partuuid | 1 + 6 files changed, 198 insertions(+), 109 deletions(-) create mode 100644 spec/fixtures/blkid_output_root_has_partuuid create mode 100644 spec/fixtures/cmdline_root_device_partuuid diff --git a/lib/facter/resolvers/mountpoints_resolver.rb b/lib/facter/resolvers/mountpoints_resolver.rb index 1af18e9c94..b9839e73d7 100644 --- a/lib/facter/resolvers/mountpoints_resolver.rb +++ b/lib/facter/resolvers/mountpoints_resolver.rb @@ -16,7 +16,21 @@ def post_resolve(fact_name) def root_device cmdline = Util::FileHelper.safe_read('/proc/cmdline') match = cmdline.match(/root=([^\s]+)/) - match&.captures&.first + root = match&.captures&.first + + if !root.nil? && root.include?('=') + # We are dealing with the PARTUUID of the partition. Need to extract partition path. + root_partition_path = convert_partuuid_to_path(root) + root = root_partition_path unless root_partition_path.nil? + end + root + end + + def convert_partuuid_to_path(root) + blkid_content = Facter::Core::Execution.execute('blkid', logger: log) + partuuid = root.split('=').last + match = blkid_content.match(/(.+)#{partuuid}/) + match&.captures&.first&.split(':')&.first end def compute_device(device) @@ -26,36 +40,47 @@ def compute_device(device) device end - # rubocop:disable Metrics/MethodLength, Metrics/AbcSize def read_mounts(fact_name) mounts = [] - FilesystemHelper.read_mountpoints.each do |fs| - device = compute_device(fs.name) - filesystem = fs.mount_type - path = fs.mount_point - options = fs.options.split(',').map(&:strip) - - next if path =~ %r{^/(proc|sys)} && filesystem != 'tmpfs' || filesystem == 'autofs' + FilesystemHelper.read_mountpoints.each do |file_system| + mount = {} + get_mount_data(file_system, mount) - stats = FilesystemHelper.read_mountpoint_stats(path) - size_bytes = stats.bytes_total.abs - available_bytes = stats.bytes_available.abs + next if mount[:path] =~ %r{^/(proc|sys)} && mount[:filesystem] != 'tmpfs' || mount[:filesystem] == 'autofs' - used_bytes = stats.bytes_used.abs - total_bytes = used_bytes + available_bytes - capacity = FilesystemHelper.compute_capacity(used_bytes, total_bytes) - - size = Facter::FactsUtils::UnitConverter.bytes_to_human_readable(size_bytes) - available = Facter::FactsUtils::UnitConverter.bytes_to_human_readable(available_bytes) - used = Facter::FactsUtils::UnitConverter.bytes_to_human_readable(used_bytes) - - mounts << Hash[FilesystemHelper::MOUNT_KEYS.zip(FilesystemHelper::MOUNT_KEYS - .map { |v| binding.local_variable_get(v) })] + get_mount_sizes(mount) + mounts << mount end + @fact_list[:mountpoints] = mounts @fact_list[fact_name] end - # rubocop:enable Metrics/MethodLength, Metrics/AbcSize + + def get_mount_data(file_system, mount) + mount[:device] = compute_device(file_system.name) + mount[:filesystem] = file_system.mount_type + mount[:path] = file_system.mount_point + mount[:options] = file_system.options.split(',').map(&:strip) + end + + def get_mount_sizes(mount) + stats = FilesystemHelper.read_mountpoint_stats(mount[:path]) + + get_bytes_data(mount, stats) + + total_bytes = mount[:used_bytes] + mount[:available_bytes] + mount[:capacity] = FilesystemHelper.compute_capacity(mount[:used_bytes], total_bytes) + + mount[:size] = Facter::FactsUtils::UnitConverter.bytes_to_human_readable(mount[:size_bytes]) + mount[:available] = Facter::FactsUtils::UnitConverter.bytes_to_human_readable(mount[:available_bytes]) + mount[:used] = Facter::FactsUtils::UnitConverter.bytes_to_human_readable(mount[:used_bytes]) + end + + def get_bytes_data(mount, stats) + mount[:size_bytes] = stats.bytes_total.abs + mount[:available_bytes] = stats.bytes_available.abs + mount[:used_bytes] = stats.bytes_used.abs + end end end end diff --git a/lib/facter/resolvers/partitions.rb b/lib/facter/resolvers/partitions.rb index eaa25acf2d..1a4de94ee8 100644 --- a/lib/facter/resolvers/partitions.rb +++ b/lib/facter/resolvers/partitions.rb @@ -15,98 +15,122 @@ def post_resolve(fact_name) end def read_partitions(fact_name) - @fact_list[:partitions] = {} - return {} unless File.readable?(BLOCK_PATH) + return unless File.readable?(BLOCK_PATH) block_devices = Dir.entries(BLOCK_PATH).reject { |dir| dir =~ /^\.+/ } + @fact_list[:partitions] = {} unless block_devices.empty? + blkid_and_lsblk = {} + block_devices.each do |block_device| block_path = "#{BLOCK_PATH}/#{block_device}" if File.directory?("#{block_path}/device") - extract_from_device(block_path) + extract_from_device(block_path, blkid_and_lsblk) elsif File.directory?("#{block_path}/dm") - extract_from_dm(block_path) + extract_from_dm(block_path, block_device, blkid_and_lsblk) elsif File.directory?("#{block_path}/loop") - extract_from_loop(block_path) + extract_from_loop(block_path, block_device, blkid_and_lsblk) end end + @fact_list[fact_name] end - def extract_from_device(block_path) + def extract_from_device(block_path, blkid_and_lsblk) subdirs = browse_subdirectories(block_path) subdirs.each do |subdir| name = "/dev/#{subdir.split('/').last}" - populate_partitions(name, subdir) + populate_partitions(name, subdir, blkid_and_lsblk) end end - def extract_from_dm(block_path) + def extract_from_dm(block_path, block_device, blkid_and_lsblk) map_name = Util::FileHelper.safe_read("#{block_path}/dm/name").chomp if map_name.empty? - populate_partitions("/dev#{block_path}", block_path) + populate_partitions("/dev/#{block_device}", block_path, blkid_and_lsblk) else - populate_partitions("/dev/mapper/#{map_name}", block_path) + populate_partitions("/dev/mapper/#{map_name}", block_path, blkid_and_lsblk) end end - def extract_from_loop(block_path) + def extract_from_loop(block_path, block_device, blkid_and_lsblk) backing_file = Util::FileHelper.safe_read("#{block_path}/loop/backing_file").chomp if backing_file.empty? - populate_partitions("/dev#{block_path}", block_path) + populate_partitions("/dev/#{block_device}", block_path, blkid_and_lsblk) else - populate_partitions("/dev#{block_path}", block_path, backing_file) + populate_partitions("/dev/#{block_device}", block_path, blkid_and_lsblk, backing_file) end end - def populate_partitions(partition_name, block_path, backing_file = nil) - @fact_list[:partitions][partition_name] = {} + def populate_partitions(partition_name, block_path, blkid_and_lsblk, backing_file = nil) size_bytes = Util::FileHelper.safe_read("#{block_path}/size", '0') .chomp.to_i * BLOCK_SIZE info_hash = { size_bytes: size_bytes, size: Facter::FactsUtils::UnitConverter.bytes_to_human_readable(size_bytes), backing_file: backing_file } - info_hash.merge!(populate_from_syscalls(partition_name)) + info_hash.merge!(populate_from_syscalls(partition_name, blkid_and_lsblk)) @fact_list[:partitions][partition_name] = info_hash.reject { |_key, value| value.nil? } end - def populate_from_syscalls(partition_name) - part_info = populate_from_blkid(partition_name) + def populate_from_syscalls(partition_name, blkid_and_lsblk) + part_info = populate_from_blkid(partition_name, blkid_and_lsblk) - return pupulate_from_lsblk(partition_name) if part_info.empty? + return populate_from_lsblk(partition_name, blkid_and_lsblk) if part_info.empty? part_info end - def populate_from_blkid(partition_name) - return {} unless blkid_command? + def browse_subdirectories(path) + dirs = Dir[File.join(path, '**', '*')].select { |p| File.directory? p } + dirs.select { |subdir| subdir.split('/').last.include?(path.split('/').last) }.reject(&:nil?) + end + + def populate_from_blkid(partition_name, blkid_and_lsblk) + return {} unless available?('blkid', blkid_and_lsblk) + + blkid_and_lsblk[:blkid] ||= execute_and_extract_blkid_info - @blkid_content ||= execute_and_extract_blkid_info - return {} unless @blkid_content[partition_name] + partition_data = blkid_and_lsblk[:blkid][partition_name] + return {} unless partition_data + + filesys = partition_data['TYPE'] + uuid = partition_data['UUID'] + label = partition_data['LABEL'] + part_uuid = partition_data['PARTUUID'] + part_label = partition_data['PARTLABEL'] - filesys = @blkid_content[partition_name]['TYPE'] - uuid = @blkid_content[partition_name]['UUID'] - label = @blkid_content[partition_name]['LABEL'] - part_uuid = @blkid_content[partition_name]['PARTUUID'] - part_label = @blkid_content[partition_name]['PARTLABEL'] { filesystem: filesys, uuid: uuid, label: label, partuuid: part_uuid, partlabel: part_label } end - def blkid_command? - return @blkid_exists unless @blkid_exists.nil? + def available?(command, blkid_and_lsblk) + command_exists_key = command == 'blkid' ? :blkid_exists : :lsblk_exists + + return blkid_and_lsblk[command_exists_key] unless blkid_and_lsblk[command_exists_key].nil? - output = Facter::Core::Execution.execute('which blkid', logger: log) + output = Facter::Core::Execution.execute("which #{command}", logger: log) - @blkid_exists = !output.empty? + blkid_and_lsblk[:command_exists_key] = !output.empty? end - def pupulate_from_lsblk(partition_name) - return {} unless lsblk_command? + def execute_and_extract_blkid_info + stdout = Facter::Core::Execution.execute('blkid', logger: log) + output_hash = Hash[*stdout.split(/^([^:]+):/)[1..-1]] + output_hash.each do |key, value| + output_hash[key] = Hash[*value.delete('"').chomp.rstrip.split(/ ([^= ]+)=/)[1..-1]] + end + end - @lsblk_content ||= Facter::Core::Execution.execute('lsblk -fp', logger: log) + def populate_from_lsblk(partition_name, blkid_and_lsblk) + return {} unless available?('lsblk', blkid_and_lsblk) - part_info = @lsblk_content.match(/#{partition_name}.*/).to_s.split(' ') + blkid_and_lsblk[:lsblk] ||= Facter::Core::Execution.execute('lsblk -fp', logger: log) + + part_info = blkid_and_lsblk[:lsblk].match(/#{partition_name}.*/).to_s.split(' ') return {} if part_info.empty? + parse_part_info(part_info) + end + + def parse_part_info(part_info) result = { filesystem: part_info[1] } if part_info.count.eql?(5) @@ -118,27 +142,6 @@ def pupulate_from_lsblk(partition_name) result end - - def lsblk_command? - return @lsblk_exists unless @lsblk_exists.nil? - - output = Facter::Core::Execution.execute('which lsblk', logger: log) - - @lsblk_exists = !output.empty? - end - - def execute_and_extract_blkid_info - stdout = Facter::Core::Execution.execute('blkid', logger: log) - output_hash = Hash[*stdout.split(/^([^:]+):/)[1..-1]] - output_hash.each do |key, value| - output_hash[key] = Hash[*value.delete('"').chomp.rstrip.split(/ ([^= ]+)=/)[1..-1]] - end - end - - def browse_subdirectories(path) - dirs = Dir[File.join(path, '**', '*')].select { |p| File.directory? p } - dirs.select { |subdir| subdir.split('/').last.include?(path.split('/').last) }.reject(&:nil?) - end end end end diff --git a/spec/facter/resolvers/mountpoints_resolver_spec.rb b/spec/facter/resolvers/mountpoints_resolver_spec.rb index 400af3ad6c..3ac5d7afe7 100644 --- a/spec/facter/resolvers/mountpoints_resolver_spec.rb +++ b/spec/facter/resolvers/mountpoints_resolver_spec.rb @@ -59,6 +59,7 @@ allow(Facter::FilesystemHelper).to receive(:read_mountpoints).and_return(ignored_mounts) result = Facter::Resolvers::Mountpoints.resolve(:mountpoints) + expect(result).to be_empty end @@ -69,6 +70,7 @@ it 'looks up the actual device if /dev/root' do result = Facter::Resolvers::Mountpoints.resolve(:mountpoints) + expect(result.first[:device]).to eq('/dev/mmcblk0p2') end @@ -81,9 +83,45 @@ it 'returns device as nil' do result = Facter::Resolvers::Mountpoints.resolve(:mountpoints) + expect(result.first[:device]).to be(nil) end end + + context 'when root device has partuuid' do + let(:log) { instance_spy(Facter::Log) } + + before do + allow(Facter::Util::FileHelper).to receive(:safe_read) + .with('/proc/cmdline') + .and_return(load_fixture('cmdline_root_device_partuuid').read) + allow(Facter::Core::Execution).to receive(:execute) + .with('blkid', logger: log) + .and_return(load_fixture('blkid_output_root_has_partuuid').read) + Facter::Resolvers::Mountpoints.instance_variable_set(:@log, log) + end + + it 'returns the path instead of the PARTUUID' do + result = Facter::Resolvers::Mountpoints.resolve(:mountpoints) + + expect(result.first[:device]).to eq('/dev/xvda1') + end + + context 'when blkid command is not available' do + before do + allow(Facter::Core::Execution).to receive(:execute) + .with('blkid', logger: log) + .and_return('blkid: command not found') + Facter::Resolvers::Mountpoints.instance_variable_set(:@log, log) + end + + it 'returns the partition path as PARTUUID' do + result = Facter::Resolvers::Mountpoints.resolve(:mountpoints) + + expect(result.first[:device]).to eq('PARTUUID=a2f52878-01') + end + end + end end describe 'resolver key not found' do diff --git a/spec/facter/resolvers/partitions_spec.rb b/spec/facter/resolvers/partitions_spec.rb index 72e0bf1874..22b76abd8a 100644 --- a/spec/facter/resolvers/partitions_spec.rb +++ b/spec/facter/resolvers/partitions_spec.rb @@ -5,9 +5,11 @@ let(:sys_block_path) { '/sys/block' } let(:sys_block_subdirs) { ['.', '..', 'sda'] } + let(:logger) { instance_spy(Facter::Log) } - after do + before do Facter::Resolvers::Partitions.invalidate_cache + Facter::Resolvers::Partitions.instance_variable_set(:@log, logger) end context 'when /sys/block is not readable' do @@ -16,7 +18,18 @@ end it 'returns empty hash' do - expect(resolver.resolve(:partitions)).to eq({}) + expect(resolver.resolve(:partitions)).to be(nil) + end + end + + context 'when /sys/block has no entries' do + before do + allow(File).to receive(:readable?).with(sys_block_path).and_return(true) + allow(Dir).to receive(:entries).with(sys_block_path).and_return(['.', '..']) + end + + it 'returns empty hash' do + expect(resolver.resolve(:partitions)).to be(nil) end end @@ -24,6 +37,14 @@ before do allow(File).to receive(:readable?).with(sys_block_path).and_return(true) allow(Dir).to receive(:entries).with(sys_block_path).and_return(sys_block_subdirs) + allow(Facter::Core::Execution).to receive(:execute) + .with('which blkid', logger: logger).and_return('/usr/bin/blkid') + allow(Facter::Core::Execution).to receive(:execute) + .with('blkid', logger: logger).and_return(load_fixture('blkid_output').read) + allow(Facter::Core::Execution).to receive(:execute) + .with('which lsblk', logger: logger).and_return('/usr/bin/lsblk') + allow(Facter::Core::Execution).to receive(:execute) + .with('lsblk -fp', logger: logger).and_return(load_fixture('lsblk_output').read) end context 'when block has a device subdir' do @@ -44,14 +65,6 @@ .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') - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'which blkid') - .and_return('/usr/bin/blkid') - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'blkid') - .and_return(load_fixture('blkid_output').read) - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'which lsblk') - .and_return('/usr/bin/lsblk') - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'lsblk -fp') - .and_return(load_fixture('lsblk_output').read) end context 'when device size files are readable' do @@ -75,13 +88,17 @@ '/dev/sda2' => { filesystem: 'LVM2_member', size: '0 bytes', size_bytes: 0, uuid: 'edi7s0-2WVa-ZBan' } } end - it 'return partitions fact with 0 sizes' do + before do allow(Facter::Util::FileHelper).to receive(:safe_read) .with("#{sys_block_path}/sda/sda2/size", '0').and_return('') allow(Facter::Util::FileHelper).to receive(:safe_read) .with("#{sys_block_path}/sda/sda1/size", '0').and_return('') + end - expect(resolver.resolve(:partitions)).to eq(partitions_with_no_sizes) + it 'return partitions fact with 0 sizes' do + result = resolver.resolve(:partitions) + + expect(result).to eq(partitions_with_no_sizes) end end end @@ -94,10 +111,6 @@ .with("#{sys_block_path}/sda/dm/name").and_return('VolGroup00-LogVol00') allow(Facter::Util::FileHelper).to receive(:safe_read) .with("#{sys_block_path}/sda/size", '0').and_return('201213') - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'which blkid') - .and_return('/usr/bin/blkid') - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'blkid') - .and_return(load_fixture('blkid_output').read) end context 'when device name file is readable' do @@ -113,14 +126,17 @@ context 'when device name file is not readable' do let(:partitions) do - { '/dev/sys/block/sda' => { size: '98.25 MiB', size_bytes: 103_021_056 } } + { '/dev/sda' => { size: '98.25 MiB', size_bytes: 103_021_056 } } + end + + before do + allow(Facter::Util::FileHelper).to receive(:safe_read).with("#{sys_block_path}/sda/dm/name").and_return('') end it 'return partitions fact with no device name' do - allow(Facter::Util::FileHelper).to receive(:safe_read) - .with("#{sys_block_path}/sda/dm/name").and_return('') + result = resolver.resolve(:partitions) - expect(resolver.resolve(:partitions)).to eq(partitions) + expect(result).to eq(partitions) end end end @@ -134,32 +150,32 @@ .with("#{sys_block_path}/sda/loop/backing_file").and_return('some_path') allow(Facter::Util::FileHelper).to receive(:safe_read) .with("#{sys_block_path}/sda/size", '0').and_return('201213') - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'which blkid') - .and_return('/usr/bin/blkid') - allow(Open3).to receive(:popen3).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'blkid') - .and_return(load_fixture('blkid_output').read) end context 'when backing_file is readable' do let(:partitions) do - { '/dev/sys/block/sda' => { backing_file: 'some_path', size: '98.25 MiB', size_bytes: 103_021_056 } } + { '/dev/sda' => { backing_file: 'some_path', size: '98.25 MiB', size_bytes: 103_021_056 } } end - it 'return partitions fact' do + it 'returns partitions fact' do expect(resolver.resolve(:partitions)).to eq(partitions) end end context 'when backing_file is not readable' do let(:partitions) do - { '/dev/sys/block/sda' => { size: '98.25 MiB', size_bytes: 103_021_056 } } + { '/dev/sda' => { size: '98.25 MiB', size_bytes: 103_021_056 } } end - it 'return partitions fact' do + before do allow(Facter::Util::FileHelper).to receive(:safe_read) .with("#{sys_block_path}/sda/loop/backing_file").and_return('') + end - expect(resolver.resolve(:partitions)).to eq(partitions) + it 'returns partitions fact' do + result = resolver.resolve(:partitions) + + expect(result).to eq(partitions) end end end diff --git a/spec/fixtures/blkid_output_root_has_partuuid b/spec/fixtures/blkid_output_root_has_partuuid new file mode 100644 index 0000000000..832af8c200 --- /dev/null +++ b/spec/fixtures/blkid_output_root_has_partuuid @@ -0,0 +1,6 @@ +/dev/xvda1: LABEL="cloudimg-rootfs" UUID="f387d281-b162-4d60-84b5-e7e94687e6b8" TYPE="ext4" PARTUUID="a2f52878-01" +/dev/loop0: TYPE="squashfs" +/dev/loop1: TYPE="squashfs" +/dev/loop2: TYPE="squashfs" +/dev/loop3: TYPE="squashfs" +/dev/loop4: TYPE="squashfs" \ No newline at end of file diff --git a/spec/fixtures/cmdline_root_device_partuuid b/spec/fixtures/cmdline_root_device_partuuid new file mode 100644 index 0000000000..1e1c3208ba --- /dev/null +++ b/spec/fixtures/cmdline_root_device_partuuid @@ -0,0 +1 @@ +BOOT_IMAGE=/boot/vmlinuz-5.4.0-1024-aws root=PARTUUID=a2f52878-01 ro console=tty1 console=ttyS0 nvme_core.io_timeout=4294967295 panic=-1 \ No newline at end of file