Skip to content

Fix packaged runc reporting incorrect version#10556

Merged
mxpv merged 1 commit intocontainerd:mainfrom
thaJeztah:fix_runc_version
Aug 6, 2024
Merged

Fix packaged runc reporting incorrect version#10556
mxpv merged 1 commit intocontainerd:mainfrom
thaJeztah:fix_runc_version

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Aug 6, 2024

runc v1.1.13 introduced an option to customize the version (as printed by the --version flag) through a VERSION Make variable / environment variable (see 1).

This variable collided with the VERSION environment variable used by containerd for the same purpose, which lead to runc binaries built using the version of containerd;

runc --version
runc version 1.7.20
commit: v1.1.13-0-g58aa9203
...

This patch explicitly sets the VERSION variable to the version of runc being built when building the binary.

Before this patch:

docker build -t containerd-test -f contrib/Dockerfile.test .
docker run -it --rm --env VERSION=1.7.20 containerd-test sh -c 'script/setup/install-runc && /usr/local/sbin/runc --version'
# ....
HEAD is now at 58aa9203 VERSION: release 1.1.13
go build -trimpath "-buildmode=pie"  -tags "seccomp" -ldflags "-X main.gitCommit=v1.1.13-0-g58aa9203 -X main.version=1.7.20 " -o runc .
install -D -m0755 runc /usr/local/sbin/runc
/go/src/github.com/containerd/containerd
runc version 1.7.20
commit: v1.1.13-0-g58aa9203
spec: 1.0.2-dev
go: go1.22.5
libseccomp: 2.5.4

With this patch:

docker build -t containerd-test -f contrib/Dockerfile.test .
docker run -it --rm --env VERSION=1.7.20 containerd-test sh -c 'script/setup/install-runc && /usr/local/sbin/runc --version'
# ....
HEAD is now at 58aa9203 VERSION: release 1.1.13
go build -trimpath "-buildmode=pie"  -tags "seccomp" -ldflags "-X main.gitCommit=v1.1.13-0-g58aa9203 -X main.version=v1.1.13 " -o runc .
install -D -m0755 runc /usr/local/sbin/runc
/go/src/github.com/containerd/containerd
runc version v1.1.13
commit: v1.1.13-0-g58aa9203
spec: 1.0.2-dev
go: go1.22.5
libseccomp: 2.5.4

@thaJeztah thaJeztah added kind/bug cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch labels Aug 6, 2024
@dosubot dosubot bot added the area/toolchain Build and Release Toolchain label Aug 6, 2024
@thaJeztah thaJeztah added cherry-pick/1.6.x cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch and removed cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch labels Aug 6, 2024
@dmcgowan dmcgowan added this pull request to the merge queue Aug 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 6, 2024
cpuguy83
cpuguy83 approved these changes Aug 6, 2024
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This is cause by this change in runc: opencontainers/runc#4270

cpuguy83
cpuguy83 requested changes Aug 6, 2024
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, is that the right var to set? Maybe unset VERSION?

akhilerm
akhilerm reviewed Aug 6, 2024
View reviewed changes
script/setup/install-runc Outdated
pushd "${TMPROOT}"/runc
git checkout "${RUNC_VERSION}"
make BUILDTAGS='seccomp' runc
make BUILDTAGS='seccomp' VERSION="${RUNC_VERSION}" runc
Copy link
Member

@akhilerm akhilerm Aug 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think unsetting the VERSION in the context of install-runc will be better, so that it falls back to the behaviour of getting version as specified in the build script of runc.

@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 6, 2024

Ah, yes, I actually first went for unsetting, then thought; perhaps we want to keep RUNC_VERSION if set (e.g. set to a specific commit), but I guess in that case the version output would still show the commit that's used, so maybe not a thing.

Let me change and push

@thaJeztah
script/setup/install-runc: fix runc using incorrect version
349d2b5 
runc v1.1.13 introduced an option to customize the version (as printed by the
`--version` flag) through a `VERSION` Make variable / environment variable
(see [1]).

This variable collided with the `VERSION` environment variable used by
containerd for the same purpose, which lead to `runc` binaries built
using the version of containerd;

    runc --version
    runc version 1.7.20
    commit: v1.1.13-0-g58aa9203
    ...

This patch unsets the `VERSION` variable to bring prevent it from being
inherited and to bring back the previous behavior.

Before this patch:

    docker build -t containerd-test -f contrib/Dockerfile.test .
    docker run -it --rm --env VERSION=1.7.20 containerd-test sh -c 'script/setup/install-runc && /usr/local/sbin/runc --version'
    # ....
    HEAD is now at 58aa9203 VERSION: release 1.1.13
    go build -trimpath "-buildmode=pie"  -tags "seccomp" -ldflags "-X main.gitCommit=v1.1.13-0-g58aa9203 -X main.version=1.7.20 " -o runc .
    install -D -m0755 runc /usr/local/sbin/runc
    /go/src/github.com/containerd/containerd
    runc version 1.7.20
    commit: v1.1.13-0-g58aa9203
    spec: 1.0.2-dev
    go: go1.22.5
    libseccomp: 2.5.4

With this patch:

    docker build -t containerd-test -f contrib/Dockerfile.test .
    docker run -it --rm --env VERSION=1.7.20 containerd-test sh -c 'script/setup/install-runc && /usr/local/sbin/runc --version'
    # ....
    HEAD is now at 58aa9203 VERSION: release 1.1.13
    go build -trimpath "-buildmode=pie"  -tags "seccomp" -ldflags "-X main.gitCommit=v1.1.13-0-g58aa9203 -X main.version=v1.1.13 " -o runc .
    install -D -m0755 runc /usr/local/sbin/runc
    /go/src/github.com/containerd/containerd
    runc version v1.1.13
    commit: v1.1.13-0-g58aa9203
    spec: 1.0.2-dev
    go: go1.22.5
    libseccomp: 2.5.4

[1]: opencontainers/runc@6f4d975

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 6, 2024

@cpuguy83 updated; PTAL 🤗

@mxpv mxpv added this pull request to the merge queue Aug 6, 2024
Merged via the queue into containerd:main with commit 10d4ee9 Aug 6, 2024
@thaJeztah thaJeztah deleted the fix_runc_version branch August 6, 2024 21:32
This was referenced Aug 6, 2024
Merged
Merged
@thaJeztah thaJeztah self-assigned this Aug 6, 2024
@thaJeztah thaJeztah added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch and removed cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Aug 6, 2024
@samuelkarp samuelkarp changed the title script/setup/install-runc: fix runc using incorrect version Fix packaged runc reporting incorrect version Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@akhilerm akhilerm akhilerm approved these changes

@fuweid fuweid fuweid approved these changes

Assignees

@thaJeztah thaJeztah

Labels

area/toolchain Build and Release Toolchain cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch kind/bug size/XS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bundled runc binary shows incorrect version

7 participants

@thaJeztah @dmcgowan @cpuguy83 @akhilerm @fuweid @mxpv @k8s-ci-robot