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

Reloadable containers #28465

Merged
merged 2 commits into from Aug 30, 2017
Merged

Reloadable containers #28465

merged 2 commits into from Aug 30, 2017

Conversation

danbst
Copy link
Contributor

@danbst danbst commented Aug 22, 2017

Motivation for this change

This makes declarative containers truly reloadable (container config changes are applied on nixos-rebuild switch). Current code already declares it:

restartIfChanged = false;

reloadIfChanged = true;

Original author: @chrisfarms in 6e36619
Most of stuff from that commit has already been ported.

Details

There is indeed a bug in how switch-to-configuraiton.pl detects template instances. That's why systemd units for containers were never considered as changed and so never where reloaded.

This obviously doesn't do anything to imperative containers.

Pitfalls

In current state containers suffer from the long-running problem - when to restart and when to reload. Because this change makes containers "reloadable-only" (was "never restart, never reload"), it imposes several problems:

  • when you change bind-mounts container won't be restarted to reflect that
  • network can go down in containers ("nixos-rebuild switch" kills container networking 2 #26342), so restart required
  • journalctl won't show container reload logs without -M $container_name_here
  • it makes activation phase slower when there are a lot of containers
Things done

[x] Test in production
[x] Nixos test for this stuff
[ ] Release notes update

…r template instances

This makes declarative containers truly reloadable. Current code already declares it:

https://github.com/NixOS/nixpkgs/blob/56904d7c423f2b13b37fbd29f39bbb4b52bc7824/nixos/modules/virtualisation/containers.nix#L488

```
  restartIfChanged = false;
```

https://github.com/NixOS/nixpkgs/blob/56904d7c423f2b13b37fbd29f39bbb4b52bc7824/nixos/modules/virtualisation/containers.nix#L540

```
  reloadIfChanged = true;
```

Original author: @chrisfarms in NixOS@6e36619
Most of stuff from that commit has already been ported.
…ig changes

are applied on `nixos-rebuild switch` invocations.
@danbst
Copy link
Contributor Author

danbst commented Aug 23, 2017

cc @volth @kampfschlaefer @fpletz who run containers

I'm not going to address "pitfalls" problems in this PR. I'd rather say that current PR is a compromise of usefulness VS correctness.

Also, pitfalls 1 and 2 are present already, so the only real problem is slower activation time when many containers are running. It may also start failing, because of real problems in container configuration.

@globin @fpletz do you think this can go to 17.09?

@danbst danbst changed the title [WIP] Reloadable containers Reloadable containers Aug 23, 2017
@danbst
Copy link
Contributor Author

danbst commented Aug 24, 2017

So, I've done some research which templated services will be affected. Here is a list of all templated:

$ grep -r "@\"" ~/nixpkgs/nixos | grep -Ev "\"(@|\\$)" | grep -E '"[[:alpha:]]' | grep -v test-driver                                                                                      
/home/danbst/nixpkgs/nixos/tests/virtualbox.nix:      systemd.services."vboxtestlog-${name}@" = {                                                                                                               
/home/danbst/nixpkgs/nixos/modules/virtualisation/containers.nix:      [{ name = "container@"; value = unit; }]
/home/danbst/nixpkgs/nixos/modules/profiles/headless.nix:  systemd.services."autovt@".enable = false;
/home/danbst/nixpkgs/nixos/modules/tasks/swraid.nix:  systemd.services."mdmon@" = {
/home/danbst/nixpkgs/nixos/modules/tasks/swraid.nix:  systemd.services."mdadm-grow-continue@" = {
/home/danbst/nixpkgs/nixos/modules/services/networking/ssh/sshd.nix:        services."sshd@" = service;
/home/danbst/nixpkgs/nixos/modules/services/networking/websockify.nix:    systemd.services."websockify@" = {
/home/danbst/nixpkgs/nixos/modules/services/networking/spiped.nix:    systemd.services."spiped@" = {
/home/danbst/nixpkgs/nixos/modules/services/networking/supplicant.nix:  serviceName = iface: "supplicant-${if (iface=="WLAN") then "wlan@" else (
/home/danbst/nixpkgs/nixos/modules/services/networking/supplicant.nix:                                     if (iface=="LAN") then "lan@" else (
/home/danbst/nixpkgs/nixos/modules/services/hardware/sane.nix:      systemd.services."saned@" = {
/home/danbst/nixpkgs/nixos/modules/services/hardware/tlp.nix:      "systemd-rfkill@".enable = false;
/home/danbst/nixpkgs/nixos/modules/services/hardware/actkbd.nix:    systemd.services."actkbd@" = {
/home/danbst/nixpkgs/nixos/modules/services/x11/terminal-server.nix:    systemd.services."terminal-server@" =
/home/danbst/nixpkgs/nixos/modules/services/ttys/agetty.nix:    systemd.services."getty@" =
/home/danbst/nixpkgs/nixos/modules/services/ttys/agetty.nix:    systemd.services."serial-getty@" =
/home/danbst/nixpkgs/nixos/modules/services/ttys/agetty.nix:    systemd.services."container-getty@" =
/home/danbst/nixpkgs/nixos/modules/services/network-filesystems/u9fs.nix:      services."u9fs@" = {
/home/danbst/nixpkgs/nixos/modules/system/boot/systemd.nix:    systemd.services."systemd-backlight@".restartIfChanged = false;
/home/danbst/nixpkgs/nixos/modules/system/boot/systemd.nix:    systemd.services."systemd-fsck@".restartIfChanged = false;
/home/danbst/nixpkgs/nixos/modules/system/boot/systemd.nix:    systemd.services."systemd-fsck@".path = [ config.system.path ];
/home/danbst/nixpkgs/nixos/modules/system/boot/systemd.nix:    systemd.services."user@".restartIfChanged = false;
/home/danbst/nixpkgs/nixos/modules/system/boot/systemd-nspawn.nix:      systemd.services."systemd-nspawn@" = {
/home/danbst/nixpkgs/nixos/modules/system/boot/networkd.nix:    systemd.services."systemd-network-wait-online@" = {

and here is list of reloadable templated services:

$ grep -E "reloadIfChanged.*=" $(grep -r "@\"" ~/nixpkgs/nixos | grep -Ev "\"(@|\\$)" | grep -E '"[[:alpha:]]' | grep -v test-driver | cut -d: -f1)                                         
/home/danbst/nixpkgs/nixos/modules/virtualisation/containers.nix:              reloadIfChanged = true;                                                                                                          
/home/danbst/nixpkgs/nixos/modules/services/network-filesystems/u9fs.nix:        reloadIfChanged = true;  

That u9fs stuff is socket activated, so I don't know how to get it reloading in my synthetic tests.

@fpletz
Copy link
Member

fpletz commented Aug 30, 2017

Cool! Thanks a lot!

@fpletz fpletz merged commit 3e18f32 into NixOS:master Aug 30, 2017
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

2 participants