docker: update service units from upstream

All the new options in detail:

Enable docker in multi-user.target make container created with restart=always
to start. We still want socket activation as it decouples dependencies between
the existing of /var/run/docker.sock and the docker daemon. This means that
services can rely on the availability of this socket. Fixes #11478 #21303

  wantedBy = ["multi-user.target"];

This allows us to remove the postStart hack, as docker reports on its own when
it is ready.

  Type=notify

The following will set unset some limits because overhead in kernel's ressource
accounting was observed. Note that these limit only apply to containerd.
Containers will have their own limit set.

  LimitNPROC=infinity
  LimitCORE=infinity
  TasksMax=infinity

Upgrades may require schema migrations. This can delay the startup of dockerd.

  TimeoutStartSec=0

Allows docker to create its own cgroup subhierarchy to apply ressource limits on
containers.

  Delegate=true

When dockerd is killed, container should be not affected to allow
`live restore` to work.

  KillMode=process

3 files changed, 56 insertions, 42 deletions

diff --git a/nixos/modules/services/web-servers/lighttpd/inginious.nix b/nixos/modules/services/web-servers/lighttpd/inginious.nix
index 43deccb6aef..669e81d0f14 100644
--- a/nixos/modules/services/web-servers/lighttpd/inginious.nix
+++ b/nixos/modules/services/web-servers/lighttpd/inginious.nix
@@ -191,9 +191,8 @@ in
         virtualisation.docker = {
           enable = true;
           # We need docker to listen on port 2375.
-          extraOptions = "-H tcp://127.0.0.1:2375 -H unix:///var/run/docker.sock";
+          listenOptions = ["127.0.0.1:2375" "/var/run/docker.sock"];
           storageDriver = mkDefault "overlay";
-          socketActivation = false;
         };
 
         users.extraUsers."lighttpd".extraGroups = [ "docker" ];
diff --git a/nixos/modules/virtualisation/docker.nix b/nixos/modules/virtualisation/docker.nix
index 92fe98f3f9c..8902799936c 100644
--- a/nixos/modules/virtualisation/docker.nix
+++ b/nixos/modules/virtualisation/docker.nix
@@ -28,16 +28,42 @@ in
             <command>docker</command> command line tool.
           '';
       };
-    socketActivation =
+
+    listenOptions =
+      mkOption {
+        type = types.listOf types.str;
+        default = ["/var/run/docker.sock"];
+        description =
+          ''
+            A list of unix and tcp docker should listen to. The format follows
+            ListenStream as described in systemd.socket(5).
+          '';
+      };
+
+    enableOnBoot =
       mkOption {
         type = types.bool;
         default = true;
         description =
           ''
-            This option enables docker with socket activation. I.e. docker will
-            start when first called by client.
+            When enabled dockerd is started on boot. This is required for
+            container, which are created with the
+            <literal>--restart=always</literal> flag, to work. If this option is
+            disabled, docker might be started on demand by socket activation.
           '';
       };
+
+    liveRestore =
+      mkOption {
+        type = types.bool;
+        default = true;
+        description =
+          ''
+            Allow dockerd to be restarted without affecting running container.
+            This option is incompatible with docker swarm.
+          '';
+      };
+
     storageDriver =
       mkOption {
         type = types.nullOr (types.enum ["aufs" "btrfs" "devicemapper" "overlay" "overlay2" "zfs"]);
@@ -69,24 +95,6 @@ in
             <command>docker</command> daemon.
           '';
       };
-
-    postStart =
-      mkOption {
-        type = types.lines;
-        default = ''
-          while ! [ -e /var/run/docker.sock ]; do
-            sleep 0.1
-          done
-        '';
-        description = ''
-          The postStart phase of the systemd service. You may need to
-          override this if you are passing in flags to docker which
-          don't cause the socket file to be created. This option is ignored
-          if socket activation is used.
-        '';
-      };
-
-
   };
 
   ###### implementation
@@ -94,44 +102,53 @@ in
   config = mkIf cfg.enable (mkMerge [
     { environment.systemPackages = [ pkgs.docker ];
       users.extraGroups.docker.gid = config.ids.gids.docker;
+      # this unit follows the one provided by upstream see: https://github.com/docker/docker/blob/master/contrib/init/systemd/docker.service
+      # comments below reflect experience from upstream.
       systemd.services.docker = {
         description = "Docker Application Container Engine";
-        wantedBy = optional (!cfg.socketActivation) "multi-user.target";
-        after = [ "network.target" ] ++ (optional cfg.socketActivation "docker.socket") ;
-        requires = optional cfg.socketActivation "docker.socket";
+        wantedBy = optional cfg.enableOnBoot "multi-user.target";
+        after = [ "network.target" "docker.socket" ];
+        requires = ["docker.socket"];
         serviceConfig = {
+          # the default is not to use systemd for cgroups because the delegate issues still
+          # exists and systemd currently does not support the cgroup feature set required
+          # for containers run by docker
           ExecStart = ''${pkgs.docker}/bin/dockerd \
-            --group=docker --log-driver=${cfg.logDriver} \
+            --group=docker \
+            --host=fd:// \
+            --log-driver=${cfg.logDriver} \
             ${optionalString (cfg.storageDriver != null) "--storage-driver=${cfg.storageDriver}"} \
-            ${optionalString cfg.socketActivation "--host=fd://"} \
+            ${optionalString cfg.liveRestore "--live-restore" } \
             ${cfg.extraOptions}
           '';
-          #  I'm not sure if that limits aren't too high, but it's what
-          #  goes in config bundled with docker itself
+          Type="notify";
+          ExecReload="${pkgs.procps}/bin/kill -s HUP $MAINPID";
           LimitNOFILE = 1048576;
-          LimitNPROC = 1048576;
+          # Having non-zero Limit*s causes performance problems due to accounting overhead
+          # in the kernel. We recommend using cgroups to do container-local accounting.
+          LimitNPROC="infinity";
+          LimitCORE="infinity";
+          TasksMax="infinity";
+          TimeoutStartSec=0;
+          # set delegate yes so that systemd does not reset the cgroups of docker containers
+          Delegate="yes";
+          # kill only the docker process, not all processes in the cgroup
+          KillMode="process";
         } // proxy_env;
 
         path = [ pkgs.kmod ] ++ (optional (cfg.storageDriver == "zfs") pkgs.zfs);
-
-        postStart = if cfg.socketActivation then "" else cfg.postStart;
-
-        # Presumably some containers are running we don't want to interrupt
-        restartIfChanged = false;
       };
-    }
-    (mkIf cfg.socketActivation {
       systemd.sockets.docker = {
         description = "Docker Socket for the API";
         wantedBy = [ "sockets.target" ];
         socketConfig = {
-          ListenStream = "/var/run/docker.sock";
+          ListenStream = cfg.listenOptions;
           SocketMode = "0660";
           SocketUser = "root";
           SocketGroup = "docker";
         };
       };
-    })
+    }
   ]);
 
 }
diff --git a/nixos/tests/docker-registry.nix b/nixos/tests/docker-registry.nix
index df24686aba8..109fca440e5 100644
--- a/nixos/tests/docker-registry.nix
+++ b/nixos/tests/docker-registry.nix
@@ -16,13 +16,11 @@ import ./make-test.nix ({ pkgs, ...} : {
 
     client1 = { config, pkgs, ...}: {
       virtualisation.docker.enable = true;
-      virtualisation.docker.socketActivation = false;
       virtualisation.docker.extraOptions = "--insecure-registry registry:8080";
     };
 
     client2 = { config, pkgs, ...}: {
       virtualisation.docker.enable = true;
-      virtualisation.docker.socketActivation = false;
       virtualisation.docker.extraOptions = "--insecure-registry registry:8080";
     };
   };