feat: add boot.kernel.sysctl and boot.kernelModules#384
feat: add boot.kernel.sysctl and boot.kernelModules#384yuxqiu wants to merge 11 commits intonumtide:mainfrom
Conversation
e0412d6 to
86089ea
Compare
jfroche
left a comment
There was a problem hiding this comment.
Thanks for these new options. If you manage to use the upstream docker module based on this, do not hesitate to upstream the required changes to related module or just a test that validates that the module works well.
| { | ||
| boot = lib.mkOption { | ||
| type = lib.types.raw; | ||
| boot = { |
There was a problem hiding this comment.
The problems is that we now implement a subset of the boot options. It doesn't cost us a lot more to stub more of boot options here. Maybe we should just copying the options defined in https://github.com/NixOS/nixpkgs/blob/80bdc1e5ce51f56b19791b52b2901187931f5353/nixos/modules/system/boot/kernel.nix) ? I guess importing that upstream file directly would have been a problem ?
There was a problem hiding this comment.
Yeah, I think importing it directly wouldn't give us much benefit anyway as a lot of those options don't really make sense in our context, and we'd still need to define extra stubs to make everything work properly.
There was a problem hiding this comment.
Thank you for adding that feature ! Could you add an extra container test for that ?
There was a problem hiding this comment.
Yes, for sure. I've pushed a commit that adds an extra test.
That said, to merge this, we might also need to update the documentation. Previously, the docs stated that we define boot with type raw. I'm not sure exactly how it should be rephrased now, so I'll leave that to you if that's alright.
|
I made a few changes and added a VM test to really test the sysctl/module feature. We still have a failing test when trying to check if the module configuration has been applied. Haven't figure it out yet |
Thanks! It is indeed a bit weird. I've added two debug prints to the VM test and found that the |
After the apply function, config.boot.kernelModules is a list, not an attrset.
Wrap the systemd-modules-load drop-in in mkIf so it is only created when boot.kernelModules is non-empty.
Wrap the systemd-sysctl drop-in in mkIf to avoid referencing sysctl.d/60-nixos.conf when it does not exist.
Implement #382 and make
bootmore extensible. To maintain backward compatibility and ensure the tests pass,boot.kernelPackages.kernel.versionis also stubbed. Some potential side effects are: if users import other modules that access additionalbootattributes downstream and those are not stubbed in this PR, they will need to provide stub values underbootthemselves.