Skip to content

Commit

Permalink
Use fifo for create/start
Browse files Browse the repository at this point in the history
This removes the use of a signal handler and SIGCONT to signal the init
process to exec the users process.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
  • Loading branch information
crosbymichael committed Jun 13, 2016
1 parent 8fbe19e commit 3aacff6
Show file tree
Hide file tree
Showing 12 changed files with 123 additions and 74 deletions.
10 changes: 8 additions & 2 deletions libcontainer/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ type BaseContainer interface {
Start(process *Process) (err error)

// Run immediatly starts the process inside the conatiner. Returns error if process
// fails to start. It does not block waiting for a SIGCONT after start returns but
// sends the signal when the process has completed.
// fails to start. It does not block waiting for the exec fifo after start returns but
// opens the fifo after start returns.
//
// errors:
// ContainerDestroyed - Container no longer exists,
Expand All @@ -148,4 +148,10 @@ type BaseContainer interface {
// errors:
// SystemError - System error.
Signal(s os.Signal) error

// Exec signals the container to exec the users process at the end of the init.
//
// errors:
// SystemError - System error.
Exec() error
}
60 changes: 44 additions & 16 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ import (

const stdioFdCount = 3

// InitContinueSignal is used to signal the container init process to
// start the users specified process after the container create has finished.
const InitContinueSignal = syscall.SIGCONT

type linuxContainer struct {
id string
root string
Expand Down Expand Up @@ -195,16 +191,39 @@ func (c *linuxContainer) Run(process *Process) error {
if err != nil {
return err
}
isInit := status == Stopped
if err := c.start(process, isInit); err != nil {
if err := c.start(process, status == Stopped); err != nil {
return err
}
if isInit {
return process.ops.signal(InitContinueSignal)
if status == Stopped {
return c.exec()
}
return nil
}

func (c *linuxContainer) Exec() error {
c.m.Lock()
defer c.m.Unlock()
return c.exec()
}

func (c *linuxContainer) exec() error {
path := filepath.Join(c.root, execFifoFilename)
f, err := os.OpenFile(path, os.O_RDONLY, 0)
if err != nil {
return newSystemErrorWithCause(err, "open exec fifo for reading")
}
defer f.Close()
data, err := ioutil.ReadAll(f)
if err != nil {
return err
}
if len(data) > 0 {
os.Remove(path)
return nil
}
return fmt.Errorf("cannot start an already running container")
}

func (c *linuxContainer) start(process *Process, isInit bool) error {
parent, err := c.newParentProcess(process, isInit)
if err != nil {
Expand Down Expand Up @@ -262,17 +281,21 @@ func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProces
if err != nil {
return nil, newSystemErrorWithCause(err, "creating new init pipe")
}
cmd, err := c.commandTemplate(p, childPipe)
rootDir, err := os.Open(c.root)
if err != nil {
return nil, err
}
cmd, err := c.commandTemplate(p, childPipe, rootDir)
if err != nil {
return nil, newSystemErrorWithCause(err, "creating new command template")
}
if !doInit {
return c.newSetnsProcess(p, cmd, parentPipe, childPipe)
return c.newSetnsProcess(p, cmd, parentPipe, childPipe, rootDir)
}
return c.newInitProcess(p, cmd, parentPipe, childPipe)
return c.newInitProcess(p, cmd, parentPipe, childPipe, rootDir)
}

func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec.Cmd, error) {
func (c *linuxContainer) commandTemplate(p *Process, childPipe, rootDir *os.File) (*exec.Cmd, error) {
cmd := &exec.Cmd{
Path: c.initPath,
Args: c.initArgs,
Expand All @@ -284,8 +307,10 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec.
if cmd.SysProcAttr == nil {
cmd.SysProcAttr = &syscall.SysProcAttr{}
}
cmd.ExtraFiles = append(p.ExtraFiles, childPipe)
cmd.Env = append(cmd.Env, fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1))
cmd.ExtraFiles = append(p.ExtraFiles, childPipe, rootDir)
cmd.Env = append(cmd.Env,
fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-2),
fmt.Sprintf("_LIBCONTAINER_STATEDIR=%d", stdioFdCount+len(cmd.ExtraFiles)-1))
// NOTE: when running a container with no PID namespace and the parent process spawning the container is
// PID1 the pdeathsig is being delivered to the container's init process by the kernel for some reason
// even with the parent still running.
Expand All @@ -295,7 +320,7 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec.
return cmd, nil
}

func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File) (*initProcess, error) {
func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe, rootDir *os.File) (*initProcess, error) {
cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initStandard))
nsMaps := make(map[configs.NamespaceType]string)
for _, ns := range c.config.Namespaces {
Expand All @@ -318,10 +343,11 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c
process: p,
bootstrapData: data,
sharePidns: sharePidns,
rootDir: rootDir,
}, nil
}

func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File) (*setnsProcess, error) {
func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe, rootDir *os.File) (*setnsProcess, error) {
cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initSetns))
state, err := c.currentState()
if err != nil {
Expand All @@ -342,6 +368,7 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe,
config: c.newInitConfig(p),
process: p,
bootstrapData: data,
rootDir: rootDir,
}, nil
}

Expand All @@ -360,6 +387,7 @@ func (c *linuxContainer) newInitConfig(process *Process) *initConfig {
AppArmorProfile: c.config.AppArmorProfile,
ProcessLabel: c.config.ProcessLabel,
Rlimits: c.config.Rlimits,
ExecFifoPath: filepath.Join(c.root, execFifoFilename),
}
if process.NoNewPrivileges != nil {
cfg.NoNewPrivileges = *process.NoNewPrivileges
Expand Down
33 changes: 20 additions & 13 deletions libcontainer/factory_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"fmt"
"os"
"os/exec"
"os/signal"
"path/filepath"
"regexp"
"runtime/debug"
Expand All @@ -24,7 +23,8 @@ import (
)

const (
stateFilename = "state.json"
stateFilename = "state.json"
execFifoFilename = "exec.fifo"
)

var (
Expand Down Expand Up @@ -168,6 +168,9 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
if err := os.MkdirAll(containerRoot, 0700); err != nil {
return nil, newGenericError(err, SystemError)
}
if err := syscall.Mkfifo(filepath.Join(containerRoot, execFifoFilename), 0666); err != nil {
return nil, newGenericError(err, SystemError)
}
c := &linuxContainer{
id: id,
root: containerRoot,
Expand Down Expand Up @@ -220,13 +223,18 @@ func (l *LinuxFactory) Type() string {
// StartInitialization loads a container by opening the pipe fd from the parent to read the configuration and state
// This is a low level implementation detail of the reexec and should not be consumed externally
func (l *LinuxFactory) StartInitialization() (err error) {
// start the signal handler as soon as we can
s := make(chan os.Signal, 1)
signal.Notify(s, InitContinueSignal)
fdStr := os.Getenv("_LIBCONTAINER_INITPIPE")
pipefd, err := strconv.Atoi(fdStr)
if err != nil {
return fmt.Errorf("error converting env var _LIBCONTAINER_INITPIPE(%q) to an int: %s", fdStr, err)
var pipefd, rootfd int
for k, v := range map[string]*int{
"_LIBCONTAINER_INITPIPE": &pipefd,
"_LIBCONTAINER_STATEDIR": &rootfd,
} {
s := os.Getenv(k)

i, err := strconv.Atoi(s)
if err != nil {
return fmt.Errorf("unable to convert %s=%s to int", k, s)
}
*v = i
}
var (
pipe = os.NewFile(uintptr(pipefd), "pipe")
Expand All @@ -235,6 +243,7 @@ func (l *LinuxFactory) StartInitialization() (err error) {
// clear the current process's environment to clean any libcontainer
// specific env vars.
os.Clearenv()

var i initer
defer func() {
// We have an error during the initialization of the container's init,
Expand All @@ -253,18 +262,16 @@ func (l *LinuxFactory) StartInitialization() (err error) {
// ensure that this pipe is always closed
pipe.Close()
}()

defer func() {
if e := recover(); e != nil {
err = fmt.Errorf("panic from initialization: %v, %v", e, string(debug.Stack()))
}
}()

i, err = newContainerInit(it, pipe)
i, err = newContainerInit(it, pipe, rootfd)
if err != nil {
return err
}
return i.Init(s)
return i.Init()
}

func (l *LinuxFactory) loadState(root string) (*State, error) {
Expand Down
12 changes: 7 additions & 5 deletions libcontainer/init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ type initConfig struct {
PassedFilesCount int `json:"passed_files_count"`
ContainerId string `json:"containerid"`
Rlimits []configs.Rlimit `json:"rlimits"`
ExecFifoPath string `json:"start_pipe_path"`
}

type initer interface {
Init(s chan os.Signal) error
Init() error
}

func newContainerInit(t initType, pipe *os.File) (initer, error) {
func newContainerInit(t initType, pipe *os.File, stateDirFD int) (initer, error) {
var config *initConfig
if err := json.NewDecoder(pipe).Decode(&config); err != nil {
return nil, err
Expand All @@ -79,9 +80,10 @@ func newContainerInit(t initType, pipe *os.File) (initer, error) {
}, nil
case initStandard:
return &linuxStandardInit{
pipe: pipe,
parentPid: syscall.Getppid(),
config: config,
pipe: pipe,
parentPid: syscall.Getppid(),
config: config,
stateDirFD: stateDirFD,
}, nil
}
return nil, fmt.Errorf("unknown init type %q", t)
Expand Down
4 changes: 2 additions & 2 deletions libcontainer/integration/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ func TestMain(m *testing.M) {
logrus.SetOutput(os.Stderr)
logrus.SetLevel(logrus.InfoLevel)

factory, err = libcontainer.New(".", libcontainer.Cgroupfs)
factory, err = libcontainer.New("/run/libctTests", libcontainer.Cgroupfs)
if err != nil {
logrus.Error(err)
os.Exit(1)
}
if systemd.UseSystemd() {
systemdFactory, err = libcontainer.New(".", libcontainer.SystemdCgroups)
systemdFactory, err = libcontainer.New("/run/libctTests", libcontainer.SystemdCgroups)
if err != nil {
logrus.Error(err)
os.Exit(1)
Expand Down
8 changes: 4 additions & 4 deletions libcontainer/integration/seccomp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ func TestSeccompPermitWriteConditional(t *testing.T) {
Args: []*configs.Arg{
{
Index: 0,
Value: 1,
Op: configs.GreaterThan,
Value: 2,
Op: configs.EqualTo,
},
},
},
Expand Down Expand Up @@ -162,8 +162,8 @@ func TestSeccompDenyWriteConditional(t *testing.T) {
Args: []*configs.Arg{
{
Index: 0,
Value: 1,
Op: configs.GreaterThan,
Value: 2,
Op: configs.EqualTo,
},
},
},
Expand Down
7 changes: 6 additions & 1 deletion libcontainer/integration/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package integration

import (
"bytes"
"crypto/md5"
"encoding/hex"
"fmt"
"io/ioutil"
"os"
Expand All @@ -11,6 +13,7 @@ import (
"strings"
"syscall"
"testing"
"time"

"github.com/opencontainers/runc/libcontainer"
"github.com/opencontainers/runc/libcontainer/configs"
Expand Down Expand Up @@ -92,7 +95,9 @@ func copyBusybox(dest string) error {
}

func newContainer(config *configs.Config) (libcontainer.Container, error) {
return newContainerWithName("testCT", config)
h := md5.New()
h.Write([]byte(time.Now().String()))
return newContainerWithName(hex.EncodeToString(h.Sum(nil)), config)
}

func newContainerWithName(name string, config *configs.Config) (libcontainer.Container, error) {
Expand Down
4 changes: 4 additions & 0 deletions libcontainer/process_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type setnsProcess struct {
fds []string
process *Process
bootstrapData io.Reader
rootDir *os.File
}

func (p *setnsProcess) startTime() (string, error) {
Expand All @@ -69,6 +70,7 @@ func (p *setnsProcess) start() (err error) {
defer p.parentPipe.Close()
err = p.cmd.Start()
p.childPipe.Close()
p.rootDir.Close()
if err != nil {
return newSystemErrorWithCause(err, "starting setns process")
}
Expand Down Expand Up @@ -186,6 +188,7 @@ type initProcess struct {
process *Process
bootstrapData io.Reader
sharePidns bool
rootDir *os.File
}

func (p *initProcess) pid() int {
Expand Down Expand Up @@ -230,6 +233,7 @@ func (p *initProcess) start() error {
err := p.cmd.Start()
p.process.ops = p
p.childPipe.Close()
p.rootDir.Close()
if err != nil {
p.process.ops = nil
return newSystemErrorWithCause(err, "starting init process command")
Expand Down
5 changes: 1 addition & 4 deletions libcontainer/setns_init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package libcontainer
import (
"fmt"
"os"
"os/signal"

"github.com/opencontainers/runc/libcontainer/apparmor"
"github.com/opencontainers/runc/libcontainer/keys"
Expand All @@ -24,7 +23,7 @@ func (l *linuxSetnsInit) getSessionRingName() string {
return fmt.Sprintf("_ses.%s", l.config.ContainerId)
}

func (l *linuxSetnsInit) Init(s chan os.Signal) error {
func (l *linuxSetnsInit) Init() error {
if !l.config.Config.NoNewKeyring {
// do not inherit the parent's session keyring
if _, err := keyctl.JoinSessionKeyring(l.getSessionRingName()); err != nil {
Expand All @@ -50,7 +49,5 @@ func (l *linuxSetnsInit) Init(s chan os.Signal) error {
if err := label.SetProcessLabel(l.config.ProcessLabel); err != nil {
return err
}
signal.Stop(s)
close(s)
return system.Execv(l.config.Args[0], l.config.Args[0:], os.Environ())
}
Loading

0 comments on commit 3aacff6

Please sign in to comment.