Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(storage): config support for generating physical volumes #1652

Merged

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Oct 1, 2024

Add support to the storage config for using a generate section for physical volumes.

"physicalVolumes": [
  { "generate": ["first-disk", "second-disk"] }
]
"physicalVolumes": [
  {
    "generate": {
      "targetDevices": ["first-disk"],
      "encryption": {
        "luks2": { "password": "12345" }
      }
    }
  }
]

The generate section allows to indicate the target devices in which to automatically create physical volumes, if needed. Agama will try to create physical volumes to allocate the defined logical volumes. If an encryption section is provided by the generate section, then the new physical volumes will be encrypted.

Notes:

  • For now, physicalVolumes can contain either device aliases or a generate. In the future it will allow both.
  • If there is more than one generate in the list of physical volumes, then only the first one will be considered (the rest ones are ignored). Note that the schema does not complain.

@joseivanlopez joseivanlopez force-pushed the storage-generate-pvs branch 3 times, most recently from d713541 to 6e01867 Compare October 4, 2024 09:34
@coveralls
Copy link

coveralls commented Oct 4, 2024

Pull Request Test Coverage Report for Build 11249392029

Details

  • 145 of 145 (100.0%) changed or added relevant lines in 28 files are covered.
  • 7 unchanged lines in 5 files lost coverage.
  • Overall coverage remained the same at 70.596%

Files with Coverage Reduction New Missed Lines %
service/lib/agama/storage/config_conversions/from_json_conversions/base.rb 1 93.75%
service/lib/agama/storage/config.rb 1 96.97%
service/lib/agama/storage/proposal.rb 1 98.2%
service/lib/y2storage/agama_proposal.rb 1 98.75%
service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb 3 94.83%
Totals Coverage Status
Change from base Build 11209402098: 0.0%
Covered Lines: 15836
Relevant Lines: 22432

💛 - Coveralls

@joseivanlopez joseivanlopez force-pushed the storage-generate-pvs branch 3 times, most recently from 8e21647 to 832a140 Compare October 7, 2024 06:29
@joseivanlopez joseivanlopez marked this pull request as ready for review October 7, 2024 06:58
{
"name": "system",
"physicalVolumes": [
"pv1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually not sure we currently support combining manually defined PVs and generated ones. I'm pretty sure it would be technically possible but probably it does not work with the current implementation of yast/yast-storage-ng#1392.

The algorithm now can combine EXISTING PVs (for a reused VG) with generated ones (for growing that existing VG). But likely no new manual PVs and automatic ones.

I will check how hard would be to add it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it would be easy, but I have to double check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked and it would be possible by modifying the yast2-storage-ng algorithm a bit. But we decided to leave it for later, as part of a separate task internally tracked at https://trello.com/c/XAaoYdCL/578-storage-profile-improve-support-for-generating-lvm-physical-volumes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have adapted the schema to allow either aliases or 'generate'.

@joseivanlopez joseivanlopez force-pushed the storage-generate-pvs branch 2 times, most recently from 387cec8 to ea76bd1 Compare October 8, 2024 10:49
@joseivanlopez joseivanlopez force-pushed the storage-generate-pvs branch 2 times, most recently from 7ea8721 to 8f737ba Compare October 9, 2024 05:46
Copy link
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joseivanlopez joseivanlopez merged commit c48515c into agama-project:master Oct 9, 2024
4 checks passed
ancorgs added a commit that referenced this pull request Oct 9, 2024
#1652 adds support for configuring the automatic generation of LVM
physical volumes. Read there the details about how the feature works.

This pull request implements a first version of the functionality
described there, including a couple of unit tests.

Depends on yast/yast-storage-ng#1392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants