Skip to content

Commit 8b87a17

Browse files
mheonrh-atomic-bot
authored andcommitted
Add tracking for exec session IDs
Exec sessions now have an ID generated and assigned to their PID and stored in the database state. This allows us to track what exec sessions are currently active. Signed-off-by: Matthew Heon <[email protected]> Closes: #412 Approved by: baude
1 parent aea4f24 commit 8b87a17

File tree

5 files changed

+135
-8
lines changed

5 files changed

+135
-8
lines changed

libpod/container_api.go

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ import (
55
"io/ioutil"
66
"os"
77
"path/filepath"
8+
"strconv"
9+
"time"
810

911
"github.com/docker/docker/daemon/caps"
12+
"github.com/docker/docker/pkg/stringid"
1013
"github.com/docker/docker/pkg/term"
1114
"github.com/pkg/errors"
1215
"github.com/projectatomic/libpod/libpod/driver"
@@ -235,7 +238,86 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
235238
capList = caps.GetAllCapabilities()
236239
}
237240

238-
return c.runtime.ociRuntime.execContainer(c, cmd, tty, user, capList, env)
241+
// Generate exec session ID
242+
// Ensure we don't conflict with an existing session ID
243+
sessionID := stringid.GenerateNonCryptoID()
244+
found := true
245+
// This really ought to be a do-while, but Go doesn't have those...
246+
for found {
247+
found = false
248+
for id, _ := range c.state.ExecSessions {
249+
if id == sessionID {
250+
found = true
251+
break
252+
}
253+
}
254+
if found == true {
255+
sessionID = stringid.GenerateNonCryptoID()
256+
}
257+
}
258+
259+
execCmd, err := c.runtime.ociRuntime.execContainer(c, cmd, capList, env, tty, user, sessionID)
260+
if err != nil {
261+
return errors.Wrapf(err, "error creating exec command for container %s", c.ID())
262+
}
263+
264+
if err := execCmd.Start(); err != nil {
265+
return errors.Wrapf(err, "error starting exec command for container %s", c.ID())
266+
}
267+
268+
pidFile := c.execPidPath(sessionID)
269+
const pidWaitTimeout = 250
270+
271+
// Wait until runc makes the pidfile
272+
// TODO: If runc errors before the PID file is created, we have to wait for timeout here
273+
if err := WaitForFile(pidFile, pidWaitTimeout * time.Millisecond); err != nil {
274+
logrus.Debugf("Timed out waiting for pidfile from runc for container %s exec", c.ID())
275+
276+
// Check if an error occurred in the process before we made a pidfile
277+
// TODO: Wait() here is a poor choice - is there a way to see if
278+
// a process has finished, instead of waiting for it to finish?
279+
if err := execCmd.Wait(); err != nil {
280+
return err
281+
}
282+
283+
return errors.Wrapf(err, "timed out waiting for runc to create pidfile for exec session in container %s", c.ID())
284+
}
285+
286+
// Pidfile exists, read it
287+
contents, err := ioutil.ReadFile(pidFile)
288+
if err != nil {
289+
// We don't know the PID of the exec session
290+
// However, it may still be alive
291+
// TODO handle this better
292+
return errors.Wrapf(err, "could not read pidfile for exec session %s in container %s", sessionID, c.ID())
293+
}
294+
pid, err := strconv.ParseInt(string(contents), 10, 32)
295+
if err != nil {
296+
// As above, we don't have a valid PID, but the exec session is likely still alive
297+
// TODO handle this better
298+
return errors.Wrapf(err, "error parsing PID of exec session %s in container %s", sessionID, c.ID())
299+
}
300+
301+
// We have the PID, add it to state
302+
if c.state.ExecSessions == nil {
303+
c.state.ExecSessions = make(map[string]int)
304+
}
305+
c.state.ExecSessions[sessionID] = int(pid)
306+
if err := c.save(); err != nil {
307+
// Now we have a PID but we can't save it in the DB
308+
// TODO handle this better
309+
return errors.Wrapf(err, "error saving exec sessions %s for container %s", sessionID, c.ID())
310+
}
311+
312+
waitErr := execCmd.Wait()
313+
314+
// Remove the exec session from state
315+
delete(c.state.ExecSessions, sessionID)
316+
if err := c.save(); err != nil {
317+
logrus.Errorf("Error removing exec session %s from container %s state: %v", sessionID, c.ID(), err)
318+
}
319+
320+
return waitErr
239321
}
240322

241323
// Attach attaches to a container

libpod/container_inspect.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ func (c *Container) getContainerInspectData(size bool, driverData *inspect.Data)
2020
args = args[1:]
2121
}
2222

23+
execIDs := []string{}
24+
for id, _ := range c.state.ExecSessions {
25+
execIDs = append(execIDs, id)
26+
}
27+
2328
data := &inspect.ContainerInspectData{
2429
ID: config.ID,
2530
Created: config.CreatedTime,
@@ -50,7 +55,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *inspect.Data)
5055
MountLabel: config.MountLabel,
5156
ProcessLabel: spec.Process.SelinuxLabel,
5257
AppArmorProfile: spec.Process.ApparmorProfile,
53-
ExecIDs: []string{}, //TODO
58+
ExecIDs: execIDs,
5459
GraphDriver: driverData,
5560
Mounts: spec.Mounts,
5661
NetworkSettings: &inspect.NetworkSettings{

libpod/container_internal.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ func (c *Container) attachSocketPath() string {
100100
return filepath.Join(c.runtime.ociRuntime.socketsDir, c.ID(), "attach")
101101
}
102102

103+
// Get PID file path for a container's exec session
104+
func (c *Container) execPidPath(sessionID string) string {
105+
return filepath.Join(c.state.RunDir, "exec_pid_" + sessionID)
106+
}
107+
103108
// Sync this container with on-disk state and runc status
104109
// Should only be called with container lock held
105110
// This function should suffice to ensure a container's state is accurate and

libpod/oci.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,15 @@ func (r *OCIRuntime) unpauseContainer(ctr *Container) error {
471471
// TODO: Add --detach support
472472
// TODO: Convert to use conmon
473473
// TODO: add --pid-file and use that to generate exec session tracking
474-
func (r *OCIRuntime) execContainer(c *Container, cmd []string, tty bool, user string, capAdd, env []string) error {
474+
func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty bool, user, sessionID string) (*exec.Cmd, error) {
475+
if len(cmd) == 0 {
476+
return nil, errors.Wrapf(ErrInvalidArg, "must provide a command to execute")
477+
}
478+
479+
if sessionID == "" {
480+
return nil, errors.Wrapf(ErrEmptyID, "must provide a session ID for exec")
481+
}
482+
475483
args := []string{}
476484

477485
// TODO - should we maintain separate logpaths for exec sessions?
@@ -481,6 +489,8 @@ func (r *OCIRuntime) execContainer(c *Container, cmd []string, tty bool, user st
481489

482490
args = append(args, "--cwd", c.config.Spec.Process.Cwd)
483491

492+
args = append(args, "--pid-file", c.execPidPath(sessionID))
493+
484494
if tty {
485495
args = append(args, "--tty")
486496
}
@@ -512,9 +522,5 @@ func (r *OCIRuntime) execContainer(c *Container, cmd []string, tty bool, user st
512522
execCmd.Stderr = os.Stderr
513523
execCmd.Stdin = os.Stdin
514524

515-
if err := execCmd.Start(); err != nil {
516-
return errors.Wrapf(err, "error starting exec command for container %s", c.ID())
517-
}
518-
519-
return execCmd.Wait()
525+
return execCmd, nil
520526
}

libpod/util.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,3 +107,32 @@ func MountExists(specMounts []spec.Mount, dest string) bool {
107107
}
108108
return false
109109
}
110+
111+
// WaitForFile waits until a file has been created or the given timeout has occurred
112+
func WaitForFile(path string, timeout time.Duration) error {
113+
done := make(chan struct{})
114+
chControl := make(chan struct{})
115+
go func() {
116+
for {
117+
select {
118+
case <-chControl:
119+
return
120+
default:
121+
_, err := os.Stat(path)
122+
if err == nil {
123+
close(done)
124+
return
125+
}
126+
time.Sleep(25 * time.Millisecond)
127+
}
128+
}
129+
}()
130+
131+
select {
132+
case <-done:
133+
return nil
134+
case <-time.After(timeout):
135+
close(chControl)
136+
return errors.Wrapf(ErrInternal, "timed out waiting for file %s", path)
137+
}
138+
}

0 commit comments

Comments
 (0)