Upgrade the frozen images to multi-arch ones #35929

arm64b · 2018-01-04T08:05:43Z

Upgrade the frozen images to multi-arch, and fix all the bugs triggered by the upgrade accordingly.

Signed-off-by: Dennis Chen [email protected]

- What I did

Upgrade the frozen images to multi-arch 2. Fix all the compatible issue of this upgrade

- How I did it
Upgrade all the legacy frozen docker images to the latest multi-arch ones, then handle the compatible issue for the newer version.

- How to verify it
We can verify it directly with make test-integration.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

boaz0

LGTM 👍 but maybe we can use Busybox v1.27 in all our testing.

dnephin

Instead of trying to support both versions can we just upgrade the frozen image to the latest version?

dnephin · 2018-01-04T16:19:10Z

integration-cli/cli/cli.go

I don't think this helper is necessary. Just call Docker(Args(...)) directly.

DockerCmd() only exists for backwards compatibility.

We can upgrade the frozen image to the latest version and only support the latest for this test case 🉑 things will be easy as it should to be...

thaJeztah · 2018-01-04T18:14:57Z

Silly question; is busybox not using SemVer? And if it does, this would be a breaking change; should this be reported upstream in the busybox issue tracker?

tophj-ibm · 2018-01-04T18:51:09Z

@thaJeztah busybox appears to use SemVar. They have their own version of ls, which they changed a while ago in busybox 1.27.0, when they replaced -e with --full-time https://git.busybox.net/busybox/commit/?id=194b2ebd2a0cfc1d27744dea10ef706975c61317 so I don't think we have to submit an issue.

The arch-specific frozen busybox images use v1.26.2 with the old functionality, so all the dockerfiles should all be updated to use the manifest list which points to v1.27.2 too.

arm64b · 2018-01-05T02:13:23Z

Thanks for the review! So according to the comments, I am happy to make below changes:

Upgrade the frozen images to the latest (we just have a multi-arch images download PR Download support of images with multi-arch manifest #35772 merged to do that)
Support the latest version of BusyBox (v1.27)

Will push the changes soon. Any comments/suggestions are appreciated, as always 👍

arm64b · 2018-01-05T07:43:37Z

Hello every maintainers, PTAL? 😵

dnephin · 2018-01-05T17:13:23Z

Thanks! The change looks good, but there are a few more test failures to fix.

For TestRunSlowStdoutConsumer I think there are two problems:

stderr is never checked, so the failure message is not helpful. It should probably check that stderr is empty somewhere.
(to fix the actual failure) change catv to cat -v

For TestListImagesWithDigests it looks like busybox now has a digest, so the last assertion is failing (only for janky, I'm not sure why it doesn't also fail on experimental).

I'm not sure about the rest of the failures, so might be flakes. When you push those fixes we'll see if the same tests fail. Adding the links to the first CI runs here:

tianon · 2018-01-05T17:53:21Z

Dockerfile.armhf

This is the one case where download-frozen-image-v2.sh won't be smart enough as-is to use the correct image (just like Docker ATM isn't smart enough as-is). See #34875 for more details (and download-frozen-image-v2.sh now uses a very similar algorithm to Docker's own for choosing).

Right! Things will be a little bit complicated when we talking about arm arch, since it acrosses wider ecosystem from embedded, mobile/IoT to the enterprise. For the topic itself, clearly we need a variant to location the exact arm image such as armhf and armel. Current problem is with GOARCH we only tell arm64 or arm, we've realized this so that's why we have a PR containerd/containerd#1575 merged into containerd to handle this, that PR uses /proc/cpuinfo to get v5 v6 v7 v8 info of arm. So IMO, probably we need to add the variant support in download-frozen-image-v2.sh in terms of arm 32bit case. currently I prefer to let it to be still existing one , and enhance that after all the multi-arch futures for Arm arch are ready. For me, I don't care about the 32-bit at all, my github account is arm64b meaning 'it's arm, but only 64b(it)` 🎃. But if you have any suggestions here, I am happy to take account of it.

arm64b · 2018-01-08T04:22:25Z

@dnephin For the 2 problems of TestRunSlowStdoutConsumer case, I guess the first one is not the case specific, actually I have to confess that I don't quite understand that well. So I use cat -v instead of catv to trigger the new around CI test (I found I have to systemctl restart docker in my local machine to pass more test cases, but I have no control for the CI system 😭 ).

For the TestListImagesWithDigests, It's very weird to get below info since we have no such digest for the change, I guess maybe there're some stale images in the system before:

08:40:14 imageReference1 = 127.0.0.1:5000/dockercli/busybox-by-dgst@sha256:9d0aa3088408437e0eb553a2e67e128e17001588e4fc09eb17b3bf6880d62c46
08:40:14 imageReference2 = 127.0.0.1:5000/dockercli/busybox-by-dgst@sha256:64a5c42230002a3dd98ecc52e149661c4718540f2a2b5539c10b6dcb84267911

arm64b · 2018-01-08T09:51:01Z

Oops! Except the z node (being scheduled so long time?), the CI test still failes on Janky and experimental. Just go through the details, looks like there're 2 categories errors:

TestListImagesWithDigests
Errors seems related with ping and top command in busybox(but I checked the 2 commands for the 2 busybox versions, they are the same, so weird).
(happened on x86 ony?)

But all those are above my head to move further to fix in short time, so I need you guys' great help to make the future better 🎉

thaJeztah · 2018-01-08T10:42:51Z

Except the z node (being scheduled so long time?)

I think there's an issue currently with network access to the z nodes @tophj-ibm do you know what the status is on that one?

tophj-ibm · 2018-01-08T14:06:24Z

@thaJeztah not much of an update, but the Z community cloud where these vms are being hosted had an outage on friday and is currently under maintenance. We have someone in that datacenter working with docker to get these machines back up

Upgrade the frozen images to the multi-arch ones. Since issue moby#35963 is not fixed yet on linux/amd64, so we keep the busybox image on amd64 untouched. Signed-off-by: Dennis Chen <[email protected]>

We don't need the test image namespace anymore since we've already upgrade those images to the latest multi-arch ones. Signed-off-by: Dennis Chen <[email protected]>

BusyBox v1.26.2 (2017-03-09 00:04:38 UTC) supports `-le` option to get the full date and time information, while BusyBox v1.27.2 (2017-11-01 23:22:25 UTC, which is used by the official multi-arch image, uses `--full-time` instead of `-e` to get the same data. As a result, we will get below error for the `DockerSuite.TestBuildLastModified` test case in case of multi-arch image used: > docker_cli_build_test.go:446: > out2 = cli.DockerCmd(c, "run", name, "ls", "-le", "/file").Combined() > o/src/github.com/docker/docker/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go:61: > t.Fatalf("at %s:%d - %s\n", filepath.Base(file), line, err.Error()) > ... Error: at cli.go:33 - > Command: /usr/local/bin/docker run testbuildlastmodified ls -le /file > ExitCode: 1 > Error: exit status 1 > Stdout: > Stderr: ls: invalid option -- e > BusyBox v1.27.2 (2017-11-01 23:22:25 UTC) multi-call binary. This PR tries to fix the above compatible issue for busybox image. Signed-off-by: Dennis Chen <[email protected]>

Use `cat -v` command instead of `catv` for the latest version of busybox(V1.28.0) with multi-arch Signed-off-by: Dennis Chen <[email protected]>

arm64b · 2018-01-12T02:00:28Z

Hi @dnephin , PTAL? Thanks @thaJeztah for the 'rebuild' over and over 🤝

dnephin

LGTM

thaJeztah

LGTM

Since for now download-frozen-image-v2.sh won't be smart enough to use the adequate image in arm platform, hence we add variant support to fix it. Relevant request has been raised in github (moby#35929 and moby#34875). Signed-off-by: Penny Zheng <[email protected]>

arm64b requested review from dnephin and vdemeester as code owners January 4, 2018 08:05

GordonTheTurtle added the status/0-triage label Jan 4, 2018

arm64b force-pushed the busybox-ls-compatible branch from a7193dd to a6c45b3 Compare January 4, 2018 08:12

boaz0 added the status/2-code-review label Jan 4, 2018

boaz0 approved these changes Jan 4, 2018

View reviewed changes

dnephin suggested changes Jan 4, 2018

View reviewed changes

thaJeztah removed the status/0-triage label Jan 4, 2018

arm64b force-pushed the busybox-ls-compatible branch from a6c45b3 to 08cd715 Compare January 5, 2018 07:39

arm64b requested a review from tianon as a code owner January 5, 2018 07:39

dnephin added the rebuild/z label Jan 5, 2018

GordonTheTurtle removed the rebuild/z label Jan 5, 2018

dnephin added the rebuild/z label Jan 5, 2018

GordonTheTurtle removed the rebuild/z label Jan 5, 2018

tianon reviewed Jan 5, 2018

View reviewed changes

dnephin added rebuild/janky labels Jan 8, 2018

GordonTheTurtle removed rebuild/experimental labels Jan 8, 2018

arm64b force-pushed the busybox-ls-compatible branch from 82c56d0 to f9c1a25 Compare January 9, 2018 02:55

arm64b changed the title ~~Fix busybox ls command compatible issue~~ Upgrade the frozen images to multi-arch ones Jan 9, 2018

arm64b force-pushed the busybox-ls-compatible branch from f4b28a0 to 0a5ac8b Compare January 10, 2018 07:55

thaJeztah added the rebuild/* label Jan 10, 2018

GordonTheTurtle removed the rebuild/* label Jan 10, 2018

arm64b force-pushed the busybox-ls-compatible branch from 0a5ac8b to d383ca8 Compare January 11, 2018 03:38

arm64b added 2 commits January 11, 2018 05:14

Upgrade the frozen images to multi-arch

eaae775

Upgrade the frozen images to the multi-arch ones. Since issue moby#35963 is not fixed yet on linux/amd64, so we keep the busybox image on amd64 untouched. Signed-off-by: Dennis Chen <[email protected]>

Removal of TEST_IMAGE_NAMESPACE

662bdb4

We don't need the test image namespace anymore since we've already upgrade those images to the latest multi-arch ones. Signed-off-by: Dennis Chen <[email protected]>

arm64b force-pushed the busybox-ls-compatible branch from d383ca8 to 5f254ce Compare January 11, 2018 05:25

arm64b added 2 commits January 11, 2018 06:34

busybox: use '-v' option for 'cat'

ec6659a

Use `cat -v` command instead of `catv` for the latest version of busybox(V1.28.0) with multi-arch Signed-off-by: Dennis Chen <[email protected]>

arm64b force-pushed the busybox-ls-compatible branch from 5f254ce to ec6659a Compare January 11, 2018 06:37

dnephin added rebuild/powerpc labels Jan 11, 2018

GordonTheTurtle removed rebuild/powerpc labels Jan 11, 2018

thaJeztah added the rebuild/powerpc label Jan 12, 2018

GordonTheTurtle removed the rebuild/powerpc label Jan 12, 2018

dnephin approved these changes Jan 12, 2018

View reviewed changes

thaJeztah approved these changes Jan 15, 2018

View reviewed changes

thaJeztah merged commit 947eb28 into moby:master Jan 15, 2018

This was referenced Jan 22, 2018

Exception observed for the base image of busybox docker-library/busybox#42

Closed

Add multi-arch image support for ARM #36121

Closed

Pennyzct mentioned this pull request Feb 6, 2018

Add variant support in download-frozen-image-v2.sh #36217

Closed

arm64b mentioned this pull request Feb 13, 2018

Fix DockerNetworkInternalMode issue #36298

Merged

arm64b deleted the busybox-ls-compatible branch February 13, 2018 05:29

Upgrade the frozen images to multi-arch ones #35929

Upgrade the frozen images to multi-arch ones #35929

Uh oh!

Conversation

arm64b commented Jan 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boaz0 left a comment • 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!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

dnephin Jan 4, 2018

Choose a reason for hiding this comment

Uh oh!

arm64b Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jan 4, 2018

Uh oh!

tophj-ibm commented Jan 4, 2018

Uh oh!

arm64b commented Jan 5, 2018

Uh oh!

arm64b commented Jan 5, 2018

Uh oh!

dnephin commented Jan 5, 2018

Uh oh!

tianon Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

arm64b Jan 8, 2018

Choose a reason for hiding this comment

Uh oh!

arm64b commented Jan 8, 2018

Uh oh!

arm64b commented Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jan 8, 2018

Uh oh!

tophj-ibm commented Jan 8, 2018

Uh oh!

arm64b commented Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

arm64b commented Jan 4, 2018 •

edited

Loading

boaz0 left a comment •

edited

Loading

arm64b commented Jan 8, 2018 •

edited

Loading

arm64b commented Jan 12, 2018 •

edited

Loading