fix: Show error when trying to put LVM volume on partition pool#531
Conversation
Trying this currently fails later with an exception so this just makes sure we show a nicer error message describing the error in the provided playbook. Resolves: RHEL-71246
Reviewer's GuideIntroduce an explicit validation in the blivet backend to ensure LVM volumes are only created on LVM pools and add a corresponding test case to verify that a clear error is raised when a type mismatch occurs. Sequence Diagram: LVM Volume Creation with Pool Type ValidationsequenceDiagram
actor User/Playbook
participant BVC as BlivetCode
participant BP as BlivetPool
participant PD as PoolDevice
User/Playbook->>BVC: Call _create() for LVM Volume
BVC->>BP: Get self._blivet_pool
BP->>PD: Get self._blivet_pool._device
PD-->>BP: return device
BP-->>BVC: return pool (with device)
BVC->>BVC: Check pool._device.type
alt Pool device type is not "lvmvg"
BVC-->>User/Playbook: raise BlivetAnsibleError("LVM volume can be placed only on LVM pool")
else Pool device type is "lvmvg"
BVC->>BVC: Proceed with LVM volume creation (original logic)
end
Class Diagram: Update to LVM Volume Creation Logic in Blivet LibraryclassDiagram
class BlivetLogicContainer {
-_blivet_pool: BlivetPool
+_create() void
note for _create "Modified to add validation: checks self._blivet_pool._device.type.\nRaises BlivetAnsibleError if type is not 'lvmvg'."
}
class BlivetPool {
-_device: PoolDevice
}
class PoolDevice {
+type: string
}
class BlivetAnsibleError {
<<Exception>>
+message: string
}
BlivetLogicContainer ..> BlivetAnsibleError : throws
BlivetLogicContainer o-- BlivetPool : uses
BlivetPool o-- PoolDevice : has
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #531 +/- ##
==========================================
- Coverage 16.54% 10.71% -5.84%
==========================================
Files 2 8 +6
Lines 284 1951 +1667
Branches 79 0 -79
==========================================
+ Hits 47 209 +162
- Misses 237 1742 +1505
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
[citest] |
|
[citest_bad] |
There was a problem hiding this comment.
Hey @vojtechtrefny - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Trying this currently fails later with an exception so this just makes sure we show a nicer error message describing the error in the provided playbook.
Resolves: RHEL-71246
Summary by Sourcery
Provide explicit error handling when attempting to create an LVM volume on a non-LVM pool and add a test to verify this scenario
Bug Fixes:
Tests: