AgamaProposal: Honor DiskAnalyzer candidate devices when searching dr…

…ives (#1765)

## Problem

When searching drives, the initial implementation of `AgamaProposal`
uses all disk (or disk-like devices, to be precise) in the system.

That is ok as a first attempt, but we always knew we might need to
refine that a bit.

Now that we are working on the new interface for storage configuration,
fully based on `AgamaProposal`, the fact that `AgamaProposal` can choose
a disk that is not considered by `DiskAnalyzer` to be an installation
candidate feels a bit odd.

## Solution

For the time being (and with minimal code changes), this adapts the
`AgamaProposal` algorithm to filter out disks that are skipped by
`DiskAnalyzer`. That typically means the disks from which the installer
is running, disks that contain installation repositories, etc.

We may need to refine that in the future again. But for now that's the
most consistent behavior we can adopt without taking big decisions.

## Testing

Added a new unit test

Manual testing done by @joseivanlopez.

service/lib/agama/storage/config_search_solver.rb

@@ -23,10 +23,12 @@ 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)
+      # @param devicegraph [Y2Storage::Devicegraph] used to find the corresponding devices that
+      #   will get associated to each search element.
+      # @param analyzer [Y2Storage::DiskAnalyzer, nil] optionally used to filter candidate disks
+      def initialize(devicegraph, analyzer)
         @devicegraph = devicegraph
+        @disk_analyzer = analyzer
       end
 
       # Solves all the search configs within a given config.
@@ -41,9 +43,12 @@ def solve(config)
 
     private
 
-      # @return [Devicegraph]
+      # @return [Y2Storage::Devicegraph]
       attr_reader :devicegraph
 
+      # @return [Y2Storage::DiskAnalyzer, nil]
+      attr_reader :disk_analyzer
+
       # @return [Array<Integer>] SIDs of the devices that are already associated to another search.
       attr_reader :sids
 
@@ -117,9 +122,19 @@ def solve_partition(original_partition, drive_device)
       def find_drives(search_config)
         candidates = candidate_devices(search_config, default: devicegraph.blk_devices)
         candidates.select! { |d| d.is?(:disk_device, :stray_blk_device) }
+        filter_by_disk_analyzer(candidates)
         next_unassigned_devices(candidates, search_config)
       end
 
+      # @see #find_drives
+      # @param devices [Array<Y2Storage::Device>] this argument is modified
+      def filter_by_disk_analyzer(devices)
+        return unless disk_analyzer
+
+        candidate_sids = disk_analyzer.candidate_disks.map(&:sid)
+        devices.select! { |d| candidate_sids.include?(d.sid) }
+      end
+
       # Finds the partitions matching the given search config, if any
       #
       # @param search_config [Agama::Storage::Configs::Search]

service/lib/agama/storage/config_solver.rb

@@ -32,11 +32,14 @@ module Storage
     # example, the sizes of a partition config taking into account its fallbacks, assigning a
     # specific device when a config has a search, etc.
     class ConfigSolver
-      # @param devicegraph [Y2Storage::Devicegraph]
-      # @param product_config [Agama::Config]
-      def initialize(devicegraph, product_config)
+      # @param devicegraph [Y2Storage::Devicegraph] initial layout of the system
+      # @param product_config [Agama::Config] configuration of the product to install
+      # @param disk_analyzer [Y2Storage::DiskAnalyzer, nil] optional extra information about the
+      #   initial layout of the system
+      def initialize(devicegraph, product_config, disk_analyzer: nil)
         @devicegraph = devicegraph
         @product_config = product_config
+        @disk_analyzer = disk_analyzer
       end
 
       # Solves the config according to the product and the system.
@@ -47,7 +50,7 @@ def initialize(devicegraph, product_config)
       def solve(config)
         ConfigEncryptionSolver.new(product_config).solve(config)
         ConfigFilesystemSolver.new(product_config).solve(config)
-        ConfigSearchSolver.new(devicegraph).solve(config)
+        ConfigSearchSolver.new(devicegraph, disk_analyzer).solve(config)
         # Sizes must be solved once the searches are solved.
         ConfigSizeSolver.new(devicegraph, product_config).solve(config)
       end
@@ -59,6 +62,9 @@ def solve(config)
 
       # @return [Agama::Config]
       attr_reader :product_config
+
+      # @return [Y2Storage::DiskAnalyzer, nil]
+      attr_reader :disk_analyzer
     end
   end
 end

service/lib/y2storage/agama_proposal.rb

@@ -93,7 +93,7 @@ def fatal_error?
     # @raise [NoDiskSpaceError] if there is no enough space to perform the installation
     def calculate_proposal
       Agama::Storage::ConfigSolver
-        .new(initial_devicegraph, product_config)
+        .new(initial_devicegraph, product_config, disk_analyzer: disk_analyzer)
         .solve(config)
 
       issues = Agama::Storage::ConfigChecker

service/package/rubygem-agama-yast.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Nov 14 13:26:23 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>
+
+- Storage: honor the candidate devices from DiskAnalyzer when
+  matching drives (gh#agama-project/agama#1765).
+
 -------------------------------------------------------------------
 Wed Nov 13 12:14:06 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>
 

service/test/agama/storage/config_solver_test.rb

@@ -100,6 +100,7 @@
   end
 
   let(:devicegraph) { Y2Storage::StorageManager.instance.probed }
+  let(:disk_analyzer) { nil }
 
   before do
     mock_storage(devicegraph: scenario)
@@ -109,7 +110,7 @@
       .and_return(true)
   end
 
-  subject { described_class.new(devicegraph, product_config) }
+  subject { described_class.new(devicegraph, product_config, disk_analyzer: disk_analyzer) }
 
   describe "#solve" do
     let(:scenario) { "empty-hd-50GiB.yaml" }
@@ -574,6 +575,31 @@
       expect(search3.device.name).to eq("/dev/vdc")
     end
 
+    context "and any of the devices are excluded from the list of candidate devices" do
+      let(:disk_analyzer) { instance_double(Y2Storage::DiskAnalyzer) }
+      before do
+        allow(disk_analyzer).to receive(:candidate_disks).and_return [
+          devicegraph.find_by_name("/dev/vdb"), devicegraph.find_by_name("/dev/vdc")
+        ]
+      end
+
+      it "sets the first unassigned candidate devices to the drive" do
+        subject.solve(config)
+        searches = config.drives.map(&:search)
+        expect(searches[0].solved?).to eq(true)
+        expect(searches[0].device.name).to eq("/dev/vdb")
+        expect(searches[1].solved?).to eq(true)
+        expect(searches[1].device.name).to eq("/dev/vdc")
+      end
+
+      it "does not set devices that are not installation candidates" do
+        subject.solve(config)
+        searches = config.drives.map(&:search)
+        expect(searches[2].solved?).to eq(true)
+        expect(searches[2].device).to be_nil
+      end
+    end
+
     context "and there is not unassigned device" do
       let(:drives) do
         [