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

[18.09 backport] test-fixes and updates, and fix images filter when use multi reference filter #198

Merged
merged 45 commits into from
Apr 23, 2019

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Apr 17, 2019

Backports for 18.09 of:

# https://github.com/moby/moby/pull/37783 Fix various spell errors
git cherry-pick -s -S -x 5c0d2a0932afb1e9c9e26326a22fdbefa1069463

# https://github.com/moby/moby/pull/37851 migrated ipc integration tests to integration/container
git cherry-pick -s -S -x febefb850d12e0ed1c2ab2a25336ece43204ba03

# https://github.com/moby/moby/pull/38171 cli: fix images filter when use multi reference filter
git cherry-pick -s -S -x 5007c36d71ac86f5b47b6ba10a5ed6e841807284

# https://github.com/moby/moby/pull/38417 Test: Replace NewClient() with NewClientT()
git cherry-pick -s -S -x 2cb7b73a1bdbf2e7ea6da7d0c050b303c2c4f5dc

    # conflict in integration/network/service_test.go because https://github.com/moby/moby/pull/38102 is not in 18.09


# https://github.com/moby/moby/pull/38473 Integration: use testenv.APIClient()
git cherry-pick -s -S -x 4d88a95d6730383624570f8730aa203a56caadc3
git cherry-pick -s -S -x 0de62d9bbcb92e9b7c73ee4cdef51c2229878e05

    # both modified:   integration/container/mounts_linux_test.go because of https://github.com/moby/moby/pull/38003 mount: add BindOptions.NonRecursive (API v1.40)
    # deleted by us:   integration/container/run_linux_test.go    because https://github.com/moby/moby/pull/37043 Add memory.kernelTCP support for linux not in 18.09


# https://github.com/moby/moby/pull/38482 Improve consistency in skipping tests
git cherry-pick -s -S -x a3948d17d330315c832112bfecfc15d5e19511b1
git cherry-pick -s -S -x 84224935ea78d93e21a21c0e1e0aa3e83a5c7853
git cherry-pick -s -S -x 69c0b7e47682a2a7a850122a9a2f711259fbb25a

    # both modified:   integration/container/mounts_linux_test.go because of https://github.com/moby/moby/pull/38003 mount: add BindOptions.NonRecursive (API v1.40)

git cherry-pick -s -S -x 263e28a830c7c0ba573f8bb6aa37bee1c3956d22

# https://github.com/moby/moby/pull/38486 Remove use of deprecated client.NewEnvClient()
git cherry-pick -s -S -x c8ff5ecc091278520a890ce58ed891ec354bf6d9

# https://github.com/moby/moby/pull/38548 Remove code duplication and consolidate networkIsRemoved
git cherry-pick -s -S -x 28b7824caa9c8f1acc0471136a8a4fd80e51f491

# https://github.com/moby/moby/pull/38557 Integration tests: remove some duplicated code, and preserve context
git cherry-pick -s -S -x 60d93aab2e68b13b3f22c43add4762d3e8108227
git cherry-pick -s -S -x 56a68c15f8a093b1761e77a74d8b7acdfbcb30a2

# https://github.com/moby/moby/pull/38603 integration-cli: remove deprecated daemonHost() utility
git cherry-pick -s -S -x 3105ca26dc3dc5273e62dec622b8c40a1ae31b91

# https://github.com/moby/moby/pull/38445 Replace deprecated client.WithDialer()
git cherry-pick -s -S -x 8d3feccfa9dd0c0b5d866eba982c89f739b5e5b2

# https://github.com/moby/moby/pull/38515 Use poll.WaitOn in authz_plugin_test.go
git cherry-pick -s -S -x 0492b0997b2fc739a9a6fe6831145297d8149e55

# https://github.com/moby/moby/pull/38554 Add missing error-check in TestAPISwarmManagerRestore
git cherry-pick -s -S -x 2e326eba7040042e7b4e9974ebd46aaa6b03dc4f

# https://github.com/moby/moby/pull/38605 Use assert.NilError() instead of assert.Assert()
git cherry-pick -s -S -x 3449b12cc7eefa8ebd0de6ec8b9803c6ee823af0

# https://github.com/moby/moby/pull/38604 Remove use of deprecated client.NewClient()
git cherry-pick -s -S -x 3a4bb96ab74125c95702b818725ba92a27e3a450

# https://github.com/moby/moby/pull/38686 Completely remove deprecated `d.NewClient` from testing tools
git cherry-pick -s -S -x e063099f918a01891db4389aaa6ba843e5d37fa9

# conflict (missing test) integration/network/service_test.go because https://github.com/moby/moby/pull/38102 is not in 18.09

# https://github.com/moby/moby/pull/37718 integration-cli: fix TestAttachDetach, rm TestAttachDetachTruncatedID
git cherry-pick -s -S -x 9f3a343a5101ab661a6a97c9e149a0b11ccc320a

# https://github.com/moby/moby/pull/37770 Windows: Go1.11: Use long path in TestBuildSymlinkBreakout
git cherry-pick -s -S -x b1b9937bc75f0db9c804838ecce9bb6792a42525

# https://github.com/moby/moby/pull/37820 TestStartReturnCorrectExitCode: show error
git cherry-pick -s -S -x 0d59f4305cbb1aaae1b0c6aab94c1e5a08b50bfb

# https://github.com/moby/moby/pull/38127 Integration tests fixes and cleanups
git cherry-pick -s -S -x 73baee2dcf546b2561bdd9a500b0af08cb62b1be
git cherry-pick -s -S -x 66cb1222d6559e120d9d1a29932aa778aa517894
git cherry-pick -s -S -x 6016520162fdcb19f50d08c4f0b54b06a7a6eac0
git cherry-pick -s -S -x 24cbb9897193894f4716583d1861091ab2fa1ae2
git cherry-pick -s -S -x 06afc2d1e6f8c5052af71e8815266d30e29ed664
git cherry-pick -s -S -x 2ed512c7faea938b0b07e69187b8a132e2ecb66a

# https://github.com/moby/moby/pull/38426 Remove old ExperimentalDaemon, NotS390X, NotPausable, SameHostDaemon checks
git cherry-pick -s -S -x 362f737e1ccf9a1c3ac9599f48b10a33fd9b0e08
git cherry-pick -s -S -x 43b15e924ffb7790ae087feac7618308a9e4b75e

# https://github.com/moby/moby/pull/38546 reduce flakiness of TestSwarmLockUnlockCluster and TestSwarmJoinPromoteLocked
git cherry-pick -s -S -x 973ca00d60712ef644b5b37abf7fa01078bb4ade

# https://github.com/moby/moby/pull/38552 Make TestEventsFilterLabels less flaky
git cherry-pick -s -S -x 0e15c02465c87c82908bcc45b0c1d6bd38f27c32

# https://github.com/moby/moby/pull/38843 TestDaemonRestartIpcMode: move to integration
git cherry-pick -s -S -x 39eaf1ef9709d79fc50095ee2a0546bd2f565ab2
git cherry-pick -s -S -x 17022b3ad2d95863e5acd17ebaf943b7761623cc
git cherry-pick -s -S -x f664df01d1836686c7a8a917e9da81d56c758d74

    # integration/internal/container/ops.go minor conflict due to https://github.com/moby/moby/pull/38003 mount: add BindOptions.NonRecursive (API v1.40)

git cherry-pick -s -S -x 9fd765f07cc08ccc2ea991d21835bf50ece9318b

# https://github.com/moby/moby/pull/37958 TestSwarmContainerEndpointOptions: fix debug
git cherry-pick -s -S -x 1921753b4b30dcca4fe772e7c1b0bc3f7bb7cd62


# https://github.com/moby/moby/pull/37981 Fix typo
git cherry-pick -s -S -x a28843150a7e926006798369f0ac31f32f028e36


# https://github.com/moby/moby/pull/39001 Remove some checkers and use gotest.tools
git cherry-pick -s -S -x 86f2ac4a6b5b746c3309b57e7e04bdbdb70d46d7
git cherry-pick -s -S -x 6345208b9b067f19f7792edcc675f59a617a3ca5

    # minor conflicts:
    # integration-cli/docker_cli_build_test.go  because of https://github.com/moby/moby/commit/20833b06a0a41602001d595b3e1785248a352991#diff-64d1c3a97fd8b1abf170680883e81aaa (https://github.com/moby/moby/pull/38541)
    # integration-cli/docker_cli_daemon_test.go because https://github.com/moby/moby/pull/35355 fix unless-stopped unexpected behavior

# https://github.com/moby/moby/pull/39095 Use existing Windows image for test instead of microsoft/nanoserver
git cherry-pick -s -S -x 6099336ae61ca43cafe9420f3188c796cb0812bc

# https://github.com/moby/moby/pull/39095 Use existing Windows image for test instead of microsoft/nanoserver
git cherry-pick -s -S -x aad7e9797b2e9a980b28d7d77bc119cb4b7e57dc 

tossmilestone and others added 5 commits April 17, 2019 21:32
Signed-off-by: Xiaoxi He <xxhe@alauda.io>
(cherry picked from commit 5c0d2a0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Arash Deshmeh <adeshmeh@ca.ibm.com>
(cherry picked from commit febefb8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: zhangyue <zy675793960@yeah.net>
(cherry picked from commit 5007c36)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2cb7b73)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`testEnv` is a package-level variable, so protecting / restoring
`testEnv` in parallel will result in "concurrent map write" errors.

This patch removes `t.Parallel()` from tests that use this
functionality (through `defer setupTest(t)()`).

Note that _subtests_ can still be run in parallel, as the defer
will be called after all subtests have completed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 4d88a95)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 18.09.6 milestone Apr 17, 2019
@thaJeztah
Copy link
Member Author

Ah;

21:00:05 integration/container/create_test.go:1::warning: file is not goimported (goimports)
21:00:05 integration/container/create_test.go:15:2:warning: "github.com/docker/docker/internal/test/request" imported but not used (gosimple)

A client is already created in testenv.New(), so we can just
as well use that one, instead of creating a new client.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 0de62d9)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah and others added 21 commits April 17, 2019 23:08
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a3948d1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These tests can only be run on a local Linux daemon, so there's
no need to build them on Windows

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8422493)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These tests are run on a local Linux daemon only, so no need
to do a platform-check.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 69c0b7e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 263e28a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit c8ff5ec)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This fix removes code duplication and consolidates networkIsRemoved
into one place.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 28b7824)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Clean up and refactor this test;

- make `serviceRunningTasksCount` to use a `desired-state` filter
- use subtests, and inline the `validNetworkVerbose` checks; also use
  asserts for the individual checks, so that any failure will log exactly
  what failed
- remove helper functions that are no longer needed

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 60d93aa)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This introduces `NoTasksForService` and `NoTasks` poller checks, that
can be used to check if no tasks are left in general, or for a specific
service.

Some redundant checks were also removed from some tests.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 56a68c1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3105ca2)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8d3fecc)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This fix uses poll.WaitOn to replace customerized
implementation in authz_plugin_test.go

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 0492b09)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2e326eb)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3449b12)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3a4bb96)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Favor `d.NewClientT` instead.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit e063099)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It looks like the logic of the test became wrong after commit
ae0883c ("Move TestAttachDetach to integration-cli").

The original logic was:
* (a few first steps skipped for clarity)
* send escape sequence to "attach";
* check "attach" is exiting (i.e. escape sequence works);
* check the container is still alive;
* kill the container.

Also, timeouts were big at that time, in the order of seconds.

The logic after the above mentioned commit and until now is:
* ...
* send escape sequence to "attach";
* check the container is running (why shouldn't it?);
* kill the container;
* checks that the "attach" has exited.

So, from the "let's check detach using escape sequence is working"
the test became something like "let's check that attach is gone
once we kill the container".

Let's fix the above test, also increasing the timeout waiting
for attach to exit (which fails from time to time on power CI).

Now, the second test, TestAttachDetachTruncatedID, does the exact
same thing, except it uses a truncated container ID. It does not
seem to be of much value, so let's remove it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 9f3a343)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…nkBreakout)

Signed-off-by: John Howard <jhoward@microsoft.com>
(cherry picked from commit b1b9937)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 0d59f43)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1. Using MNT_FORCE flag does not make sense for nsfs. Using MNT_DETACH
though might help.

2. When -check.vv is added to TESTFLAGS, there are a lot of messages
like this one:

> unmount of /tmp/dxr/d847fd103a4ba/netns failed: invalid argument

and some like

> unmount of /tmp/dxr/dd245af642d94/netns failed: no such file or directory

The first one means directory is not a mount point, the second one
means it's gone. Do ignore both of these.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 73baee2)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since commit 17173ef checkSwarmLockedToUnlocked() no longer
require its third argument, so remove it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 66cb122)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A timer is leaking on every daemon start and stop.
Probably nothing major, but given the amount of
daemon starts/stops during tests, it's better to
be accurate about it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 6016520)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
kolyshkin and others added 8 commits April 17, 2019 23:09
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 39eaf1e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since container.Create() already initializes HostConfig
to be non-nil, there is no need for this code. Remove it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 17022b3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
NOTE TestUpdateRestartPolicy is left as is as otherwise
it will decrease its readability.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit f664df0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move the test case from integration-cli to integration.

The test logic itself has not changed, except these
two things:

* the new test sets default-ipc-mode via command line
  rather than via daemon.json (less code);
* the new test uses current API version.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 9fd765f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In case of failure, stale out was printed.

Fixes: 6212ea6

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 1921753)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Hiroyuki Sasagawa <hs19870702@gmail.com>
(cherry picked from commit a288431)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 86f2ac4)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 6345208)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Ah, must've missed one

github.com/docker/docker/integration-cli
21:26:20 integration-cli/docker_cli_push_test.go:260:25: undefined: checker
21:26:20 integration-cli/docker_cli_push_test.go:272:25: undefined: checker

@thaJeztah
Copy link
Member Author

Ah; it's a test that's removed on master, that's why DockerSchema1RegistrySuite

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Power and s390 passed (except for DockerSuite.TestPullWindowsImageFailsOnLinux, which is expected)

Experimental; https://jenkins.dockerproject.org/job/Docker-PRs-experimental/44998/console

DockerSwarmSuite.TestSwarmClusterRotateUnlockKey (known flaky), and this one looks to be flaky as well; mentioned here; moby#38806 and moby#38482 (comment)

23:51:53 FAIL: docker_cli_run_test.go:1792: DockerSuite.TestRunInteractiveWithRestartPolicy
23:51:53
23:51:53 assertion failed:
23:51:53 Command:  /usr/local/cli/docker run -i --name test-inter-restart --restart=always busybox sh
23:51:53 ExitCode: 0
23:51:53 Error:    <nil>
23:51:53 Stdout:
23:51:53 Stderr:
23:51:53
23:51:53 Failures:
23:51:53 ExitCode was 0 expected 11

restarting janky and experimental

@thaJeztah
Copy link
Member Author

Added moby#39095 Use existing Windows image for test instead of microsoft/nanoserver

to fix the DockerSuite.TestPullWindowsImageFailsOnLinux failures

@thaJeztah
Copy link
Member Author

Ah, dang, of course the fix won't work without #197

09:54:54 FAIL: docker_cli_pull_test.go:273: DockerSuite.TestPullWindowsImageFailsOnLinux
09:54:54 
09:54:54 assertion failed: expected error to contain "no matching manifest for linux", got 
09:54:54 Command:  /usr/local/cli/docker pull mcr.microsoft.com/windows/servercore:ltsc2019
09:54:54 ExitCode: 1
09:54:54 Error:    exit status 1
09:54:54 Stdout:   ltsc2019: Pulling from windows/servercore
09:54:54 
09:54:54 Stderr:   no matching manifest for unknown in the manifest list entries
09:54:54 
09:54:54 
09:54:54 Failures:
09:54:54 ExitCode was 1 expected 0
09:54:54 Expected no error

StefanScherer and others added 2 commits April 18, 2019 18:05
Signed-off-by: Stefan Scherer <scherer_stefan@icloud.com>
(cherry picked from commit 4b9db20)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 6099336)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Stefan Scherer <stefan.scherer@docker.com>
(cherry picked from commit aad7e97)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Added moby#39095 Use existing Windows image for test instead of microsoft/nanoserver

@thaJeztah
Copy link
Member Author

Only failure on experimental is a flaky test moby#38521

01:49:37 FAIL: docker_cli_start_test.go:190: DockerSuite.TestStartReturnCorrectExitCode
01:49:37 
01:49:37 assertion failed: expected an error, got nil
01:49:39 
01:49:39 ------------------------------------------------

@thaJeztah
Copy link
Member Author

ping @kolyshkin @vdemeester @andrewhsu PTAL 🤗

Copy link

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM for test fixes

Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
10 participants