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.

Sign up for GitHub

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

Jump to bottom

Add support for dm-integrity on lvm raids #106

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Add support for dm-integrity on lvm raids #106

merged 4 commits into from
Sep 24, 2024

Conversation

huettner94
Copy link
Contributor

This adds support for automated raid integrity checking and can thereby help preventing bit rot. It however costs some read and write performance.

Details can be found in man lvmraid or here.

The bump of alpine to 3.17 is needed as otherwise the lvm2 cli does not yet know this option.

@majst01
Copy link
Contributor

majst01 commented Sep 12, 2024

Hi @huettner94 thanks for this PR, much appreciated. This is a useful addition.

We are currently trying to bring #100 to a state where it passes all integration test, i would like to merge yours after #100 was merged if this is ok for you.

@huettner94
Copy link
Contributor Author

That sounds good for me. Then i can also skip the alpine bump.
I will watch #100 and rebase on top of it afterwards.

Thanks a lot

@majst01
Copy link
Contributor

majst01 commented Sep 13, 2024

#100 was merged

@huettner94
Copy link
Contributor Author

Ok i now rebased.

Also i included a modprobe call in the ci job to ensure the dm-raid and dm-integrity modules are loaded. I think that caused the test error previously.

huettner94 and others added 2 commits September 13, 2024 19:13
@huettner94
Add support for dm-integrity on mirrors
5a17493 
This adds support for automated raid integrity checking and can thereby
help preventing bit rot. It however costs some read and write
performance.

Details can be found in `man lvmraid`.

To run these tests we need the dm-raid and dm-integrity kernel modules
loaded.
@vknabel
test: create storageclass for integrity
d88c575
@vknabel
Copy link

vknabel commented Sep 16, 2024

FYI I added an apply for the storageclass and did some cleanup after the tests run. Now the CI is happy, too. 🙂

@Gerrit91
Copy link
Contributor

Is this PR then "Ready for review" now?

@huettner94 huettner94 marked this pull request as ready for review September 16, 2024 19:55
@huettner94 huettner94 requested a review from a team as a code owner September 16, 2024 19:55
@huettner94
Copy link
Contributor Author

Thanks a lot @vknabel

Gerrit91
Gerrit91 reviewed Sep 17, 2024
View reviewed changes
Copy link
Contributor

@Gerrit91 Gerrit91 left a comment

Choose a reason for hiding this comment

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

I think this is pretty close. Just few minor remarks. :D

pkg/lvm/controllerserver.go Outdated Show resolved Hide resolved
tests/files/pvc.mirror-integrity.yaml Outdated Show resolved Hide resolved
@Gerrit91 Gerrit91 merged commit 4d0f260 into metal-stack:master Sep 24, 2024
3 checks passed
@Gerrit91
Copy link
Contributor

Thanks for hanging in there! ⭐

@huettner94 huettner94 deleted the dm_integrity branch September 24, 2024 17:19
@huettner94
Copy link
Contributor Author

Thanks a lot <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gerrit91 Gerrit91 Gerrit91 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@huettner94 @majst01 @vknabel @Gerrit91