(FACT-2829) Fixed partitions naming and convert root partition partuu…

…id to path

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

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

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