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.

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

Running a container twice does not produce output the second time #4367

Closed
hmahmood opened this issue Mar 23, 2017 · 10 comments
Closed

Running a container twice does not produce output the second time #4367

hmahmood opened this issue Mar 23, 2017 · 10 comments
Labels
kind/defect Behavior that is inconsistent with what's intended priority/p2
Milestone

Comments

@hmahmood
Copy link
Contributor

docker run -t --name test busybox ls
docker start -a test

Second command does not produce output sometimes. At times when it does not produce output, running docker start -a test again would produce expected output from the second run immediately (even before the container has powered on).

@hmahmood
Copy link
Contributor Author

The problem is that we're reusing the ssh channel for attach from the previous run, when we should be tearing it down on container VM power off. There is race where attach will get the old connection first, and finally when the container attaches the old connection is rewritten in the connection cache with the new connection. We just need to remove the connection from the connection cache on container VM power off.

@caglar10ur
Copy link
Contributor

assigning this to myself as I believe I'm fix this as a part of exec attach work

@caglar10ur caglar10ur removed their assignment Mar 24, 2017
@caglar10ur
Copy link
Contributor

unassigned as I realized this is not in the sprint. @hmahmood @hickeng @sflxn what to do here?

@mhagen-vmware mhagen-vmware added kind/defect Behavior that is inconsistent with what's intended priority/p0 labels Mar 27, 2017
@mhagen-vmware
Copy link
Contributor

@caglar10ur It will be top priority for next sprint. If you have all your work done for this sprint already, go ahead and pull it in to current sprint. Thanks!

@hmahmood
Copy link
Contributor Author

hmahmood commented Mar 27, 2017

Another variation:

docker run -i --name test busybox sort <<< one
docker start -ai test <<< one

Should output one for both commands, but will intermittently fail if start is run more than once.

hickeng pushed a commit to hickeng/vic that referenced this issue Mar 29, 2017
@caglar10ur caglar10ur self-assigned this Mar 29, 2017
@caglar10ur caglar10ur added this to the Sprint 6 milestone Mar 29, 2017
@hickeng
Copy link
Member

hickeng commented Mar 30, 2017

In the case of the following:

docker run -i --name test busybox sort <<< one

CloseStdin is being called before the input data is ever copied to the container - see the reported EOF at the end of this snippet and the ordering of the calls to CloseStdin and Stdin:

Mar  7 2017 11:33:22.432Z DEBUG [BEGIN] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).CloseStdin:104]
Mar  7 2017 11:33:22.432Z DEBUG vspc read 68 bytes from the  remote system connection
Mar  7 2017 11:33:22.432Z DEBUG vspc relayed the read data to the containerVM
Mar  7 2017 11:33:22.470Z DEBUG (vspc) fsm read 1 bytes from the containerVM
Mar  7 2017 11:33:22.513Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/exec.(*containerBase).start:181] [11.519496193s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:22.513Z DEBUG Set container e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537 state: Running
Mar  7 2017 11:33:22.513Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/exec.(*Container).start:336] [11.519532998s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:22.513Z DEBUG [BEGIN] [github.com/vmware/vic/lib/portlayer/exec.(*Container).RefreshFromHandle:319] fc07130d1a29cd2231211d3722c13cb2
Mar  7 2017 11:33:22.513Z DEBUG Current ChangeVersion: 2017-03-07T11:33:11.25953Z
Mar  7 2017 11:33:22.513Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/exec.(*Container).RefreshFromHandle:319] [5.466µs] fc07130d1a29cd2231211d3722c13cb2
Mar  7 2017 11:33:22.513Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/exec.Commit:35] [11.745639561s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:22.513Z DEBUG [ END ] [github.com/vmware/vic/lib/apiservers/portlayer/restapi/handlers.(*ContainersHandlersImpl).CommitHandler:213] [11.745852095s] handle(fc07130d1a29cd2231211d3722c13cb2)
Mar  7 2017 11:33:22.525Z DEBUG Found 2 subscribers to events.ContainerEvent: Container e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537 Started
Mar  7 2017 11:33:22.525Z DEBUG Event manager calling back to PLE-4b74f1219a486f4e5493aff4aacaf26fab6e7e3934ed436faa72e00d3c787113
Mar  7 2017 11:33:22.525Z DEBUG Event manager calling back to netCtx(0xc420d1cc60)
Mar  7 2017 11:33:22.525Z DEBUG (vspc) fsm read 51 bytes from the containerVM
Mar  7 2017 11:33:22.525Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).CloseStdin:104] [93.859497ms]
Mar  7 2017 11:33:22.525Z DEBUG [ END ] [github.com/vmware/vic/lib/apiservers/portlayer/restapi/handlers.(*InteractionHandlersImpl).ContainerCloseStdinHandler:233] [11.822801621s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:22.526Z DEBUG vspc read 120 bytes from the  remote system connection
Mar  7 2017 11:33:22.526Z DEBUG vspc relayed the read data to the containerVM
Mar  7 2017 11:33:22.619Z DEBUG (vspc) fsm read 52 bytes from the containerVM
Mar  7 2017 11:33:22.619Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).Unblock:184] [295.241081ms]
Mar  7 2017 11:33:22.619Z INFO  attach connector: Unblocked e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537, returning
Mar  7 2017 11:33:22.619Z DEBUG attach connector: Found connection for e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537: 0xc42040f7d0
Mar  7 2017 11:33:22.619Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*Connector).Interaction:70] [11.917009996s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:22.619Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*Server).Interaction:105] [11.917017144s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:22.619Z DEBUG [BEGIN] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).Stdout:126]
Mar  7 2017 11:33:22.619Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).Stdout:126] [11.463µs]
Mar  7 2017 11:33:22.619Z DEBUG [ END ] [github.com/vmware/vic/lib/apiservers/portlayer/restapi/handlers.(*InteractionHandlersImpl).ContainerGetStdoutHandler:257] [11.917054593s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:22.620Z DEBUG vspc read 68 bytes from the  remote system connection
Mar  7 2017 11:33:22.620Z DEBUG vspc relayed the read data to the containerVM
Mar  7 2017 11:33:23.017Z DEBUG (vspc) fsm read 1 bytes from the containerVM
Mar  7 2017 11:33:23.055Z DEBUG (vspc) fsm read 103 bytes from the containerVM
Mar  7 2017 11:33:23.056Z DEBUG Finished streaming stdout for e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:23.056Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).Unblock:184] [731.829041ms]
Mar  7 2017 11:33:23.056Z INFO  attach connector: Unblocked e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537, returning
Mar  7 2017 11:33:23.056Z DEBUG attach connector: Found connection for e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537: 0xc42040f7d0
Mar  7 2017 11:33:23.056Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*Connector).Interaction:70] [12.353481347s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:23.056Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*Server).Interaction:105] [12.353487329s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537
Mar  7 2017 11:33:23.056Z DEBUG [BEGIN] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).Stdin:138]
Mar  7 2017 11:33:23.056Z DEBUG [ END ] [github.com/vmware/vic/lib/portlayer/attach/communication.(*interaction).Stdin:138] [4.149µs]
Mar  7 2017 11:33:23.056Z DEBUG Found primer bytes, port layer client might be personality server
Mar  7 2017 11:33:23.056Z ERROR Copy@ContainerSetStdinHandler returned EOF
Mar  7 2017 11:33:23.056Z DEBUG Done copying stdin
Mar  7 2017 11:33:23.056Z DEBUG [ END ] [github.com/vmware/vic/lib/apiservers/portlayer/restapi/handlers.(*InteractionHandlersImpl).ContainerSetStdinHandler:172] [12.353536622s] e5aca52fafe71b2a18fe1e946334682faf94068e1bf39e76819af964139ac537

@hmahmood
Copy link
Contributor Author

That is correct. #4352 fixes it.

@hickeng
Copy link
Member

hickeng commented Mar 30, 2017

@hmahmood which part of #4352 ?

@hmahmood
Copy link
Contributor Author

hmahmood commented Mar 30, 2017

Changes to cmd/tether/attach.go combined with only calling CloseWrite() on the channel when stdin closes: https://github.com/vmware/vic/pull/4352/files#diff-68824f0584e18cbd83689d34c87021c4R227

@mdubya66
Copy link
Contributor

connected to attach work. moving to medium

caglar10ur added a commit that referenced this issue Apr 27, 2017
* Attach support for exec'ed session

Includes @hmahmood's and @hickeng's various fixed to dio/persona and portlayer

Fixes #4263
Fixes #4367
Fixes #4598
Fixes #4410
Fixes #4339
Fixes #4881

Requires #4288

Implements exec functionality for personality
Those changes live in lib/apiservers/engine/backends/ directory. There are changes in container_proxy functions since exec and attach shares some common code paths. We started to pass ids instead of viccontainer structs around to them to support both workflows. AttachStreams function splits the cancelation logic into different parts. Closing stdin cancels other active streams. We also ensure that the API client connection is shutdown on exit.

Implements exec support for portlayer rest api
Those changes live in lib/apiservers/portlayer/ directory. One important change there is context changes. In the past we were calculating the timeouts manually and then passing timeouts as a parameter to Get call.
session, err := attachServer.Get(context.Background(), params.ID, timeout)

now each stream is creating its own context and passing it to the function - eliminating manual calculations and redundant timeout value.

session, err := i.server.Interaction(ctx, params.ID)

Portlayer attach subsystem re-organization
Those changes live in lib/portlayer/attach/. Some of them just renames and splitting monolithic file (they are now bind.go/join.go etc.) into pieces. Connector has new set of features. We now have a different unblock signal for signaling tether (called unblock) for starting the processes inside the container. We also now have a SSH channel mux running on client side to receive container ids from tether. Normally portlayer initiates the connection and asks container ids. This is still the case for the primary session. But after that primary session start event, any exec'ed session will cause tether to restart and as a part of that tether will push container ids to the portlayer. Additionally we now started to use channel mux to evict closed connections from the connection map (#4367). Lastly we started to use singleflight to make sure that there is only one call in-flight for each ID at any given time.

tether support for exec
As noted above, tether now pushes container ids to portlayer when it receives reload signal. It supports new unblock signal. Fixes a bug caused by wrong usage of cleanup functions. Fixes races causing incorrect behaviors.

dio changes
dio reader now has a PropagateEOF function toggles whether to return EOF when all readers return EOF. Setting this to true will result in an EOF if there are no readers available when Read is next called. dio writer now calls CloseWrite if the writes implements CloseWriter.
@mdubya66 mdubya66 added this to the Sprint 8 milestone May 1, 2017
matthewavery pushed a commit to matthewavery/vic that referenced this issue May 3, 2017
* Attach support for exec'ed session

Includes @hmahmood's and @hickeng's various fixed to dio/persona and portlayer

Fixes vmware#4263
Fixes vmware#4367
Fixes vmware#4598
Fixes vmware#4410
Fixes vmware#4339
Fixes vmware#4881

Requires vmware#4288

Implements exec functionality for personality
Those changes live in lib/apiservers/engine/backends/ directory. There are changes in container_proxy functions since exec and attach shares some common code paths. We started to pass ids instead of viccontainer structs around to them to support both workflows. AttachStreams function splits the cancelation logic into different parts. Closing stdin cancels other active streams. We also ensure that the API client connection is shutdown on exit.

Implements exec support for portlayer rest api
Those changes live in lib/apiservers/portlayer/ directory. One important change there is context changes. In the past we were calculating the timeouts manually and then passing timeouts as a parameter to Get call.
session, err := attachServer.Get(context.Background(), params.ID, timeout)

now each stream is creating its own context and passing it to the function - eliminating manual calculations and redundant timeout value.

session, err := i.server.Interaction(ctx, params.ID)

Portlayer attach subsystem re-organization
Those changes live in lib/portlayer/attach/. Some of them just renames and splitting monolithic file (they are now bind.go/join.go etc.) into pieces. Connector has new set of features. We now have a different unblock signal for signaling tether (called unblock) for starting the processes inside the container. We also now have a SSH channel mux running on client side to receive container ids from tether. Normally portlayer initiates the connection and asks container ids. This is still the case for the primary session. But after that primary session start event, any exec'ed session will cause tether to restart and as a part of that tether will push container ids to the portlayer. Additionally we now started to use channel mux to evict closed connections from the connection map (vmware#4367). Lastly we started to use singleflight to make sure that there is only one call in-flight for each ID at any given time.

tether support for exec
As noted above, tether now pushes container ids to portlayer when it receives reload signal. It supports new unblock signal. Fixes a bug caused by wrong usage of cleanup functions. Fixes races causing incorrect behaviors.

dio changes
dio reader now has a PropagateEOF function toggles whether to return EOF when all readers return EOF. Setting this to true will result in an EOF if there are no readers available when Read is next called. dio writer now calls CloseWrite if the writes implements CloseWriter.
fdawg4l pushed a commit to fdawg4l/vic that referenced this issue May 8, 2017
* Attach support for exec'ed session

Includes @hmahmood's and @hickeng's various fixed to dio/persona and portlayer

Fixes vmware#4263
Fixes vmware#4367
Fixes vmware#4598
Fixes vmware#4410
Fixes vmware#4339
Fixes vmware#4881

Requires vmware#4288

Implements exec functionality for personality
Those changes live in lib/apiservers/engine/backends/ directory. There are changes in container_proxy functions since exec and attach shares some common code paths. We started to pass ids instead of viccontainer structs around to them to support both workflows. AttachStreams function splits the cancelation logic into different parts. Closing stdin cancels other active streams. We also ensure that the API client connection is shutdown on exit.

Implements exec support for portlayer rest api
Those changes live in lib/apiservers/portlayer/ directory. One important change there is context changes. In the past we were calculating the timeouts manually and then passing timeouts as a parameter to Get call.
session, err := attachServer.Get(context.Background(), params.ID, timeout)

now each stream is creating its own context and passing it to the function - eliminating manual calculations and redundant timeout value.

session, err := i.server.Interaction(ctx, params.ID)

Portlayer attach subsystem re-organization
Those changes live in lib/portlayer/attach/. Some of them just renames and splitting monolithic file (they are now bind.go/join.go etc.) into pieces. Connector has new set of features. We now have a different unblock signal for signaling tether (called unblock) for starting the processes inside the container. We also now have a SSH channel mux running on client side to receive container ids from tether. Normally portlayer initiates the connection and asks container ids. This is still the case for the primary session. But after that primary session start event, any exec'ed session will cause tether to restart and as a part of that tether will push container ids to the portlayer. Additionally we now started to use channel mux to evict closed connections from the connection map (vmware#4367). Lastly we started to use singleflight to make sure that there is only one call in-flight for each ID at any given time.

tether support for exec
As noted above, tether now pushes container ids to portlayer when it receives reload signal. It supports new unblock signal. Fixes a bug caused by wrong usage of cleanup functions. Fixes races causing incorrect behaviors.

dio changes
dio reader now has a PropagateEOF function toggles whether to return EOF when all readers return EOF. Setting this to true will result in an EOF if there are no readers available when Read is next called. dio writer now calls CloseWrite if the writes implements CloseWriter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/defect Behavior that is inconsistent with what's intended priority/p2
Projects
None yet
Development

No branches or pull requests

5 participants