Allow overriding sysctl parameters via /etc/sysctl.d files #3883
Conversation
Relocate current content of /etc/sysctl.d to /usr/lib and make the /etc folder writable via a bind mount.
|
One of the use cases can also be configuration of swappiness discussed in #968 (via |
📝 WalkthroughWalkthroughA new systemd mount unit file named Changes
Sequence Diagram(s)sequenceDiagram
participant Target as hassos-bind.target
participant MntOverlay as mnt-overlay.mount
participant SysctlMount as etc-sysctl.d.mount
participant SysctlService as systemd-sysctl.service
Target->>MntOverlay: Trigger mount initialization
MntOverlay->>SysctlMount: Enable dependent mount (After condition)
SysctlMount->>SysctlService: Ensure mount is active (Before condition)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
buildroot-external/rootfs-overlay/usr/lib/systemd/system/etc-sysctl.d.mount (1)
1-15: Consider documenting the security implications.While the implementation is correct, it would be helpful to document:
- The security implications of allowing sysctl parameter overrides
- Any validation or restrictions on the parameters that can be overridden
- Usage guidelines for system administrators
Would you like me to help draft documentation covering these aspects?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
buildroot-external/rootfs-overlay/usr/lib/systemd/system/etc-sysctl.d.mount(1 hunks)
🔇 Additional comments (3)
buildroot-external/rootfs-overlay/usr/lib/systemd/system/etc-sysctl.d.mount (3)
1-6: LGTM! Unit dependencies and ordering are correctly defined.The unit section properly ensures that:
- The mount happens after the overlay mount is ready
- The mount is complete before systemd-sysctl.service starts reading parameters
13-14: LGTM! Proper integration with hassos-bind.target.The mount unit is correctly integrated with hassos-bind.target, ensuring it's activated at the appropriate time during system initialization.
7-12:❓ Verification inconclusive
Verify the mount configuration and directory existence.
The mount configuration looks correct for a bind mount. However, we should verify that:
- The source directory exists or is created during system initialization
- The target directory exists or is created during system initialization
🏁 Script executed:
#!/bin/bash # Description: Check if the source and target directories are properly set up in the system # Check if the source directory exists in the rootfs overlay echo "Checking source directory in rootfs overlay..." fd "sysctl.d$" buildroot-external/rootfs-overlay/ # Check if there are any references to creating these directories in build scripts echo "Checking for directory creation in scripts..." rg -A 2 "mkdir.*sysctl\.d"Length of output: 384
Action: Reassess the Bind Mount Source Directory for /mnt/overlay/etc/sysctl.d
The bind mount unit in
buildroot-external/rootfs-overlay/usr/lib/systemd/system/etc-sysctl.d.mount
appears syntactically correct. However, during verification we observed:
The mount unit specifies
What=/mnt/overlay/etc/sysctl.d, but a search of the overlay revealed directories at:
buildroot-external/rootfs-overlay/etc/sysctl.d/buildroot-external/rootfs-overlay/usr/lib/sysctl.d/There was no evidence of a directory matching
/mnt/overlay/etc/sysctl.d, which raises a question about whether the intended source directory actually exists in the final system image.The absence of any build-script references (e.g., via
mkdir) for creating a/mnt/overlay/etc/sysctl.dsuggests that this directory might neither be present by default nor generated during initialization.Please verify that:
- The source directory (
What=/mnt/overlay/etc/sysctl.d) is either provided by the overlay or is created at runtime.- The target directory (
Where=/etc/sysctl.d) is available as expected.Manual confirmation is recommended to ensure that the paths in the mount unit align with the actual directory structure in the built system.
After relocation of kernel parameters in #3883, move swappiness parameter to file that overrides only this value. This is to potentially avoid the risk of losing other parameters that could be added there when the file is overridden in /etc/sysctl.d (either manually or using future os-agent interface). Refs #968
Relocate current content of /etc/sysctl.d to /usr/lib and make the /etc folder writable via a bind mount.
Summary by CodeRabbit