Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

[18.03] backport updates to tests, and fix empty LogPath with non-blocking logging mode#512

Merged
andrewhsu merged 24 commits into
docker-archive:18.03from
thaJeztah:18.03-backport-tests
Apr 17, 2018
Merged

[18.03] backport updates to tests, and fix empty LogPath with non-blocking logging mode#512
andrewhsu merged 24 commits into
docker-archive:18.03from
thaJeztah:18.03-backport-tests

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Apr 17, 2018

Backports for 18.03 of:

This PR also contains a fix; I think it would be ok to include that

After this is merged, #468 should be rebased; after that one is merged, there's more changes to the tests we can backport, including the "big one"; moby/moby#36507 Replace testify/assert with gotestyourself/assert

yongtang and others added 24 commits April 16, 2018 14:19
@yongtang @thaJeztah
Migrate volumes tests in integration-cli to api tests
1331808 
This fix migrates volumes tests in integration-cli to api tests
in integration/

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit d896f87)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Migrate several docker rm tests to api tests
15237fd 
This fix migrates several docker rm tests to api tests

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 6bd4f48)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Update docker-py
7aa342f 
This fix update docker-py so that containers from the tests run
could be cleaned up during teardown:
```diff
-ENV DOCKER_PY_COMMIT 5e28dcaace5f7b70cbe44c313b7a3b288fa38916
+ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f
```

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 66935a0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@arm64b @thaJeztah
Clean the teardown process of network test
fd75fd2 
We need to clean the resources created in some test cases, else
in some cases we'll get below error for other tests:

> FAIL: docker_experimental_network_test.go:37: DockerNetworkSuite.TestDockerNetworkMacvlanPersistance
>  docker_experimental_network_test.go:44:
> ...
> Command:  ip link add dm-dummy0 type dummy
> ExitCode: 2
> Error:    exit status 2
> Stdout:
> Stderr:   RTNETLINK answers: File exists
> ...

Logically, each test case should be independent, the failure of previous
test case should not have side-effect for the test cases followed.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
(cherry picked from commit 57d85e7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Move containerIsStopped/containerIsInState to integration/internal/co…
811b93a 
…ntainer

This fix moves helper functions containerIsStopped and
containerIsInState to integration/internal/container,
so that they could be used outside of integration/container.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit eda311c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Migrate some config secret tests to api test
89f42b5 
This fix migrates some secret create tests to api tests,
and remove redundant TestConfigCreate.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 99e2818)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@braintreeps @thaJeztah
Fix empty LogPath with non-blocking logging mode
b2ff741 
This fixes an issue where the container LogPath was empty when the
non-blocking logging mode was enabled. This change sets the LogPath on
the container as soon as the path is generated, instead of setting the
LogPath on a logger struct and then attempting to pull it off that
logger at a later point. That attempt to pull the LogPath off the logger
was error prone since it assumed that the logger would only ever be a
single type.

Prior to this change docker inspect returned an empty string for
LogPath. This caused issues with tools that rely on docker inspect
output to discover container logs, e.g. Kubernetes.

This commit also removes some LogPath methods that are now unnecessary
and are never invoked.

Signed-off-by: junzhe and mnussbaum <code@getbraintree.com>
(cherry picked from commit 20ca612)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@mnussbaum @thaJeztah
Clean-up after container unit test
bae8ccd 
Remove temp directories and close file loggers in container unit tests.

Signed-off-by: mnussbaum <michael.nussbaum@getbraintree.com>
(cherry picked from commit 07d5446)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Migrate config inspect test to api test
e4022bf 
This fix migrates config inspect test in integration-cli
to api test.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 4b99d78)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Migrate events tests to api tests
8b54bcc 
This fix migrates events tests in integration-cli to api tests.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 3a74915)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Remove docker_cli_secret_inspect_test.go
0fd259a 
as the test (TestSecretInspectMultiple) seems to have been covered pretty well in cli:
https://github.com/docker/cli/blob/master/cli/command/secret/inspect_test.go

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 3d38adb)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Enhancement of replacing ContainerCreate with helper funcs in tests
53a10ae 
This fix is a minor enhancement to replace several ContainerCreate with
helper funcs of `container.Create` in tests.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 6ad4720)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Fix "expected" and "actual" being reversed
d4444e3 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a2517cb)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Migrate export tests to api tests
e695ffe 
This fix migrates export tests in integration-cli to api tests.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 4e702cf)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@vdemeester @thaJeztah
Fixes some integration/container test to run on remote daemon
1f529b2 
```
docker build -f Dockerfile.e2e -t moby-e2e .
docker run -v /var/run/docker.sock:/var/run/docker.sock \
           -e TEST_INTEGRATION_DIR=/tests/integration/container \
           -e DOCKER_API_VERSION=1.36 moby-e2e
```

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 18dd1d9)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Improvement in integration tests
618f290 
This fix adds several improvement:
1. No need for explicit ContainerRemove as it has been handled in setupTest()
2. Added `container.WithImage` helper function and used it in commit tests.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 6ab4658)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Migrate docker rm tests to api tests
6fbd783 
This fix migrates docker rm test in integration-cli
to api tests.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit ed58ba9)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Remove unnecessary container.WithName in kill test
b3929fa 
This fix removes several unnecessary `container.WithName`
usage in docker kill integration test.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 1778719)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@yongtang @thaJeztah
Address expected vs actual in integration tests
04ebff9 
This fix addresses `expected` vs `actual` in integration tests
so that they match `assert.Equal(t, expected, actual)`

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 8a854e9)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@cpuguy83 @thaJeztah
Adds a unit test for plugin request timeout
e945f93 
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 7ca971f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Remove duplicate TestServiceUpdatePort
df6211f 
The TestAPIServiceUpdatePort test performs exactly
the same steps.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 36e1646)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@dnephin @thaJeztah
Remove unnecessary diff tests
ae56939 
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 038f3ad)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@kolyshkin @thaJeztah
TestLinksEtcHostsContentMatch: use container.Exec()
c25858e 
I am not quite sure why but this test is sometimes failing like this:

> 15:21:41 --- FAIL: TestLinksEtcHostsContentMatch (0.53s)
> 15:21:41 	assertions.go:226:
>
> 	Error Trace:	links_linux_test.go:46
> 15:21:41
> 	Error:      	Not equal:
> 15:21:41
> 	            	expected: "127.0.0.1\tlocalhost\n::1\tlocalhost
> ip6-localhost
> ip6-loopback\nfe00::0\tip6-localnet\nff00::0\tip6-mcastprefix\nff02::1\tip6-allnodes\nff02::2\tip6-allrouters\n172.17.0.2\tf53feb6df161\n"
> 15:21:41
> 	            	received: ""

To eliminate some possible failures (like ignoring stderr from `cat` or
its exit code), let's use container.Exec() to read a file from a container.

Fixes: e6bd20e ("Migrate some integration-cli test to api tests")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit ad2f88d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@cpuguy83 @thaJeztah
Add some tests to the volume store
a354096 
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 834d0e2)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Apr 17, 2018

This PR overlaps with #512 and #510, so we can probably close those. Opened as a separate PR for now to discuss and see if all tests go green

@thaJeztah thaJeztah mentioned this pull request Apr 17, 2018
Closed
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Apr 17, 2018

Not entirely sure what the failure is in s390x; could it be that there's no more tests in that suite for s390x? https://jenkins.dockerproject.org/job/docker-ce-pr-s390x/549/execution/node/621/log/

01:36:52 + make -f Makefile-docker-ce-pr log-DockerAuthzV2Suite.tgz
01:36:52 Makefile-docker-ce-pr:6: warning: overriding recipe for target 'docker-ce'
01:36:52 Makefile-docker-ce:29: warning: ignoring old recipe for target 'docker-ce'
01:36:52 docker run --rm -v /home/ubuntu/workspace/docker-ce-pr-s390x:/v -w /v busybox chown -R 1000:1000 bundles
01:36:53 find bundles -name '*.log' -o -name '*.prof' -o -name integration.test | xargs tar -czf log-DockerAuthzV2Suite.tgz
01:36:53 tar: Cowardly refusing to create an empty archive
01:36:53 Try 'tar --help' or 'tar --usage' for more information.
01:36:53 Makefile-docker-ce:67: recipe for target 'log-DockerAuthzV2Suite.tgz' failed
01:36:53 make: *** [log-DockerAuthzV2Suite.tgz] Error 123

@andrewhsu
Copy link
Copy Markdown
Contributor

andrewhsu commented Apr 17, 2018

I think the error is with getting the image to run the tests:

docker run --rm --privileged --name jenkins-docker-ce-pr-s390x-549-0-a0b317 \
	-v "/home/ubuntu/workspace/docker-ce-pr-s390x/bundles:/go/src/github.com/docker/docker/bundles" \
	-v "/home/ubuntu/workspace/docker-ce-pr-s390x/docker-ce/components/cli/build:/usr/local/cli" \
	-e DOCKER_CLI_PATH=docker \
	-e DOCKER_GITCOMMIT=a3540962fd234faa30385d5f6ef3a96c61610ccd \
	-e TESTFLAGS='-check.f DockerAuthzV2Suite.*' \
	-e KEEPBUNDLE=1 \
	dockerbuildbot/docker@sha256:a19c21e8f86a9c1345789a72436cde7189a524c52f3ebbc6e3b0b0087845d605 hack/make.sh test-integration-cli
Unable to find image 'dockerbuildbot/docker@sha256:a19c21e8f86a9c1345789a72436cde7189a524c52f3ebbc6e3b0b0087845d605' locally
docker: Error response from daemon: Get https://registry-1.docker.io/v2/dockerbuildbot/docker/manifests/sha256:a19c21e8f86a9c1345789a72436cde7189a524c52f3ebbc6e3b0b0087845d605: EOF.

Kicking build again.

vdemeester
vdemeester approved these changes Apr 17, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Apr 17, 2018

s390x failure is https://jenkins.dockerproject.org/job/docker-ce-pr-s390x/550/execution/node/627/log/

05:19:52 ----------------------------------------------------------------------
05:19:52 FAIL: check_test.go:365: DockerSwarmSuite.TearDownTest
05:19:52 
05:19:52 unmount of /tmp/docker-execroot/d1f2cc7589c78/netns failed: invalid argument
05:19:52 unmount of /tmp/docker-execroot/d1f2cc7589c78/netns failed: no such file or directory
05:19:52 check_test.go:370:
05:19:52     d.Stop(c)
05:19:52 daemon/daemon.go:389:
05:19:52     t.Fatalf("Error while stopping the daemon %s : %v", d.id, err)
05:19:52 ... Error: Error while stopping the daemon d87d39ab26572 : exit status 130
05:19:52 
05:19:52 
05:19:52 ----------------------------------------------------------------------
05:19:52 PANIC: docker_api_swarm_test.go:262: DockerSwarmSuite.TestAPISwarmLeaderProxy
05:19:52 
05:19:52 [d1f2cc7589c78] waiting for daemon to start
05:19:52 [d1f2cc7589c78] daemon started
05:19:52 
05:19:52 [d87d39ab26572] waiting for daemon to start
05:19:52 [d87d39ab26572] daemon started
05:19:52 
05:19:52 [d3485c7d636d5] waiting for daemon to start
05:19:52 [d3485c7d636d5] daemon started
05:19:52 
05:19:52 [d1f2cc7589c78] exiting daemon
05:19:52 [d87d39ab26572] daemon started
05:19:52 Attempt #2: daemon is still running with pid 22578
05:19:52 Attempt #3: daemon is still running with pid 22578
05:19:52 Attempt #4: daemon is still running with pid 22578
05:19:52 [d87d39ab26572] exiting daemon
05:19:52 ... Panic: Fixture has panicked (see related PANIC)
05:19:52

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Apr 17, 2018

I think that flakiness should be fixed by moby/moby#36511, but I see it needs a rebase

cpuguy83
cpuguy83 approved these changes Apr 17, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

Wow, LGTM.

@andrewhsu andrewhsu merged commit ec3221a into docker-archive:18.03 Apr 17, 2018
@thaJeztah thaJeztah deleted the 18.03-backport-tests branch April 17, 2018 22:06
This was referenced Apr 17, 2018
Merged
Closed
Merged
kolyshkin pushed a commit to kolyshkin/docker-ce that referenced this pull request May 10, 2018
@albers
Fix docker-archive#512 Bash autocompletion works incorrect with inspect
25c006f 
Signed-off-by: Harald Albers <github@albersweb.de>
Upstream-commit: a2d0b6e122021cfac87a99a46d3d02339552d281
Component: cli
kolyshkin pushed a commit to kolyshkin/docker-ce that referenced this pull request May 10, 2018
@dnephin
Merge pull request #713 from albers/completion-512
a67c7bc 
Fix docker-archive#512 Bash autocompletion works incorrect with inspect
Upstream-commit: 718a245b6e6c21b9f3694bb541d5e93fd3ebddaa
Component: cli
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 24, 2020
@albers @thaJeztah
Fix docker-archive#512 Bash autocompletion works incorrect with inspect
dee0f92 
Signed-off-by: Harald Albers <github@albersweb.de>
(cherry picked from commit a2d0b6e122021cfac87a99a46d3d02339552d281)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 27, 2020
@albers @thaJeztah
Fix docker-archive#512 Bash autocompletion works incorrect with inspect
eb42272 
Signed-off-by: Harald Albers <github@albersweb.de>
(cherry picked from commit a2d0b6e122021cfac87a99a46d3d02339552d281)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@vdemeester vdemeester vdemeester approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@thaJeztah @andrewhsu @vdemeester @cpuguy83 @yongtang @arm64b @braintreeps @mnussbaum @dnephin @kolyshkin