From 1b1a42ba6dc7953db742959a54fd19124348f3fc Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 6 Dec 2018 15:26:58 -0800 Subject: [PATCH] A sandbox process should wait until it has not been moved into cgroups PiperOrigin-RevId: 224418900 Change-Id: I53cf4d7c1c70117875b6920f8fd3d58a3b1497e9 --- runsc/cmd/boot.go | 24 ++++++++++++++- runsc/sandbox/sandbox.go | 65 ++++++++++++++++++++++++++-------------- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index 5f5dca109..192df7d3c 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -69,6 +69,9 @@ type Boot struct { // userLogFD is the file descriptor to write user logs to. userLogFD int + + // startSyncFD is the file descriptor to synchronize runsc and sandbox. + startSyncFD int } // Name implements subcommands.Command.Name. @@ -99,12 +102,13 @@ func (b *Boot) SetFlags(f *flag.FlagSet) { f.IntVar(&b.cpuNum, "cpu-num", 0, "number of CPUs to create inside the sandbox") f.Uint64Var(&b.totalMem, "total-memory", 0, "sets the initial amount of total memory to report back to the container") f.IntVar(&b.userLogFD, "user-log-fd", 0, "file descriptor to write user logs to. 0 means no logging.") + f.IntVar(&b.startSyncFD, "start-sync-fd", -1, "required FD to used to synchronize sandbox startup") } // 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() != 1 { + if b.specFD == -1 || b.controllerFD == -1 || b.startSyncFD == -1 || f.NArg() != 1 { f.Usage() return subcommands.ExitUsageError } @@ -155,6 +159,14 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) panic("setCapsAndCallSelf must never return success") } + // Wait until this process has been moved into cgroups. + startSyncFile := os.NewFile(uintptr(b.startSyncFD), "start-sync file") + defer startSyncFile.Close() + buf := make([]byte, 1) + if r, err := startSyncFile.Read(buf); err != nil || r != 1 { + Fatalf("Unable to read from the start-sync descriptor: %v", err) + } + // Create the loader. bootArgs := boot.Args{ ID: f.Arg(0), @@ -173,9 +185,19 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) if err != nil { Fatalf("error creating loader: %v", err) } + // Fatalf exits the process and doesn't run defers. 'l' must be destroyed // explicitly! + // Notify the parent process the controller has been created. + if w, err := startSyncFile.Write(buf); err != nil || w != 1 { + l.Destroy() + Fatalf("Unable to write into the start-sync descriptor: %v", err) + } + // startSyncFile is closed here to be sure that starting with this point + // the runsc process will not write anything into it. + startSyncFile.Close() + // Notify other processes the loader has been created. l.NotifyLoaderCreated() diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 3f00eba94..0798aef9b 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -60,7 +60,7 @@ type Sandbox struct { // is running in. Chroot string `json:"chroot"` - // Ccroup has the cgroup configuration for the sandbox. + // Cgroup has the cgroup configuration for the sandbox. Cgroup *cgroup.Cgroup `json:"cgroup"` } @@ -69,7 +69,7 @@ type Sandbox struct { func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSocket, userLog string, ioFiles []*os.File) (*Sandbox, error) { s := &Sandbox{ID: id} // The Cleanup object cleans up partially created sandboxes when an error occurs. - // Any errors occuring during cleanup itself are ignored. + // Any errors occurring during cleanup itself are ignored. c := specutils.MakeCleanup(func() { _ = s.destroy() }) defer c.Clean() @@ -82,13 +82,25 @@ func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSo } } - // Create the sandbox process. - if err := s.createSandboxProcess(spec, conf, bundleDir, consoleSocket, userLog, ioFiles); err != nil { - return nil, err + // Create a socket pair to synchronize runsc and sandbox processes. + // It is used for the following: + // * to notify the sandbox process when it has been moved into cgroups. + // * to wait for the controller socket. + fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_SEQPACKET, 0) + if err != nil { + return nil, fmt.Errorf("error creating a start-sync socket pair %q: %v", s.ID, err) } + startSyncFile := os.NewFile(uintptr(fds[0]), "start-sync socket") + defer startSyncFile.Close() - // Wait for the control server to come up (or timeout). - if err := s.waitForCreated(20 * time.Second); err != nil { + sandboxSyncFile := os.NewFile(uintptr(fds[1]), "sandbox start-sync socket") + + // Create the sandbox process. + err = s.createSandboxProcess(spec, conf, bundleDir, consoleSocket, userLog, ioFiles, sandboxSyncFile) + // sandboxSyncFile has to be closed to be able to detect + // when the sandbox process exits unexpectedly. + sandboxSyncFile.Close() + if err != nil { return nil, err } @@ -98,6 +110,24 @@ func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSo } } + b := make([]byte, 1) + // Notify the sandbox process it has been moved into cgroups. + if l, err := startSyncFile.Write(b); err != nil || l != 1 { + return nil, fmt.Errorf("error writing into the start-sync descriptor: %v", err) + } + // Wait until the sandbox process has initialized the controller socket. + if l, err := startSyncFile.Read(b); err != nil || l != 1 { + return nil, fmt.Errorf("error reading from the start-sync descriptor: %v", err) + } + // startSyncFile is closed here to be sure that starting with this point + // the sandbox process will not write anything into it. + startSyncFile.Close() + + // Wait for the control server to come up. + if err := s.waitForCreated(); err != nil { + return nil, err + } + c.Release() return s, nil } @@ -282,7 +312,7 @@ func (s *Sandbox) connError(err error) error { // createSandboxProcess starts the sandbox as a subprocess by running the "boot" // command, passing in the bundle dir. -func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bundleDir, consoleSocket, userLog string, ioFiles []*os.File) error { +func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bundleDir, consoleSocket, userLog string, ioFiles []*os.File, startSyncFile *os.File) error { // nextFD is used to get unused FDs that we can pass to the sandbox. It // starts at 3 because 0, 1, and 2 are taken by stdin/out/err. nextFD := 3 @@ -346,6 +376,10 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund cmd.Args = append(cmd.Args, "--spec-fd="+strconv.Itoa(nextFD)) nextFD++ + cmd.ExtraFiles = append(cmd.ExtraFiles, startSyncFile) + cmd.Args = append(cmd.Args, "--start-sync-fd="+strconv.Itoa(nextFD)) + nextFD++ + // If there is a gofer, sends all socket ends to the sandbox. for _, f := range ioFiles { defer f.Close() @@ -581,21 +615,8 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund // waitForCreated waits for the sandbox subprocess control server to be // running and for the loader to have been created, at which point the sandbox // is in Created state. -func (s *Sandbox) waitForCreated(timeout time.Duration) error { +func (s *Sandbox) waitForCreated() error { log.Debugf("Waiting for sandbox %q creation", s.ID) - - ready := func() (bool, error) { - c, err := client.ConnectTo(boot.ControlSocketAddr(s.ID)) - if err != nil { - return false, nil - } - // It's alive! - c.Close() - return true, nil - } - if err := specutils.WaitForReady(s.Pid, timeout, ready); err != nil { - return fmt.Errorf("unexpected error waiting for sandbox %q, err: %v", s.ID, err) - } conn, err := s.sandboxConnect() if err != nil { return err