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

Successive docker starts do not start container every time #4410

Closed
hmahmood opened this issue Mar 27, 2017 · 5 comments
Closed

Successive docker starts do not start container every time #4410

hmahmood opened this issue Mar 27, 2017 · 5 comments
Milestone

Comments

@hmahmood
Copy link
Contributor

One docker start call after another in quick succession on the same container sometimes will return 125 exit code from the docker cli.

Steps to reproduce:

  1. docker create --name test busybox echo foo
  2. run in a tight loop: docker start -a test; should return "foo" and exit code 0 every time, but sometimes will return 125 with or without the correct output
@hmahmood
Copy link
Contributor Author

hmahmood commented Mar 27, 2017

For at least the case where we get a 125 and no output, I found out that the start command does not start the container. We get the powered off event for the previous start call late, and set the container state to stopped for the current call before we can actually power on the VM, which results in the container not getting started.

@hickeng
Copy link
Member

hickeng commented Mar 28, 2017

Where do we abort a start call based on the container state? It's supposed to be an idempotent operation, so portlayer would just do nothing if the container is already in the desired state - but that should be being determined based on the VM runtime structure, not the container cache state, which is primarily for faking the transition states.

@mhagen-vmware
Copy link
Contributor

@hmahmood Please prioritize and estimate and move to backlog.

@caglar10ur
Copy link
Contributor

vspc holds the connection even after the vm powers off up to 10 sec. so starting the vm again before this loops run causes issues.

267 // monitorVMConnections is to monitor connections and delete VM recoreds from the vmManager map when the containerVM exits
268 func (vspc *Vspc) monitorVMConnections() {
269     ticker := time.NewTicker(VMHealthCheckPeriod)
270     go func() {
271         for {
272             select {
273             case <-ticker.C:
274                 vspc.vmManagerMu.Lock()
275
276                 for k, vm := range vspc.vmManager {
277                     vm.Lock()
278                     if !vm.inVmotion && vm.containerConn.IsClosed() { // vm just shut down
279                         log.Debugf("(vspc) detected closed connection for VM %s", k)
280                         log.Debugf("(vspc) closing connection to the AttachServer")
281                         vm.remoteConn.Close()
282                         log.Debugf("(vspc) deleting vm records from the vm manager %s", k)
283                         delete(vspc.vmManager, k)
284                     }
285                     vm.Unlock()
286                 }
287                 vspc.vmManagerMu.Unlock()
288             case <-vspc.doneCh:
289                 ticker.Stop()
290                 return
291             }
292         }
293     }()
294 }

@stuclem
Copy link
Contributor

stuclem commented Apr 12, 2017

@mhagen-vmware should this go in the Release Notes for 1.1?

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
Projects
None yet
Development

No branches or pull requests

6 participants