update slirp4netns (1.3.2), containerd (2.0.4), runc (1.2.6)...; verify the git tags with the commit hashes #4017
Conversation
35895dc to
7b931ef
Compare
There was a problem hiding this comment.
Checking if we can have a new release of BuildKit soon as well:
…hashes Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
| ;; | ||
| esac | ||
|
|
||
| "$GIT" checkout "$TAG" |
There was a problem hiding this comment.
Could be a separate PR, but we should:
- quiet the output
- consider shallow clones instead of full checkouts (listed in [CI] Epic: possible enhancements to current Dockerfile #4021).
eg: git clone --quiet --depth 1 --branch "$VERSION"
There was a problem hiding this comment.
LGTM - but if you feel like doing it in this PR, suggesting we do quiet and shallow clone.
There was a problem hiding this comment.
Cloning is out of the scope of this script.
Probably dockerfile's ADD git:// instruction should be used for cache optimality
There was a problem hiding this comment.
Sure, we can discuss separately.
re: ADD.
I do not know what ADD git:// does under the hood. Is it still only on dockerfile/labs or generally available? Also, need to check what it does wrt quiet and --depth.
There was a problem hiding this comment.
Cloning is out of the scope of this script.
To clarify: there should be no need for a separate checkout operation at all, which is why I am bringing it up here.
There was a problem hiding this comment.
git clone --quiet --depth 1 --branch "$VERSION"
LGTM, the cmd is sandboxed in the ci container, no cache is left and no need to fetch the git entire history of the the tag.
There was a problem hiding this comment.
ADD git:// is available
since 2022 (moby/buildkit#2799) and out of experimental since 2023 (moby/buildkit#3979)
https://docs.docker.com/reference/dockerfile/#adding-files-from-a-git-repository
This might be more cache-efficient when running tests with several containerd versions
There was a problem hiding this comment.
Moving that part of the conversation to #4022
| ;; | ||
| esac | ||
|
|
||
| "$GIT" checkout "$TAG" |
There was a problem hiding this comment.
git clone --quiet --depth 1 --branch "$VERSION"
LGTM, the cmd is sandboxed in the ci container, no cache is left and no need to fetch the git entire history of the the tag.
| else | ||
| if [ "$HEAD" != "$HASH" ]; then | ||
| echo >&2 "ERROR: ${TAG}: expected ${HASH}, got ${HEAD}" | ||
| exit 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
shallow fetch makes the else block useless. The following cond is always true:
"$HEAD" == "$HASH"
There was a problem hiding this comment.
Doesn't seem so?
$ git clone --depth 1 --branch v2.0.4 https://github.com/containerd/containerd
$ cd containerd
$ ~/gopath/src/github.com/containerd/nerdctl/hack/git-checkout-tag-with-hash.sh v2.0.4@wrong-hash
HEAD is now at 1a43cb6 Merge commit from fork
ERROR: v2.0.4: expected wrong-hash, got 1a43cb6a1035441f9aca8f5666a9b3ef9e70ab20| # @BINARY: the binary checksums are verified via Dockerfile.d/SHA256SUMS.d/<COMPONENT>-<VERSION> | ||
| ARG CONTAINERD_VERSION=v2.0.4@1a43cb6a1035441f9aca8f5666a9b3ef9e70ab20 | ||
| ARG RUNC_VERSION=v1.2.6@e89a29929c775025419ab0d218a43588b4c12b9a | ||
| ARG CNI_PLUGINS_VERSION=v1.6.2@BINARY |
There was a problem hiding this comment.
(not blocker) a comment to mention that module is BINARY is not enough ?
There was a problem hiding this comment.
the @BINARY suffix is a bit disturbing
|
There are people waiting for the release, let's focus on |
Suggesting we just merge this now. |
|
Merging and making a release, but we will probably to make another new release soon with a new release of BuildKit |
Will release v2.0.4 after merging this