Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

flaky test integration-cli/DockerSuite.TestPostContainersAttach() #36611

Closed
kolyshkin opened this issue Mar 16, 2018 · 6 comments · Fixed by #36612
Closed

flaky test integration-cli/DockerSuite.TestPostContainersAttach() #36611

kolyshkin opened this issue Mar 16, 2018 · 6 comments · Fixed by #36612
Labels
area/testing status/needs-attention Calls for a collective discussion during a review session

Comments

@kolyshkin
Copy link
Contributor

kolyshkin commented Mar 16, 2018 •
edited
Loading

Looking at recent PRs CI, there are a few failures like this:

11:03:32 
11:03:32 ----------------------------------------------------------------------
11:03:32 FAIL: docker_api_attach_test.go:98: DockerSuite.TestPostContainersAttach
11:03:32 
11:03:32 docker_api_attach_test.go:211:
11:03:32     c.Assert(actualStdout.Bytes(), checker.DeepEquals, []byte("hello\nsuccess"), check.Commentf("Attach didn't return the expected data from stdout"))
11:03:32 ... obtained []uint8 = []byte{0x73, 0x75, 0x63, 0x63, 0x65, 0x73, 0x73}
11:03:32 ... expected []uint8 = []byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0xa, 0x73, 0x75, 0x63, 0x63, 0x65, 0x73, 0x73}
11:03:32 ... Attach didn't return the expected data from stdout
11:03:32

in particular:
@kolyshkin kolyshkin mentioned this issue Mar 16, 2018
Merged
@thaJeztah thaJeztah added status/needs-attention Calls for a collective discussion during a review session area/testing labels Mar 16, 2018
This was referenced Mar 16, 2018
Merged
Merged
@ncdc
Copy link
Contributor

ncdc commented Mar 16, 2018

This sounds like something has regressed w.r.t. attach+logs. The test is confirming this flow:

  1. Start container
  2. Container writes some data to stdout
  3. Attach to container with logs=true
  4. Write data to stdin of attach
  5. The output from attach should have the data written from step 2 (thus verifying logs=true) and from step 4 (verifying data emitted post-attach)

Is it possible the data from step 2 is somehow not yet written/available?

@thaJeztah
Copy link
Member

Commented on the PR, but wondering if there could be a relation with #36517, which was merged recently

@kolyshkin
Copy link
Contributor Author

kolyshkin commented Mar 16, 2018 •
edited
Loading

Is it possible the data from step 2 is somehow not yet written/available?

Well, echo is obviously executed before cat, and I can't think of a scenario in which echo output would appear later than the one from cat. So maybe this test failure reveals a real bug.

@kolyshkin kolyshkin mentioned this issue Mar 20, 2018
Merged
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Mar 20, 2018 •
edited
Loading

OK, one difference is io.ReadFull() reads the exact predefined amount of bytes (the size of the buffer passed), while the StdCopy() reads until EOF (which apparently it never gets as stdin is closed later) or until timeout (which happens in a second and which should be ignored by the test.

Still I can't see how this can result in missing hello\n string.

@anusha-ragunathan
Copy link
Contributor

One more failure https://jenkins.dockerproject.org/job/Docker-PRs/48603/console

@anusha-ragunathan anusha-ragunathan mentioned this issue Mar 20, 2018
Merged
@thaJeztah thaJeztah mentioned this issue Mar 22, 2018
Merged
@kolyshkin
Copy link
Contributor Author

This was presumably fixed by #36663. Let's reopen if we'll see more failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/testing status/needs-attention Calls for a collective discussion during a review session
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

TestPostContainersAttach: minor improvements kolyshkin/moby
4 participants
@ncdc @thaJeztah @kolyshkin @anusha-ragunathan