Consider bootable RAIDs when resolving an omitted boot device

service/lib/agama/storage/config_solver.rb

@@ -45,12 +45,12 @@ def initialize(product_config, storage_system)
       #
       # @param config [Config]
       def solve(config)
-        ConfigSolvers::Boot.new(product_config).solve(config)
         ConfigSolvers::Encryption.new(product_config).solve(config)
         ConfigSolvers::Filesystem.new(product_config).solve(config)
         ConfigSolvers::DrivesSearch.new(storage_system).solve(config)
         ConfigSolvers::MdRaidsSearch.new(storage_system).solve(config)
-        # Sizes must be solved once the searches are solved.
+        # Sizes and boot must be solved once the searches are solved.
+        ConfigSolvers::Boot.new(product_config, storage_system).solve(config)
         ConfigSolvers::Size.new(product_config).solve(config)
       end
 

service/lib/agama/storage/config_solvers/boot.rb

@@ -26,6 +26,13 @@ module Storage
     module ConfigSolvers
       # Solver for the boot config.
       class Boot < Base
+        # @param product_config [Agama::Config]
+        # @param storage_system [Storage::System]
+        def initialize(product_config, storage_system)
+          super(product_config)
+          @storage_system = storage_system
+        end
+
         # Solves the boot config within a given config.
         #
         # @note The config object is modified.
@@ -38,14 +45,18 @@ def solve(config)
 
       private
 
+        # @return [Storage::System]
+        attr_reader :storage_system
+
         # Finds a device for booting and sets its alias, if needed.
         #
         # A boot device cannot be automatically inferred in the following scenarios:
         #   * The root partition or logical volume is missing.
         #   * A disk is directly formated and mounted as root.
         #   * The volume group allocating the root logical volume uses whole drives as physical
         #     volumes.
-        #   * The MD RAID allocating root uses whole drives as member devices.
+        #   * The MD RAID allocating root uses whole drives as member devices and is not directly
+        #     bootable (see System#candidate_md_raids).
         def solve_device_alias
           return unless config.boot.configure? && config.boot.device.default?
 
@@ -65,72 +76,94 @@ def solve_device_alias
         #
         # The boot device is recursively searched until reaching a drive.
         #
-        # @return [Configs::Drive, nil] nil if the boot device cannot be inferred from the config.
+        # @return [Configs::Drive, Configs::MdRaid, nil] nil if the boot device cannot be inferred
+        #   from the config.
         def boot_device
           root_device = config.root_drive || config.root_md_raid || config.root_volume_group
           return unless root_device
 
-          partitioned_drive_from_device(root_device)
+          partitionable_from_device(root_device)
         end
 
-        # Recursively looks for the first partitioned drive from the given list of devices.
+        # Recursively looks for the first partitioned config from the given list of devices.
         #
         # @param devices [Array<Configs::Drive, Configs::MdRaid, Configs::VolumeGroup>]
         # @param is_target [Boolean] Whether the devices are target for automatically creating
         #   partitions (e.g., for creating physical volumes).
         #
-        # @return [Configs::Drive, nil]
-        def partitioned_drive_from_devices(devices, is_target: false)
+        # @return [Configs::Drive, Configs::MdRaid, nil]
+        def partitionable_from_devices(devices, is_target: false)
           devices.each do |device|
-            drive = partitioned_drive_from_device(device, is_target: is_target)
+            drive = partitionable_from_device(device, is_target: is_target)
             return drive if drive
           end
 
           nil
         end
 
-        # Recursively looks for the first partitioned drive from the given device.
+        # Recursively looks for the first partitioned config from the given device.
         #
         # @param device [Configs::Drive, Configs::MdRaid, Configs::VolumeGroup]
         # @param is_target [Boolean] See {#partitioned_drive_from_devices}
         #
-        # @return [Configs::Drive, nil]
-        def partitioned_drive_from_device(device, is_target: false)
+        # @return [Configs::Drive, Configs::MdRaid, nil]
+        def partitionable_from_device(device, is_target: false)
           case device
           when Configs::Drive
             (device.partitions? || is_target) ? device : nil
           when Configs::MdRaid
-            partitioned_drive_from_md_raid(device)
+            partitionable_from_md_raid(device)
           when Configs::VolumeGroup
-            partitioned_drive_from_volume_group(device)
+            partitionable_from_volume_group(device)
           end
         end
 
+        # Recursively looks for the first partitioned config from the given MD RAID.
+        #
+        # @param md_raid [Configs::MdRaid]
+        # @return [Configs::Drive, Configs::MdRaid, nil]
+        def partitionable_from_md_raid(md_raid)
+          return partitionable_from_found_md_raid(md_raid) if md_raid.found_device
+
+          partitioned_drive_from_new_md_raid(md_raid)
+        end
+
+        # Recursively looks for the first partitioned config from the given MD RAID.
+        #
+        # @param md_raid [Configs::MdRaid]
+        # @return [Configs::Drive, Configs::MdRaid, nil]
+        def partitionable_from_found_md_raid(md_raid)
+          return md_raid if storage_system.candidate?(md_raid.found_device)
+
+          # TODO: find the correct underlying disk devices for the MD RAID (note they may lack
+          # a corresponding drive entry at the configuration)
+          nil
+        end
+
         # Recursively looks for the first partitioned drive from the given MD RAID.
         #
         # @param md_raid [Configs::MdRaid]
         # @return [Configs::Drive, nil]
-        def partitioned_drive_from_md_raid(md_raid)
+        def partitioned_drive_from_new_md_raid(md_raid)
           devices = find_devices(md_raid.devices)
-          partitioned_drive_from_devices(devices)
+          partitionable_from_devices(devices)
         end
 
-        # Recursively looks for the first partitioned drive from the given volume group.
+        # Recursively looks for the first partitioned config from the given volume group.
         #
         # @param volume_group [Configs::VolumeGroup]
-        # @return [Configs::Drive, nil]
-        def partitioned_drive_from_volume_group(volume_group)
+        # @return [Configs::Drive, Configs::MdRaid, nil]
+        def partitionable_from_volume_group(volume_group)
           pv_devices = find_devices(volume_group.physical_volumes_devices, is_target: true)
           pvs = find_devices(volume_group.physical_volumes)
 
-          partitioned_drive_from_devices(pv_devices, is_target: true) ||
-            partitioned_drive_from_devices(pvs)
+          partitionable_from_devices(pv_devices, is_target: true) || partitionable_from_devices(pvs)
         end
 
         # Finds the devices with the given aliases or containing the given aliases.
         #
         # @param aliases [Array<String>]
-        # @param is_target [Boolean] See {#partitioned_drive_from_devices}
+        # @param is_target [Boolean] See {#partitionable_from_devices}
         #
         # @return [Array<Configs::Drive, Configs::MdRaid, Configs::VolumeGroup>]
         def find_devices(aliases, is_target: false)

service/lib/agama/storage/config_solvers/drives_search.rb

@@ -44,7 +44,7 @@ def initialize(storage_system)
         # @param config [Storage::Config]
         # @return [Storage::Config]
         def solve(config)
-          config.drives = super(config.drives, storage_system.candidate_drives)
+          config.drives = super(config.drives, storage_system.available_drives)
           solve_partitions_search(config.drives)
           config
         end

service/lib/agama/storage/config_solvers/md_raids_search.rb

@@ -44,7 +44,7 @@ def initialize(storage_system)
         # @param config [Storage::Config]
         # @return [Storage::Config]
         def solve(config)
-          config.md_raids = super(config.md_raids, storage_system.candidate_md_raids)
+          config.md_raids = super(config.md_raids, storage_system.available_md_raids)
           solve_partitions_search(config.md_raids)
           config
         end

service/lib/agama/storage/system.rb

@@ -35,23 +35,71 @@ def initialize(devicegraph = nil)
         @disk_analizer = Y2Storage::DiskAnalyzer.new(devicegraph)
       end
 
-      # Candidate drives for installation.
+      # All devices that can be referenced by a drive entry at the Agama config
       #
-      # @return [Array<Y2Storage::Drive, Y2Storage::StrayBlkDevice>]
-      def candidate_drives
+      # This excludes devices with any mounted filesystem and devices that contain a repository
+      # for installation.
+      #
+      # @return [Array<Y2Storage::Partitionable>]
+      def available_drives
         return [] unless devicegraph
 
         drives = devicegraph.disk_devices + devicegraph.stray_blk_devices
-        drives.select { |d| analyzer.candidate_device?(d) }
+        drives.select { |d| available?(d) }
       end
 
-      # Candidate MD RAIDs for installation.
+      # All drive devices that are considered as a valid target for the boot partitions and,
+      # as such, as candidates for a typical installation.
       #
-      # @return [Array<Y2Storage::Md]
-      def candidate_md_raids
+      # @return [Array<Y2Storage::Partitionable>]
+      def candidate_drives
+        available_drives.select { |d| analyzer.supports_boot_partitions?(d) }
+      end
+
+      # All devices that can be referenced by an mdRaid entry at the Agama config
+      #
+      # This excludes devices with any mounted filesystem and devices that contain a repository
+      # for installation.
+      #
+      # @return [Array<Y2Storage::Md>]
+      def available_md_raids
         return [] unless devicegraph
 
-        devicegraph.md_raids.select { |d| analyzer.candidate_device?(d) }
+        devicegraph.software_raids.select { |r| available?(r) }
+      end
+
+      # All mdRaid devices that are considered as a valid target for the boot partitions and,
+      # as such, as candidates for a typical installation.
+      #
+      # Although it could diverge in the future, this relies in the historical YaST heuristics
+      # that considers software RAIDs with partition table or without children as candidates for
+      # installation, but only when booting in EFI mode.
+      #
+      # Check Y2Storage::DiskAnalyzer for some historical background.
+      #
+      # @return [Array<Y2Storage::Md>]
+      def candidate_md_raids
+        available_md_raids.select { |r| analyzer.supports_boot_partitions?(r) }
+      end
+
+      # Whether the device is usable as drive or mdRaid
+      #
+      # See {#available_drives} and {#available_md_raids}
+      #
+      # @param device [Y2Storage::Partitionable, Y2Storage::Md]
+      # @return [Boolean]
+      def available?(device)
+        analyzer.available_device?(device)
+      end
+
+      # Whether the device can be used for installation, including the boot partitions
+      #
+      # See {#candidate_drives} and {#candidate_md_raids}
+      #
+      # @param device [Y2Storage::Partitionable, Y2Storage::Md]
+      # @return [Boolean]
+      def candidate?(device)
+        analyzer.supports_boot_partitions?(device) && available?(device)
       end
 
       # Devicegraph representing the system.

service/package/gem2rpm.yml

@@ -23,7 +23,7 @@
     Requires:       yast2-iscsi-client >= 4.5.7
     Requires:       yast2-network
     Requires:       yast2-proxy
-    Requires:       yast2-storage-ng >= 5.0.30
+    Requires:       yast2-storage-ng >= 5.0.31
     Requires:       yast2-users
     %ifarch s390 s390x
     Requires:       yast2-s390 >= 4.6.4

service/test/agama/storage/config_solvers/boot_test.rb

@@ -23,11 +23,13 @@
 require "agama/config"
 require "agama/storage/config_conversions/from_json"
 require "agama/storage/config_solvers/boot"
+require "agama/storage/system"
 
 describe Agama::Storage::ConfigSolvers::Boot do
-  subject { described_class.new(product_config) }
+  subject { described_class.new(product_config, storage_system) }
 
   let(:product_config) { Agama::Config.new({}) }
+  let(:storage_system) { Agama::Storage::System.new }
 
   describe "#solve" do
     let(:config_json) { nil }
@@ -122,7 +124,7 @@
             let(:md_raids) do
               [
                 {
-                  alias:      "root",
+                  alias:      raid_alias,
                   partitions: [
                     {
                       filesystem: { path: "/" }
@@ -133,6 +135,7 @@
               ]
             end
 
+            let(:raid_alias) { "raid" }
             let(:device_alias) { "root" }
             let(:md_devices) { [] }
 
@@ -190,6 +193,49 @@
                 expect(config.boot.device.device_alias).to be_nil
               end
             end
+
+            context "and it corresponds to an existing (reused) RAID" do
+              let(:md_device) { instance_double(Y2Storage::Md) }
+
+              before do
+                allow(config.md_raids.first).to receive(:found_device).and_return md_device
+                allow(storage_system).to receive(:candidate?).with(md_device).and_return candidate
+              end
+
+              context "if the RAID is considered bootable (candidate)" do
+                let(:candidate) { true }
+
+                it "sets the alias of the mdRaid as boot device alias" do
+                  subject.solve(config)
+                  expect(config.boot.device.device_alias).to eq("raid")
+                end
+
+                context "and the mdRaid configuration has no alias" do
+                  let(:raid_alias) { nil }
+
+                  it "sets an alias to the mdRaid" do
+                    subject.solve(config)
+                    raid = config.md_raids.first
+                    expect(raid.alias).to_not be_nil
+                  end
+
+                  it "sets the alias of the mdRaid as boot device alias" do
+                    subject.solve(config)
+                    raid = config.md_raids.first
+                    expect(config.boot.device.device_alias).to eq(raid.alias)
+                  end
+                end
+              end
+
+              context "if the RAID is a regular one (not candidate)" do
+                let(:candidate) { false }
+
+                it "does not set a boot device alias" do
+                  subject.solve(config)
+                  expect(config.boot.device.device_alias).to be_nil
+                end
+              end
+            end
           end
 
           context "and there is a root logical volume" do

service/test/agama/storage/config_solvers/drives_search_test.rb

@@ -69,9 +69,11 @@
         expect(drive3.search.device.name).to eq("/dev/vdc")
       end
 
-      context "and any of the devices is not a candidate device" do
+      context "and any of the devices is not available" do
         before do
-          allow(storage_system.analyzer).to receive(:candidate_device?) { |d| d.name != "/dev/vda" }
+          allow(storage_system.analyzer).to receive(:available_device?) do |dev|
+            dev.name != "/dev/vda"
+          end
         end
 
         it "sets the first unassigned candidate device to the drive config" do

service/test/agama/storage/config_solvers/md_raids_search_test.rb

@@ -66,9 +66,11 @@
         expect(md2.search.device.name).to eq("/dev/md1")
       end
 
-      context "and any of the devices is not a candidate device" do
+      context "and any of the devices is not available" do
         before do
-          allow(storage_system.analyzer).to receive(:candidate_device?) { |d| d.name != "/dev/md0" }
+          allow(storage_system.analyzer).to receive(:available_device?) do |dev|
+            dev.name != "/dev/md0"
+          end
         end
 
         it "sets the first unassigned candidate device to the MD RAID config" do