From 1b5b9aa124a8acc5aaf126db8956b585fc5527f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Thu, 31 Mar 2016 02:16:32 -0400 Subject: [PATCH 1/2] Use fork for command execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This introduces a new "forkexec" sub-command which is used for all exec calls. The thought is that the random lockups we were seeing before this were caused by a race between go-lxc calling liblxc which called fork() and some of the other goroutines. Closes #1803 Signed-off-by: Stéphane Graber --- lxc/exec.go | 3 +- lxd/container.go | 4 +- lxd/container_exec.go | 65 ++++++++++++-------------------- lxd/container_lxc.go | 52 +++++++++++++++++++------ lxd/containers.go | 88 ++++++++++++++++++++++++++++++++++++++++++- lxd/main.go | 8 ++++ 6 files changed, 162 insertions(+), 58 deletions(-) diff --git a/lxc/exec.go b/lxc/exec.go index 846bbc523b70..87427eda2d84 100644 --- a/lxc/exec.go +++ b/lxc/exec.go @@ -162,7 +162,6 @@ func (c *execCmd) run(config *lxd.Config, args []string) error { termios.Restore(cfd, oldttystate) } - /* we get the result of waitpid() here so we need to transform it */ - os.Exit(ret >> 8) + os.Exit(ret) return fmt.Errorf(i18n.G("unreachable return reached")) } diff --git a/lxd/container.go b/lxd/container.go index 5e216a649ede..b3d8ac5f1b39 100644 --- a/lxd/container.go +++ b/lxd/container.go @@ -346,6 +346,9 @@ type container interface { FilePull(srcpath string, dstpath string) error FilePush(srcpath string, dstpath string, uid int, gid int, mode os.FileMode) error + // Command execution + Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File) (int, error) + // Status Render() (interface{}, error) RenderState() (*shared.ContainerState, error) @@ -383,7 +386,6 @@ type container interface { LogPath() string // FIXME: Those should be internal functions - LXContainerGet() *lxc.Container StorageStart() error StorageStop() error Storage() storage diff --git a/lxd/container_exec.go b/lxd/container_exec.go index 6fedf9ec9d81..2e706050dc0e 100644 --- a/lxd/container_exec.go +++ b/lxd/container_exec.go @@ -12,7 +12,6 @@ import ( "github.com/gorilla/mux" "github.com/gorilla/websocket" - "gopkg.in/lxc/go-lxc.v2" "github.com/lxc/lxd/shared" ) @@ -26,22 +25,13 @@ type commandPostContent struct { Height int `json:"height"` } -func runCommand(container *lxc.Container, command []string, options lxc.AttachOptions) (int, error) { - status, err := container.RunCommandStatus(command, options) - if err != nil { - shared.Debugf("Failed running command: %q", err.Error()) - return 0, err - } - - return status, nil -} - type execWs struct { - command []string - container *lxc.Container + command []string + container container + env map[string]string + rootUid int rootGid int - options lxc.AttachOptions conns map[int]*websocket.Conn connsLock sync.Mutex allConnected chan bool @@ -109,13 +99,18 @@ func (s *execWs) Do(op *operation) error { var ttys []*os.File var ptys []*os.File + var stdin *os.File + var stdout *os.File + var stderr *os.File + if s.interactive { ttys = make([]*os.File, 1) ptys = make([]*os.File, 1) ptys[0], ttys[0], err = shared.OpenPty(s.rootUid, s.rootGid) - s.options.StdinFd = ttys[0].Fd() - s.options.StdoutFd = ttys[0].Fd() - s.options.StderrFd = ttys[0].Fd() + + stdin = ttys[0] + stdout = ttys[0] + stderr = ttys[0] if s.width > 0 && s.height > 0 { shared.SetSize(int(ptys[0].Fd()), s.width, s.height) @@ -129,9 +124,10 @@ func (s *execWs) Do(op *operation) error { return err } } - s.options.StdinFd = ptys[0].Fd() - s.options.StdoutFd = ttys[1].Fd() - s.options.StderrFd = ttys[2].Fd() + + stdin = ptys[0] + stdout = ttys[1] + stderr = ttys[2] } controlExit := make(chan bool) @@ -216,11 +212,7 @@ func (s *execWs) Do(op *operation) error { } } - cmdResult, cmdErr := runCommand( - s.container, - s.command, - s.options, - ) + cmdResult, cmdErr := s.container.Exec(s.command, s.env, stdin, stdout, stderr) for _, tty := range ttys { tty.Close() @@ -274,22 +266,17 @@ func containerExecPost(d *Daemon, r *http.Request) Response { return BadRequest(err) } - opts := lxc.DefaultAttachOptions - opts.ClearEnv = true - opts.Env = []string{} + env := map[string]string{} for k, v := range c.ExpandedConfig() { if strings.HasPrefix(k, "environment.") { - opts.Env = append(opts.Env, fmt.Sprintf("%s=%s", strings.TrimPrefix(k, "environment."), v)) + env[strings.TrimPrefix(k, "environment.")] = v } } if post.Environment != nil { for k, v := range post.Environment { - if k == "HOME" { - opts.Cwd = v - } - opts.Env = append(opts.Env, fmt.Sprintf("%s=%s", k, v)) + env[k] = v } } @@ -310,7 +297,6 @@ func containerExecPost(d *Daemon, r *http.Request) Response { ws.allConnected = make(chan bool, 1) ws.controlConnected = make(chan bool, 1) ws.interactive = post.Interactive - ws.options = opts for i := -1; i < len(ws.conns)-1; i++ { ws.fds[i], err = shared.RandomCryptoString() if err != nil { @@ -319,7 +305,9 @@ func containerExecPost(d *Daemon, r *http.Request) Response { } ws.command = post.Command - ws.container = c.LXContainerGet() + ws.container = c + ws.env = env + ws.width = post.Width ws.height = post.Height @@ -340,13 +328,8 @@ func containerExecPost(d *Daemon, r *http.Request) Response { return err } defer nullDev.Close() - nullfd := nullDev.Fd() - - opts.StdinFd = nullfd - opts.StdoutFd = nullfd - opts.StderrFd = nullfd - _, cmdErr := runCommand(c.LXContainerGet(), post.Command, opts) + _, cmdErr := c.Exec(post.Command, env, nil, nil, nil) return cmdErr } diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go index b78a77a2140f..8918160287f8 100644 --- a/lxd/container_lxc.go +++ b/lxd/container_lxc.go @@ -2921,6 +2921,46 @@ func (c *containerLXC) FilePush(srcpath string, dstpath string, uid int, gid int return nil } +func (c *containerLXC) Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File) (int, error) { + envSlice := []string{} + + for k, v := range env { + envSlice = append(envSlice, fmt.Sprintf("%s=%s", k, v)) + } + + args := []string{c.daemon.execPath, "forkexec", c.name, c.daemon.lxcpath, filepath.Join(c.LogPath(), "lxc.conf")} + + args = append(args, "--") + args = append(args, "env") + args = append(args, envSlice...) + + args = append(args, "--") + args = append(args, "cmd") + args = append(args, command...) + + cmd := exec.Cmd{} + cmd.Path = c.daemon.execPath + cmd.Args = args + cmd.Stdin = stdin + cmd.Stdout = stdout + cmd.Stderr = stderr + + err := cmd.Run() + if err != nil { + exitErr, ok := err.(*exec.ExitError) + if ok { + status, ok := exitErr.Sys().(syscall.WaitStatus) + if ok { + return status.ExitStatus(), nil + } + } + + return -1, err + } + + return 0, nil +} + func (c *containerLXC) diskState() map[string]shared.ContainerStateDisk { disk := map[string]shared.ContainerStateDisk{} @@ -4332,18 +4372,6 @@ func (c *containerLXC) LastIdmapSet() (*shared.IdmapSet, error) { return lastIdmap, nil } -func (c *containerLXC) LXContainerGet() *lxc.Container { - // FIXME: This function should go away - - // Load the go-lxc struct - err := c.initLXC() - if err != nil { - return nil - } - - return c.c -} - func (c *containerLXC) Daemon() *Daemon { // FIXME: This function should go away return c.daemon diff --git a/lxd/containers.go b/lxd/containers.go index a7a8351c649a..e39983deafd8 100644 --- a/lxd/containers.go +++ b/lxd/containers.go @@ -5,6 +5,7 @@ import ( "os" "sort" "strconv" + "strings" "sync" "syscall" "time" @@ -198,8 +199,6 @@ func containerDeleteSnapshots(d *Daemon, cname string) error { * This is called by lxd when called as "lxd forkstart " * 'forkstart' is used instead of just 'start' in the hopes that people * do not accidentally type 'lxd start' instead of 'lxc start' - * - * We expect to read the lxcconfig over fd 3. */ func startContainer(args []string) error { if len(args) != 4 { @@ -246,3 +245,88 @@ func startContainer(args []string) error { return c.Start() } + +/* + * This is called by lxd when called as "lxd forkexec " + */ +func execContainer(args []string) (int, error) { + if len(args) < 6 { + return -1, fmt.Errorf("Bad arguments: %q", args) + } + + name := args[1] + lxcpath := args[2] + configPath := args[3] + + c, err := lxc.NewContainer(name, lxcpath) + if err != nil { + return -1, fmt.Errorf("Error initializing container for start: %q", err) + } + + err = c.LoadConfigFile(configPath) + if err != nil { + return -1, fmt.Errorf("Error opening startup config file: %q", err) + } + + syscall.Dup3(int(os.Stdin.Fd()), 200, 0) + syscall.Dup3(int(os.Stdout.Fd()), 201, 0) + syscall.Dup3(int(os.Stderr.Fd()), 202, 0) + + syscall.Close(int(os.Stdin.Fd())) + syscall.Close(int(os.Stdout.Fd())) + syscall.Close(int(os.Stderr.Fd())) + + opts := lxc.DefaultAttachOptions + opts.ClearEnv = true + opts.StdinFd = 200 + opts.StdoutFd = 201 + opts.StderrFd = 202 + + logPath := shared.LogPath(name, "forkexec.log") + if shared.PathExists(logPath) { + os.Remove(logPath) + } + + logFile, err := os.OpenFile(logPath, os.O_WRONLY|os.O_CREATE|os.O_SYNC, 0644) + if err == nil { + syscall.Dup3(int(logFile.Fd()), 1, 0) + syscall.Dup3(int(logFile.Fd()), 2, 0) + } + + env := []string{} + cmd := []string{} + + section := "" + for _, arg := range args[5:len(args)] { + if arg == "--" { + section = "" + continue + } + + if section == "" { + section = arg + continue + } + + if section == "env" { + fields := strings.SplitN(arg, "=", 2) + if len(fields) == 2 && fields[0] == "HOME" { + opts.Cwd = fields[1] + } + env = append(env, arg) + } else if section == "cmd" { + cmd = append(cmd, arg) + } else { + return -1, fmt.Errorf("Invalid exec section: %s", section) + } + } + + opts.Env = env + + status, err := c.RunCommandStatus(cmd, opts) + if err != nil { + return -1, fmt.Errorf("Failed running command: %q", err) + } + + return status >> 8, nil +} diff --git a/lxd/main.go b/lxd/main.go index 0a4bbd4da825..ca1245c5fd5b 100644 --- a/lxd/main.go +++ b/lxd/main.go @@ -138,6 +138,8 @@ func run() error { fmt.Printf(" How long to wait before failing\n") fmt.Printf("\n\nInternal commands (don't call these directly):\n") + fmt.Printf(" forkexec\n") + fmt.Printf(" Execute a command in a container\n") fmt.Printf(" forkgetnet\n") fmt.Printf(" Get container network information\n") fmt.Printf(" forkgetfile\n") @@ -219,6 +221,12 @@ func run() error { return MigrateContainer(os.Args[1:]) case "forkstart": return startContainer(os.Args[1:]) + case "forkexec": + ret, err := execContainer(os.Args[1:]) + if err != nil { + fmt.Fprintf(os.Stderr, "error: %v\n", err) + } + os.Exit(ret) } } From d76c549acc73ba1be88815c8742e3439ef156845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Thu, 31 Mar 2016 12:29:33 -0400 Subject: [PATCH 2/2] Tweak parser to allow -- in commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- lxd/containers.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lxd/containers.go b/lxd/containers.go index e39983deafd8..4231399b4e69 100644 --- a/lxd/containers.go +++ b/lxd/containers.go @@ -298,7 +298,8 @@ func execContainer(args []string) (int, error) { section := "" for _, arg := range args[5:len(args)] { - if arg == "--" { + // The "cmd" section must come last as it may contain a -- + if arg == "--" && section != "cmd" { section = "" continue }