-
Notifications
You must be signed in to change notification settings - Fork 59
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
fix: Fix recreate check for formats without labelling support #435
fix: Fix recreate check for formats without labelling support #435
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #435 +/- ##
==========================================
- Coverage 16.54% 11.65% -4.90%
==========================================
Files 2 8 +6
Lines 284 1784 +1500
Branches 79 0 -79
==========================================
+ Hits 47 208 +161
- Misses 237 1576 +1339
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
tests/tests_volume_relabel.yml
Outdated
fs_type: lvmpv | ||
disks: "{{ unused_disks }}" | ||
|
||
- name: Rerun to checks that we don't try to relabel LVMPV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does rerunning the role verifies this? Shouldn't it be some kind of test instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LVMPV doesn't support labels so if the role tries to set label on it, the action will simply fail, so there isn't a real result to check here.
The first step is just to create a device formatted as LVMPV (I could do that manually with pvcreate
) and the second step is checking that if we see a device formatted to LVMPV we won't try to do anything related to filesystem labels on it. It's a regression test specific to RHEL-29874, but I didn't want to create a separate test just for that so I added it here (I'll update the description to mention the issue).
ca4f4b2
to
f6a9be6
Compare
Formats like LUKS or LVMPV don't support labels so we need to skip the label check in BlivetVolume._reformat. Resolves: RHEL-29874
`populate()` is method of DeviceTree, not Blivet.
f6a9be6
to
01e09cf
Compare
Formats like LUKS or LVMPV don't support labels so we need to skip
the label check in BlivetVolume._reformat.
Resolves: RHEL-29874
(The second commit is just partially related to this, I found that issue when testing with LUKS which also doesn't have label support in blivet.)