Skip to content
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

Merged
merged 1 commit into from Sep 21, 2019
Merged

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Sep 5, 2019

Motivation for this change

We need a CRI-O service definition to be able to run it via systemd.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
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.

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😻

@saschagrunert saschagrunert force-pushed the cri-o-service branch 7 times, most recently from 2bc5fce to 22f1081 Compare September 8, 2019 13:06
@saschagrunert
Copy link
Member Author

I made everything more configurable and added a first network config.

@saschagrunert saschagrunert force-pushed the cri-o-service branch 3 times, most recently from 0bc0c32 to c5c2c84 Compare September 8, 2019 13:24
@nixos-discourse
Copy link

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

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better ❤️

@lheckemann lheckemann added this to the 20.03 milestone Sep 10, 2019
Copy link
Member

@zimbatm zimbatm left a 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";
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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 ];
};

Copy link
Member Author

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.

@saschagrunert saschagrunert force-pushed the cri-o-service branch 3 times, most recently from 2fca7bd to eff4b94 Compare September 19, 2019 15:10
Signed-off-by: Sascha Grunert <sgrunert@suse.com>
@saschagrunert
Copy link
Member Author

@zimbatm PTAL 😇

manage_network_ns_lifecycle = true
'';
environment.etc."containers/policy.json".text = ''
{"default": [{"type": "insecureAcceptAnything"}]}
Copy link
Member

@arianvp arianvp Sep 19, 2019

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

Copy link
Member Author

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?

Copy link
Member

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.

Copy link
Member

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

Copy link
Member Author

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).

Copy link
Member

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.

@zimbatm zimbatm merged commit 2c3dcbb into NixOS:master Sep 21, 2019
@saschagrunert saschagrunert deleted the cri-o-service branch September 21, 2019 15:05
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Sep 21, 2019
Signed-off-by: Sascha Grunert <sgrunert@suse.com>
(cherry picked from commit 2c3dcbb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants