Merge pull request #2146 from Filipovici-Andrei/FACT-2829

(FACT-2829) Fixed partitions and mount points facts

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

@@ -26,9 +26,9 @@ def read_partitions(fact_name)
             if File.directory?("#{block_path}/device")
               extract_from_device(block_path, blkid_and_lsblk)
             elsif File.directory?("#{block_path}/dm")
-              extract_from_dm(block_path, blkid_and_lsblk)
+              extract_from_dm(block_path, block_device, blkid_and_lsblk)
             elsif File.directory?("#{block_path}/loop")
-              extract_from_loop(block_path, blkid_and_lsblk)
+              extract_from_loop(block_path, block_device, blkid_and_lsblk)
             end
           end
 
@@ -43,21 +43,21 @@ def extract_from_device(block_path, blkid_and_lsblk)
           end
         end
 
-        def extract_from_dm(block_path, blkid_and_lsblk)
+        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, blkid_and_lsblk)
+            populate_partitions("/dev/#{block_device}", block_path, blkid_and_lsblk)
           else
             populate_partitions("/dev/mapper/#{map_name}", block_path, blkid_and_lsblk)
           end
         end
 
-        def extract_from_loop(block_path, blkid_and_lsblk)
+        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, blkid_and_lsblk)
+            populate_partitions("/dev/#{block_device}", block_path, blkid_and_lsblk)
           else
-            populate_partitions("/dev#{block_path}", block_path, blkid_and_lsblk, backing_file)
+            populate_partitions("/dev/#{block_device}", block_path, blkid_and_lsblk, backing_file)
           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

spec/facter/resolvers/partitions_spec.rb

@@ -126,7 +126,7 @@
 
       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
@@ -154,7 +154,7 @@
 
       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 'returns partitions fact' do
@@ -164,7 +164,7 @@
 
       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
 
         before do

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"

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