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.
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
nixos/lxd: add productionSetup option #71978
Conversation
nixos/modules/virtualisation/lxd.nix
Outdated
@@ -69,8 +81,9 @@ in | |||
ExecStart = "@${pkgs.lxd.bin}/bin/lxd lxd --group lxd"; | |||
Type = "simple"; | |||
KillMode = "process"; # when stopping, leave the containers alone | |||
LimitMEMLOCK = mkIf cfg.productionSetup "infinity"; | |||
LimitNOFILE = mkIf cfg.productionSetup "1048576"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these service settings should always be added (not only in production). The debian lxd packages seems to do that too https://github.com/lxc/lxd-pkg-ubuntu/blob/dpm-bionic/debian/lxd.service#L15.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've added some stuff from https://github.com/lxc/lxd-pkg-ubuntu/blob/dpm-xenial/debian/lxd.service, how does this look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I would only rename the option from productionSetup
to recommendedSysctlSettings
in the style of the recommended*
nginx options in nixos: https://nixos.org/nixos/options.html#nginx.reco
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@mt-caret I am not sure who we should be ping to review and merge this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Motivation for this change
Adds an option to fix errors that may come up for ZFS storage backend users.
See https://discuss.linuxcontainers.org/t/weird-error-for-zfs-storage-backend/6008/3 for details.
Things done
Tested on own machine.
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)