diff --git a/rust/agama-lib/share/examples/storage.json b/rust/agama-lib/share/examples/storage.json index 3c80bc5e76..aa39914563 100644 --- a/rust/agama-lib/share/examples/storage.json +++ b/rust/agama-lib/share/examples/storage.json @@ -6,13 +6,12 @@ }, "drives": [ { - "search": { - "name": "/dev/vda" - }, + "search": "/dev/vda", "ptableType": "gpt", "partitions": [ { - "search": { "name": "/dev/vda2" }, + "id": "linux", + "size": "10 GiB", "encryption": { "luks1": { "password": "notsecret" @@ -31,12 +30,16 @@ } }, { - "id": "linux", - "size": "10 GiB", + "search": { + "condition": { + "name": "/dev/vda2" + }, + "ifNotFound": "skip" + }, "encryption": { "luks2": { "password": "notsecret", - "label": "data" + "label": "home" } }, "filesystem": { @@ -45,7 +48,7 @@ } }, { - "size": "2 GiB", + "search": {}, "encryption": "random_swap", "filesystem": { "type": "swap", @@ -56,7 +59,10 @@ }, { "search": { - "name": "/dev/vdb" + "condition": { + "name": "/dev/vda" + }, + "ifNotFound": "error" }, "filesystem": { "type": "ext4", diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index d61873f068..729fad49f9 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -671,19 +671,45 @@ } ] }, - "search": { - "title": "Search options", + "searchName": { + "title": "Device name", + "type": "string", + "examples": ["/dev/vda", "/dev/disk/by-id/ata-WDC_WD3200AAKS-75L9"] + }, + "searchByName": { + "title": "Search by name condition", "type": "object", "additionalProperties": false, "required": ["name"], "properties": { "name": { - "title": "Device name", - "type": "string", - "examples": ["/dev/vda"] + "$ref": "#/$defs/searchName" } } }, + "search": { + "anyOf": [ + { + "$ref": "#/$defs/searchName" + }, + { + "title": "Search options", + "type": "object", + "additionalProperties": false, + "properties": { + "condition": { + "$ref": "#/$defs/searchByName" + }, + "ifNotFound": { + "title": "Not found action", + "description": "How to handle the section if the device is not found.", + "enum": ["skip", "error"], + "default": "error" + } + } + } + ] + }, "boot": { "title": "Boot options", "description": "Allows configuring boot partitions automatically.", diff --git a/service/lib/agama/storage/config_conversions.rb b/service/lib/agama/storage/config_conversions.rb index c17ee67683..0412053670 100644 --- a/service/lib/agama/storage/config_conversions.rb +++ b/service/lib/agama/storage/config_conversions.rb @@ -21,13 +21,12 @@ require "agama/storage/config_conversions/block_device" require "agama/storage/config_conversions/drive" -require "agama/storage/config_conversions/encrypt" +require "agama/storage/config_conversions/encryption" require "agama/storage/config_conversions/filesystem" -require "agama/storage/config_conversions/format" require "agama/storage/config_conversions/from_json" -require "agama/storage/config_conversions/mount" require "agama/storage/config_conversions/partition" require "agama/storage/config_conversions/partitionable" +require "agama/storage/config_conversions/search" require "agama/storage/config_conversions/size" module Agama diff --git a/service/lib/agama/storage/config_conversions/block_device/from_json.rb b/service/lib/agama/storage/config_conversions/block_device/from_json.rb index 768c2ba358..fdf1d30f16 100644 --- a/service/lib/agama/storage/config_conversions/block_device/from_json.rb +++ b/service/lib/agama/storage/config_conversions/block_device/from_json.rb @@ -45,11 +45,13 @@ def initialize(blk_device_json, settings:, volume_builder:) # Performs the conversion from Hash according to the JSON schema. # - # @param config [#encrypt=, #format=, #mount=] - def convert(config) - config.encryption = convert_encrypt - config.filesystem = convert_filesystem - config + # @param default [Configs::Drive, Configs::Partition] + # @return [Configs::Drive, Configs::Partition] + def convert(default) + default.dup.tap do |config| + config.encryption = convert_encrypt + config.filesystem = convert_filesystem + end end private diff --git a/service/lib/agama/storage/config_conversions/drive/from_json.rb b/service/lib/agama/storage/config_conversions/drive/from_json.rb index 698e74c69e..94dbfafbac 100644 --- a/service/lib/agama/storage/config_conversions/drive/from_json.rb +++ b/service/lib/agama/storage/config_conversions/drive/from_json.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "agama/storage/config_conversions/block_device/from_json" +require "agama/storage/config_conversions/search/from_json" require "agama/storage/config_conversions/partitionable/from_json" require "agama/storage/configs/drive" @@ -42,11 +43,14 @@ def initialize(drive_json, settings:, volume_builder:) # Performs the conversion from Hash according to the JSON schema. # + # @param default [Configs::Drive, nil] # @return [Configs::Drive] - def convert - Configs::Drive.new.tap do |config| - convert_block_device(config) - convert_partitionable(config) + def convert(default = nil) + default_config = default.dup || Configs::Drive.new + + convert_drive(default_config).tap do |config| + search = convert_search(config.search) + config.search = search if search end end @@ -61,6 +65,14 @@ def convert # @return [VolumeTemplatesBuilder] attr_reader :volume_builder + # @param config [Configs::Drive] + # @return [Configs::Drive] + def convert_drive(config) + convert_block_device( + convert_partitionable(config) + ) + end + # @param config [Configs::Drive] def convert_block_device(config) converter = BlockDevice::FromJSON.new(drive_json, @@ -76,6 +88,16 @@ def convert_partitionable(config) converter.convert(config) end + + # @param config [Configs::Search] + # @return [Configs::Search, nil] + def convert_search(config) + search_json = drive_json[:search] + return unless search_json + + converter = Search::FromJSON.new(search_json) + converter.convert(config) + end end end end diff --git a/service/lib/agama/storage/config_conversions/filesystem/from_json.rb b/service/lib/agama/storage/config_conversions/filesystem/from_json.rb index 2e2d442595..b3a6056f86 100644 --- a/service/lib/agama/storage/config_conversions/filesystem/from_json.rb +++ b/service/lib/agama/storage/config_conversions/filesystem/from_json.rb @@ -37,7 +37,7 @@ def initialize(filesystem_json) # Performs the conversion from Hash according to the JSON schema. # # @param default [Configs::Filesystem, nil] - # @return [Configs::Format] + # @return [Configs::Filesystem] def convert(default = nil) default_config = default.dup || Configs::Filesystem.new diff --git a/service/lib/agama/storage/config_conversions/partition/from_json.rb b/service/lib/agama/storage/config_conversions/partition/from_json.rb index 47e7736d4e..b1117d359a 100644 --- a/service/lib/agama/storage/config_conversions/partition/from_json.rb +++ b/service/lib/agama/storage/config_conversions/partition/from_json.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "agama/storage/config_conversions/block_device/from_json" +require "agama/storage/config_conversions/search/from_json" require "agama/storage/config_conversions/size/from_json" require "agama/storage/configs/partition" require "y2storage/partition_id" @@ -43,12 +44,19 @@ def initialize(partition_json, settings:, volume_builder:) # Performs the conversion from Hash according to the JSON schema. # + # @param default [Configs::Partition, nil] # @return [Configs::Partition] - def convert - Configs::Partition.new.tap do |config| - config.id = convert_id - config.size = convert_size - convert_block_device(config) + def convert(default = nil) + default_config = default.dup || Configs::Partition.new + + convert_block_device(default_config).tap do |config| + search = convert_search(config.search) + id = convert_id + size = convert_size(config.size) + + config.search = search if search + config.id = id if id + config.size = size if size end end @@ -63,6 +71,25 @@ def convert # @return [VolumeTemplatesBuilder] attr_reader :volume_builder + # @param config [Configs::Partition] + # @return [Configs::Partition] + def convert_block_device(config) + converter = BlockDevice::FromJSON.new(partition_json, + settings: settings, volume_builder: volume_builder) + + converter.convert(config) + end + + # @param config [Configs::Search] + # @return [Configs::Search, nil] + def convert_search(config) + search_json = partition_json[:search] + return unless search_json + + converter = Search::FromJSON.new(search_json) + converter.convert(config) + end + # @return [Y2Storage::PartitionId, nil] def convert_id value = partition_json[:id] @@ -71,20 +98,13 @@ def convert_id Y2Storage::PartitionId.find(value) end - # @return [Configs::Size] - def convert_size + # @param config [Configs::Size] + # @return [Configs::Size, nil] + def convert_size(config) size_json = partition_json[:size] - return Configs::Size.new unless size_json + return unless size_json - Size::FromJSON.new(size_json).convert - end - - # @param config [Configs::Partition] - def convert_block_device(config) - converter = BlockDevice::FromJSON.new(partition_json, - settings: settings, volume_builder: volume_builder) - - converter.convert(config) + Size::FromJSON.new(size_json).convert(config) end end end diff --git a/service/lib/agama/storage/config_conversions/partitionable/from_json.rb b/service/lib/agama/storage/config_conversions/partitionable/from_json.rb index fd6141b249..330ef37802 100644 --- a/service/lib/agama/storage/config_conversions/partitionable/from_json.rb +++ b/service/lib/agama/storage/config_conversions/partitionable/from_json.rb @@ -41,11 +41,13 @@ def initialize(partitionable_json, settings:, volume_builder:) # Performs the conversion from Hash according to the JSON schema. # - # @param config [#ptable_type=, #partitions=] - def convert(config) - config.ptable_type = convert_ptable_type - config.partitions = convert_partitions - config + # @param default [Configs::Drive] + # @return [Configs::Drive] + def convert(default) + default.dup.tap do |config| + config.ptable_type = convert_ptable_type + config.partitions = convert_partitions + end end private diff --git a/service/lib/agama/storage/config_conversions/search.rb b/service/lib/agama/storage/config_conversions/search.rb new file mode 100644 index 0000000000..5a6c6d44f3 --- /dev/null +++ b/service/lib/agama/storage/config_conversions/search.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "agama/storage/config_conversions/search/from_json" + +module Agama + module Storage + module ConfigConversions + # Conversions for search. + module Search + end + end + end +end diff --git a/service/lib/agama/storage/config_conversions/search/from_json.rb b/service/lib/agama/storage/config_conversions/search/from_json.rb new file mode 100644 index 0000000000..a03be629d0 --- /dev/null +++ b/service/lib/agama/storage/config_conversions/search/from_json.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +module Agama + module Storage + module ConfigConversions + module Search + # Search conversion from JSON hash according to schema. + class FromJSON + # @param search_json [Hash, String] + def initialize(search_json) + @search_json = search_json + end + + # Performs the conversion from Hash according to the JSON schema. + # + # @param default [Configs::Search, nil] + # @return [Configs::Search] + def convert(default = nil) + default_config = default.dup || Configs::Search.new + + default_config.tap do |config| + name = convert_name + not_found = convert_not_found + + config.name = name if name + config.if_not_found = not_found if not_found + end + end + + private + + # @return [Hash, String] + attr_reader :search_json + + # @return [String, nil] + def convert_name + return search_json if search_json.is_a?(String) + + search_json.dig(:condition, :name) + end + + # @return [Symbol, nil] + def convert_not_found + return if search_json.is_a?(String) + + value = search_json[:ifNotFound] + return unless value + + value.to_sym + end + end + end + end + end +end diff --git a/service/lib/agama/storage/config_conversions/size/from_json.rb b/service/lib/agama/storage/config_conversions/size/from_json.rb index f140ff9ed4..02fe5a6102 100644 --- a/service/lib/agama/storage/config_conversions/size/from_json.rb +++ b/service/lib/agama/storage/config_conversions/size/from_json.rb @@ -35,11 +35,12 @@ def initialize(size_json) # Performs the conversion from Hash according to the JSON schema. # + # @param default [Configs::Size, nil] # @return [Configs::Size] - def convert - return default_size if size_json.is_a?(String) && size_json.casecmp?("default") + def convert(default = nil) + default_config = default.dup || Configs::Size.new - Configs::Size.new.tap do |config| + default_config.tap do |config| config.default = false config.min = convert_size(:min) config.max = convert_size(:max) || Y2Storage::DiskSize.unlimited @@ -71,10 +72,6 @@ def convert_size(field) # JSON schema validations should prevent this from happening end end - - def default_size - Configs::Size.new.tap { |c| c.default = true } - end end end end diff --git a/service/lib/agama/storage/configs/drive.rb b/service/lib/agama/storage/configs/drive.rb index 36ddff9351..ff1b8260f5 100644 --- a/service/lib/agama/storage/configs/drive.rb +++ b/service/lib/agama/storage/configs/drive.rb @@ -27,7 +27,7 @@ module Configs # Section of the configuration representing a device that is expected to exist in the target # system and that can be used as a regular disk. class Drive - # @return [Search, nil] + # @return [Search] attr_accessor :search # @return [Encryption, nil] @@ -45,33 +45,17 @@ class Drive # Constructor def initialize @partitions = [] + # All drives are expected to match a real device in the system, so let's ensure a search. + @search = Search.new end - # Resolves the search, so a devices of the given devicegraph is associated to the drive if - # possible - # - # Since all drives are expected to match a real device in the system, this creates a default - # search if that was ommited. + # Assigned device according to the search. # - # @param devicegraph [Y2Storage::Devicegraph] source of the search - # @param used_sids [Array] SIDs of the devices that are already associated to - # another drive, so they cannot be associated to this - def search_device(devicegraph, used_sids) - @search ||= default_search - devs = devicegraph.blk_devices.select { |d| d.is?(:disk_device, :stray_blk_device) } - search.find(devs, used_sids) - end - - # @return [Search] - def default_search - Search.new - end - - # Device resulting from a previous call to {#search_device} + # @see Y2Storage::Proposal::AgamaSearcher # # @return [Y2Storage::Device, nil] def found_device - search&.device + search.device end # Whether the drive definition contains partition definitions diff --git a/service/lib/agama/storage/configs/partition.rb b/service/lib/agama/storage/configs/partition.rb index fbe38f6335..254e517164 100644 --- a/service/lib/agama/storage/configs/partition.rb +++ b/service/lib/agama/storage/configs/partition.rb @@ -19,6 +19,8 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. +require "agama/storage/configs/size" + module Agama module Storage module Configs @@ -30,7 +32,7 @@ class Partition # @return [Y2Storage::PartitionId, nil] attr_accessor :id - # @return [Size, nil] can be nil for reused partitions + # @return [Size] attr_accessor :size # @return [Encryption, nil] @@ -39,19 +41,13 @@ class Partition # @return [Filesystem, nil] attr_accessor :filesystem - # Resolves the search if the partition specification contains any, associating a partition - # of the given device if possible - # - # @param partitionable [Y2Storage::Partitionable] scope for the search - # @param used_sids [Array] SIDs of the devices that are already associated to - # another partition definition, so they cannot be associated to this - def search_device(partitionable, used_sids) - return unless search - - search.find(partitionable.partitions, used_sids) + def initialize + @size = Size.new end - # Device resulting from a previous call to {#search_device} + # Assigned device according to the search. + # + # @see Y2Storage::Proposal::AgamaSearcher # # @return [Y2Storage::Device, nil] def found_device diff --git a/service/lib/agama/storage/configs/search.rb b/service/lib/agama/storage/configs/search.rb index 2c99bf7f77..0f886c4cd9 100644 --- a/service/lib/agama/storage/configs/search.rb +++ b/service/lib/agama/storage/configs/search.rb @@ -29,38 +29,47 @@ class Search # @return [Y2Storage::Device, nil] attr_reader :device + # Name of the device to find. + # @return [String, nil] + attr_accessor :name + # What to do if the search does not match with the expected number of devices - # @return [Symbol] :create, :skip or :error + # @return [:create, :skip, :error] attr_accessor :if_not_found # Constructor def initialize - @if_not_found = :skip + @if_not_found = :error end - # Whether {#find} was already called + # Whether the search does not define any specific condition. + # + # @return [Boolean] + def any_device? + name.nil? + end + + # Whether the search was already resolved. # # @return [Boolean] def resolved? !!@resolved end + # Resolves the search with the given device. + # + # @param device [Y2Storage::Device, nil] + def resolve(device = nil) + @device = device + @resolved = true + end + # Whether the section containing the search should be skipped # # @return [Boolean] def skip_device? resolved? && device.nil? && if_not_found == :skip end - - # Resolve the search, associating the corresponding device to {#device} - # - # @param candidate_devs [Array] candidate devices - # @param used_sids [Array] SIDs of the devices that are already used elsewhere - def find(candidate_devs, used_sids) - devices = candidate_devs.reject { |d| used_sids.include?(d.sid) } - @resolved = true - @device = devices.min_by(&:name) - end end end end diff --git a/service/lib/agama/storage/proposal_strategies/agama.rb b/service/lib/agama/storage/proposal_strategies/agama.rb index a984f9eaac..c34e134f7c 100644 --- a/service/lib/agama/storage/proposal_strategies/agama.rb +++ b/service/lib/agama/storage/proposal_strategies/agama.rb @@ -45,19 +45,24 @@ def initialize(config, logger, storage_config) # @see Base#calculate def calculate - proposal = agama_proposal - proposal.propose + @proposal = agama_proposal + @proposal.propose ensure - storage_manager.proposal = proposal + storage_manager.proposal = @proposal end # @see Base#issues def issues - storage_manager.proposal.issues_list + return [] unless proposal + + proposal.issues_list end private + # @return [Y2Storage::AgamaProposal, nil] Proposal used. + attr_reader :proposal + # Instance of the Y2Storage proposal to be used to run the calculation. # # @return [Y2Storage::AgamaProposal] diff --git a/service/lib/y2storage/agama_proposal.rb b/service/lib/y2storage/agama_proposal.rb index 1590074104..e70e8a7bd7 100644 --- a/service/lib/y2storage/agama_proposal.rb +++ b/service/lib/y2storage/agama_proposal.rb @@ -30,7 +30,35 @@ require "y2storage/planned" module Y2Storage - # Class to calculate a storage proposal for autoinstallation using Agama + # Class to calculate a storage proposal for auto-installation using Agama. + # + # @note The storage config (initial_settings param in constructor) is modified in several ways: + # * The search configs are resolved. + # * Every config with an unfound search (e.g., a drive config, a partition config) is removed if + # its search has #if_not_found set to skip. + # + # It would be preferable to work over a copy instead of modifying the given config. In some + # cases, the config object is needed to generate its JSON format. The JSON result would not + # be 100% accurate if some elements are removed. + # + # The original config without removing elements is needed if: + # * The current proposal is the initial proposal automatically calculated by Agama. In + # this case, the config is generated from the product definition. The config JSON format is + # obtained by converting the config object to JSON. + # * The current proposal was calculated from a settings following the guided schema. This + # usually happens when a proposal is calculated from the UI. In this case, a config is + # generated from the guided settings. The config JSON format is obtained by converting the + # config object to JSON. + # + # In those two cases (initial proposal and proposal from guided settings) no elements are + # removed from the config because it has no searches with skip: + # * The config from the product definition has a drive that fails with unfound search (i.e., + # there is no candidate device for installing the system). + # * The config from the guided settings has all drives and partitions with search set to + # error. The proposal fails if the selected devices are not found. + # + # In the future there could be any other scenario in which it would be needed to keep all the + # elements from an initial config containing searches with skip. # # @example Creating a proposal from the current Agama configuration # config = Agama::Storage::Config.new_from_json(config_json) @@ -84,7 +112,13 @@ def fatal_error? # # @raise [NoDiskSpaceError] if there is no enough space to perform the installation def calculate_proposal - Proposal::AgamaSearcher.new.search(initial_devicegraph, settings, issues_list) + # TODO: Could the search be moved to the devices planner? If so, the settings object might + # keep untouched, directly generating planned devices associated to the found device and + # skipping planned devices for searches with skip if not found. + Proposal::AgamaSearcher + .new(initial_devicegraph) + .search(settings, issues_list) + if fatal_error? # This means some IfNotFound is set to "error" and we failed to find a match @devices = nil diff --git a/service/lib/y2storage/proposal/agama_device_planner.rb b/service/lib/y2storage/proposal/agama_device_planner.rb index e696a7d968..82cb340f64 100644 --- a/service/lib/y2storage/proposal/agama_device_planner.rb +++ b/service/lib/y2storage/proposal/agama_device_planner.rb @@ -58,6 +58,17 @@ def planned_devices(_setting) private + # @param planned [Planned::Disk, Planned::Partition] + # @param settings [#found_device] + def configure_reuse(planned, settings) + device = settings.found_device + return unless device + + planned.assign_reuse(device) + # TODO: Allow mounting without reformatting. + planned.reformat = true + end + # @param planned [Planned::Disk, Planned::Partition] # @param settings [#encryption, #filesystem] def configure_device(planned, settings) @@ -189,6 +200,7 @@ def configure_partitions(planned, settings) def planned_partition(settings) Planned::Partition.new(nil, nil).tap do |planned| planned.partition_id = settings.id + configure_reuse(planned, settings) configure_device(planned, settings) configure_size(planned, settings.size) end diff --git a/service/lib/y2storage/proposal/agama_drive_planner.rb b/service/lib/y2storage/proposal/agama_drive_planner.rb index c8f0be0203..2503cb8a92 100644 --- a/service/lib/y2storage/proposal/agama_drive_planner.rb +++ b/service/lib/y2storage/proposal/agama_drive_planner.rb @@ -48,7 +48,7 @@ def planned_drive(settings) # @return [Planned::Disk] def planned_full_drive(settings) Planned::Disk.new.tap do |planned| - configure_drive(planned, settings) + configure_reuse(planned, settings) configure_device(planned, settings) end end @@ -57,16 +57,10 @@ def planned_full_drive(settings) # @return [Planned::Disk] def planned_partitioned_drive(settings) Planned::Disk.new.tap do |planned| - configure_drive(planned, settings) + configure_reuse(planned, settings) configure_partitions(planned, settings) end end - - # @param planned [Planned::Disk] - # @param settings [Agama::Storage::Configs::Drive] - def configure_drive(planned, settings) - planned.assign_reuse(settings.found_device) - end end end end diff --git a/service/lib/y2storage/proposal/agama_searcher.rb b/service/lib/y2storage/proposal/agama_searcher.rb index 614f57d38e..06d968cc9f 100644 --- a/service/lib/y2storage/proposal/agama_searcher.rb +++ b/service/lib/y2storage/proposal/agama_searcher.rb @@ -28,46 +28,106 @@ class AgamaSearcher include Yast::Logger include Yast::I18n - # Constructor - def initialize + # @param devicegraph [Devicegraph] used to find the corresponding devices that will get + # associated to each search element. + def initialize(devicegraph) textdomain "agama" + + @devicegraph = devicegraph end # Resolve all the 'search' elements within a given configuration # - # The second argument (the storage configuration) gets modified in several ways: + # The first argument (the storage configuration) gets modified in several ways: # # - All its 'search' elements get resolved, associating devices from the devicegraph # (first argument) if some is found. # - Some device definitions can get removed if configured to be skipped in absence of a # corresponding device # - # The third argument (the list of issues) gets modified by adding any found problem. + # The second argument (the list of issues) gets modified by adding any found problem. # - # @param devicegraph [Devicegraph] used to find the corresponding devices that will get - # associated to each search element - # @param settings [Agama::Storage::Config] storage configuration containing device definitions + # @param config [Agama::Storage::Config] storage configuration containing device definitions # like drives, volume groups, etc. # @param issues_list [Array] - def search(devicegraph, settings, issues_list) + def search(config, issues_list) @sids = [] - settings.drives.each do |drive| - drive.search_device(devicegraph, @sids) - process_element(drive, settings.drives, issues_list) + config.drives.each do |drive_config| + device = find_drive(drive_config.search) + drive_config.search.resolve(device) - next unless drive.found_device && drive.partitions? + process_element(drive_config, config.drives, issues_list) - drive.partitions.each do |part| - next unless part.search + next unless drive_config.found_device && drive_config.partitions? - part.search_device(drive.found_device, @sids) - process_element(part, drive.partitions, issues_list) + drive_config.partitions.each do |partition_config| + next unless partition_config.search + + partition = find_partition(partition_config.search, drive_config.found_device) + partition_config.search.resolve(partition) + process_element(partition_config, drive_config.partitions, issues_list) end end end private + # @return [Devicegraph] + attr_reader :devicegraph + + # @return [Array] SIDs of the devices that are already associated to another search. + attr_reader :sids + + # Finds a drive matching the given search config. + # + # @param search_config [Agama::Storage::Configs::Search] + # @return [Y2Storage::Device, nil] + def find_drive(search_config) + candidates = candidate_devices(search_config, default: devicegraph.blk_devices) + candidates.select! { |d| d.is?(:disk_device, :stray_blk_device) } + next_unassigned_device(candidates) + end + + # Finds a partitions matching the given search config. + # + # @param search_config [Agama::Storage::Configs::Search] + # @return [Y2Storage::Device, nil] + def find_partition(search_config, device) + candidates = candidate_devices(search_config, default: device.partitions) + candidates.select! { |d| d.is?(:partition) } + next_unassigned_device(candidates) + end + + # Candidate devices for the given search config. + # + # @param search_config [Agama::Storage::Configs::Search] + # @param default [Array] Candidates if the search does not indicate + # conditions. + # @return [Array] + def candidate_devices(search_config, default: []) + return default if search_config.any_device? + + [find_device(search_config)].compact + end + + # Performs a search in the devicegraph to find a device matching the given search config. + # + # @param search_config [Agama::Storage::Configs::Search] + # @return [Y2Storage::Device] + def find_device(search_config) + devicegraph.find_by_any_name(search_config.name) + end + + # Next unassigned device from the given list. + # + # @param devices [Array] + # @return [Y2Storage::Device, nil] + def next_unassigned_device(devices) + devices + .reject { |d| sids.include?(d.sid) } + .min_by(&:name) + end + # @see #search def process_element(element, collection, issues_list) found = element.found_device diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 2c1ea3eb19..f358a01272 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Sep 3 08:14:23 UTC 2024 - José Iván López González + +- Storage: add support for searching by device in the storage + config (gh#openSUSE/agama#1560). + ------------------------------------------------------------------- Tue Aug 27 15:16:17 UTC 2024 - José Iván López González diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index a5204cce91..6f899caeac 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -138,6 +138,215 @@ partition = drive.partitions.first expect(partition.filesystem.path).to eq "/" end + + context "omitting search for a drive" do + let(:config_json) do + { + drives: [ + { + partitions: [] + } + ] + } + end + + it "sets the default search" do + config = subject.convert + drive = config.drives.first + expect(drive.search).to be_a(Agama::Storage::Configs::Search) + expect(drive.search.name).to be_nil + expect(drive.search.if_not_found).to eq(:error) + end + end + + context "specifying search for a drive" do + let(:config_json) do + { + drives: [ + { + search: search, + partitions: [] + } + ] + } + end + + context "with a device name" do + let(:search) { "/dev/vda" } + + it "sets the expected search" do + config = subject.convert + drive = config.drives.first + expect(drive.search).to be_a(Agama::Storage::Configs::Search) + expect(drive.search.name).to eq("/dev/vda") + expect(drive.search.if_not_found).to eq(:error) + end + end + + context "with a search section" do + let(:search) do + { + condition: { name: "/dev/vda" }, + ifNotFound: "skip" + } + end + + it "sets the expected search" do + config = subject.convert + drive = config.drives.first + expect(drive.search).to be_a(Agama::Storage::Configs::Search) + expect(drive.search.name).to eq("/dev/vda") + expect(drive.search.if_not_found).to eq(:skip) + end + end + end + end + + context "specifying a filesystem for a drive" do + let(:config_json) do + { + drives: [{ filesystem: filesystem }] + } + end + + let(:filesystem) do + { + path: "/", + type: "xfs", + label: "root", + mkfsOptions: ["version=2"], + mountOptions: ["rw"], + mountBy: "label" + } + end + + it "uses the specified attributes" do + config = subject.convert + filesystem = config.drives.first.filesystem + expect(filesystem.path).to eq "/" + expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::XFS + expect(filesystem.label).to eq "root" + expect(filesystem.mkfs_options).to eq ["version=2"] + expect(filesystem.mount_options).to eq ["rw"] + expect(filesystem.mount_by).to eq Y2Storage::Filesystems::MountByType::LABEL + end + + context "if the filesystem specification only contains a path" do + let(:filesystem) { { path: "/" } } + + it "uses the default type and btrfs attributes for that path" do + config = subject.convert + filesystem = config.drives.first.filesystem + expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS + expect(filesystem.type.btrfs.snapshots).to eq true + expect(filesystem.type.btrfs.default_subvolume).to eq "@" + expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["home", "opt", "root", "srv"] + end + end + + context "if the filesystem specification contains some btrfs settings" do + let(:filesystem) do + { path: "/", + type: { btrfs: { snapshots: false, default_subvolume: "", subvolumes: ["tmp"] } } } + end + + it "uses the specified btrfs attributes" do + config = subject.convert + filesystem = config.drives.first.filesystem + expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS + expect(filesystem.type.btrfs.snapshots).to eq false + # TODO: none of the following attributes are specified at the schema. Intentional? + # expect(filesystem.type.btrfs.default_subvolume).to eq "" + # expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["tmp"] + end + + context "and the default filesystem type is not btrfs" do + let(:filesystem) do + { path: "/home", type: { btrfs: { snapshots: false } } } + end + + it "uses btrfs filesystem" do + config = subject.convert + filesystem = config.drives.first.filesystem + expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS + end + end + end + end + + context "omitting search for a partition" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + filesystem: { + path: "/" + } + } + ] + } + ] + } + end + + it "does not set a search" do + config = subject.convert + drive = config.drives.first + partition = drive.partitions.first + expect(partition.search).to be_nil + end + end + + context "specifying search for a partition" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: search, + filesystem: { + path: "/" + } + } + ] + } + ] + } + end + + context "with a device name" do + let(:search) { "/dev/vda1" } + + it "sets the expected search" do + config = subject.convert + drive = config.drives.first + partition = drive.partitions.first + expect(partition.search).to be_a(Agama::Storage::Configs::Search) + expect(partition.search.name).to eq("/dev/vda1") + expect(partition.search.if_not_found).to eq(:error) + end + end + + context "with a search section" do + let(:search) do + { + condition: { name: "/dev/vda1" }, + ifNotFound: "skip" + } + end + + it "sets the expected search" do + config = subject.convert + drive = config.drives.first + partition = drive.partitions.first + expect(partition.search).to be_a(Agama::Storage::Configs::Search) + expect(partition.search.name).to eq("/dev/vda1") + expect(partition.search.if_not_found).to eq(:skip) + end + end end context "omitting sizes for the partitions" do @@ -346,43 +555,8 @@ end end - context "using 'default' as size for some partitions and size limit for others" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { path: "/", size: "default" } - }, - { - filesystem: { path: "/opt" }, - size: { min: "6 GiB", max: "22 GiB" } - } - ] - } - ] - } - end - - it "uses the appropriate sizes for each partition" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: true, min: 40.GiB, - max: Y2Storage::DiskSize.unlimited) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: false, min: 6.GiB, max: 22.GiB) - ) - ) - end - end - - context "using 'default' as size for some partitions and size limit for others" do + # TODO: "default" is not currently accepted by the schema. + xcontext "using 'default' as size for some partitions and size limit for others" do let(:config_json) do { drives: [ @@ -418,7 +592,8 @@ end end - context "using 'default' for a partition that is fallback for others" do + # TODO: "default" is not currently accepted by the schema. + xcontext "using 'default' for a partition that is fallback for others" do let(:config_json) { { drives: [{ partitions: partitions }] } } let(:root) do { filesystem: { path: "/", type: { btrfs: { snapshots: false } } }, size: "default" } @@ -457,78 +632,6 @@ end end - context "specifying a filesystem for a drive" do - let(:config_json) do - { - drives: [{ filesystem: filesystem }] - } - end - - let(:filesystem) do - { - path: "/", - type: "xfs", - label: "root", - mkfsOptions: ["version=2"], - mountOptions: ["rw"], - mountBy: "label" - } - end - - it "uses the specified attributes" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.path).to eq "/" - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::XFS - expect(filesystem.label).to eq "root" - expect(filesystem.mkfs_options).to eq ["version=2"] - expect(filesystem.mount_options).to eq ["rw"] - expect(filesystem.mount_by).to eq Y2Storage::Filesystems::MountByType::LABEL - end - - context "if the filesystem specification only contains a path" do - let(:filesystem) { { path: "/" } } - - it "uses the default type and btrfs attributes for that path" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - expect(filesystem.type.btrfs.snapshots).to eq true - expect(filesystem.type.btrfs.default_subvolume).to eq "@" - expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["home", "opt", "root", "srv"] - end - end - - context "if the filesystem specification contains some btrfs settings" do - let(:filesystem) do - { path: "/", - type: { btrfs: { snapshots: false, default_subvolume: "", subvolumes: ["tmp"] } } } - end - - it "uses the specified btrfs attributes" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - expect(filesystem.type.btrfs.snapshots).to eq false - # TODO: none of the following attributes are specified at the schema. Intentional? - # expect(filesystem.type.btrfs.default_subvolume).to eq "" - # expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["tmp"] - end - - context "and the default filesystem type is not btrfs" do - let(:filesystem) do - { path: "/home", type: { btrfs: { snapshots: false } } } - end - - it "uses btrfs filesystem" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - end - end - end - end - context "configuring partial information for several mount points" do let(:config_json) { { drives: [{ partitions: partitions }] } } let(:partitions) do diff --git a/service/test/fixtures/partitioned_disk.yaml b/service/test/fixtures/partitioned_disk.yaml new file mode 100644 index 0000000000..bfe5219833 --- /dev/null +++ b/service/test/fixtures/partitioned_disk.yaml @@ -0,0 +1,23 @@ +--- +- disk: + name: "/dev/vda" + size: 50 GiB + partition_table: gpt + partitions: + - partition: + size: 2 MiB + name: "/dev/vda1" + id: bios_boot + - partition: + size: 20 GiB + name: "/dev/vda2" + id: linux + file_system: btrfs + - partition: + size: 10 GiB + name: "/dev/vda3" + id: linux + file_system: xfs +- disk: + name: "/dev/vdb" + size: 50 GiB diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 5af850cdac..8c4a49283d 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -27,22 +27,20 @@ describe Y2Storage::AgamaProposal do include Agama::RSpec::StorageHelpers - before do - mock_storage(devicegraph: "empty-hd-50GiB.yaml") - end - - subject(:proposal) do - described_class.new(initial_settings, issues_list: issues_list) - end let(:initial_settings) do Agama::Storage::Config.new.tap do |settings| settings.drives = drives end end + let(:issues_list) { [] } + let(:drives) { [drive0] } + let(:drive0) { Agama::Storage::Configs::Drive.new.tap { |d| d.partitions = partitions0 } } + let(:partitions0) { [root_partition] } + let(:root_partition) do Agama::Storage::Configs::Partition.new.tap do |part| part.filesystem = Agama::Storage::Configs::Filesystem.new.tap do |fs| @@ -58,6 +56,31 @@ end end + let(:home_partition) do + Agama::Storage::Configs::Partition.new.tap do |part| + part.filesystem = Agama::Storage::Configs::Filesystem.new.tap do |fs| + fs.path = "/home" + fs.type = Agama::Storage::Configs::FilesystemType.new.tap do |type| + type.fs_type = Y2Storage::Filesystems::Type::EXT4 + end + end + part.size = Agama::Storage::Configs::Size.new.tap do |size| + size.min = Y2Storage::DiskSize.GiB(10) + size.max = Y2Storage::DiskSize.unlimited + end + end + end + + before do + mock_storage(devicegraph: scenario) + end + + subject(:proposal) do + described_class.new(initial_settings, issues_list: issues_list) + end + + let(:scenario) { "empty-hd-50GiB.yaml" } + describe "#propose" do context "when only the root partition is specified" do context "if no configuration about boot devices is specified" do @@ -117,22 +140,6 @@ context "when encrypting some devices" do let(:partitions0) { [root_partition, home_partition] } - let(:home_partition) do - Agama::Storage::Configs::Partition.new.tap do |part| - part.filesystem = Agama::Storage::Configs::Filesystem.new.tap do |fs| - fs.path = "/home" - fs.type = Agama::Storage::Configs::FilesystemType.new.tap do |type| - type.fs_type = Y2Storage::Filesystems::Type::EXT4 - end - end - part.size = Agama::Storage::Configs::Size.new.tap do |size| - size.min = Y2Storage::DiskSize.GiB(10) - size.max = Y2Storage::DiskSize.unlimited - end - part.encryption = home_encryption - end - end - let(:home_encryption) do Agama::Storage::Configs::Encryption.new.tap do |enc| enc.password = "notSecreT" @@ -145,6 +152,7 @@ before do allow(encryption_method).to receive(:available?).and_return(available?) if encryption_method + home_partition.encryption = home_encryption end context "if the encryption settings contain all the detailed information" do @@ -279,9 +287,56 @@ end end - context "when searching for a non-existent partition" do - let(:partitions0) { [root_partition, existing_partition] } - let(:existing_partition) do + context "when searching for an existent drive" do + let(:scenario) { "partitioned_disk.yaml" } + + before do + drive0.search.name = "/dev/vdb" + end + + it "uses the drive" do + proposal.propose + + root = proposal.devices.partitions.find do |part| + part.filesystem&.mount_path == "/" + end + + expect(root.disk.name).to eq("/dev/vdb") + end + end + + context "when searching for any drive" do + let(:scenario) { "partitioned_disk.yaml" } + + let(:drives) { [drive0, drive1] } + + let(:drive0) do + Agama::Storage::Configs::Drive.new.tap { |d| d.partitions = [root_partition] } + end + + let(:drive1) do + Agama::Storage::Configs::Drive.new.tap { |d| d.partitions = [home_partition] } + end + + it "uses the first unassigned drive" do + proposal.propose + + root = proposal.devices.partitions.find do |part| + part.filesystem&.mount_path == "/" + end + + home = proposal.devices.partitions.find do |part| + part.filesystem&.mount_path == "/home" + end + + expect(root.disk.name).to eq("/dev/vda") + expect(home.disk.name).to eq("/dev/vdb") + end + end + + context "when searching for a missing partition" do + let(:partitions0) { [root_partition, missing_partition] } + let(:missing_partition) do Agama::Storage::Configs::Partition.new.tap do |part| part.search = Agama::Storage::Configs::Search.new.tap do |search| search.if_not_found = if_not_found @@ -323,5 +378,61 @@ end end end + + context "when searching for an existent partition" do + let(:scenario) { "partitioned_disk.yaml" } + + let(:partitions0) { [root_partition, home_partition] } + + before do + home_partition.search = Agama::Storage::Configs::Search.new.tap do |search| + search.name = "/dev/vda3" + end + end + + it "reuses the partition" do + vda3 = Y2Storage::StorageManager.instance.probed.find_by_name("/dev/vda3") + proposal.propose + + partition = proposal.devices.find_by_name("/dev/vda3") + expect(partition.sid).to eq(vda3.sid) + expect(partition.filesystem.mount_path).to eq("/home") + end + end + + context "when searching for any partition" do + let(:scenario) { "partitioned_disk.yaml" } + + let(:partitions0) { [root_partition, home_partition] } + + before do + home_partition.search = Agama::Storage::Configs::Search.new + end + + # TODO: Is this correct? The first partition (boot partition) is reused for home. + it "reuses the first unassigned partition" do + vda1 = Y2Storage::StorageManager.instance.probed.find_by_name("/dev/vda1") + proposal.propose + + partition = proposal.devices.find_by_name("/dev/vda1") + expect(partition.sid).to eq(vda1.sid) + expect(partition.filesystem.mount_path).to eq("/home") + end + + it "does not reuse the same partition twice" do + vda1 = Y2Storage::StorageManager.instance.probed.find_by_name("/dev/vda1") + vda2 = Y2Storage::StorageManager.instance.probed.find_by_name("/dev/vda2") + root_partition.search = Agama::Storage::Configs::Search.new + proposal.propose + + root = proposal.devices.find_by_name("/dev/vda1") + expect(root.sid).to eq(vda1.sid) + expect(root.filesystem.mount_path).to eq("/") + + home = proposal.devices.find_by_name("/dev/vda2") + expect(home.sid).to eq(vda2.sid) + expect(home.filesystem.mount_path).to eq("/home") + end + end end end