Fix detection of default runc version due upstream to switch to go.mod by thaJeztah · Pull Request #224 · docker/containerd-packaging

thaJeztah · 2021-03-08T10:25:10Z

The code that was used to automatically select the default version of runc to use for containerd was still assuming the containerd code to have a vendor.conf.

Containerd "master", and v1.5 have switched to go modules, so should now look in the go.mod file to get the recommended / default version.

Also change the order in which we check out source-code, so that the containerd source-code is checked out before we determine the runc version; this allows us to always use the local source of containerd, without having to use curl to get the version from GitHub.

Finally, a small modification was made for the "make validate" target, which now uses a generic "alpine" Golang image, so that validation can be done before the containerd source code was checked out (which was used to detect the Go version to use).

Before this patch:

$ make docker.io/library/ubuntu:focal
...
curl: (22) The requested URL returned error: 404 Not Found
--------------------------------------------------------------------
Building packages on docker.io/library/ubuntu:focal

containerd   : HEAD (commit: a72fe7d)
runc         :  (commit: 59ad417)
architecture : x86_64
build image  : docker.io/library/ubuntu:focal
golang image : docker.io/library/golang:1.15.8-buster

With this patch:

$ make docker.io/library/ubuntu:focal
...
Building packages on docker.io/library/ubuntu:focal

containerd   : HEAD (commit: a72fe7d)
runc         : v1.0.0-rc93 (commit: 59ad417)
architecture : x86_64
build image  : docker.io/library/ubuntu:focal
golang image : docker.io/library/golang:1.15.8-buster

thaJeztah · 2021-03-08T10:27:49Z

Hmm... looks like make validate now fails; let me fix that

+ make validate
grep: src/github.com/containerd/containerd/contrib/Dockerfile.test: No such file or directory
docker run --rm -v /home/ubuntu/workspace/containerd-packaging_PR-224:/work -w /work docker.io/library/golang:-buster bash -c 'go get -u github.com/kunalkushwaha/ltag && ./scripts/validate/fileheader'
docker: invalid reference format.
See 'docker run --help'.
Makefile:113: recipe for target 'validate' failed
make: *** [validate] Error 125
script returned exit code 2

thaJeztah · 2021-03-08T10:47:05Z

Fixed the make validate step to no longer depend on the containerd source code, and instead just use golang:alpine

thaJeztah · 2021-03-08T10:49:55Z

Looks good now; https://ci-next.docker.com/teams-core/blue/rest/organizations/jenkins/pipelines/containerd-packaging/branches/PR-224/runs/2/nodes/88/steps/605/log/?start=0

+ git -C src/github.com/opencontainers/runc checkout -q FETCH_HEAD
# upstream systemd unit uses /usr/local/bin, whereas our packages use /usr/bin
sed 's#/usr/local/bin/containerd#/usr/bin/containerd#g' src/github.com/containerd/containerd/containerd.service > common/containerd.service
--------------------------------------------------------------------
Building packages on docker.io/library/ubuntu:focal

containerd   : HEAD (commit: a72fe7d)
runc         : v1.0.0-rc93 (commit: 59ad417)
architecture : aarch64
build image  : docker.io/library/ubuntu:focal
golang image : docker.io/library/golang:1.15.8-buster
--------------------------------------------------------------------
focal: Pulling from library/ubuntu

thaJeztah · 2021-03-08T11:37:16Z

@silvin-lubecki @fuweid @StefanScherer @tianon @tiborvass ptal

note that we'll have to update this script once containerd/containerd#5036 has been accepted upstreeam

thaJeztah · 2021-03-08T11:38:18Z

+	fi
+
+	# shellcheck disable=SC2164
+	repo_abspath="$(cd -- "$(dirname "$0")/.." >/dev/null 2>&1 ; pwd -P)"


don't know if there's a cleaner way; this variant seems to be portable (as in; it works on macOS as well, which doesn't support readlink -f)

Just curiosity about what -- is used for. look likes repo_abspath="$(cd "$(dirname "$0")/.." >/dev/null 2>&1 ; pwd -P)" can work.

Good point; TBH I was testing some variations that didn't work on macOS, and found this one, which did work, but was also wondering why the -- was there, and forgot to test without; let me try

Looks like it's the preferred way to do this (to be most portable); there's some discussion in https://go-review.googlesource.com/c/sys/+/210157/, which at some point suggests adding the -- instead of skipping it.

silvin-lubecki

Looks good 👍

fuweid

LGTM(nb)

The code that was used to automatically select the default version of runc to use for containerd was still assuming the containerd code to have a vendor.conf. Containerd "master", and v1.5 have switched to go modules, so should now look in the go.mod file to get the recommended / default version. Also change the order in which we check out source-code, so that the containerd source-code is checked out before we determine the runc version; this allows us to always use the local source of containerd, without having to use curl to get the version from GitHub. Finally, a small modification was made for the "make validate" target, which now uses a generic "alpine" Golang image, so that validation can be done before the containerd source code was checked out (which was used to detect the Go version to use). Before this patch: $ make docker.io/library/ubuntu:focal ... curl: (22) The requested URL returned error: 404 Not Found -------------------------------------------------------------------- Building packages on docker.io/library/ubuntu:focal containerd : HEAD (commit: a72fe7d) runc : (commit: 59ad417) architecture : x86_64 build image : docker.io/library/ubuntu:focal golang image : docker.io/library/golang:1.15.8-buster With this patch: $ make docker.io/library/ubuntu:focal INFO: detected runc version from script/setup/runc-version ... Building packages on docker.io/library/ubuntu:focal containerd : HEAD (commit: a72fe7d) runc : v1.0.0-rc93 (commit: 59ad417) architecture : x86_64 build image : docker.io/library/ubuntu:focal golang image : docker.io/library/golang:1.15.8-buster Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2021-03-20T14:48:24Z

Updated to take the changes from upstream containerd/containerd#5036 into account

silvin-lubecki

LGTM

thaJeztah mentioned this pull request Mar 8, 2021

Add changelog for containerd v1.4.4-1 #223

Merged

thaJeztah force-pushed the fix_runc_version_select branch from c9a3ea9 to 7e78bf1 Compare March 8, 2021 10:45

thaJeztah force-pushed the fix_runc_version_select branch from 7e78bf1 to 330ff9a Compare March 8, 2021 11:14

thaJeztah commented Mar 8, 2021

View reviewed changes

fuweid reviewed Mar 8, 2021

View reviewed changes

Comment thread common/common.mk

thaJeztah force-pushed the fix_runc_version_select branch from 330ff9a to 2f5633a Compare March 8, 2021 15:05

silvin-lubecki previously approved these changes Mar 8, 2021

View reviewed changes

fuweid previously approved these changes Mar 9, 2021

View reviewed changes

thaJeztah dismissed stale reviews from fuweid and silvin-lubecki via 6a2e254 March 17, 2021 11:50

thaJeztah force-pushed the fix_runc_version_select branch from 2f5633a to 6a2e254 Compare March 17, 2021 11:50

silvin-lubecki approved these changes Mar 22, 2021

View reviewed changes

tiborvass approved these changes Mar 23, 2021

View reviewed changes

tiborvass merged commit 5627b67 into docker:master Mar 23, 2021

thaJeztah deleted the fix_runc_version_select branch March 23, 2021 21:35

Uh oh!

Conversation

thaJeztah commented Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Mar 8, 2021

Uh oh!

thaJeztah commented Mar 8, 2021

Uh oh!

thaJeztah commented Mar 8, 2021

Uh oh!

thaJeztah commented Mar 8, 2021

Uh oh!

thaJeztah Mar 8, 2021

Choose a reason for hiding this comment

Uh oh!

fuweid Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah Mar 8, 2021

Choose a reason for hiding this comment

Uh oh!

thaJeztah Mar 8, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

silvin-lubecki left a comment

Choose a reason for hiding this comment

Uh oh!

fuweid left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Mar 20, 2021

Uh oh!

silvin-lubecki left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

thaJeztah commented Mar 8, 2021 •

edited

Loading

fuweid Mar 8, 2021 •

edited

Loading