Fix detection of default runc version due upstream to switch to go.mod #224

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

scripts/determine-runc-version

+	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.

common/common.mk

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 <github@gone.nl>

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

common/common.mk Show resolved Hide resolved

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix detection of default runc version due upstream to switch to go.mod #224

Fix detection of default runc version due upstream to switch to go.mod #224

thaJeztah commented Mar 8, 2021 •

edited

Loading

thaJeztah commented Mar 8, 2021

thaJeztah commented Mar 8, 2021

thaJeztah commented Mar 8, 2021

thaJeztah commented Mar 8, 2021

thaJeztah Mar 8, 2021

fuweid Mar 8, 2021 •

edited

Loading

thaJeztah Mar 8, 2021

thaJeztah Mar 8, 2021

silvin-lubecki left a comment

fuweid left a comment

thaJeztah commented Mar 20, 2021

silvin-lubecki left a comment

Fix detection of default runc version due upstream to switch to go.mod #224

Fix detection of default runc version due upstream to switch to go.mod #224

Conversation

thaJeztah commented Mar 8, 2021 • edited Loading

thaJeztah commented Mar 8, 2021

thaJeztah commented Mar 8, 2021

thaJeztah commented Mar 8, 2021

thaJeztah commented Mar 8, 2021

thaJeztah Mar 8, 2021

Choose a reason for hiding this comment

fuweid Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

thaJeztah Mar 8, 2021

Choose a reason for hiding this comment

thaJeztah Mar 8, 2021

Choose a reason for hiding this comment

silvin-lubecki left a comment

Choose a reason for hiding this comment

fuweid left a comment

Choose a reason for hiding this comment

thaJeztah commented Mar 20, 2021

silvin-lubecki left a comment

Choose a reason for hiding this comment

thaJeztah commented Mar 8, 2021 •

edited

Loading

fuweid Mar 8, 2021 •

edited

Loading