Resolve search sections that match several devices (#1691)

## Problem

The search functionality described at `auto_storage.md` is pretty
powerful. But the current implementation only allows to:

- search by name
- return 0 or 1 devices per search

## Solution

This implements several parts of the missing functionality, like:

- Ability to match more than one device per `search` section
- Limit the number of matches by using `max`.

This pull request does not add the ability to use other conditions or
matching order beyond `name`. So it's only possible to search either for
a device with a given name, either for all the devices sorted by name.
The syntax for the latter would be this (a search with no conditions):

```
{ "search": {} }
```

For readability, the following alternative syntax is also introduced at
this pull request:

```
{ "search": "*" }
```

The new default for drive entries (if omitted) becomes:

```
{ "search": { "max": 1 } }
```

If several devices match with a given `search` section, the
corresponding configuration containing the `search` is replicated as
many times as needed. Eg. in a system with two disks, the following
config...

```
{
  "storage": {
    "drives": [
      { "search": "*" }
    ]
  }
}
```

...is expanded into:

```
{
  "storage": {
    "drives": [
      { "search": "*" },
      { "search": "*" }
    ]
  }
}
```

This also makes sure everything keeps working when an `alias` is used
together with a `search` that matches several devices (so all the
resulting config objects share the same alias).

## Testing

- Adapted existing unit tests and added a new ones regarding the
`search` functionality
- Added a new unit test to check that resolving a config into several
ones (potentially with a shared alias) works as expected.

rust/agama-lib/share/examples/storage/drives.json

@@ -67,9 +67,7 @@
             "deleteIfNeeded": true
           },
           {
-            "search": {
-              "ifNotFound": "skip"
-            },
+            "search": "*",
             "delete": true
           },
           {

rust/agama-lib/share/profile.schema.json

@@ -1029,6 +1029,11 @@
       "type": "string",
       "examples": ["/dev/vda", "/dev/disk/by-id/ata-WDC_WD3200AAKS-75L9"]
     },
+    "searchAll": {
+      "title": "Search all devices",
+      "description": "Shortcut to match all devices if there is any (equivalent to specify no conditions and to skip the entry if no device is found).",
+      "const": "*"
+    },
     "searchByName": {
       "title": "Search by name condition",
       "type": "object",
@@ -1042,6 +1047,9 @@
     },
     "search": {
       "anyOf": [
+        {
+          "$ref": "#/$defs/searchAll"
+        },
         {
           "$ref": "#/$defs/searchName"
         },

service/lib/agama/storage/config_conversions.rb

@@ -26,6 +26,9 @@ module Agama
   module Storage
     # Conversions for the storage config.
     module ConfigConversions
+      # Reserved string for Configs::Search meaning 'match all devices if there is any'
+      SEARCH_ANYTHING_STRING = "*"
+      private_constant :SEARCH_ANYTHING_STRING
     end
   end
 end

service/lib/agama/storage/config_conversions/from_json_conversions/search.rb

@@ -41,27 +41,20 @@ def convert
           # @see Base#conversions
           # @return [Hash]
           def conversions
+            return convert_string if search_json.is_a?(String)
+
             {
-              name:         convert_name,
-              if_not_found: convert_not_found
+              name:         search_json.dig(:condition, :name),
+              max:          search_json[:max],
+              if_not_found: search_json[:ifNotFound]&.to_sym
             }
           end
 
-          # @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
+          # @return [String]
+          def convert_string
+            return { if_not_found: :skip } if search_json == SEARCH_ANYTHING_STRING
 
-            value.to_sym
+            { name: search_json }
           end
         end
       end

service/lib/agama/storage/config_conversions/to_json_conversions/search.rb

@@ -33,13 +33,21 @@ def self.config_type
             Configs::Search
           end
 
+          # @see Base#convert
+          def convert
+            return SEARCH_ANYTHING_STRING if config.all_if_any?
+
+            super
+          end
+
         private
 
           # @see Base#conversions
           def conversions
             {
               condition:  convert_condition,
-              ifNotFound: config.if_not_found.to_s
+              ifNotFound: config.if_not_found.to_s,
+              max:        config.max
             }
           end
 

service/lib/agama/storage/config_search_solver.rb

@@ -36,22 +36,7 @@ def initialize(devicegraph)
       # @param config [Agama::Storage::Config]
       def solve(config)
         @sids = []
-        config.drives.each do |drive_config|
-          device = find_drive(drive_config.search)
-          drive_config.search.solve(device)
-
-          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.solve(partition)
-            add_found(partition_config)
-          end
-        end
+        config.drives = config.drives.flat_map { |d| solve_drive(d) }
       end
 
     private
@@ -62,24 +47,87 @@ def solve(config)
       # @return [Array<Integer>] SIDs of the devices that are already associated to another search.
       attr_reader :sids
 
-      # Finds a drive matching the given search config.
+      # @see #solve
+      #
+      # @note The given drive object can be modified
+      #
+      # @param original_drive [Configs::Drive]
+      # @return [Configs::Drive, Array<Configs::Drive>]
+      def solve_drive(original_drive)
+        devices = find_drives(original_drive.search)
+        return without_device(original_drive) if devices.empty?
+
+        devices.map do |device|
+          drive_copy(original_drive, device)
+        end
+      end
+
+      # Marks the search of the given config object as solved
+      #
+      # @note The config object is modified.
+      #
+      # @param config [Configs::Drive, Configs::Partition]
+      # @return [Configs::Drive, Configs::Partition]
+      def without_device(config)
+        config.search.solve
+        config
+      end
+
+      # see #solve_drive
+      def drive_copy(original_drive, device)
+        drive_config = original_drive.copy
+        drive_config.search.solve(device)
+        add_found(drive_config)
+
+        return drive_config unless drive_config.partitions?
+
+        drive_config.partitions = drive_config.partitions.flat_map do |partition_config|
+          solve_partition(partition_config, device)
+        end
+
+        drive_config
+      end
+
+      # see #solve_drive
+      #
+      # @note The given partition object can be modified
+      #
+      # @param original_partition [Configs::Partition]
+      # @param drive_device [Y2Storage::Partitionable]
+      # @return [Configs::Partition, Array<Configs::Partition>]
+      def solve_partition(original_partition, drive_device)
+        return original_partition unless original_partition.search
+
+        partitions = find_partitions(original_partition.search, drive_device)
+        return without_device(original_partition) if partitions.empty?
+
+        partitions.map do |partition|
+          partition_config = original_partition.copy
+          partition_config.search.solve(partition)
+          add_found(partition_config)
+
+          partition_config
+        end
+      end
+
+      # Finds the drives matching the given search config.
       #
       # @param search_config [Agama::Storage::Configs::Search]
       # @return [Y2Storage::Device, nil]
-      def find_drive(search_config)
+      def find_drives(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)
+        next_unassigned_devices(candidates, search_config)
       end
 
-      # Finds a partitions matching the given search config.
+      # Finds the partitions matching the given search config, if any
       #
       # @param search_config [Agama::Storage::Configs::Search]
       # @return [Y2Storage::Device, nil]
-      def find_partition(search_config, device)
+      def find_partitions(search_config, device)
         candidates = candidate_devices(search_config, default: device.partitions)
         candidates.select! { |d| d.is?(:partition) }
-        next_unassigned_device(candidates)
+        next_unassigned_devices(candidates, search_config)
       end
 
       # Candidate devices for the given search config.
@@ -89,7 +137,7 @@ def find_partition(search_config, device)
       #   conditions.
       # @return [Array<Y2Storage::Device>]
       def candidate_devices(search_config, default: [])
-        return default if search_config.any_device?
+        return default if search_config.always_match?
 
         [find_device(search_config)].compact
       end
@@ -102,14 +150,16 @@ def find_device(search_config)
         devicegraph.find_by_any_name(search_config.name)
       end
 
-      # Next unassigned device from the given list.
+      # Next unassigned devices from the given list.
       #
       # @param devices [Array<Y2Storage::Device>]
+      # @param search [Config::Search]
       # @return [Y2Storage::Device, nil]
-      def next_unassigned_device(devices)
+      def next_unassigned_devices(devices, search)
         devices
           .reject { |d| sids.include?(d.sid) }
-          .min_by(&:name)
+          .sort_by(&:name)
+          .first(search.max || devices.size)
       end
 
       # @see #search

service/lib/agama/storage/configs/drive.rb

@@ -48,7 +48,7 @@ class Drive
         def initialize
           @partitions = []
           # All drives are expected to match a real device in the system, so let's ensure a search.
-          @search = Search.new
+          @search = Search.new.tap { |s| s.max = 1 }
         end
 
         # Whether the drive definition contains partition definitions

service/lib/agama/storage/configs/search.rb

@@ -37,6 +37,12 @@ class Search
         # @return [:create, :skip, :error]
         attr_accessor :if_not_found
 
+        # Optional max number of devices to match
+        #
+        # return [Integer, nil] nil means no limit, ie. all devices that meet the condition are
+        #   matched
+        attr_accessor :max
+
         # Constructor
         def initialize
           @solved = false
@@ -61,10 +67,17 @@ def solve(device = nil)
         # Whether the search does not define any specific condition.
         #
         # @return [Boolean]
-        def any_device?
+        def always_match?
           name.nil?
         end
 
+        # Whether the search matches all the available devices, skipping if none is found
+        #
+        # @return [Boolean]
+        def all_if_any?
+          always_match? && max.nil? && if_not_found == :skip
+        end
+
         # Whether the section containing the search should be skipped
         #
         # @return [Boolean]

service/lib/agama/storage/configs/with_search.rb

@@ -35,6 +35,14 @@ module WithSearch
         def found_device
           search&.device
         end
+
+        # Creates a deep copy of the config element
+        #
+        # Needed when a search returns multiple devices and the configuration needs to be replicated
+        # for each one.
+        def copy
+          Marshal.load(Marshal.dump(self))
+        end
       end
     end
   end

service/lib/y2storage/proposal/agama_vg_planner.rb

@@ -59,8 +59,8 @@ def planned_vg(vg_config, config)
       # @param config [Agama::Storage::Config]
       # @return [Array<String>]
       def devices_for_pvs(vg_config, config)
-        drives = vg_config.physical_volumes_devices.map do |dev_alias|
-          config.drives.find { |d| d.alias?(dev_alias) }
+        drives = vg_config.physical_volumes_devices.flat_map do |dev_alias|
+          config.drives.select { |d| d.alias?(dev_alias) }
         end.compact
 
         drives.map { |d| d.found_device.name }

service/package/rubygem-agama-yast.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Oct 24 13:07:50 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>
+
+- Storage: support to match several devices with every 'search'
+  section (gh#agama-project/agama#1691).
+
 -------------------------------------------------------------------
 Tue Oct 22 09:48:57 UTC 2024 - José Iván López González <[email protected]>
 

service/test/agama/storage/config_checker_test.rb

@@ -244,8 +244,8 @@
 
   before do
     mock_storage(devicegraph: scenario)
-    # To speed-up the tests
-    allow(Y2Storage::EncryptionMethod::TPM_FDE)
+    # To speed-up the tests. Use #allow_any_instance because #allow introduces marshaling problems
+    allow_any_instance_of(Y2Storage::EncryptionMethod::TpmFde)
       .to(receive(:possible?))
       .and_return(true)
   end
@@ -255,6 +255,8 @@
       # Solves the config before checking.
       devicegraph = Y2Storage::StorageManager.instance.probed
 
+      allow(Y2Storage::BlkDevice).to receive(:find_by_any_name)
+
       Agama::Storage::ConfigSolver
         .new(devicegraph, product_config)
         .solve(config)

service/test/agama/storage/config_conversions/from_json_test.rb

@@ -21,7 +21,7 @@
 
 require_relative "../../../test_helper"
 require "agama/config"
-require "agama/storage/config_conversions/from_json"
+require "agama/storage/config_conversions"
 require "y2storage/encryption_method"
 require "y2storage/filesystems/mount_by_type"
 require "y2storage/filesystems/type"
@@ -107,6 +107,18 @@
     end
   end
 
+  context "with an asterisk" do
+    let(:search) { "*" }
+
+    it "sets #search to the expected value" do
+      config = config_proc.call(subject.convert)
+      expect(config.search).to be_a(Agama::Storage::Configs::Search)
+      expect(config.search.name).to be_nil
+      expect(config.search.if_not_found).to eq(:skip)
+      expect(config.search.max).to be_nil
+    end
+  end
+
   context "with a search section" do
     let(:search) do
       {
@@ -120,6 +132,24 @@
       expect(config.search).to be_a(Agama::Storage::Configs::Search)
       expect(config.search.name).to eq("/dev/vda1")
       expect(config.search.if_not_found).to eq(:skip)
+      expect(config.search.max).to be_nil
+    end
+  end
+
+  context "with a search section including a max" do
+    let(:search) do
+      {
+        ifNotFound: "error",
+        max:        3
+      }
+    end
+
+    it "sets #search to the expected value" do
+      config = config_proc.call(subject.convert)
+      expect(config.search).to be_a(Agama::Storage::Configs::Search)
+      expect(config.search.name).to be_nil
+      expect(config.search.if_not_found).to eq(:error)
+      expect(config.search.max).to eq 3
     end
   end
 end