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

@@ -53,18 +53,30 @@ def issues
  # @param config [Configs::Drive]
  # @return [Array<Issue>]
  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<Issue>]
+ def partitions_issues(config)
+ config.partitions.flat_map { |p| partition_issues(p) }
  end
 
  # Issues from a partition config.
  #
  # @param config [Configs::Partition]
  # @return [Array<Issue>]
  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]

service/lib/y2storage/proposal/agama_searcher.rb → service/lib/agama/storage/config_search_solver.rb

@@ -19,53 +19,37 @@
 # 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<Agama::Issue>]
- 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?
 
  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)
+ 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

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

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

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<Agama::Issue>] 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<Agama::Issue] Array to register issues found during the process
- def initialize(initial_config, devicegraph: nil, disk_analyzer: nil, issues_list: nil)
+ # @param 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<Agama::Issue] Array to register issues found during the process.
+ def initialize(config, devicegraph: nil, disk_analyzer: nil, issues_list: nil)
  super(devicegraph: devicegraph, disk_analyzer: disk_analyzer)
  @issues_list = issues_list || []
- @config = initial_config
+ @config = config
  end
 
  private
@@ -112,12 +85,9 @@ def fatal_error?
  #
  # @raise [NoDiskSpaceError] if there is no enough space to perform the installation
  def calculate_proposal
- # TODO: Could the search be moved to the devices planner? If so, the config object can 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(config, issues_list)
+ Agama::Storage::ConfigSolver.new(initial_devicegraph).solve(config)
+ issues = Agama::Storage::ConfigChecker.new(config).issues
+ issues_list.concat(issues)
 
  if fatal_error?
  # This means some IfNotFound is set to "error" and we failed to find a match