diff --git a/service/lib/agama/storage/config_solver.rb b/service/lib/agama/storage/config_solver.rb index fe76fa879c..e959a9d23b 100644 --- a/service/lib/agama/storage/config_solver.rb +++ b/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 diff --git a/service/lib/agama/storage/config_solvers/boot.rb b/service/lib/agama/storage/config_solvers/boot.rb index 9d1608d4b4..5d43b55afd 100644 --- a/service/lib/agama/storage/config_solvers/boot.rb +++ b/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,6 +45,9 @@ 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: @@ -45,7 +55,8 @@ def solve(config) # * 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] # @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] - # @param is_target [Boolean] See {#partitioned_drive_from_devices} + # @param is_target [Boolean] See {#partitionable_from_devices} # # @return [Array] def find_devices(aliases, is_target: false) diff --git a/service/lib/agama/storage/config_solvers/drives_search.rb b/service/lib/agama/storage/config_solvers/drives_search.rb index 0dac9c2862..9b1377766c 100644 --- a/service/lib/agama/storage/config_solvers/drives_search.rb +++ b/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 diff --git a/service/lib/agama/storage/config_solvers/md_raids_search.rb b/service/lib/agama/storage/config_solvers/md_raids_search.rb index caf5571baf..2b1ff2381b 100644 --- a/service/lib/agama/storage/config_solvers/md_raids_search.rb +++ b/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 diff --git a/service/lib/agama/storage/system.rb b/service/lib/agama/storage/system.rb index 2088ff5169..76ce7b9513 100644 --- a/service/lib/agama/storage/system.rb +++ b/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] - def candidate_drives + # This excludes devices with any mounted filesystem and devices that contain a repository + # for installation. + # + # @return [Array] + 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] + 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] + 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] + 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. diff --git a/service/package/gem2rpm.yml b/service/package/gem2rpm.yml index fdf59c2183..8ce853ba52 100644 --- a/service/package/gem2rpm.yml +++ b/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 diff --git a/service/test/agama/storage/config_solvers/boot_test.rb b/service/test/agama/storage/config_solvers/boot_test.rb index dc39249a6d..4dcc52e10b 100644 --- a/service/test/agama/storage/config_solvers/boot_test.rb +++ b/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 diff --git a/service/test/agama/storage/config_solvers/drives_search_test.rb b/service/test/agama/storage/config_solvers/drives_search_test.rb index 984110694e..565c40867a 100644 --- a/service/test/agama/storage/config_solvers/drives_search_test.rb +++ b/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 diff --git a/service/test/agama/storage/config_solvers/md_raids_search_test.rb b/service/test/agama/storage/config_solvers/md_raids_search_test.rb index 5659518404..04f3844718 100644 --- a/service/test/agama/storage/config_solvers/md_raids_search_test.rb +++ b/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 diff --git a/service/test/agama/storage/system_test.rb b/service/test/agama/storage/system_test.rb index fa626c5a89..e60984785b 100644 --- a/service/test/agama/storage/system_test.rb +++ b/service/test/agama/storage/system_test.rb @@ -33,7 +33,7 @@ let(:scenario) { "disks.yaml" } before do - allow(disk_analyzer).to receive(:candidate_device?) { |d| d.name != "/dev/vdb" } + allow(disk_analyzer).to receive(:available_device?) { |d| d.name != "/dev/vdb" } end it "includes all drives suitable for installation" do @@ -44,7 +44,7 @@ let(:scenario) { "md_raids.yaml" } before do - allow(disk_analyzer).to receive(:candidate_device?).and_call_original + allow(disk_analyzer).to receive(:available_device?).and_call_original end it "does not include MD RAIDs" do @@ -53,15 +53,27 @@ end end + describe "#available_md_raids" do + let(:scenario) { "md_raids.yaml" } + + before do + allow(disk_analyzer).to receive(:available_device?) { |d| d.name != "/dev/md0" } + end + + it "includes all software RAIDs that are not in use" do + expect(subject.available_md_raids.map(&:name)).to contain_exactly("/dev/md1", "/dev/md2") + end + end + describe "#candidate_md_raids" do let(:scenario) { "md_raids.yaml" } before do - allow(disk_analyzer).to receive(:candidate_device?) { |d| d.name != "/dev/md0" } + allow(disk_analyzer).to receive(:supports_boot_partitions?) { |d| d.name == "/dev/md1" } end - it "includes all MD RAIDs suitable for installation" do - expect(subject.candidate_md_raids.map(&:name)).to contain_exactly("/dev/md1", "/dev/md2") + it "includes all MD RAIDs considered by Y2Storage as suitable for installation" do + expect(subject.candidate_md_raids.map(&:name)).to contain_exactly("/dev/md1") end end end