Add LVM global_filter configuration to avoid scanning user devices #315
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@NickVolynkin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds documentation updates to include a new Talos patch that overwrites /etc/lvm/lvm.conf (mode 0644), disabling LVM backups/archives and applying a global_filter to exclude drbd, dm, and zd devices. This patch is added alongside the existing CRI customization file creation in provisioning examples. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant T as talosctl
participant A as Talos API
participant N as Node (Talos)
participant FS as Node FS
rect #E6F4EA
U->>T: Apply machine config with patches
T->>A: Submit patch operations
A->>N: Execute file overwrite/creation
N->>FS: Overwrite /etc/lvm/lvm.conf (0644)
N->>FS: Create/ensure /etc/cri/conf.d/20-customization.part
end
note over FS: lvm.conf sets backup=0, archive=0,<br/>devices.global_filter excludes drbd, dm, zd
FS-->>U: Files updated per patch
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kvaps
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @kvaps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the robustness of Kubernetes installations using Talos by configuring LVM to ignore certain device types. By implementing a global_filter, the system avoids conflicts and ensures stable operation when other storage solutions are in use, ultimately improving the reliability of the underlying storage infrastructure.
Highlights
- LVM Configuration Update: Introduces a global_filter in /etc/lvm/lvm.conf to prevent LVM from scanning specific device types (DRBD, device mapper, ZFS devices). This ensures LVM does not interfere with storage managed by other systems, improving stability and performance.
- Documentation Integration: Updates the Talos Kubernetes installation guides (talos-bootstrap.md and talosctl.md) to include the new LVM configuration, ensuring users apply these recommended settings during setup.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for talos-bootstrap and talosctl to include a configuration for LVM. The added lvm.conf disables backups and sets a global_filter to prevent LVM from scanning devices managed by DRBD, ZFS, and device-mapper. This is a good practice to avoid conflicts and potential data corruption. My review includes a minor suggestion to improve the indentation in the provided code examples for consistency.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
content/en/docs/install/kubernetes/talosctl.md (1)
100-110: LVM overwrite patch looks good; add an inline caveat comment for dm-crypt/multipath users.
- Syntax and placement are valid: backup/archive belong under the backup section; global_filter belongs under devices. (docs.redhat.com)
- Talos supports files.op=overwrite and octal permissions like 0o644. (talos.dev)
- Optional: add brief comments in lvm.conf to explain intent and warn that rejecting /dev/dm-* can hide dm-crypt or multipath devices in environments that rely on them. Devices rejected by global_filter are not opened by LVM. (docs.redhat.com)
Apply within this hunk:
- op: overwrite path: /etc/lvm/lvm.conf permissions: 0o644 content: | + # Cozystack: prevent LVM from scanning user devices (drbd, dm-*, zvol) + # and reduce noise by disabling metadata backups/archives. + # WARNING: If you use dm-crypt (LUKS) or multipath, adjust/remove the dm-* reject. backup { backup = 0 archive = 0 } devices { global_filter = [ "r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|" ] }content/en/docs/install/kubernetes/talos-bootstrap.md (1)
82-92: Same here: config is correct; consider embedding a short warning comment.
- Structure matches lvm.conf semantics (backup/archive in backup; global_filter in devices). (docs.redhat.com)
- Talos machine.files supports overwrite and octal permissions. (talos.dev)
- Optional: add an inline note that rejecting /dev/dm-* can impact dm-crypt/multipath setups; global_filter prevents those devices from being opened. (docs.redhat.com)
Patch within this block:
- op: overwrite path: /etc/lvm/lvm.conf permissions: 0o644 content: | + # Cozystack: limit LVM scans (drbd, dm-*, zvol) and disable metadata backups/archives. + # WARNING: If you rely on dm-crypt (LUKS) or multipath, review the dm-* reject rule. backup { backup = 0 archive = 0 } devices { global_filter = [ "r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|" ] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
content/en/docs/install/kubernetes/talos-bootstrap.md(1 hunks)content/en/docs/install/kubernetes/talosctl.md(1 hunks)
NickVolynkin
force-pushed
the
lvm-filter
branch
from
061096b to
6628e43
Compare
…ices Same change is already included in Talm: cozystack/talm#76 Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
NickVolynkin
force-pushed
the
lvm-filter
branch
from
6628e43 to
d743b3d
Compare
linked with Talm
Summary by CodeRabbit