From 966eb34b0c75d2c3e6de3fa24ba298c42967c025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 18 Sep 2024 15:26:12 +0100 Subject: [PATCH] refactor(storage): improve agama proposal - Do not remove configs if a search is not found. - Introduce ConfigSolver class. - Extend ConfigChecker to generate search issues. --- service/lib/agama/storage/config_checker.rb | 52 ++++++++++- .../storage/config_search_solver.rb} | 86 ++++--------------- service/lib/agama/storage/config_solver.rb | 48 +++++++++++ service/lib/agama/storage/configs/search.rb | 27 +++--- service/lib/y2storage/agama_proposal.rb | 70 +++++---------- .../proposal/agama_device_planner.rb | 5 +- .../proposal/agama_devices_planner.rb | 4 - .../y2storage/proposal/agama_drive_planner.rb | 2 + 8 files changed, 151 insertions(+), 143 deletions(-) rename service/lib/{y2storage/proposal/agama_searcher.rb => agama/storage/config_search_solver.rb} (57%) create mode 100644 service/lib/agama/storage/config_solver.rb diff --git a/service/lib/agama/storage/config_checker.rb b/service/lib/agama/storage/config_checker.rb index e05d920f05..281fb44479 100644 --- a/service/lib/agama/storage/config_checker.rb +++ b/service/lib/agama/storage/config_checker.rb @@ -53,10 +53,19 @@ def issues # @param config [Configs::Drive] # @return [Array] def drive_issues(config) - issues = encryption_issues(config) - partitions_issues = config.partitions.flat_map { |p| partition_issues(p) } + [ + search_issue(config), + encryption_issues(config), + partitions_issues(config) + ].flatten.compact + end - issues + partitions_issues + # Issues from partitions. + # + # @param config [Configs::Drive] + # @return [Array] + def partitions_issues(config) + config.partitions.flat_map { |p| partition_issues(p) } end # Issues from a partition config. @@ -64,7 +73,10 @@ def drive_issues(config) # @param config [Configs::Partition] # @return [Array] def partition_issues(config) - encryption_issues(config) + [ + search_issue(config), + encryption_issues(config) + ].flatten.compact end # Issues from a volume group config. @@ -91,6 +103,26 @@ def logical_volume_issues(lv_config, vg_config) ].compact.flatten end + # Issue for not found device. + # + # @param config [Configs::Drive, Configs::Partition] + # @return [Agama::Issue] + def search_issue(config) + return if !config.search || config.found_device + + if config.is_a?(Agama::Storage::Configs::Drive) + if config.search.skip_device? + warning(_("No device found for an optional drive")) + else + error(_("No device found for a mandatory drive")) + end + elsif config.search.skip_device? + warning(_("No device found for an optional partition")) + else + error(_("No device found for a mandatory partition")) + end + end + # @see #logical_volume_issues # # @param lv_config [Configs::LogicalVolume] @@ -200,6 +232,18 @@ def wrong_encryption_method_issue(config) ) end + # Creates a warning issue. + # + # @param message [String] + # @return [Issue] + def warning(message) + Agama::Issue.new( + message, + source: Agama::Issue::Source::CONFIG, + severity: Agama::Issue::Severity::WARN + ) + end + # Creates an error issue. # # @param message [String] diff --git a/service/lib/y2storage/proposal/agama_searcher.rb b/service/lib/agama/storage/config_search_solver.rb similarity index 57% rename from service/lib/y2storage/proposal/agama_searcher.rb rename to service/lib/agama/storage/config_search_solver.rb index 06d968cc9f..72a772b13a 100644 --- a/service/lib/y2storage/proposal/agama_searcher.rb +++ b/service/lib/agama/storage/config_search_solver.rb @@ -19,44 +19,28 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "agama/issue" - -module Y2Storage - module Proposal - # Auxiliary class to handle the 'search' elements within a storage configuration - class AgamaSearcher - include Yast::Logger - include Yast::I18n - +module Agama + module Storage + # Solver for the search configs. + class ConfigSearchSolver # @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 first argument (the storage configuration) gets modified in several ways: + # Solves all the search configs within a given config. # - # - 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 + # @note The config object is modified. # - # The second argument (the list of issues) gets modified by adding any found problem. - # - # @param config [Agama::Storage::Config] storage configuration containing device definitions - # like drives, volume groups, etc. - # @param issues_list [Array] - def search(config, issues_list) + # @param config [Agama::Storage::Config] + def solve(config) @sids = [] config.drives.each do |drive_config| device = find_drive(drive_config.search) - drive_config.search.resolve(device) + drive_config.search.solve(device) - process_element(drive_config, config.drives, issues_list) + add_found(drive_config) next unless drive_config.found_device && drive_config.partitions? @@ -64,8 +48,8 @@ def search(config, issues_list) 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) + partition_config.search.solve(partition) + add_found(partition_config) end end end @@ -129,48 +113,10 @@ def next_unassigned_device(devices) end # @see #search - def process_element(element, collection, issues_list) - found = element.found_device - if found - @sids << found.sid - else - issues_list << not_found_issue(element) - collection.delete(element) if element.search.skip_device? - end - end - - # Issue generated if a corresponding device is not found for the given element - # - # @param element [Agama::Storage::Configs::Drive, Agama::Storage::Configs::Partition] - # @return [Agama::Issue] - def not_found_issue(element) - Agama::Issue.new( - issue_message(element), - source: Agama::Issue::Source::CONFIG, - severity: issue_severity(element.search) - ) - end - - # @see #not_found_issue - def issue_message(element) - if element.is_a?(Agama::Storage::Configs::Drive) - if element.search.skip_device? - _("No device found for an optional drive") - else - _("No device found for a mandatory drive") - end - elsif element.search.skip_device? - _("No device found for an optional partition") - else - _("No device found for a mandatory partition") - end - end - - # @see #not_found_issue - def issue_severity(search) - return Agama::Issue::Severity::WARN if search.skip_device? - - Agama::Issue::Severity::ERROR + # @param config [#found_device] + def add_found(config) + found = config.found_device + @sids << found.sid if found end end end diff --git a/service/lib/agama/storage/config_solver.rb b/service/lib/agama/storage/config_solver.rb new file mode 100644 index 0000000000..e03989d5a3 --- /dev/null +++ b/service/lib/agama/storage/config_solver.rb @@ -0,0 +1,48 @@ +# 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_search_solver" + +module Agama + module Storage + # Class for solving a storage config. + class ConfigSolver + # @param devicegraph [Y2Storage::Devicegraph] + def initialize(devicegraph) + @devicegraph = devicegraph + end + + # Solves the given config with information from the devicegrah. + # + # @note The config object is modified. + # + # @param config [Config] + def solve(config) + ConfigSearchSolver.new(devicegraph).solve(config) + end + + private + + # @return [Y2Storage::Devicegraph] + attr_reader :devicegraph + end + end +end diff --git a/service/lib/agama/storage/configs/search.rb b/service/lib/agama/storage/configs/search.rb index 0f886c4cd9..207a48a289 100644 --- a/service/lib/agama/storage/configs/search.rb +++ b/service/lib/agama/storage/configs/search.rb @@ -39,36 +39,37 @@ class Search # Constructor def initialize + @solved = false @if_not_found = :error end - # Whether the search does not define any specific condition. + # Whether the search was already solved. # # @return [Boolean] - def any_device? - name.nil? + def solved? + @solved end - # Whether the search was already resolved. + # Solves the search with the given device. # - # @return [Boolean] - def resolved? - !!@resolved + # @param device [Y2Storage::Device, nil] + def solve(device = nil) + @device = device + @solved = true end - # Resolves the search with the given device. + # Whether the search does not define any specific condition. # - # @param device [Y2Storage::Device, nil] - def resolve(device = nil) - @device = device - @resolved = true + # @return [Boolean] + def any_device? + name.nil? end # Whether the section containing the search should be skipped # # @return [Boolean] def skip_device? - resolved? && device.nil? && if_not_found == :skip + solved? && device.nil? && if_not_found == :skip end end end diff --git a/service/lib/y2storage/agama_proposal.rb b/service/lib/y2storage/agama_proposal.rb index 0212912bd9..5dca7796db 100644 --- a/service/lib/y2storage/agama_proposal.rb +++ b/service/lib/y2storage/agama_proposal.rb @@ -19,46 +19,19 @@ # 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_checker" +require "agama/storage/config_solver" require "yast" +require "y2storage/exceptions" +require "y2storage/planned" require "y2storage/proposal" -require "y2storage/proposal/agama_searcher" -require "y2storage/proposal/agama_space_maker" -require "y2storage/proposal/agama_devices_planner" require "y2storage/proposal/agama_devices_creator" +require "y2storage/proposal/agama_devices_planner" +require "y2storage/proposal/agama_space_maker" require "y2storage/proposal/planned_devices_handler" -require "y2storage/exceptions" -require "y2storage/planned" module Y2Storage - # 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. + # Class to calculate a storage proposal for Agama. # # @example Creating a proposal from the current Agama configuration # config = Agama::Storage::Config.new_from_json(config_json) @@ -81,19 +54,19 @@ class AgamaProposal < Proposal::Base # @return [Array] List of found issues attr_reader :issues_list - # Constructor + # @note The storage config (first param) is modified in several ways: + # * The search configs are solved. # - # @param initial_config [Agama::Storage::Config] Agama storage config - # @param devicegraph [Devicegraph] starting point. If nil, then probed devicegraph - # will be used - # @param disk_analyzer [DiskAnalyzer] by default, the method will create a new one - # based on the initial devicegraph or will use the one from the StorageManager if - # starting from probed (i.e. 'devicegraph' argument is also missing) - # @param issues_list [Array] + def planned_devices raise NotImplementedError end @@ -149,6 +149,7 @@ def configure_partitions(planned, device_config, config) partition_configs = device_config.partitions .reject(&:delete?) .reject(&:delete_if_needed?) + .reject { |c| c.search&.skip_device? } planned.partitions = partition_configs.map do |partition_config| planned_partition(partition_config, device_config, config) diff --git a/service/lib/y2storage/proposal/agama_devices_planner.rb b/service/lib/y2storage/proposal/agama_devices_planner.rb index 3624f6f21b..a8788e0402 100644 --- a/service/lib/y2storage/proposal/agama_devices_planner.rb +++ b/service/lib/y2storage/proposal/agama_devices_planner.rb @@ -19,7 +19,6 @@ # 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_checker" require "y2storage/planned/devices_collection" require "y2storage/proposal/agama_drive_planner" require "y2storage/proposal/agama_vg_planner" @@ -45,9 +44,6 @@ def initialize(devicegraph, issues_list) # @param config [Agama::Storage::Config] # @return [Planned::DevicesCollection] def planned_devices(config) - checker = Agama::Storage::ConfigChecker.new(config) - issues_list.concat(checker.issues) - # In the future this will also include planned devices that are equivalent to # those typically generated by the Guided Proposal. For those, note that: # - For dedicated VGs it creates a Planned VG containing a Planned LV, but no PVs diff --git a/service/lib/y2storage/proposal/agama_drive_planner.rb b/service/lib/y2storage/proposal/agama_drive_planner.rb index a99e573fb0..b5b30282b5 100644 --- a/service/lib/y2storage/proposal/agama_drive_planner.rb +++ b/service/lib/y2storage/proposal/agama_drive_planner.rb @@ -31,6 +31,8 @@ class AgamaDrivePlanner < AgamaDevicePlanner # # @return [Array] def planned_devices(drive_config, config) + return [] if drive_config.search&.skip_device? + [planned_drive(drive_config, config)] end