Merge pull request #4592 from kolyshkin/exec-nits

Improvements to how `runc exec` is handled

Author: lifubang

exec.go

@@ -158,19 +158,7 @@ func execProcess(context *cli.Context) (int, error) {
 	if status == libcontainer.Paused && !context.Bool("ignore-paused") {
 		return -1, errors.New("cannot exec in a paused container (use --ignore-paused to override)")
 	}
-	path := context.String("process")
-	if path == "" && len(context.Args()) == 1 {
-		return -1, errors.New("process args cannot be empty")
-	}
-	state, err := container.State()
-	if err != nil {
-		return -1, err
-	}
-	bundle, ok := utils.SearchLabels(state.Config.Labels, "bundle")
-	if !ok {
-		return -1, errors.New("bundle not found in labels")
-	}
-	p, err := getProcess(context, bundle)
+	p, err := getProcess(context, container)
 	if err != nil {
 		return -1, err
 	}
@@ -196,7 +184,7 @@ func execProcess(context *cli.Context) (int, error) {
 	return r.run(p)
 }
 
-func getProcess(context *cli.Context, bundle string) (*specs.Process, error) {
+func getProcess(context *cli.Context, c *libcontainer.Container) (*specs.Process, error) {
 	if path := context.String("process"); path != "" {
 		f, err := os.Open(path)
 		if err != nil {
@@ -209,7 +197,11 @@ func getProcess(context *cli.Context, bundle string) (*specs.Process, error) {
 		}
 		return &p, validateProcessSpec(&p)
 	}
-	// process via cli flags
+	// Process from config.json and CLI flags.
+	bundle, ok := utils.SearchLabels(c.Config().Labels, "bundle")
+	if !ok {
+		return nil, errors.New("bundle not found in labels")
+	}
 	if err := os.Chdir(bundle); err != nil {
 		return nil, err
 	}
@@ -218,7 +210,11 @@ func getProcess(context *cli.Context, bundle string) (*specs.Process, error) {
 		return nil, err
 	}
 	p := spec.Process
-	p.Args = context.Args()[1:]
+	args := context.Args()
+	if len(args) < 2 {
+		return nil, errors.New("exec args cannot be empty")
+	}
+	p.Args = args[1:]
 	// Override the cwd, if passed.
 	if cwd := context.String("cwd"); cwd != "" {
 		p.Cwd = cwd

libcontainer/container_linux.go

@@ -432,11 +432,11 @@ func (c *Container) signal(s os.Signal) error {
 }
 
 func (c *Container) createExecFifo() (retErr error) {
-	rootuid, err := c.Config().HostRootUID()
+	rootuid, err := c.config.HostRootUID()
 	if err != nil {
 		return err
 	}
-	rootgid, err := c.Config().HostRootGID()
+	rootgid, err := c.config.HostRootGID()
 	if err != nil {
 		return err
 	}

utils_linux.go

@@ -43,9 +43,8 @@ func getDefaultImagePath() string {
 	return filepath.Join(cwd, "checkpoint")
 }
 
-// newProcess returns a new libcontainer Process with the arguments from the
-// spec and stdio from the current process.
-func newProcess(p specs.Process) (*libcontainer.Process, error) {
+// newProcess converts [specs.Process] to [libcontainer.Process].
+func newProcess(p *specs.Process) (*libcontainer.Process, error) {
 	lp := &libcontainer.Process{
 		Args:            p.Args,
 		Env:             p.Env,
@@ -89,7 +88,7 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
 }
 
 // setupIO modifies the given process config according to the options.
-func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, detach bool, sockpath string) (*tty, error) {
+func setupIO(process *libcontainer.Process, container *libcontainer.Container, createTTY, detach bool, sockpath string) (*tty, error) {
 	if createTTY {
 		process.Stdin = nil
 		process.Stdout = nil
@@ -135,6 +134,17 @@ func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, det
 		inheritStdio(process)
 		return &tty{}, nil
 	}
+
+	config := container.Config()
+	rootuid, err := config.HostRootUID()
+	if err != nil {
+		return nil, err
+	}
+	rootgid, err := config.HostRootGID()
+	if err != nil {
+		return nil, err
+	}
+
 	return setupProcessPipes(process, rootuid, rootgid)
 }
 
@@ -210,7 +220,7 @@ func (r *runner) run(config *specs.Process) (int, error) {
 	if err = r.checkTerminal(config); err != nil {
 		return -1, err
 	}
-	process, err := newProcess(*config)
+	process, err := newProcess(config)
 	if err != nil {
 		return -1, err
 	}
@@ -232,20 +242,12 @@ func (r *runner) run(config *specs.Process) (int, error) {
 		}
 		process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i)))
 	}
-	rootuid, err := r.container.Config().HostRootUID()
-	if err != nil {
-		return -1, err
-	}
-	rootgid, err := r.container.Config().HostRootGID()
-	if err != nil {
-		return -1, err
-	}
 	detach := r.detach || (r.action == CT_ACT_CREATE)
 	// Setting up IO is a two stage process. We need to modify process to deal
 	// with detaching containers, and then we get a tty after the container has
 	// started.
 	handler := newSignalHandler(r.enableSubreaper, r.notifySocket)
-	tty, err := setupIO(process, rootuid, rootgid, config.Terminal, detach, r.consoleSocket)
+	tty, err := setupIO(process, r.container, config.Terminal, detach, r.consoleSocket)
 	if err != nil {
 		return -1, err
 	}