Skip to content

Fix sporadic failures of AttachContainerCmdIT tests#1483

Merged
bsideup merged 6 commits intodocker-java:masterfrom
tejksat:alexander.koshevoy/fixSporadicFailuresOfAttachContainerCmdIT
Nov 6, 2020
Merged

Fix sporadic failures of AttachContainerCmdIT tests#1483
bsideup merged 6 commits intodocker-java:masterfrom
tejksat:alexander.koshevoy/fixSporadicFailuresOfAttachContainerCmdIT

Conversation

@tejksat
Copy link
Copy Markdown
Contributor

@tejksat tejksat commented Nov 2, 2020

Fix AttachContainerCmdIT, which used to fail for me from time to time because of the race condition. I believe the canonical way is to: (1) create the container, (2) attach to it, and then (3) start it. The last two actions have been previously shifted. This led to the race condition: the container might have finished its execution before the attach actually happened.

Please note that the test com.github.dockerjava.cmd.AttachContainerCmdIT.attachContainerWithTTY might fail now with the exception:

java.lang.AssertionError: 
Expected: a string containing "stdout\r\nstderr"
     but: was ""
<Click to see difference>


	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at com.github.dockerjava.cmd.AttachContainerCmdIT.attachContainerWithTTY(AttachContainerCmdIT.java:175)
	...

This happens because docker-java-stream-... worker thread is stuck on recv() system call while receiving HTTP headers response for attach request to the container:

"docker-java-stream--616211391@3896" daemon prio=5 tid=0x1b nid=NA runnable
  java.lang.Thread.State: RUNNABLE
	  at com.github.dockerjava.okhttp.UnixDomainSocket.recv(UnixDomainSocket.java:-1)
	  at com.github.dockerjava.okhttp.UnixDomainSocket$UnixSocketInputStream.read(UnixDomainSocket.java:240)
	  at java.io.FilterInputStream.read(FilterInputStream.java:133)
	  at com.github.dockerjava.okhttp.UnixSocketFactory$1$1.read(UnixSocketFactory.java:45)
	  at okio.Okio$2.read(Okio.java:140)
	  at okio.AsyncTimeout$2.read(AsyncTimeout.java:237)
	  at okio.RealBufferedSource.indexOf(RealBufferedSource.java:358)
	  at okio.RealBufferedSource.readUtf8LineStrict(RealBufferedSource.java:230)
	  at okhttp3.internal.http1.Http1ExchangeCodec.readHeaderLine(Http1ExchangeCodec.java:242)
	  at okhttp3.internal.http1.Http1ExchangeCodec.readHeaders(Http1ExchangeCodec.java:251)
	  at okhttp3.internal.http1.Http1ExchangeCodec.readResponseHeaders(Http1ExchangeCodec.java:219)
	  at okhttp3.internal.connection.Exchange.readResponseHeaders(Exchange.java:115)
	  at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:94)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:117)
	  at com.github.dockerjava.okhttp.HijackingInterceptor.intercept(HijackingInterceptor.java:20)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)
	  at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:43)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:117)
	  at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:94)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:117)
	  at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)
	  at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:88)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:142)
	  at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:117)
	  at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:229)
	  at okhttp3.RealCall.execute(RealCall.java:81)
	  at com.github.dockerjava.okhttp.OkDockerHttpClient$OkResponse.<init>(OkDockerHttpClient.java:251)
	  at com.github.dockerjava.okhttp.OkDockerHttpClient.execute(OkDockerHttpClient.java:225)
	  at com.github.dockerjava.cmd.TrackingDockerHttpClient.execute(TrackingDockerHttpClient.java:27)
	  at com.github.dockerjava.core.DefaultInvocationBuilder.execute(DefaultInvocationBuilder.java:228)
	  at com.github.dockerjava.core.DefaultInvocationBuilder.lambda$executeAndStream$1(DefaultInvocationBuilder.java:269)
	  at com.github.dockerjava.core.DefaultInvocationBuilder$$Lambda$82.1028811481.run(Unknown Source:-1)
	  at java.lang.Thread.run(Thread.java:834)

There is the PR with the fix for it #1476.

@tejksat tejksat changed the title Alexander.koshevoy/fix sporadic failures of attach container cmd it Fix sporadic failures of AttachContainerCmdIT Nov 2, 2020
@tejksat tejksat changed the title Fix sporadic failures of AttachContainerCmdIT Fix sporadic failures of AttachContainerCmdIT tests Nov 2, 2020
@tejksat tejksat mentioned this pull request Nov 3, 2020
Merged
@bsideup bsideup added this to the next milestone Nov 3, 2020
@bsideup
Copy link
Copy Markdown
Member

bsideup commented Nov 3, 2020

What's interesting is that, despite it working, the docs suggest that the container should be running:
https://docs.docker.com/engine/reference/commandline/attach/#description

But the tests are passing, so I think this is just some (outdated?) issue with the docs.

Another thing is that, if we attach before starting, sleep 1 can be removed and we won't miss anything - could you please try removing it and check? Thanks!

@tejksat
Copy link
Copy Markdown
Contributor Author

tejksat commented Nov 6, 2020 •
edited
Loading

What's interesting is that, despite it working, the docs suggest that the container should be running:
https://docs.docker.com/engine/reference/commandline/attach/#description

@bsideup after playing with docker attach in the Terminal this seems to be true for the CLI command. But it is evidently not the case for the Docker REST API command /containers/{id}/attach. docker command-line tool uses the same approach: (1) create the container (at line 128), (2) attach to the container (at line 156), (3) start the container (at line 167).

@tejksat
Copy link
Copy Markdown
Contributor Author

tejksat commented Nov 6, 2020

Another thing is that, if we attach before starting, sleep 1 can be removed and we won't miss anything - could you please try removing it and check?

Done! Please take a look at the two latest commits.

@bsideup
Copy link
Copy Markdown
Member

bsideup commented Nov 6, 2020

@tejksat great! Thanks for verifying 💯

bsideup
bsideup approved these changes Nov 6, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@bsideup bsideup left a comment

Choose a reason for hiding this comment

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

👍

@bsideup bsideup merged commit ce5e9eb into docker-java:master Nov 6, 2020
@tejksat
Copy link
Copy Markdown
Contributor Author

tejksat commented Nov 6, 2020

Yay!:) Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bsideup bsideup bsideup approved these changes

Assignees

No one assigned

Labels

type/housekeeping

Projects

None yet

Milestone

3.2.6

Development

Successfully merging this pull request may close these issues.

2 participants

@tejksat @bsideup