Improve Dockerfile.e2e by thaJeztah · Pull Request #39116 · moby/moby

thaJeztah · 2019-04-19T14:31:07Z

follow-up to #39115 (first commit is from that PR)

See individual commits for details

To test (if you're running on Docker 18.09 (API v1.39)):

docker build -t moby-e2e-test -f Dockerfile.e2e .

docker run -it --rm \
  -v /var/run/docker.sock:/var/run/docker.sock \
  -e DOCKER_API_VERSION=1.39 \
  moby-e2e-test

thaJeztah · 2019-04-19T14:31:20Z

ping @vdemeester @cpuguy83 PTAL

thaJeztah · 2019-04-19T15:51:57Z

Looking at some weirdness when running these tests; looks like there might be a bug somewhere (not introduced in this PR)

codecov · 2019-04-19T16:40:48Z

Codecov Report

Merging #39116 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #39116      +/-   ##
==========================================
- Coverage   37.05%   37.02%   -0.04%     
==========================================
  Files         612      612              
  Lines       45400    45400              
==========================================
- Hits        16824    16809      -15     
- Misses      26294    26307      +13     
- Partials     2282     2284       +2

thaJeztah · 2019-04-19T18:49:10Z

Added various fixes/skips for situations where this image is run against an older daemon

thaJeztah · 2019-04-19T18:50:28Z

/cc @seemethere

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Older versions of the daemon would concatenate hostname and domainname, so hostname "foobar" and domainname "baz.cyphar.com" would produce `foobar.baz.cyphar.com` as hostname. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

TestNISDomainname in the integration suite covers this Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

The daemon may already have other volumes, so filter out those when running the test. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Older versions did not use an UUID as ID Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

andrewhsu

LGTM

seemethere · 2019-04-25T20:39:00Z

Disregard this, just figured out I was connecting to a remote daemon running 18.09.5

This actually fails for me when I try to run it locally:

 => ERROR [builder 2/3] RUN hack/make.sh build-integration-test-binary                          0.6s
------
 > [builder 2/3] RUN hack/make.sh build-integration-test-binary:
#32 0.488
#32 0.505 error: .git directory missing and DOCKER_GITCOMMIT not specified
#32 0.505   Please either build with the .git directory accessible, or specify the
#32 0.505   exact (--short) commit hash you are building using DOCKER_GITCOMMIT for
#32 0.505   future accountability in diagnosing build issues.  Thanks!
------
executor failed running [/bin/sh -c hack/make.sh build-integration-test-binary]: exit code: 1

EDIT: Actually this only fails for me when I try running with DOCKER_BUILDKIT=1

kolyshkin · 2019-04-26T00:41:13Z

--- FAIL: TestUpdatePidsLimit (4.45s)
    --- FAIL: TestUpdatePidsLimit/update_from_none (0.47s)
        update_linux_test.go:154: failed to parse source file: /go/src/github.com/docker/docker/integration/container/update_linux_test.go: open /go/src/github.com/docker/docker/integration/container/update_linux_test.go: no such file or directory

(most probably the failure comes because I don't run it as root, I don't care)

It looks like test case sources should be copied to the final image, because stuff like assert.Equal() etc tries to parse (reflect) the source file in case of failure to print info about what kind of variable and what value is expected and what is provided, stuff like that.

thaJeztah · 2019-04-26T01:11:45Z

It looks like test case sources should be copied to the final image, because stuff like assert.Equal() etc tries to parse (reflect) the source file in case of failure to print info about what kind of variable and what value is expected and what is provided, stuff like that.

Yes, I actually had a branch that does so, but I was in doubt to include that;

the image size actually doubles
I wanted to have a look at the gotest.tools repository to see if there's a way to make that reflecting/parsing optional (in situations where the source code is not available)

This test case requires not just daemon >= 1.40, but also client API >= 1.40. In case older client is used, we'll get failure from the very first check: > ipcmode_linux_test.go:313: assertion failed: shareable (string) != private (string) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

thaJeztah · 2019-04-26T23:51:04Z

@kolyshkin LGTY?

kolyshkin

Yes, LGTM (I guess I can followup with gotest.tools stuff later)

thaJeztah added status/2-code-review area/testing labels Apr 19, 2019

GordonTheTurtle added the status/0-triage label Apr 19, 2019

thaJeztah removed the status/0-triage label Apr 19, 2019

thaJeztah added 14 commits April 24, 2019 17:51

Dockerfile.e2e: use alpine 3.9

2026268

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Dockerfile.e2e: move frozen-images to a separate stage

5554bd1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Dockerfile.e2e: re-order steps for caching

63aefbf

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Dockerfile.e2e: use /build to be consistent with main Dockerfile

045beed

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Dockerfile.e2e: move dockercli to a separate build-stage

e7784a6

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Dockerfile.e2e: move "contrib" to a separate build-stage

3ededb8

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Dockerfile.e2e: builder: change output directory to simplify copy

b73e340

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Skip TestNISDomainname on API < 1.40

c91c377

Older versions of the daemon would concatenate hostname and domainname, so hostname "foobar" and domainname "baz.cyphar.com" would produce `foobar.baz.cyphar.com` as hostname. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Remove TestContainerAPICreateWithHostName

2b5880c

TestNISDomainname in the integration suite covers this Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Fix TestVolumesCreateAndList when running against a shared daemon

566eea1

The daemon may already have other volumes, so filter out those when running the test. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Skip TestBuildWithEmptyDockerfile on API < v1.40

0e7b46a

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Skip TestPingCacheHeaders on API < v1.40

d080a86

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Skip TestUUIDGeneration on API < v1.40

05bd995

Older versions did not use an UUID as ID Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Skip TestImagesFilterMultiReference on API < v1.40

83ac2b4

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the improve_e2e_image branch from ff15310 to 83ac2b4 Compare April 25, 2019 00:53

andrewhsu approved these changes Apr 25, 2019

View reviewed changes

seemethere approved these changes Apr 25, 2019

View reviewed changes

kolyshkin self-requested a review April 27, 2019 03:36

kolyshkin approved these changes Apr 27, 2019

View reviewed changes

tiborvass approved these changes Apr 27, 2019

View reviewed changes

tiborvass merged commit 32daacb into moby:master Apr 27, 2019

thaJeztah deleted the improve_e2e_image branch April 27, 2019 03:54

kolyshkin mentioned this pull request Apr 27, 2019

Dockerfile.e2e: copy test sources #39150

Merged

thaJeztah mentioned this pull request May 13, 2019

[19.03 backport] backport Dockerfile.e2e fixes docker-archive/engine#207

Merged

thaJeztah added the process/cherry-picked label Jun 7, 2019

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Dockerfile.e2e#39116

Improve Dockerfile.e2e#39116
tiborvass merged 15 commits intomoby:masterfrom
thaJeztah:improve_e2e_image

thaJeztah commented Apr 19, 2019 •

edited

Loading

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

codecov bot commented Apr 19, 2019 •

edited

Loading

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

andrewhsu left a comment

Uh oh!

seemethere commented Apr 25, 2019 •

edited

Loading

Uh oh!

kolyshkin commented Apr 26, 2019

Uh oh!

thaJeztah commented Apr 26, 2019

Uh oh!

thaJeztah commented Apr 26, 2019

Uh oh!

kolyshkin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

thaJeztah commented Apr 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

codecov bot commented Apr 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

thaJeztah commented Apr 19, 2019

Uh oh!

andrewhsu left a comment

Choose a reason for hiding this comment

Uh oh!

seemethere commented Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kolyshkin commented Apr 26, 2019

Uh oh!

thaJeztah commented Apr 26, 2019

Uh oh!

thaJeztah commented Apr 26, 2019

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thaJeztah commented Apr 19, 2019 •

edited

Loading

codecov bot commented Apr 19, 2019 •

edited

Loading

seemethere commented Apr 25, 2019 •

edited

Loading