diff options
author | Daniel J Walsh <dwalsh@redhat.com> | 2020-10-28 13:16:42 -0400 |
---|---|---|
committer | Daniel J Walsh <dwalsh@redhat.com> | 2020-10-30 05:34:04 -0400 |
commit | 831d7fb0d7ee007fc04b1b0ff24b77c7d5635f5e (patch) | |
tree | 2888d983aea9538c421bcb2cbfce818a1904dd7a /libpod | |
parent | 228396a99dc88fc828f23d4072a46ca8de90282f (diff) | |
download | podman-831d7fb0d7ee007fc04b1b0ff24b77c7d5635f5e.tar.gz podman-831d7fb0d7ee007fc04b1b0ff24b77c7d5635f5e.tar.bz2 podman-831d7fb0d7ee007fc04b1b0ff24b77c7d5635f5e.zip |
Stop excessive wrapping of errors
Most of the builtin golang functions like os.Stat and
os.Open report errors including the file system object
path. We should not wrap these errors and put the file path
in a second time, causing stuttering of errors when they
get presented to the user.
This patch tries to cleanup a bunch of these errors.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Diffstat (limited to 'libpod')
-rw-r--r-- | libpod/container_internal.go | 4 | ||||
-rw-r--r-- | libpod/container_internal_linux.go | 46 | ||||
-rw-r--r-- | libpod/healthcheck.go | 2 | ||||
-rw-r--r-- | libpod/oci_conmon_linux.go | 2 | ||||
-rw-r--r-- | libpod/options.go | 2 | ||||
-rw-r--r-- | libpod/runtime_migrate.go | 2 | ||||
-rw-r--r-- | libpod/util.go | 8 |
7 files changed, 33 insertions, 33 deletions
diff --git a/libpod/container_internal.go b/libpod/container_internal.go index cafe70b80..0aeaae43d 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -578,10 +578,10 @@ func (c *Container) refresh() error { if len(c.config.IDMappings.UIDMap) != 0 || len(c.config.IDMappings.GIDMap) != 0 { info, err := os.Stat(c.runtime.config.Engine.TmpDir) if err != nil { - return errors.Wrapf(err, "cannot stat `%s`", c.runtime.config.Engine.TmpDir) + return err } if err := os.Chmod(c.runtime.config.Engine.TmpDir, info.Mode()|0111); err != nil { - return errors.Wrapf(err, "cannot chmod `%s`", c.runtime.config.Engine.TmpDir) + return err } root := filepath.Join(c.runtime.config.Engine.TmpDir, "containers-root", c.ID()) if err := os.MkdirAll(root, 0755); err != nil { diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 57d5100cf..b26a60146 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -309,7 +309,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { fallthrough case "Z": if err := label.Relabel(m.Source, c.MountLabel(), label.IsShared(o)); err != nil { - return nil, errors.Wrapf(err, "relabel failed %q", m.Source) + return nil, err } default: @@ -360,11 +360,11 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { for _, overlayVol := range c.config.OverlayVolumes { contentDir, err := overlay.TempDir(c.config.StaticDir, c.RootUID(), c.RootGID()) if err != nil { - return nil, errors.Wrapf(err, "failed to create TempDir in the %s directory", c.config.StaticDir) + return nil, err } overlayMount, err := overlay.Mount(contentDir, overlayVol.Source, overlayVol.Dest, c.RootUID(), c.RootGID(), c.runtime.store.GraphOptions()) if err != nil { - return nil, errors.Wrapf(err, "creating overlay failed %q", overlayVol.Source) + return nil, errors.Wrapf(err, "mounting overlay failed %q", overlayVol.Source) } g.AddMount(overlayMount) } @@ -811,7 +811,7 @@ func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) error { return errors.Wrapf(err, "error creating delete files list file %q", deleteFilesList) } if err := ioutil.WriteFile(deleteFilesList, formatJSON, 0600); err != nil { - return errors.Wrapf(err, "error creating delete files list file %q", deleteFilesList) + return errors.Wrap(err, "error creating delete files list file") } includeFiles = append(includeFiles, "deleted.files") @@ -835,7 +835,7 @@ func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) error { defer outFile.Close() if err := os.Chmod(dest, 0600); err != nil { - return errors.Wrapf(err, "cannot chmod %q", dest) + return err } _, err = io.Copy(outFile, input) @@ -1059,7 +1059,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti if n.Sandbox != "" { MAC, err = net.ParseMAC(n.Mac) if err != nil { - return errors.Wrapf(err, "failed to parse MAC %v", n.Mac) + return err } break } @@ -1163,14 +1163,14 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti return errors.Wrapf(err, "failed to read deleted files file") } if err := json.Unmarshal(deletedFilesJSON, &deletedFiles); err != nil { - return errors.Wrapf(err, "failed to read deleted files file %s", deletedFilesPath) + return errors.Wrapf(err, "failed to unmarshal deleted files file %s", deletedFilesPath) } for _, deleteFile := range deletedFiles { // Using RemoveAll as deletedFiles, which is generated from 'podman diff' // lists completely deleted directories as a single entry: 'D /root'. err = os.RemoveAll(filepath.Join(c.state.Mountpoint, deleteFile)) if err != nil { - return errors.Wrapf(err, "failed to delete file %s from container %s during restore", deletedFilesPath, c.ID()) + return errors.Wrapf(err, "failed to delete files from container %s during restore", c.ID()) } } } @@ -1209,7 +1209,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti // Make standard bind mounts to include in the container func (c *Container) makeBindMounts() error { if err := os.Chown(c.state.RunDir, c.RootUID(), c.RootGID()); err != nil { - return errors.Wrapf(err, "cannot chown run directory %s", c.state.RunDir) + return errors.Wrap(err, "cannot chown run directory") } if c.state.BindMounts == nil { @@ -1227,13 +1227,13 @@ func (c *Container) makeBindMounts() error { if c.config.NetNsCtr == "" { if resolvePath, ok := c.state.BindMounts["/etc/resolv.conf"]; ok { if err := os.Remove(resolvePath); err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "error removing container %s resolv.conf", c.ID()) + return errors.Wrapf(err, "container %s", c.ID()) } delete(c.state.BindMounts, "/etc/resolv.conf") } if hostsPath, ok := c.state.BindMounts["/etc/hosts"]; ok { if err := os.Remove(hostsPath); err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "error removing container %s hosts", c.ID()) + return errors.Wrapf(err, "container %s", c.ID()) } delete(c.state.BindMounts, "/etc/hosts") } @@ -1433,7 +1433,7 @@ func (c *Container) generateResolvConf() (string, error) { if err == nil { resolvConf = definedPath } else if !os.IsNotExist(err) { - return "", errors.Wrapf(err, "failed to stat %s", definedPath) + return "", err } } break @@ -1455,7 +1455,7 @@ func (c *Container) generateResolvConf() (string, error) { contents, err := ioutil.ReadFile(resolvPath) // resolv.conf doesn't have to exists if err != nil && !os.IsNotExist(err) { - return "", errors.Wrapf(err, "unable to read %s", resolvPath) + return "", err } // Ensure that the container's /etc/resolv.conf is compatible with its @@ -1524,7 +1524,7 @@ func (c *Container) generateResolvConf() (string, error) { destPath := filepath.Join(c.state.RunDir, "resolv.conf") if err := os.Remove(destPath); err != nil && !os.IsNotExist(err) { - return "", errors.Wrapf(err, "error removing resolv.conf for container %s", c.ID()) + return "", errors.Wrapf(err, "container %s", c.ID()) } // Build resolv.conf @@ -1544,7 +1544,7 @@ func (c *Container) generateResolvConf() (string, error) { func (c *Container) generateHosts(path string) (string, error) { orig, err := ioutil.ReadFile(path) if err != nil { - return "", errors.Wrapf(err, "unable to read %s", path) + return "", err } hosts := string(orig) hosts += c.getHosts() @@ -1947,7 +1947,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) { } orig, err := ioutil.ReadFile(originPasswdFile) if err != nil && !os.IsNotExist(err) { - return "", "", errors.Wrapf(err, "unable to read passwd file %s", originPasswdFile) + return "", "", err } passwdFile, err := c.writeStringToStaticDir("passwd", string(orig)+passwdEntry) if err != nil { @@ -1966,7 +1966,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) { f, err := os.OpenFile(containerPasswd, os.O_APPEND|os.O_WRONLY, 0600) if err != nil { - return "", "", errors.Wrapf(err, "error opening container %s /etc/passwd", c.ID()) + return "", "", errors.Wrapf(err, "container %s", c.ID()) } defer f.Close() @@ -1993,7 +1993,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) { } orig, err := ioutil.ReadFile(originGroupFile) if err != nil && !os.IsNotExist(err) { - return "", "", errors.Wrapf(err, "unable to read group file %s", originGroupFile) + return "", "", err } groupFile, err := c.writeStringToStaticDir("group", string(orig)+groupEntry) if err != nil { @@ -2012,7 +2012,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) { f, err := os.OpenFile(containerGroup, os.O_APPEND|os.O_WRONLY, 0600) if err != nil { - return "", "", errors.Wrapf(err, "error opening container %s /etc/group", c.ID()) + return "", "", errors.Wrapf(err, "container %s", c.ID()) } defer f.Close() @@ -2033,13 +2033,13 @@ func (c *Container) copyOwnerAndPerms(source, dest string) error { if os.IsNotExist(err) { return nil } - return errors.Wrapf(err, "cannot stat `%s`", dest) + return err } if err := os.Chmod(dest, info.Mode()); err != nil { - return errors.Wrapf(err, "cannot chmod `%s`", dest) + return err } if err := os.Chown(dest, int(info.Sys().(*syscall.Stat_t).Uid), int(info.Sys().(*syscall.Stat_t).Gid)); err != nil { - return errors.Wrapf(err, "cannot chown `%s`", dest) + return err } return nil } @@ -2130,7 +2130,7 @@ func (c *Container) checkFileExistsInRootfs(file string) (bool, error) { if os.IsNotExist(err) { return false, nil } - return false, errors.Wrapf(err, "error accessing container %s file %q", c.ID(), file) + return false, errors.Wrapf(err, "container %s", c.ID()) } if stat.IsDir() { return false, nil diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go index bd55b852e..f77075893 100644 --- a/libpod/healthcheck.go +++ b/libpod/healthcheck.go @@ -223,7 +223,7 @@ func (c *Container) GetHealthCheckLog() (define.HealthCheckResults, error) { } b, err := ioutil.ReadFile(c.healthCheckLogPath()) if err != nil { - return healthCheck, errors.Wrapf(err, "failed to read health check log file %s", c.healthCheckLogPath()) + return healthCheck, errors.Wrap(err, "failed to read health check log file") } if err := json.Unmarshal(b, &healthCheck); err != nil { return healthCheck, errors.Wrapf(err, "failed to unmarshal existing healthcheck results in %s", c.healthCheckLogPath()) diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 94630e57b..89d64537d 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -120,7 +120,7 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime if os.IsNotExist(err) { continue } - return nil, errors.Wrapf(err, "cannot stat OCI runtime %s path %q", name, path) + return nil, errors.Wrapf(err, "cannot stat OCI runtime %s path", name) } if !stat.Mode().IsRegular() { continue diff --git a/libpod/options.go b/libpod/options.go index 5d1ce8755..060887b7e 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1296,7 +1296,7 @@ func WithRootFS(rootfs string) CtrCreateOption { return define.ErrCtrFinalized } if _, err := os.Stat(rootfs); err != nil { - return errors.Wrapf(err, "error checking path %q", rootfs) + return err } ctr.config.Rootfs = rootfs return nil diff --git a/libpod/runtime_migrate.go b/libpod/runtime_migrate.go index 3dc38f442..1ad32fe9c 100644 --- a/libpod/runtime_migrate.go +++ b/libpod/runtime_migrate.go @@ -29,7 +29,7 @@ func stopPauseProcess() error { if os.IsNotExist(err) { return nil } - return errors.Wrapf(err, "cannot read pause process pid file %s", pausePidPath) + return errors.Wrap(err, "cannot read pause process pid file") } pausePid, err := strconv.Atoi(string(data)) if err != nil { diff --git a/libpod/util.go b/libpod/util.go index 585b07aca..c26039c50 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -74,7 +74,7 @@ func WaitForFile(path string, chWait chan error, timeout time.Duration) (bool, e return false, nil } if !os.IsNotExist(err) { - return false, errors.Wrapf(err, "checking file %s", path) + return false, err } case <-time.After(25 * time.Millisecond): // Check periodically for the file existence. It is needed @@ -86,7 +86,7 @@ func WaitForFile(path string, chWait chan error, timeout time.Duration) (bool, e return false, nil } if !os.IsNotExist(err) { - return false, errors.Wrapf(err, "checking file %s", path) + return false, err } case <-timeoutChan: return false, errors.Wrapf(define.ErrInternal, "timed out waiting for file %s", path) @@ -184,11 +184,11 @@ func DefaultSeccompPath() (string, error) { return config.SeccompOverridePath, nil } if !os.IsNotExist(err) { - return "", errors.Wrapf(err, "can't check if %q exists", config.SeccompOverridePath) + return "", err } if _, err := os.Stat(config.SeccompDefaultPath); err != nil { if !os.IsNotExist(err) { - return "", errors.Wrapf(err, "can't check if %q exists", config.SeccompDefaultPath) + return "", err } return "", nil } |