Skip to content

Fix sporadic failures of AttachContainerCmdIT tests - #1483

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

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

Conversation

@tejksat

@tejksat tejksat commented Nov 2, 2020

Copy link
Copy Markdown
Contributor

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

bsideup commented Nov 3, 2020

Copy link
Copy Markdown
Member

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

tejksat commented Nov 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

tejksat commented Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

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

bsideup commented Nov 6, 2020

Copy link
Copy Markdown
Member

@tejksat great! Thanks for verifying 💯

bsideup
bsideup approved these changes Nov 6, 2020
View reviewed changes

@bsideup bsideup left a comment

Copy link
Copy Markdown
Member

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

tejksat commented Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

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