-
Notifications
You must be signed in to change notification settings - Fork 18.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix docker-proxy and ctr not statically linked #43492
Conversation
cross for
Afaik there is no |
b0222ef
to
e541b86
Compare
I see some binaries are stripped and others not; wondering if we should strip the proxy one 🤔 (not directly related to this change, just wondering) |
Yes indeed and we should also |
Seeing some failures, which (from a quick glance) look related;
|
hack/make/binary-proxy
Outdated
@@ -2,9 +2,16 @@ | |||
|
|||
set -e | |||
|
|||
case "$(go env GOOS)/$(go env GOARCH)" in | |||
windows/arm64) ;; | |||
# TODO remove windows/arm64 when aarch64-w64-mingw32-gcc toolchain is available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need gcc for docker-proxy.
But windows/arm64 is a new golang target anyway.
It would help to review this without the changes that mess with the new-lines vs flattened into one line. |
Related ticket in containerd; containerd/containerd#5824 |
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
e541b86
to
d07cc5c
Compare
I did a quick rebase and reduced the diff by removing the whitespace changes, and some other changes that didn't seem immediately related to the fix; https://github.com/moby/moby/compare/e541b867b97e7d26741e354f61cf7fc90fd4c3cc..d07cc5c85b7942d5c06c182234f91c6f8c45b112 |
Looks like things are segfaulting; think that's the containerd binary failing;
Let me open a quick PR with only the docker-proxy commit |
1 similar comment
Looks like things are segfaulting; think that's the containerd binary failing;
Let me open a quick PR with only the docker-proxy commit |
opened #43621 - let's see if that one does go green; the ctr binary is not super-important (only for debugging purposes) |
- What I did
fix docker-proxy and ctr (containerd cli) not statically linked while building against the binary or cross targets.
- How I did it
while adding a static check in docker/docker-ce-packaging#665 (see docker/docker-ce-packaging@835cc08) for our bundles, I discovered some of our artifacts were not statically linked anymore: https://github.com/docker/docker-ce-packaging/runs/6032211065?check_suite_focus=true#step:5:1183
Seems to be a regression on the master branch of moby, not reproducible on 20.10 branch.
Let's take a look at the current static bundle with the latest stable release:
As we can see all shipped artifacts are statically linked (except ctr).
If we create a static build on docker-ce-packaging against the master branch we have the following result:
As you can see there is an issue with docker-proxy and ctr.
For docker-proxy it seems to be linked to #42539 where
-linkmode=external
is missing as it was previously. Adding it back looks good but pie mode might be enough? WDYT @tonistiigi @cpuguy83?Same for ctr where
-linkmode external
is missing according to the documentation: https://github.com/containerd/containerd/blob/main/BUILDING.md#static-binaries cc @AkihiroSudaIt seems ctr is not statically linked since 20.10.8:
- How to verify it
- Description for the changelog
fix docker-proxy and ctr not statically linked