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

docker: update service units from upstream #21326

Merged
merged 1 commit into from
Dec 23, 2016
Merged

docker: update service units from upstream #21326

merged 1 commit into from
Dec 23, 2016

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Dec 21, 2016

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

Motivation for this change
Things done

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 NixOS#11478 NixOS#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
@mention-bot
Copy link

@Mic92, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Fuuzetsu, @bennofs and @tailhook to be potential reviewers.

@Mic92
Copy link
Member Author

Mic92 commented Dec 21, 2016

ping also @offlinehacker

@Mic92
Copy link
Member Author

Mic92 commented Dec 21, 2016

A future TODO, would be to look into user namespaces: https://docs.docker.com/engine/reference/commandline/dockerd/#daemon-user-namespace-options

@jgillich
Copy link
Member

The purpose of #21303 was not to start Docker and all containers on boot, but to work around the fact that socketActivation breaks container networking (#11478). The enableOnBoot seems very redundant, what exactly happens when I set enableOnBoot = true AND socketActivation = true? I think we should just default socketActivation to false and if someone really really wants it, they can enable it.

@Mic92
Copy link
Member Author

Mic92 commented Dec 23, 2016

@jgillich read first paragraph, why i want to keep socket activation. If both are enabled, systemd already allocates the socket before starting any service. Unix socket connections can be established and are handled by docker as soon as it start up - same thing happens if docker is restarted (no connection lost). enableOnBoot finally starts docker as part of the multi-user.target to start container with property restart=always set. Socket activation has nothing to do container networking - if you take a look at the error message, it fails to load kernel modules for bridges and Nat, because it cannot find modprobe.
I am not sure if this is the reason why network setup failed because usually calling iptables or creating bridges automatically load the corresponding modules. For me it seems that most of the people reporting this issue actually ran into service timeouts, this is also addressed by this pull request.

@Mic92 Mic92 merged commit c23032a into NixOS:master Dec 23, 2016
@Mic92 Mic92 deleted the docker branch December 23, 2016 20:39
@ericsagnes
Copy link
Contributor

@Mic92

This change breaks existing configurations if virtualisation.docker.socketActivation is set without any instruction on what to do.
Consider to add a mkRemovedOptionModule or mkRenamedOptionModule entry for smoother upgrades.

@Mic92
Copy link
Member Author

Mic92 commented Jan 1, 2017

@ericsagnes done in ce99e34

@ericsagnes
Copy link
Contributor

Thank you!

@jgillich
Copy link
Member

jgillich commented Jan 2, 2017

@Mic92 Yea my last comment makes no sense at all, no idea what I was thinking. Great to see the socket issue finally being fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants