Resolve search sections that match several devices

[ancorgs]

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.

[coveralls]

Pull Request Test Coverage Report for Build 11480395585

Details

• 51 of 51 (100.0%) changed or added relevant lines in 7 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.05%) to 71.446%

---

Totals
Change from base Build 11460749266:	0.05%
Covered Lines:	16927
Relevant Lines:	23692

---

💛 - Coveralls

[joseivanlopez]

Some notes we already talked about:

• Adapt schema to include max in the search section.
• Adapt schema to include and document "*" const as possible value for search.
• Extend conversion to JSON to include max in search.

[joseivanlopez]

Note for myself: use Copyable mixing introduced by #1692.

[ancorgs]

@joseivanlopez I updated the PR (using force push, sorry) to address all concerns. Please re-review

[joseivanlopez]

Some comments. Feel free to ignore the NPs.

Changelog entry is missing.

[joseivanlopez]

I am not sure yet if we should generate wildcard. I think this is the only case in which we do it. Let's keep it for now.

[ancorgs]

I was not sure either. Our exporting is a bit weird in some aspects (like always adding the device name for found devices). I guess we will have to reconsider all that soon.

[joseivanlopez]

I would say that adding the device name is kind of expected for a solved config. I mean, independently of the initial conditions, I would expect the solved config should be a search by name. For example:

{ 
  search: { 
    condition: { size: { bigger: "10 GiB" } }
  }
}

Resolves to:

{ 
  search: "/dev/vdc" 
}

But of course, we can reconsider it.

[joseivanlopez]

NP: I think we can follow the same implementation pattern as the rest of conversions:

          # @see Base#conversions
          # @return [Hash]
          def conversions
            {
              name:         convert_name,
              max:          search_json[:max],
              if_not_found: convert_not_found
            }
          end

          # @return [String, nil]
          def convert_name
            return if search_json == SEARCH_ANYTHING_STRING
            return search_json if search_json.is_a?(String)

            search_json.dig(:condition, :name)
          end

          # @return [Symbol, nil]
          def convert_not_found
            return :skip if search_json == SEARCH_ANYTHING_STRING
            return if search_json.is_a?(String)
            
            search_json[:ifNotFound]&.to_sym          
          end

[ancorgs]

I find that much harder to follow.

We typically handle a hash except an special case in which a string can be used as shortcut.

With the implementation in this PR, that special case is handled at its own method.

With the implementation you suggest, the management is spread over several methods, all of them with its own exception for the string case. And the more attributes we add, the more methods we have with a dedicated if at the beginning. I don't see the point.

[joseivanlopez]

I think we see it differently. In the conversions I prefer to focus on each attribute and assign a value according to the value of the search_json. But it is a NP anyway, so let keep your implementation.

[joseivanlopez]

LGTM

[ancorgs]

BTW, I think we should reconsider this behavior. I don't think it's worth a warning. It can generate noise in many situations.

[ancorgs]

For the record: we agreed it makes no sense and it will be changed in a subsequent pull request.