Automated rollback of changelist 210995199

PiperOrigin-RevId: 211116429
Change-Id: I446d149c822177dc9fc3c64ce5e455f7f029aa82

8 files changed, 37 insertions, 113 deletions

diff --git a/runsc/boot/config.go b/runsc/boot/config.go
index 212f5b003..efb8563ea 100644
--- a/runsc/boot/config.go
+++ b/runsc/boot/config.go
@@ -204,9 +204,6 @@ type Config struct {
 	// TODO: Remove this when multiple container is fully supported.
 	MultiContainer bool
 
-	// SpecFile is the file containing the OCI spec.
-	SpecFile string
-
 	// WatchdogAction sets what action the watchdog takes when triggered.
 	WatchdogAction watchdog.Action
 
diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go
index 4bd6fa12a..4e08dafc8 100644
--- a/runsc/cmd/boot.go
+++ b/runsc/cmd/boot.go
@@ -32,12 +32,9 @@ import (
 // Boot implements subcommands.Command for the "boot" command which starts a
 // new sandbox. It should not be called directly.
 type Boot struct {
-	// bundleDir is the directory containing the OCI spec.
+	// bundleDir is the path to the bundle directory.
 	bundleDir string
 
-	// specFD is the file descriptor that the spec will be read from.
-	specFD int
-
 	// controllerFD is the file descriptor of a stream socket for the
 	// control server that is donated to this process.
 	controllerFD int
@@ -71,7 +68,7 @@ func (*Boot) Usage() string {
 
 // SetFlags implements subcommands.Command.SetFlags.
 func (b *Boot) SetFlags(f *flag.FlagSet) {
-	f.IntVar(&b.specFD, "spec-fd", -1, "required fd with the container spec")
+	f.StringVar(&b.bundleDir, "bundle", "", "required path to the root of the bundle directory")
 	f.IntVar(&b.controllerFD, "controller-fd", -1, "required FD of a stream socket for the control server that must be donated to this process")
 	f.Var(&b.ioFDs, "io-fds", "list of FDs to connect 9P clients. They must follow this order: root first, then mounts as defined in the spec")
 	f.BoolVar(&b.console, "console", false, "set to true if the sandbox should allow terminal ioctl(2) syscalls")
@@ -81,7 +78,7 @@ func (b *Boot) SetFlags(f *flag.FlagSet) {
 // Execute implements subcommands.Command.Execute.  It starts a sandbox in a
 // waiting state.
 func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus {
-	if b.specFD == -1 || b.controllerFD == -1 || f.NArg() != 0 {
+	if b.bundleDir == "" || b.controllerFD == -1 || f.NArg() != 0 {
 		f.Usage()
 		return subcommands.ExitUsageError
 	}
@@ -89,10 +86,8 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
 	// Ensure that if there is a panic, all goroutine stacks are printed.
 	debug.SetTraceback("all")
 
-	// Get the spec from the specFD.
-	specFile := os.NewFile(uintptr(b.specFD), "spec file")
-	defer specFile.Close()
-	spec, err := specutils.ReadSpecFromFile(specFile)
+	// Get the spec from the bundleDir.
+	spec, err := specutils.ReadSpec(b.bundleDir)
 	if err != nil {
 		Fatalf("error reading spec: %v", err)
 	}
diff --git a/runsc/cmd/create.go b/runsc/cmd/create.go
index 38ae03e7a..94a889077 100644
--- a/runsc/cmd/create.go
+++ b/runsc/cmd/create.go
@@ -15,8 +15,6 @@
 package cmd
 
 import (
-	"path/filepath"
-
 	"context"
 	"flag"
 	"github.com/google/subcommands"
@@ -85,7 +83,6 @@ func (c *Create) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}
 		Fatalf("error reading spec: %v", err)
 	}
 	specutils.LogSpec(spec)
-	conf.SpecFile = filepath.Join(bundleDir, "config.json")
 
 	// Create the container. A new sandbox will be created for the
 	// container unless the metadata specifies that it should be run in an
diff --git a/runsc/cmd/run.go b/runsc/cmd/run.go
index 92aa6bc40..681112f30 100644
--- a/runsc/cmd/run.go
+++ b/runsc/cmd/run.go
@@ -15,7 +15,6 @@
 package cmd
 
 import (
-	"path/filepath"
 	"syscall"
 
 	"context"
@@ -72,8 +71,6 @@ func (r *Run) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) s
 	if err != nil {
 		Fatalf("error reading spec: %v", err)
 	}
-	specutils.LogSpec(spec)
-	conf.SpecFile = filepath.Join(bundleDir, "config.json")
 
 	ws, err := container.Run(id, spec, conf, bundleDir, r.consoleSocket, r.pidFile)
 	if err != nil {
diff --git a/runsc/main.go b/runsc/main.go
index 0c9b9af78..773ec6486 100644
--- a/runsc/main.go
+++ b/runsc/main.go
@@ -17,11 +17,13 @@
 package main
 
 import (
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
 	"strings"
 	"syscall"
+	"time"
 
 	"context"
 	"flag"
@@ -30,7 +32,6 @@ import (
 	"gvisor.googlesource.com/gvisor/pkg/log"
 	"gvisor.googlesource.com/gvisor/runsc/boot"
 	"gvisor.googlesource.com/gvisor/runsc/cmd"
-	"gvisor.googlesource.com/gvisor/runsc/specutils"
 )
 
 var (
@@ -47,8 +48,6 @@ var (
 	// Debugging flags.
 	debugLogDir = flag.String("debug-log-dir", "", "additional location for logs. It creates individual log files per command")
 	logPackets  = flag.Bool("log-packets", false, "enable network packet logging")
-	logFD       = flag.Int("log-fd", -1, "file descriptor to log to.  If set, the 'log' flag is ignored.")
-	debugLogFD  = flag.Int("debug-log-fd", -1, "file descriptor to write debug logs to.  If set, the 'debug-log-dir' flag is ignored.")
 
 	// Debugging flags: strace related
 	strace         = flag.Bool("strace", false, "enable strace")
@@ -65,7 +64,6 @@ var (
 	panicSignal    = flag.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.")
 )
 
-// gitRevision is set during linking.
 var gitRevision = ""
 
 func main() {
@@ -154,9 +152,7 @@ func main() {
 	}
 
 	var logFile io.Writer = os.Stderr
-	if *logFD > -1 {
-		logFile = os.NewFile(uintptr(*logFD), "log file")
-	} else if *logFilename != "" {
+	if *logFilename != "" {
 		// We must set O_APPEND and not O_TRUNC because Docker passes
 		// the same log file for all commands (and also parses these
 		// log files), so we can't destroy them on each command.
@@ -177,17 +173,18 @@ func main() {
 		cmd.Fatalf("invalid log format %q, must be 'json' or 'text'", *logFormat)
 	}
 
-	if *debugLogFD > -1 {
-		f := os.NewFile(uintptr(*debugLogFD), "debug log file")
-		e = log.MultiEmitter{e, log.GoogleEmitter{&log.Writer{Next: f}}}
-	} else if *debugLogDir != "" {
+	if *debugLogDir != "" {
 		if err := os.MkdirAll(*debugLogDir, 0775); err != nil {
 			cmd.Fatalf("error creating dir %q: %v", *debugLogDir, err)
 		}
-		subcommand := flag.CommandLine.Arg(0)
-		f, err := specutils.DebugLogFile(*debugLogDir, subcommand)
+
+		// Format: <debug-log-dir>/runsc.log.<yyymmdd-hhmmss.uuuuuu>.<command>
+		scmd := flag.CommandLine.Arg(0)
+		filename := fmt.Sprintf("runsc.log.%s.%s", time.Now().Format("20060102-150405.000000"), scmd)
+		path := filepath.Join(*debugLogDir, filename)
+		f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664)
 		if err != nil {
-			cmd.Fatalf("error opening debug log file in %q: %v", *debugLogDir, err)
+			cmd.Fatalf("error opening log file %q: %v", filename, err)
 		}
 		e = log.MultiEmitter{e, log.GoogleEmitter{&log.Writer{Next: f}}}
 	}
diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go
index f58916574..f14a2f8c9 100644
--- a/runsc/sandbox/sandbox.go
+++ b/runsc/sandbox/sandbox.go
@@ -233,6 +233,16 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund
 	// starts at 3 because 0, 1, and 2 are taken by stdin/out/err.
 	nextFD := 3
 
+	// Create control server socket here and donate FD to child process because
+	// it may be in a different network namespace and won't be reachable from
+	// outside.
+	addr := boot.ControlSocketAddr(s.ID)
+	fd, err := server.CreateSocket(addr)
+	log.Infof("Creating sandbox process with addr: %s", addr[1:]) // skip "\00".
+	if err != nil {
+		return fmt.Errorf("error creating control server socket for sandbox %q: %v", s.ID, err)
+	}
+
 	consoleEnabled := consoleSocket != ""
 
 	binPath, err := specutils.BinPath()
@@ -241,61 +251,16 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund
 	}
 	cmd := exec.Command(binPath, conf.ToFlags()...)
 	cmd.SysProcAttr = &syscall.SysProcAttr{}
+	cmd.Args = append(cmd.Args,
+		"boot",
+		"--bundle", bundleDir,
+		"--controller-fd="+strconv.Itoa(nextFD),
+		"--console="+strconv.FormatBool(consoleEnabled))
+	nextFD++
 
-	// Open the log files to pass to the sandbox as FDs.
-	//
-	// These flags must come BEFORE the "boot" command in cmd.Args.
-	if conf.LogFilename != "" {
-		logFile, err := os.OpenFile(conf.LogFilename, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
-		if err != nil {
-			return fmt.Errorf("error opening log file %q: %v", conf.LogFilename, err)
-		}
-		defer logFile.Close()
-		cmd.ExtraFiles = append(cmd.ExtraFiles, logFile)
-		cmd.Args = append(cmd.Args, "--log-fd="+strconv.Itoa(nextFD))
-		nextFD++
-	}
-	if conf.DebugLogDir != "" {
-		debugLogFile, err := specutils.DebugLogFile(conf.DebugLogDir, "boot")
-		if err != nil {
-			return fmt.Errorf("error opening debug log file in %q: %v", conf.DebugLogDir, err)
-		}
-		defer debugLogFile.Close()
-		cmd.ExtraFiles = append(cmd.ExtraFiles, debugLogFile)
-		cmd.Args = append(cmd.Args, "--debug-log-fd="+strconv.Itoa(nextFD))
-		nextFD++
-	}
-
-	// Add the "boot" command to the args.
-	//
-	// All flags after this must be for the boot command
-	cmd.Args = append(cmd.Args, "boot", "--console="+strconv.FormatBool(consoleEnabled))
-
-	// Create a socket for the control server and donate it to the sandbox.
-	addr := boot.ControlSocketAddr(s.ID)
-	sockFD, err := server.CreateSocket(addr)
-	log.Infof("Creating sandbox process with addr: %s", addr[1:]) // skip "\00".
-	if err != nil {
-		return fmt.Errorf("error creating control server socket for sandbox %q: %v", s.ID, err)
-	}
-	controllerFile := os.NewFile(uintptr(sockFD), "control_server_socket")
+	controllerFile := os.NewFile(uintptr(fd), "control_server_socket")
 	defer controllerFile.Close()
 	cmd.ExtraFiles = append(cmd.ExtraFiles, controllerFile)
-	cmd.Args = append(cmd.Args, "--controller-fd="+strconv.Itoa(nextFD))
-	nextFD++
-
-	// Open the spec file to donate to the sandbox.
-	if conf.SpecFile == "" {
-		return fmt.Errorf("conf.SpecFile must be set")
-	}
-	specFile, err := os.Open(conf.SpecFile)
-	if err != nil {
-		return fmt.Errorf("error opening spec file %q: %v", conf.SpecFile, err)
-	}
-	defer specFile.Close()
-	cmd.ExtraFiles = append(cmd.ExtraFiles, specFile)
-	cmd.Args = append(cmd.Args, "--spec-fd="+strconv.Itoa(nextFD))
-	nextFD++
 
 	// If there is a gofer, sends all socket ends to the sandbox.
 	for _, f := range ioFiles {
@@ -392,11 +357,6 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund
 		nss = append(nss, specs.LinuxNamespace{Type: specs.UserNamespace})
 	}
 
-	// Log the fds we are donating to the sandbox process.
-	for i, f := range cmd.ExtraFiles {
-		log.Debugf("Donating FD %d: %q", i+3, f.Name())
-	}
-
 	log.Debugf("Starting sandbox: %s %v", binPath, cmd.Args)
 	if err := specutils.StartInNS(cmd, nss); err != nil {
 		return err
diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go
index 477409112..5fb53edb2 100644
--- a/runsc/specutils/specutils.go
+++ b/runsc/specutils/specutils.go
@@ -108,24 +108,14 @@ func ValidateSpec(spec *specs.Spec) error {
 // ReadSpec reads an OCI runtime spec from the given bundle directory.
 func ReadSpec(bundleDir string) (*specs.Spec, error) {
 	// The spec file must be in "config.json" inside the bundle directory.
-	specPath := filepath.Join(bundleDir, "config.json")
-	specFile, err := os.Open(specPath)
+	specFile := filepath.Join(bundleDir, "config.json")
+	specBytes, err := ioutil.ReadFile(specFile)
 	if err != nil {
-		return nil, fmt.Errorf("error opening spec file %q: %v", specPath, err)
-	}
-	defer specFile.Close()
-	return ReadSpecFromFile(specFile)
-}
-
-// ReadSpecFromFile reads an OCI runtime spec from the given File.
-func ReadSpecFromFile(specFile *os.File) (*specs.Spec, error) {
-	specBytes, err := ioutil.ReadAll(specFile)
-	if err != nil {
-		return nil, fmt.Errorf("error reading spec from file %q: %v", specFile.Name(), err)
+		return nil, fmt.Errorf("error reading spec from file %q: %v", specFile, err)
 	}
 	var spec specs.Spec
 	if err := json.Unmarshal(specBytes, &spec); err != nil {
-		return nil, fmt.Errorf("error unmarshaling spec from file %q: %v\n %s", specFile.Name(), err, string(specBytes))
+		return nil, fmt.Errorf("error unmarshaling spec from file %q: %v\n %s", specFile, err, string(specBytes))
 	}
 	if err := ValidateSpec(&spec); err != nil {
 		return nil, err
@@ -356,11 +346,3 @@ func WaitForReady(pid int, timeout time.Duration, ready func() (bool, error)) er
 	}
 	return backoff.Retry(op, b)
 }
-
-// DebugLogFile opens a file in logDir based on the timestamp and subcommand
-// for writing.
-func DebugLogFile(logDir, subcommand string) (*os.File, error) {
-	// Format: <debug-log-dir>/runsc.log.<yyyymmdd-hhmmss.uuuuuu>.<command>
-	filename := fmt.Sprintf("runsc.log.%s.%s", time.Now().Format("20060102-150405.000000"), subcommand)
-	return os.OpenFile(filepath.Join(logDir, filename), os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664)
-}
diff --git a/runsc/test/testutil/testutil.go b/runsc/test/testutil/testutil.go
index 4429b981b..25987d040 100644
--- a/runsc/test/testutil/testutil.go
+++ b/runsc/test/testutil/testutil.go
@@ -176,7 +176,6 @@ func SetupContainerInRoot(rootDir string, spec *specs.Spec, conf *boot.Config) (
 	}
 
 	conf.RootDir = rootDir
-	conf.SpecFile = filepath.Join(bundleDir, "config.json")
 	return bundleDir, nil
 }