Re: Bug #909644: docker.io: dockerd warning: failed to retrieve docker-runc version: unknown output format: runc version spec: 1.0.1
Hi Arnaud,
Thanks for the summary.
On Wed, Mar 6, 2019 at 9:51 AM Arnaud Rebillout
<arnaud.rebillout@collabora.com> wrote:
>
> Dear Go team,
>
> So let met sum up this bug now to avoid you reading the backlog:
>
> In short, docker doesn't recognize the output of `runc --version`, and
> then it misbehaves and flood the log forever. To be more accurate,
> docker is not happy because it wants to know the git commit that was
> used to build runc. This information may or may not be part of the
> output of `runc --version`: it depends on how runc was built. Actually,
> there are two fields, `version` and `gitcommit`, that can be passed
> optionally to runc during the build (as ldflags). Therefore the output
> of `runc --version` varies accordingly.
>
> So there's two ways out of that:
>
> **Solution 1**
>
> One is to patch docker, and fix their code so that it can handle various
> runc version outputs. This is the way chosen by buildroot for example:
>
> https://github.com/buildroot/buildroot/blob/master/package/docker-engine/0001-Fix-faulty-runc-version-commit-scrape.patch
>
> Note that this patch is not the definitive solution: it just allows
> docker to handle the output of runc *as built in buildroot*, and not in
> every case. If we go this way, we'll do something similar, supporting
> "debian's runc" output, and end up with a patch that has no chance to
> make it upstream.
>
> Maybe the core of the issue is that the `runc --version` output is
> difficult to parse properly, and that's why docker upstream don't want
> to bother fixing their code...
>
>
> **Solution 2**
>
> The other way is to modify the runc build so that we include the git
> commit. I've pushed such a change on a branch at:
>
> https://salsa.debian.org/go-team/packages/runc/commit/033a60b549b0935861c92bf4e5118d03ef443a8f
>
> It's actually not that bad, no patch involved, it's really just adding
> ldflags. The only downside I see is that there's an additional step when
> a maintainer wants to release a new upstream version: he has to look for
> the corresponding commit and add it to the file
> `debian/upstream-version-gitcommits`. I provided a helper script to do
> that, so it's really a small overhead.
I think the runc should be fixed.
But I don't like the patch you suggested. It's confused to user. If
you set the git commit to the upstream one, like
ccb5efd37fb7c86364786e9137e22948751de7ed for 1.0.0-rc6, the user would
think it's 1.0.0-rc6 indeed, but apparently it's not, it's 1.0.0-rc6
with CVE-2019-5736 patch.
So I would suggest to use the debian package version in the commit
field. More specifically:
diff --git a/debian/rules b/debian/rules
index 81df53b..0087b6b 100755
--- a/debian/rules
+++ b/debian/rules
@@ -5,7 +5,11 @@
export DH_GOPKG := github.com/opencontainers/runc
export DH_GOLANG_INSTALL_EXTRA := libcontainer/seccomp/fixtures
+
+include /usr/share/dpkg/pkg-info.mk
+
TAGS=apparmor seccomp selinux ambient
+LDFLAGS := -X main.version=$(DEB_VERSION_UPSTREAM) -X
main.gitCommit=$(DEB_VERSION)
%:
dh $@ --buildsystem=golang --with=golang --builddirectory=_build
@@ -33,7 +37,7 @@ override_dh_auto_configure:
# ln -svrf vendor/github.com/opencontainers/specs
_build/src/github.com/opencontainers/
override_dh_auto_build:
- dh_auto_build -- -tags "$(TAGS)"
+ dh_auto_build -- -tags "$(TAGS)" -ldflags "$(LDFLAGS)"
override_dh_auto_test:
DH_GOLANG_EXCLUDES="libcontainer/integration" \
>
>
> **What do you think?**
>
> I'm in favor of solution 2, it seems the path of least trouble, and is
> also a correct thing to do IMHO. I'd like to have this fixed for buster,
> as this issue has been opened for a long while.
>
> If you agree with solution 2 I'll need some DD to upload a new version
> of runc with this change (sorry to bother you...). Also this bug should
> be re-assigned to runc if we solve it in the runc package.
>
> For more details you can also look at upstream discussion:
> https://github.com/moby/moby/issues/38709
>
>
And we're late to fix this before hard freeze. If we want this fix
included in buster, we should ask release team to unblock.
--
Shengjing Zhu
Reply to: