Update to docker 1.13.x #20656

vdemeester · 2016-11-23T16:41:15Z

Motivation for this change

docker 1.13.0 is going to be released next month (more or less). This shouldn't be merged before official release but gives time to review. This change the package quite a bit, so let me describe the changes :

There is more binaries now : docker, dockerd, docker-proxy, docker-runc, docker-containerd, docker-containerd-shim and docker-init — from 5 projects : docker/docker, docker/runc (to follow docker release), docker/containerd, docker-libnetwork and krallin/tini.
The package builds all the pieces itself instead of depending to other packages like runc and containerd. This makes sure we have the same binary (from the same git commit) than the official release and thus have no weird bugs because of version mismatch.
The man pages of docker are in the process of beeing automatically generate so there is some missing man page in the current package. This aims to fix that as well (still working on it).
This adds a experimental options to the docker service too – to start docker with experimental features (using --experimental).

There is still a few things to do :

~~[ ] Fix missing man pages~~ Currently, part of the manpages are generated in a docker container. I have no clean way to build manpage otherwise for now.. 😓

Handle version and build time better (docker version and docker info)

 containerd version:  (expected: 03e5862ec0d8d3b3f750e19fca3ee367e13c090e)
 runc version: N/A (expected: 51371867a01c467f08af739783b8beafc154c4d7)
 init version: N/A (expected: 949e6facb77383876aeff8a6944dde66b3089574)

Update the package to 1.13.0 when released (and on each rc too)

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

Update docker version to 1.13
Build containerd, runc, tini, docker-proxy from the right
version (little duplicated from conf in docker scripts)

mention-bot · 2016-11-23T16:41:17Z

@vdemeester, thanks for your PR! By analyzing the history of the files in this pull request, we identified @offlinehacker, @NeQuissimus and @benley to be potential reviewers.

vdemeester · 2016-11-23T16:43:43Z

pkgs/applications/virtualization/docker/default.nix

  DOCKER_BUILDTAGS = []
    ++ optional (systemd != null) [ "journald" ]
    ++ optional (btrfs-progs == null) "exclude_graphdriver_btrfs"
    ++ optional (devicemapper == null) "exclude_graphdriver_devicemapper";

+    postUnpack = ''
+    cp -fR ${dockerSrc} docker


I did get errors with chmod… I'm not used to multiple sources build so I hacked around for that… There might be a better solution 👼

vdemeester · 2016-11-23T16:46:18Z

pkgs/applications/virtualization/docker/default.nix

-, go, containerd, runc
-, sqlite, iproute, bridge-utils, devicemapper, systemd
+{ stdenv, lib, fetchFromGitHub, makeWrapper, cmake, pkgconfig
+, go-md2man, libapparmor, apparmor-parser, libseccomp, git


runc needs git (for the commit hash I guess). I really wish there was another way like what we do for docker/docker but it doesn't seem so…

offlinehacker · 2016-11-24T07:13:21Z

But we have separated packages for runc and containerd, why did you put them all into docker nix expression? You can update runc and containerd, and use closest stable release, the same as docker team should do. Even if exact "versions" are needed, the runc and containerd packages can be overrided and overrides can be put in all-package.nix, or if that looks kinda ugly, they can be also put into docker expression, but i would really like to reuse packages as much as possible. Also spearate package should be made for libnetwork and for tiny

vdemeester · 2016-11-24T09:47:26Z

But we have separated packages for runs and containers, why did you put them all into docker nix expression?

So about that, there is a few reasons why I made that change.

For each docker release, it depends on specific version of containerd, runc, etc.. If you're not running the right version of it, you might encounter weird bugs and it might be tricky to debug or get help/support because of that.

Although package containerd and runc on their own is a good idea — because they can be used without docker — I'm don't really like the way it's currently done :

containerd and runc are copied in docker-runc and docker-containerd/docker-containerd-shim and packaged in the final docker package — this feels weird to me. Going on that path, it would make more sence to not do that (and not including docker-runc and docker-containerd* in the package), and use daemon flags to tell docker where to find the runtime and the containerd socket).
(I might be wrong on that one) as far as I can tell, I cannot pin a certain version of runc and containerd for this specific package. Let say I update runc and containerd to the right version (and I think I will, probably in separate PRs) and I depend on it. As containerd and runc are separate project, they could be released differently (most often, …). They could (sholud?) be updated separately from docker in nixpkgs (especially runc which is an oci project). And thus, I have no guarantee that the docker-runc and docker-containerd for this specific version of docker will stay the same and the one that work and should be shipped with docker. And then again, you could run into really weird bugs.

You can update runc and containerd, and use latest stable release. If docker's release process is bad, it does not mean we have to do equal.

I don't think there is good or bad release process. See this (packaging) as an embedded use of runc, containerd, etc..

offlinehacker · 2016-11-24T21:45:57Z

Please look on GitHub, I have rephrased my comment a bit, first it was written in a rush(and GitHub does not send emails for updates). Sorry for that.

…

On Thu, Nov 24, 2016, 10:47 AM Vincent Demeester ***@***.***> wrote: But we have separated packages for runs and containers, why did you put them all into docker nix expression? So about that, there is a few reasons why I made that change. For each docker release, it depends on specific version of containerd, runc, etc.. If you're not running the right version of it, you might encounter weird bugs and it might be tricky to debug or get help/support because of that. Although package containerd and runc on their own is a good idea — because they can be used without docker — I'm don't really like the way it's currently done : - containerd and runc are *copied* in docker-runc and docker-containerd /docker-containerd-shim and packaged in the final docker package — this feels weird to me. Going on that path, it would make more sence to not do that (and not including docker-runc and docker-containerd* in the package), and use daemon flags to tell docker where to find the runtime and the containerd socket). - (*I might be wrong on that one*) as far as I can tell, I cannot *pin* a certain version of runc and containerd for this specific package. Let say I update runc and containerd to the right version (and I think I will, probably in separate PRs) and I depend on it. As containerd and runc are separate project, they could be released differently (most often, …). They could (sholud?) be updated separately from docker in nixpkgs (especially runc which is an oci project). And thus, I have no guarantee that the docker-runc and docker-containerd for this specific version of docker will stay the same and the one that work and should be shipped with docker. And then again, you could run into really weird bugs. You can update runc and containerd, and use latest stable release. If docker's release process is bad, it does not mean we have to do equal. I don't think there is good or bad release process. See this (packaging) as an embedded use of runc, containerd, etc.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20656 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjvS6kQ-QabuIDTrW8a46wbcyHtNHIPks5rBV0ygaJpZM4K6zkA> .

offlinehacker · 2016-11-24T21:57:24Z

The one other thing I forgot to mention, if overriding derivations does not work(look overrideDerivation), we can ship multiple versions of runc and containerd(copy expressions, with a new name, expose in all-packages.nix), and name them differently like we have for many packages(nodejs as one example). We could name them for example runc-docker and containerd-docker.

…

On Thu, Nov 24, 2016, 10:45 PM Jaka Hudoklin ***@***.***> wrote: Please look on GitHub, I have rephrased my comment a bit, first it was written in a rush(and GitHub does not send emails for updates). Sorry for that. On Thu, Nov 24, 2016, 10:47 AM Vincent Demeester ***@***.***> wrote: But we have separated packages for runs and containers, why did you put them all into docker nix expression? So about that, there is a few reasons why I made that change. For each docker release, it depends on specific version of containerd, runc, etc.. If you're not running the right version of it, you might encounter weird bugs and it might be tricky to debug or get help/support because of that. Although package containerd and runc on their own is a good idea — because they can be used without docker — I'm don't really like the way it's currently done : - containerd and runc are *copied* in docker-runc and docker-containerd /docker-containerd-shim and packaged in the final docker package — this feels weird to me. Going on that path, it would make more sence to not do that (and not including docker-runc and docker-containerd* in the package), and use daemon flags to tell docker where to find the runtime and the containerd socket). - (*I might be wrong on that one*) as far as I can tell, I cannot *pin* a certain version of runc and containerd for this specific package. Let say I update runc and containerd to the right version (and I think I will, probably in separate PRs) and I depend on it. As containerd and runc are separate project, they could be released differently (most often, …). They could (sholud?) be updated separately from docker in nixpkgs (especially runc which is an oci project). And thus, I have no guarantee that the docker-runc and docker-containerd for this specific version of docker will stay the same and the one that work and should be shipped with docker. And then again, you could run into really weird bugs. You can update runc and containerd, and use latest stable release. If docker's release process is bad, it does not mean we have to do equal. I don't think there is good or bad release process. See this (packaging) as an embedded use of runc, containerd, etc.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20656 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjvS6kQ-QabuIDTrW8a46wbcyHtNHIPks5rBV0ygaJpZM4K6zkA> .

vdemeester · 2016-11-25T08:42:41Z

@offlinehacker ah, I think I see what you mean. I'll try to update accordingly soon 😉

vdemeester · 2016-12-06T11:09:05Z

Alright, RC3 is out, I'll update this 👼

vdemeester · 2016-12-06T13:50:47Z

Updated with 1.13.0-rc3 and using overrideDerivation instead, introducing docker-proxy.
I also update tini and make sure we can build tini-static. I'm not sure it's the right way to do it though ?

offlinehacker · 2016-12-06T16:01:39Z

pkgs/applications/virtualization/docker/proxy.nix

+  '';
+
+  meta = {
+    description = "Docker garbage collection of containers and images";


Please fix meta data

Oh wow, I guess I was a little bit tired 😂

offlinehacker · 2016-12-06T16:02:57Z

pkgs/applications/virtualization/docker/default.nix

+      sha256 = "0zj4kdis1vvc6dwn4gplqna0bs7v6d1y2zc8v80s3zi018inhznw";
+    };
+
+    patchPhase = ''


Why is this needed?

In tini the cmake file is patch to remove static from it. But in the case of docker, tini (docker-init in the end) have to be static. This is to override the patchPhase — wasn't sure it was the right move to it though.

offlinehacker · 2016-12-06T16:04:33Z

pkgs/applications/virtualization/docker/proxy.nix

+
+with lib;
+
+stdenv.mkDerivation rec {


Can you use buildGoPackage instead? I'm ok with merging as is, but can be change somewhere in the future.

Oh right, I'll do that 👼

offlinehacker · 2016-12-07T01:20:23Z

Besides comnents above, looks good to me :)

vdemeester · 2016-12-07T10:32:37Z

@offlinehacker thanks 👍 I still need to fix the manpages (as they are incompletely generate for now) and then we'll just have to wait for the release 👼

offlinehacker · 2016-12-07T23:25:04Z

Nice :)

vdemeester · 2017-01-02T17:33:03Z

Updated to 1.13.0-rc4… I'll update for 1.13.0-rc5 this week.
Man page is gonna be a little bit tricky 😅 Gonna need some go library in build input as part of the man are generated by a go program..

aneeshusa · 2017-01-03T22:37:36Z

overrideAttrs is a new alternative to overrideDerivation which is now preferred to use.

- Update docker version to 1.13.0. - Introduce now docker-proxy package (from libnetmork). - Use overrideDerivation to set the correct version for docker. - Update tini to make sure we can build it static. Signed-off-by: Vincent Demeester <vincent@sbr.pm>

phanimahesh · 2017-01-21T09:14:46Z

docker 1.13 has been released few days ago. I just started locally patching the derivation and found this when I was halfway.

@vdemeester Will you be updatingthis to use buildGoPackage instead of mkDerivation?
@offlinehacker Any further comments or is this LGTM?

offlinehacker · 2017-01-21T11:18:45Z

LGTM

vdemeester commented Nov 23, 2016

View reviewed changes

vdemeester force-pushed the docker_1_13 branch 3 times, most recently from 1a84108 to 9e36c54 Compare November 23, 2016 16:58

pSub added the 8.has: package (update) label Nov 28, 2016

vdemeester force-pushed the docker_1_13 branch from 9e36c54 to 8a6a2f0 Compare December 6, 2016 13:48

vdemeester force-pushed the docker_1_13 branch 3 times, most recently from e1b49a1 to 9c73079 Compare December 6, 2016 14:12

offlinehacker reviewed Dec 7, 2016

View reviewed changes

vdemeester force-pushed the docker_1_13 branch 3 times, most recently from d4c0bdf to 5f7082b Compare December 31, 2016 09:34

vdemeester force-pushed the docker_1_13 branch 3 times, most recently from 46390dc to 8e3e656 Compare January 17, 2017 14:49

vdemeester force-pushed the docker_1_13 branch from 8e3e656 to 690b4bd Compare January 17, 2017 15:03

docker: 1.12.6 -> 1.13.0

74d4d3e

- Update docker version to 1.13.0. - Introduce now docker-proxy package (from libnetmork). - Use overrideDerivation to set the correct version for docker. - Update tini to make sure we can build it static. Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester force-pushed the docker_1_13 branch from 690b4bd to 74d4d3e Compare January 18, 2017 20:34

vdemeester changed the title ~~WIP: Update to docker 1.13.x~~ Update to docker 1.13.x Jan 19, 2017

offlinehacker merged commit 4884fa4 into NixOS:master Jan 21, 2017

vdemeester deleted the docker_1_13 branch January 21, 2017 12:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to docker 1.13.x #20656

Update to docker 1.13.x #20656

vdemeester commented Nov 23, 2016 •

edited

mention-bot commented Nov 23, 2016

vdemeester Nov 23, 2016

vdemeester Nov 23, 2016

offlinehacker commented Nov 24, 2016 •

edited

vdemeester commented Nov 24, 2016

offlinehacker commented Nov 24, 2016 via email

offlinehacker commented Nov 24, 2016 via email

vdemeester commented Nov 25, 2016

vdemeester commented Dec 6, 2016

vdemeester commented Dec 6, 2016

offlinehacker Dec 6, 2016

vdemeester Dec 7, 2016

offlinehacker Dec 6, 2016

vdemeester Dec 7, 2016

offlinehacker Dec 6, 2016

vdemeester Dec 7, 2016

offlinehacker commented Dec 7, 2016

vdemeester commented Dec 7, 2016

offlinehacker commented Dec 7, 2016

vdemeester commented Jan 2, 2017

aneeshusa commented Jan 3, 2017

phanimahesh commented Jan 21, 2017

offlinehacker commented Jan 21, 2017

Update to docker 1.13.x #20656

Update to docker 1.13.x #20656

Conversation

vdemeester commented Nov 23, 2016 • edited

Motivation for this change

Things done

mention-bot commented Nov 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

offlinehacker commented Nov 24, 2016 • edited

vdemeester commented Nov 24, 2016

offlinehacker commented Nov 24, 2016 via email

offlinehacker commented Nov 24, 2016 via email

vdemeester commented Nov 25, 2016

vdemeester commented Dec 6, 2016

vdemeester commented Dec 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

offlinehacker commented Dec 7, 2016

vdemeester commented Dec 7, 2016

offlinehacker commented Dec 7, 2016

vdemeester commented Jan 2, 2017

aneeshusa commented Jan 3, 2017

phanimahesh commented Jan 21, 2017

offlinehacker commented Jan 21, 2017

vdemeester commented Nov 23, 2016 •

edited

offlinehacker commented Nov 24, 2016 •

edited