search

linux-system-roles / storage

Ansible role for linux storage management
https://linux-system-roles.github.io/storage/
MIT License
104 stars 59 forks source link

feat: Added support for blivet fstab handling #425

Closed japokorn closed 8 months ago

japokorn commented 9 months ago

Blivet can now handle fstab, so handling it in the role became redundant. This adapts the role for the blivet change and removes no longer needed code.

Issue Tracker Tickets: #20182

richm commented 9 months ago

Will the role still work on systems that do not have the right version of blivet? That is, for managing older rhel7, rhel8, and rhel9 systems, will the role still need the "old" fstab handling?

japokorn commented 9 months ago

Will the role still work on systems that do not have the right version of blivet? That is, for managing older rhel7, rhel8, and rhel9 systems, will the role still need the "old" fstab handling?

No. Both blivet and storage role have to be updated for them to work.

codecov[bot] commented 9 months ago

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (533145b) 16.54% compared to head (67944b2) 13.65%.

Files Patch % Lines
library/blivet.py 0.00% 8 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #425 +/- ## ========================================== - Coverage 16.54% 13.65% -2.89% ========================================== Files 2 8 +6 Lines 284 1735 +1451 Branches 79 79 ========================================== + Hits 47 237 +190 - Misses 237 1498 +1261 ``` | [Flag](https://app.codecov.io/gh/linux-system-roles/storage/pull/425/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles) | Coverage Δ | | |---|---|---| | [sanity](https://app.codecov.io/gh/linux-system-roles/storage/pull/425/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles) | `16.54% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

richm commented 9 months ago

Will the role still work on systems that do not have the right version of blivet? That is, for managing older rhel7, rhel8, and rhel9 systems, will the role still need the "old" fstab handling?

No. Both blivet and storage role have to be updated for them to work.

We can't break the storage role for rhel7 and rhel8 users. If we want to use the new blivet fstab feature, we will have to change the role to support it, and leave the old fstab handling in the blivet module.

japokorn commented 8 months ago

Will the role still work on systems that do not have the right version of blivet? That is, for managing older rhel7, rhel8, and rhel9 systems, will the role still need the "old" fstab handling?

No. Both blivet and storage role have to be updated for them to work.

We can't break the storage role for rhel7 and rhel8 users. If we want to use the new blivet fstab feature, we will have to change the role to support it, and leave the old fstab handling in the blivet module.

The latest changes in blivet fstab handling should allow to keep the storage role as is.

In these circumstances, adapting SR to handle new blivet fstab and also making it working with the older version would mean creating redundancies in the role. The better way is just to let the role always handle the fstab the old way.

I'll give this a bit of grace period to make sure everything actually works. If it does I am going to close this PR.