-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Add cri-o service to modules #68153
Add cri-o service to modules #68153
Conversation
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 😻
2bc5fce
to
22f1081
Compare
I made everything more configurable and added a first network config. |
0bc0c32
to
c5c2c84
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/kubernetes-the-nixos-module-of-the-future/3922/6 |
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.
Even better ❤️
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.
👍
Type = "notify"; | ||
ExecStart = "${pkgs.cri-o}/bin/crio"; | ||
ExecReload = "/bin/kill -s HUP $MAINPID"; | ||
Environment= "PATH=/run/current-system/sw/bin:$PATH"; |
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.
Is it necessary to add all system executables? This might mean that the service could work on some system configurations and not others.
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.
In which case would it not work? CRI-O needs some executables like conmon
and runc
in $PATH
, so I thought this would be an usual way to solve 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.
You can use
systemd.services.<name>.path
Packages added to the service's PATH environment variable. Both the bin and sbin subdirectories of each package are added.
Type: unspecified
Default: [ ]
for that.
e.g.
systemd.services.crio = {
path = [ pkgs.conmon pkgs.runc ];
};
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.
Thanks, that's nice. I also had to add utillinux
for nsenter
.
2fca7bd
to
eff4b94
Compare
Signed-off-by: Sascha Grunert <sgrunert@suse.com>
eff4b94
to
4e24af1
Compare
@zimbatm PTAL 😇 |
manage_network_ns_lifecycle = true | ||
''; | ||
environment.etc."containers/policy.json".text = '' | ||
{"default": [{"type": "insecureAcceptAnything"}]} |
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 know nothing about cri-o but it just sounds a bit scary to me on first sight? Is this the correct default?
Please ignore me if this is a stupid question
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.
Thanks for pointing that out, since I think it is important. For now I would leave it as it is, see the documentation here: https://github.com/containers/image/blob/master/docs/containers-policy.json.5.md#insecureacceptanything
What do you think?
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.
Seems like good default. It will behave just like docker where images can be pulled left and right. Let's start like this and then add a new nixos option later, if there is demand to configure it.
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.
At some point, we want to move this line to a virtualization.runc
module, as for example podman
will try to look for this file too, as both podman, cri-o and many other CRI implementations use /etc/containers
and conmon
for tasks. But we can do that once we try to package those. We should just not forget to do that at some point
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.
Other distributions provide a dedicated package for these configuration files, do we want to do this as well? Because the policy file is not directly related to runc and other runtimes could be used with CRI-O as well (like kata containers).
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.
Yes we can create a etc.containers
nixos module and then have all the other modules depend on it.
Signed-off-by: Sascha Grunert <sgrunert@suse.com> (cherry picked from commit 2c3dcbb)
Motivation for this change
We need a CRI-O service definition to be able to run it via systemd.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @vdemeester can you please have a look? I would like to push the topic of replacing docker with CRI-O forward in terms of the Kubernetes module.