-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
docker-containers: Add enableOnBoot option #76480
Conversation
@@ -161,11 +161,20 @@ let | |||
["--network=host"] | |||
''; | |||
}; | |||
|
|||
enableOnBoot = mkOption { |
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.
maybe autoStart
(similar to https://nixos.org/nixos/options.html#containers.%3Cname%3E.autostart)
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 struggled with finding a good name for it (it's a programmers greatest enemy) and decided to follow https://nixos.org/nixos/options.html#virtualisation.docker.enableonboot in the end.
Thanks for pointing it out, I forgot to ask about the name in the PR.
Docker makes a distinction between container creation and container start, so an option called Docker natively supports starting containers on boot via So I think EDIT: the other options I named don't have to be included in this PR necessarily. |
I think @danbst's suggestion is what we should go with, it's unambiguous and matches what nixos-containers use. |
Interesting.
Sure, but should a user of this module care about these implementation details?
No it's not. The "Enable" is used in NixOS (a lot) to define things into existence (or inhibit their existence). Suppose I have a container for a service that I only run when a certain event happens. Certainly this container has to be "enabled": I want it to be downloaded, held onto by the system and to be ready to run. However, it should not be started on boot, because it's designed to be triggered by my event. |
|
Don't quote me out of context like that, the sentence only makes sense together with the one preceding it.
If it directly affects how their configuration option makes their system behave, yes. It should be documented however.
@danbst's suggestion contains none of those words.
A container being available to the system is implied when you declare it in your system configuration. |
I didn't mean to offend you and that was needlessly polarizing. We're on the same team :) (team Nix) I'm trying to help you with naming, as you've indicated that it needed attention.
Yes, always creating the container is a good idea (or good default*). What should be configurable is after which events to start it. (* An |
I'm not offended, no worries.
You're more than welcome to do and continue doing that as far as I'm concerned.
That's exactly right and made me think of a good point just now: This PR's option is only binary.
Might be a good idea to open a separate issue/PR for that option. |
d9a544e
to
ebff997
Compare
ebff997
to
1a6bb32
Compare
This option allows the user to control whether or not the docker container is automatically started on boot. The previous default behavior (true) is preserved
1a6bb32
to
5399f9b
Compare
@Atemu thanks, finally got hands to test this, works fine |
This option allows the user to control whether or not the docker container is automatically started on boot. The previous default behavior (true) is preserved (cherry picked from commit 08ac06e)
Motivation for this change
A user might want to declare a docker container that can be run on-demand but does not get started automatically on boot.
Resolves #73111.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @