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

WIP [17.07] fix tests #140

Closed
wants to merge 7 commits into from
Closed

WIP [17.07] fix tests #140

wants to merge 7 commits into from

Conversation

andrewhsu
Copy link
Contributor

@andrewhsu andrewhsu commented Jul 25, 2017

Fixes the following tests:

Punting on:

Test verified by compiling cli from docker-ce 17.07 branch and using that when running test-integration-cli test from engine component of docker-ce 17.07 branch, e.g.

$ make BIND_DIR=. \
  DOCKER_CLI_PATH=/path/to/cli/build/docker-linux-amd64 \
  TESTFLAGS='-check.f DockerSuite.TestRmiContainerImageNotFound' \
  test-integration-cli
PASS: docker_cli_rmi_test.go:227: DockerSuite.TestRmiContainerImageNotFound	2.680s
OK: 1 passed

@seemethere
Copy link
Contributor

Re-kicking the windows test, failed with a weird error:

00:01:17.208 re-exec error: exit status 1: output: time="2017-07-26T00:00:16Z" level=error msg="hcsshim::ImportLayer failed in Win32: The system cannot find the path specified. (0x3) layerId=\\\\?\\D:\\control\\windowsfilter\\5d0ffc3f5a5700b571e8688e653501f634989926441ac1e52dc7627125283cfb flavour=1 folder=d:\\temp\\hcs591301103" 
00:01:17.209 hcsshim::ImportLayer failed in Win32: The system cannot find the path specified. (0x3) layerId=\\?\D:\control\windowsfilter\5d0ffc3f5a5700b571e8688e653501f634989926441ac1e52dc7627125283cfb flavour=1 folder=d:\temp\hcs591301103
00:01:17.209 
00:01:17.209 
00:01:17.210 ERROR: Failed 'ERROR: Failed to build image from Dockerfile.windows' at 07/26/2017 00:00:27

Possibly related to this:

00:00:53.153 INFO: Container count on control daemon to delete is 1
00:00:58.514 Error response from daemon: driver "windowsfilter" failed to remove root filesystem for a7fe2d1a2db2be19dd76ec62002888cb536e430e6d2fde72ca5c012f20aea8c3: rename D:\control\windowsfilter\a7fe2d1a2db2be19dd76ec62002888cb536e430e6d2fde72ca5c012f20aea8c3 D:\control\windowsfilter\a7fe2d1a2db2be19dd76ec62002888cb536e430e6d2fde72ca5c012f20aea8c3-removing: Access is denied.

andrewhsu and others added 3 commits July 25, 2017 17:07
…i` instead of `rmi -f`

Because now the cli will always return 0 for `rmi -f`.

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
Because now the cli will always return 0 for `rmi -f`.

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
…TestUpdateWithNanoCPUs

Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@andrewhsu andrewhsu changed the title WIP run test with rmi instead of rmi -f WIP fix tests Jul 26, 2017
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
To fix tests:
- DockerSwarmSuite.TestSwarmNetworkPluginV2
- DockerSwarmSuite.TestServiceLogs

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@andrewhsu andrewhsu changed the title WIP fix tests [17.07] WIP fix tests Jul 26, 2017
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@andrewhsu andrewhsu changed the title [17.07] WIP fix tests WIP [17.07] fix tests Jul 26, 2017
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

Thanks for tracking these down

out, _, err := dockerCmdWithError("rmi", "-f", imgID)
// rmi -f should not delete image with running containers
out, _, err := dockerCmdWithError("rmi", imgID)
// rmi should not delete image with running containers
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're interested in preserving the intent of this test I think the correct change is to check that this text is in stderr, but there is no error. It should continue to use -f

@@ -243,7 +243,7 @@ func (s *DockerSuite) TestRmiContainerImageNotFound(c *check.C) {
dockerCmd(c, "rmi", "-f", imageIds[1])

// Try to remove the image of the running container and see if it fails as expected.
out, _, err := dockerCmdWithError("rmi", "-f", imageIds[0])
out, _, err := dockerCmdWithError("rmi", imageIds[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@vieux
Copy link
Contributor

vieux commented Jul 27, 2017

looks like @tiborvass's fix works

@dnephin
Copy link
Contributor

dnephin commented Jul 27, 2017

I think the test fixes for rmi -f are wrong. I think we should revert docker/cli#160 in this branch instead. We need to revisit the -f behaviour.

I opened docker/cli#394 to track the problem. I can prepare a revert commit of you agree.

@tiborvass
Copy link
Contributor

LGTM

@andrewhsu
Copy link
Contributor Author

no longer needed because 17.07 is eol

@andrewhsu andrewhsu closed this Oct 5, 2017
@andrewhsu andrewhsu deleted the fix-rmi-f-test branch October 5, 2017 21:04
docker-jenkins pushed a commit that referenced this pull request Aug 17, 2018
docker-jenkins pushed a commit that referenced this pull request Dec 17, 2018
[18.09] libcontainerd: prevent exec delete locking
Upstream-commit: 484a3c3ad0fdb59ab9bb83ef2ff79184e216313f
Component: engine
akrasnov-drv pushed a commit to drivenets/docker-ce that referenced this pull request Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants