diff options
author | Fabricio Voznika <fvoznika@google.com> | 2019-01-18 17:35:09 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2019-01-18 17:36:02 -0800 |
commit | c1be25b78d89a3a55a32a7aa10724134eda9813d (patch) | |
tree | bb5c09cb94ab212c158041b260ed0bb480e2a9d5 /runsc/boot | |
parent | c0a981629cf44688687548490c5e665d851afe06 (diff) |
Scrub runsc error messages
Removed "error" and "failed to" prefix that don't add value
from messages. Adjusted a few other messages. In particular,
when the container fail to start, the message returned is easier
for humans to read:
$ docker run --rm --runtime=runsc alpine foobar
docker: Error response from daemon: OCI runtime start failed: <path> did not terminate sucessfully: starting container: starting root container [foobar]: starting sandbox: searching for executable "foobar", cwd: "/", $PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin": no such file or directory
Closes #77
PiperOrigin-RevId: 230022798
Change-Id: I83339017c70dae09e4f9f8e0ea2e554c4d5d5cd1
Diffstat (limited to 'runsc/boot')
-rw-r--r-- | runsc/boot/controller.go | 12 | ||||
-rw-r--r-- | runsc/boot/fs.go | 43 | ||||
-rw-r--r-- | runsc/boot/loader.go | 81 |
3 files changed, 73 insertions, 63 deletions
diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 36e9d2c6b..989f49388 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -163,7 +163,7 @@ func (cm *containerManager) StartRoot(cid *string, _ *struct{}) error { // Tell the root container to start and wait for the result. cm.startChan <- struct{}{} if err := <-cm.startResultChan; err != nil { - return fmt.Errorf("failed to start sandbox: %v", err) + return fmt.Errorf("starting sandbox: %v", err) } return nil } @@ -319,7 +319,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { p, err := createPlatform(cm.l.conf, int(deviceFile.Fd())) if err != nil { - return fmt.Errorf("error creating platform: %v", err) + return fmt.Errorf("creating platform: %v", err) } k := &kernel.Kernel{ Platform: p, @@ -330,14 +330,14 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { fds := &fdDispenser{fds: cm.l.goferFDs} renv, err := createRestoreEnvironment(cm.l.spec, cm.l.conf, fds) if err != nil { - return fmt.Errorf("error creating RestoreEnvironment: %v", err) + return fmt.Errorf("creating RestoreEnvironment: %v", err) } fs.SetRestoreEnvironment(*renv) // Prepare to load from the state file. networkStack, err := newEmptyNetworkStack(cm.l.conf, k) if err != nil { - return fmt.Errorf("failed to create network: %v", err) + return fmt.Errorf("creating network: %v", err) } if eps, ok := networkStack.(*epsocket.Stack); ok { stack.StackFromEnv = eps.Stack // FIXME @@ -347,7 +347,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { return err } if info.Size() == 0 { - return fmt.Errorf("error file was empty") + return fmt.Errorf("file cannot be empty") } // Load the state. @@ -385,7 +385,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { // Tell the root container to start and wait for the result. cm.startChan <- struct{}{} if err := <-cm.startResultChan; err != nil { - return fmt.Errorf("failed to start sandbox: %v", err) + return fmt.Errorf("starting sandbox: %v", err) } return nil diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index e0c8291ac..5c5e650ca 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -98,11 +98,11 @@ func setupRootContainerFS(userCtx context.Context, rootCtx context.Context, spec fds := &fdDispenser{fds: goferFDs} rootInode, err := createRootMount(rootCtx, spec, conf, fds, mounts) if err != nil { - return fmt.Errorf("failed to create root mount: %v", err) + return fmt.Errorf("creating root mount: %v", err) } mns, err := fs.NewMountNamespace(userCtx, rootInode) if err != nil { - return fmt.Errorf("failed to create root mount namespace: %v", err) + return fmt.Errorf("creating root mount namespace: %v", err) } setMountNS(mns) @@ -183,7 +183,7 @@ func createRootMount(ctx context.Context, spec *specs.Spec, conf *Config, fds *f opts := p9MountOptions(fd, conf.FileAccess) rootInode, err = p9FS.Mount(ctx, rootDevice, mf, strings.Join(opts, ",")) if err != nil { - return nil, fmt.Errorf("failed to generate root mount point: %v", err) + return nil, fmt.Errorf("creating root mount point: %v", err) } // We need to overlay the root on top of a ramfs with stub directories @@ -192,7 +192,7 @@ func createRootMount(ctx context.Context, spec *specs.Spec, conf *Config, fds *f submounts := append(subtargets("/", mounts), "/dev", "/sys", "/proc", "/tmp") rootInode, err = addSubmountOverlay(ctx, rootInode, submounts) if err != nil { - return nil, fmt.Errorf("error adding submount overlay: %v", err) + return nil, fmt.Errorf("adding submount overlay: %v", err) } if conf.Overlay && !spec.Root.Readonly { @@ -204,7 +204,7 @@ func createRootMount(ctx context.Context, spec *specs.Spec, conf *Config, fds *f } } - log.Infof("Mounted %q to \"/\" type root", spec.Root.Path) + log.Infof("Mounted %q to %q type root", spec.Root.Path, "/") return rootInode, nil } @@ -222,7 +222,7 @@ func addOverlay(ctx context.Context, conf *Config, lower *fs.Inode, name string, // Create overlay on top of mount dir. upper, err := tmpFS.Mount(ctx, name+"-upper", lowerFlags, "") if err != nil { - return nil, fmt.Errorf("failed to create tmpfs overlay: %v", err) + return nil, fmt.Errorf("creating tmpfs overlay: %v", err) } return fs.NewOverlayRoot(ctx, upper, lower, lowerFlags) } @@ -311,7 +311,7 @@ func mountSubmount(ctx context.Context, conf *Config, mns *fs.MountNamespace, ro inode, err := filesystem.Mount(ctx, mountDevice(m), mf, strings.Join(opts, ",")) if err != nil { - return fmt.Errorf("failed to create mount with source %q: %v", m.Source, err) + return fmt.Errorf("creating mount with source %q: %v", m.Source, err) } // If there are submounts, we need to overlay the mount on top of a @@ -321,7 +321,7 @@ func mountSubmount(ctx context.Context, conf *Config, mns *fs.MountNamespace, ro log.Infof("Adding submount overlay over %q", m.Destination) inode, err = addSubmountOverlay(ctx, inode, submounts) if err != nil { - return fmt.Errorf("error adding submount overlay: %v", err) + return fmt.Errorf("adding submount overlay: %v", err) } } @@ -336,11 +336,11 @@ func mountSubmount(ctx context.Context, conf *Config, mns *fs.MountNamespace, ro maxTraversals := uint(0) dirent, err := mns.FindInode(ctx, root, root, m.Destination, &maxTraversals) if err != nil { - return fmt.Errorf("failed to find mount destination %q: %v", m.Destination, err) + return fmt.Errorf("can't find mount destination %q: %v", m.Destination, err) } defer dirent.DecRef() if err := mns.Mount(ctx, dirent, inode); err != nil { - return fmt.Errorf("failed to mount at destination %q: %v", m.Destination, err) + return fmt.Errorf("mount %q error: %v", m.Destination, err) } log.Infof("Mounted %q to %q type %s", m.Source, m.Destination, m.Type) @@ -503,11 +503,11 @@ func addSubmountOverlay(ctx context.Context, inode *fs.Inode, submounts []string msrc := fs.NewPseudoMountSource() mountTree, err := ramfs.MakeDirectoryTree(ctx, msrc, submounts) if err != nil { - return nil, fmt.Errorf("error creating mount tree: %v", err) + return nil, fmt.Errorf("creating mount tree: %v", err) } overlayInode, err := fs.NewOverlayRoot(ctx, inode, mountTree, fs.MountSourceFlags{}) if err != nil { - return nil, fmt.Errorf("failed to make mount overlay: %v", err) + return nil, fmt.Errorf("adding mount overlay: %v", err) } return overlayInode, err } @@ -544,7 +544,7 @@ func setupContainerFS(procArgs *kernel.CreateProcessArgs, spec *specs.Spec, conf // fd. fdm, err := createFDMap(ctx, k, ls, console, stdioFDs) if err != nil { - return fmt.Errorf("error importing fds: %v", err) + return fmt.Errorf("importing fds: %v", err) } // CreateProcess takes a reference on FDMap if successful. We @@ -595,7 +595,7 @@ func setupContainerFS(procArgs *kernel.CreateProcessArgs, spec *specs.Spec, conf fds := &fdDispenser{fds: goferFDs} rootInode, err := createRootMount(rootCtx, spec, conf, fds, nil) if err != nil { - return fmt.Errorf("error creating filesystem for container: %v", err) + return fmt.Errorf("creating filesystem for container: %v", err) } // Mount the container's root filesystem to the newly created mount point. @@ -630,9 +630,10 @@ func setupContainerFS(procArgs *kernel.CreateProcessArgs, spec *specs.Spec, conf // executable matching the procArgs.Argv[0]. func setExecutablePath(ctx context.Context, mns *fs.MountNamespace, procArgs *kernel.CreateProcessArgs) error { paths := fs.GetPath(procArgs.Envv) - f, err := mns.ResolveExecutablePath(ctx, procArgs.WorkingDirectory, procArgs.Argv[0], paths) + exe := procArgs.Argv[0] + f, err := mns.ResolveExecutablePath(ctx, procArgs.WorkingDirectory, exe, paths) if err != nil { - return err + return fmt.Errorf("searching for executable %q, cwd: %q, $PATH=%q: %v", exe, procArgs.WorkingDirectory, strings.Join(paths, ":"), err) } procArgs.Filename = f return nil @@ -666,7 +667,7 @@ func destroyContainerFS(ctx context.Context, cid string, k *kernel.Kernel) error return nil } if err != nil { - return fmt.Errorf("error finding container root directory %q: %v", containerRoot, err) + return fmt.Errorf("finding container root directory %q: %v", containerRoot, err) } defer containerRootDirent.DecRef() @@ -682,7 +683,7 @@ func destroyContainerFS(ctx context.Context, cid string, k *kernel.Kernel) error log.Debugf("Unmounting container submount %q", root.BaseName()) m.FlushDirentRefs() if err := mns.Unmount(ctx, root, true /* detach only */); err != nil && err != syserror.EINVAL { - return fmt.Errorf("error unmounting container submount %q: %v", root.BaseName(), err) + return fmt.Errorf("unmounting container submount %q: %v", root.BaseName(), err) } } @@ -690,7 +691,7 @@ func destroyContainerFS(ctx context.Context, cid string, k *kernel.Kernel) error log.Debugf("Unmounting container root %q", containerRoot) containerRootDirent.Inode.MountSource.FlushDirentRefs() if err := mns.Unmount(ctx, containerRootDirent, true /* detach only */); err != nil { - return fmt.Errorf("error unmounting container root mount %q: %v", containerRootDirent.BaseName(), err) + return fmt.Errorf("unmounting container root mount %q: %v", containerRootDirent.BaseName(), err) } // Get a reference to the parent directory and remove the root @@ -698,12 +699,12 @@ func destroyContainerFS(ctx context.Context, cid string, k *kernel.Kernel) error maxTraversals = 0 containersDirDirent, err := mns.FindInode(ctx, mnsRoot, nil, ChildContainersDir, &maxTraversals) if err != nil { - return fmt.Errorf("error finding containers directory %q: %v", ChildContainersDir, err) + return fmt.Errorf("finding containers directory %q: %v", ChildContainersDir, err) } defer containersDirDirent.DecRef() log.Debugf("Deleting container root %q", containerRoot) if err := containersDirDirent.RemoveDirectory(ctx, mnsRoot, cid); err != nil { - return fmt.Errorf("error removing directory %q: %v", containerRoot, err) + return fmt.Errorf("removing directory %q: %v", containerRoot, err) } return nil diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 71a2ab962..f3dc15f00 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -173,17 +173,17 @@ func New(args Args) (*Loader, error) { // We initialize the rand package now to make sure /dev/urandom is pre-opened // on kernels that do not support getrandom(2). if err := rand.Init(); err != nil { - return nil, fmt.Errorf("error setting up rand: %v", err) + return nil, fmt.Errorf("setting up rand: %v", err) } if err := usage.Init(); err != nil { - return nil, fmt.Errorf("error setting up memory usage: %v", err) + return nil, fmt.Errorf("setting up memory usage: %v", err) } // Create kernel and platform. p, err := createPlatform(args.Conf, args.DeviceFD) if err != nil { - return nil, fmt.Errorf("error creating platform: %v", err) + return nil, fmt.Errorf("creating platform: %v", err) } k := &kernel.Kernel{ Platform: p, @@ -194,18 +194,18 @@ func New(args Args) (*Loader, error) { // Pass k as the platform since it is savable, unlike the actual platform. vdso, err := loader.PrepareVDSO(k) if err != nil { - return nil, fmt.Errorf("error creating vdso: %v", err) + return nil, fmt.Errorf("creating vdso: %v", err) } // Create timekeeper. tk, err := kernel.NewTimekeeper(k, vdso.ParamPage.FileRange()) if err != nil { - return nil, fmt.Errorf("error creating timekeeper: %v", err) + return nil, fmt.Errorf("creating timekeeper: %v", err) } tk.SetClocks(time.NewCalibratedClocks()) if err := enableStrace(args.Conf); err != nil { - return nil, fmt.Errorf("failed to enable strace: %v", err) + return nil, fmt.Errorf("enabling strace: %v", err) } // Create an empty network stack because the network namespace may be empty at @@ -214,13 +214,13 @@ func New(args Args) (*Loader, error) { // Run(). networkStack, err := newEmptyNetworkStack(args.Conf, k) if err != nil { - return nil, fmt.Errorf("failed to create network: %v", err) + return nil, fmt.Errorf("creating network: %v", err) } // Create capabilities. caps, err := specutils.Capabilities(args.Spec.Process.Capabilities) if err != nil { - return nil, fmt.Errorf("error creating capabilities: %v", err) + return nil, fmt.Errorf("converting capabilities: %v", err) } // Convert the spec's additional GIDs to KGIDs. @@ -262,7 +262,7 @@ func New(args Args) (*Loader, error) { RootIPCNamespace: kernel.NewIPCNamespace(creds.UserNamespace), RootAbstractSocketNamespace: kernel.NewAbstractSocketNamespace(), }); err != nil { - return nil, fmt.Errorf("error initializing kernel: %v", err) + return nil, fmt.Errorf("initializing kernel: %v", err) } // Turn on packet logging if enabled. @@ -279,11 +279,11 @@ func New(args Args) (*Loader, error) { procArgs, err := newProcess(args.ID, args.Spec, creds, k) if err != nil { - return nil, fmt.Errorf("failed to create init process for root container: %v", err) + return nil, fmt.Errorf("creating init process for root container: %v", err) } if err := initCompatLogs(args.UserLogFD); err != nil { - return nil, fmt.Errorf("init compat logs: %v", err) + return nil, fmt.Errorf("initializing compat logs: %v", err) } eid := execID{cid: args.ID} @@ -303,7 +303,7 @@ func New(args Args) (*Loader, error) { // We don't care about child signals; some platforms can generate a // tremendous number of useless ones (I'm looking at you, ptrace). if err := sighandling.IgnoreChildStop(); err != nil { - return nil, fmt.Errorf("failed to ignore child stop signals: %v", err) + return nil, fmt.Errorf("ignore child stop signals failed: %v", err) } // Handle signals by forwarding them to the root container process @@ -353,7 +353,7 @@ func newProcess(id string, spec *specs.Spec, creds *auth.Credentials, k *kernel. // Create initial limits. ls, err := createLimitSet(spec) if err != nil { - return kernel.CreateProcessArgs{}, fmt.Errorf("error creating limits: %v", err) + return kernel.CreateProcessArgs{}, fmt.Errorf("creating limits: %v", err) } // Create the process arguments. @@ -441,7 +441,7 @@ func (l *Loader) run() error { ControllerFD: l.ctrl.srv.FD(), } if err := filter.Install(opts); err != nil { - return fmt.Errorf("Failed to install seccomp filters: %v", err) + return fmt.Errorf("installing seccomp filters: %v", err) } } @@ -465,13 +465,13 @@ func (l *Loader) run() error { rootCtx := l.rootProcArgs.NewContext(l.k) rootMns := l.k.RootMountNamespace() if err := setExecutablePath(rootCtx, rootMns, &l.rootProcArgs); err != nil { - return fmt.Errorf("error setting executable path for %+v: %v", l.rootProcArgs, err) + return err } // Create the root container init task. _, _, err := l.k.CreateProcess(l.rootProcArgs) if err != nil { - return fmt.Errorf("failed to create init process: %v", err) + return fmt.Errorf("creating init process: %v", err) } // CreateProcess takes a reference on FDMap if successful. @@ -521,7 +521,7 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config // Create capabilities. caps, err := specutils.Capabilities(spec.Process.Capabilities) if err != nil { - return fmt.Errorf("error creating capabilities: %v", err) + return fmt.Errorf("creating capabilities: %v", err) } // Convert the spec's additional GIDs to KGIDs. @@ -544,7 +544,7 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config procArgs, err := newProcess(cid, spec, creds, l.k) if err != nil { - return fmt.Errorf("failed to create new process: %v", err) + return fmt.Errorf("creating new process: %v", err) } // Can't take ownership away from os.File. dup them to get a new FDs. @@ -570,20 +570,20 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config procArgs.Limits, k, cid); err != nil { - return fmt.Errorf("failed to create new process: %v", err) + return fmt.Errorf("configuring container FS: %v", err) } // setFileSystemForProcess dup'd stdioFDs, so we can close them. for i, fd := range stdioFDs { if err := syscall.Close(fd); err != nil { - return fmt.Errorf("failed to close stdioFD #%d: %v", i, fd) + return fmt.Errorf("closing stdio FD #%d: %v", i, fd) } } ctx := procArgs.NewContext(l.k) mns := k.RootMountNamespace() if err := setExecutablePath(ctx, mns, &procArgs); err != nil { - return fmt.Errorf("error setting executable path for %+v: %v", procArgs, err) + return fmt.Errorf("setting executable path for %+v: %v", procArgs, err) } l.mu.Lock() @@ -596,7 +596,7 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config tg, _, err := l.k.CreateProcess(procArgs) if err != nil { - return fmt.Errorf("failed to create process in sentry: %v", err) + return fmt.Errorf("creating process: %v", err) } // CreateProcess takes a reference on FDMap if successful. procArgs.FDMap.DecRef() @@ -615,7 +615,7 @@ func (l *Loader) destroyContainer(cid string) error { if _, _, err := l.threadGroupFromIDLocked(execID{cid: cid}); err == nil { // If the container has started, kill and wait for all processes. if err := l.signalAllProcesses(cid, int32(linux.SIGKILL)); err != nil { - return fmt.Errorf("failed to SIGKILL all container processes: %v", err) + return fmt.Errorf("sending SIGKILL to all container processes: %v", err) } } @@ -628,7 +628,7 @@ func (l *Loader) destroyContainer(cid string) error { ctx := l.rootProcArgs.NewContext(l.k) if err := destroyContainerFS(ctx, cid, l.k); err != nil { - return fmt.Errorf("failed to destroy filesystem for container %q: %v", cid, err) + return fmt.Errorf("destroying filesystem for container %q: %v", cid, err) } // We made it! @@ -715,11 +715,11 @@ func (l *Loader) waitPID(tgid kernel.ThreadID, cid string, clearStatus bool, wai // In this case, find the process in the container's PID namespace. initTG, _, err := l.threadGroupFromID(execID{cid: cid}) if err != nil { - return fmt.Errorf("failed to wait for PID %d: %v", tgid, err) + return fmt.Errorf("waiting for PID %d: %v", tgid, err) } tg := initTG.PIDNamespace().ThreadGroupWithID(tgid) if tg == nil { - return fmt.Errorf("failed to wait for PID %d: no such process", tgid) + return fmt.Errorf("waiting for PID %d: no such process", tgid) } if tg.Leader().ContainerID() != cid { return fmt.Errorf("process %d is part of a different container: %q", tgid, tg.Leader().ContainerID()) @@ -778,15 +778,21 @@ func newEmptyNetworkStack(conf *Config, clock tcpip.Clock) (inet.Stack, error) { // processes in the container, or to the foreground process group. func (l *Loader) signal(cid string, pid, signo int32, mode SignalDeliveryMode) error { if pid < 0 { - return fmt.Errorf("failed to signal container %q PID %d: PID must be positive", cid, pid) + return fmt.Errorf("PID (%d) must be positive", pid) } switch mode { case DeliverToProcess: - return l.signalProcess(cid, kernel.ThreadID(pid), signo) + if err := l.signalProcess(cid, kernel.ThreadID(pid), signo); err != nil { + return fmt.Errorf("signaling process in container %q PID %d: %v", cid, pid, err) + } + return nil case DeliverToForegroundProcessGroup: - return l.signalForegrondProcessGroup(cid, kernel.ThreadID(pid), signo) + if err := l.signalForegrondProcessGroup(cid, kernel.ThreadID(pid), signo); err != nil { + return fmt.Errorf("signaling foreground process group in container %q PID %d: %v", cid, pid, err) + } + return nil case DeliverToAllProcesses: if pid != 0 { @@ -795,12 +801,15 @@ func (l *Loader) signal(cid string, pid, signo int32, mode SignalDeliveryMode) e // Check that the container has actually started before signaling it. _, _, err := l.threadGroupFromID(execID{cid: cid}) if err != nil { - return fmt.Errorf("failed to signal container %q: %v", cid, err) + return err } - return l.signalAllProcesses(cid, signo) + if err := l.signalAllProcesses(cid, signo); err != nil { + return fmt.Errorf("signaling all processes in container %q: %v", cid, err) + } + return nil default: - panic(fmt.Sprintf("unknown signal signal delivery mode %v", mode)) + panic(fmt.Sprintf("unknown signal delivery mode %v", mode)) } } @@ -816,11 +825,11 @@ func (l *Loader) signalProcess(cid string, tgid kernel.ThreadID, signo int32) er // signal it. initTG, _, err := l.threadGroupFromID(execID{cid: cid}) if err != nil { - return fmt.Errorf("failed to signal container %q: %v", cid, err) + return fmt.Errorf("no thread group found: %v", err) } tg := initTG.PIDNamespace().ThreadGroupWithID(tgid) if tg == nil { - return fmt.Errorf("failed to signal container %q PID %d: no such process", cid, tgid) + return fmt.Errorf("no such process with PID %d", tgid) } if tg.Leader().ContainerID() != cid { return fmt.Errorf("process %d is part of a different container: %q", tgid, tg.Leader().ContainerID()) @@ -833,10 +842,10 @@ func (l *Loader) signalForegrondProcessGroup(cid string, tgid kernel.ThreadID, s // and send the signal to it. tg, tty, err := l.threadGroupFromID(execID{cid: cid, pid: tgid}) if err != nil { - return fmt.Errorf("failed to signal foreground process group for container %q PID %d: %v", cid, tgid, err) + return fmt.Errorf("no thread group found: %v", err) } if tty == nil { - return fmt.Errorf("failed to signal foreground process group in container %q PID %d: no TTY attached", cid, tgid) + return fmt.Errorf("no TTY attached") } pg := tty.ForegroundProcessGroup() if pg == nil { |