Remove excessive error wrapping

In case os.Open[File], os.Mkdir[All], ioutil.ReadFile and the like
fails, the error message already contains the file name and the
operation that fails, so there is no need to wrap the error with
something like "open %s failed".

While at it

 - replace a few places with os.Open, ioutil.ReadAll with
   ioutil.ReadFile.

 - replace errors.Wrapf with errors.Wrap for cases where there
   are no %-style arguments.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin
2020-10-05 12:33:53 -07:00
parent 1b16fcfd14
commit 4878dff3e2
26 changed files with 59 additions and 75 deletions

View File

@ -210,7 +210,7 @@ func build(cmd *cobra.Command, args []string) error {
if cmd.Flag("logfile").Changed {
logfile, err := os.OpenFile(buildOpts.Logfile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
if err != nil {
return errors.Errorf("error opening logfile %q: %v", buildOpts.Logfile, err)
return err
}
defer logfile.Close()
}

View File

@ -496,7 +496,7 @@ func (c *Container) setupStorage(ctx context.Context) error {
artifacts := filepath.Join(c.config.StaticDir, artifactsDir)
if err := os.MkdirAll(artifacts, 0755); err != nil {
return errors.Wrapf(err, "error creating artifacts directory %q", artifacts)
return errors.Wrap(err, "error creating artifacts directory")
}
return nil
@ -1820,7 +1820,7 @@ func (c *Container) appendStringToRundir(destFile, output string) (string, error
f, err := os.OpenFile(destFileName, os.O_APPEND|os.O_WRONLY, 0600)
if err != nil {
return "", errors.Wrapf(err, "unable to open %s", destFileName)
return "", err
}
defer f.Close()

View File

@ -835,13 +835,13 @@ func (c *Container) checkpointRestoreLabelLog(fileName string) error {
logFile, err := os.OpenFile(dumpLog, os.O_CREATE, 0600)
if err != nil {
return errors.Wrapf(err, "failed to create CRIU log file %q", dumpLog)
return errors.Wrap(err, "failed to create CRIU log file")
}
if err := logFile.Close(); err != nil {
logrus.Errorf("unable to close log file: %q", err)
logrus.Error(err)
}
if err = label.SetFileLabel(dumpLog, c.MountLabel()); err != nil {
return errors.Wrapf(err, "failed to label CRIU log file %q", dumpLog)
return err
}
return nil
}
@ -919,7 +919,7 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO
func (c *Container) importCheckpoint(input string) error {
archiveFile, err := os.Open(input)
if err != nil {
return errors.Wrapf(err, "Failed to open checkpoint archive %s for import", input)
return errors.Wrap(err, "Failed to open checkpoint archive for import")
}
defer archiveFile.Close()
@ -1116,7 +1116,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
// Only do this if a rootfs-diff.tar actually exists
rootfsDiffFile, err := os.Open(rootfsDiffPath)
if err != nil {
return errors.Wrapf(err, "Failed to open root file-system diff file %s", rootfsDiffPath)
return errors.Wrap(err, "Failed to open root file-system diff file")
}
defer rootfsDiffFile.Close()
if err := c.runtime.ApplyDiffTarStream(c.ID(), rootfsDiffFile); err != nil {
@ -1125,16 +1125,10 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
}
deletedFilesPath := filepath.Join(c.bundlePath(), "deleted.files")
if _, err := os.Stat(deletedFilesPath); err == nil {
deletedFilesFile, err := os.Open(deletedFilesPath)
if err != nil {
return errors.Wrapf(err, "Failed to open deleted files file %s", deletedFilesPath)
}
defer deletedFilesFile.Close()
var deletedFiles []string
deletedFilesJSON, err := ioutil.ReadAll(deletedFilesFile)
deletedFilesJSON, err := ioutil.ReadFile(deletedFilesPath)
if err != nil {
return errors.Wrapf(err, "Failed to read deleted files file %s", deletedFilesPath)
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)

View File

@ -26,7 +26,7 @@ func CreateFileLock(path string) (*FileLocks, error) {
return nil, errors.Wrapf(syscall.EEXIST, "directory %s exists", path)
}
if err := os.MkdirAll(path, 0711); err != nil {
return nil, errors.Wrapf(err, "cannot create %s", path)
return nil, err
}
locks := new(FileLocks)
@ -40,7 +40,7 @@ func CreateFileLock(path string) (*FileLocks, error) {
func OpenFileLock(path string) (*FileLocks, error) {
_, err := os.Stat(path)
if err != nil {
return nil, errors.Wrapf(err, "accessing directory %s", path)
return nil, err
}
locks := new(FileLocks)
@ -84,7 +84,7 @@ func (locks *FileLocks) AllocateLock() (uint32, error) {
if os.IsExist(err) {
continue
}
return 0, errors.Wrapf(err, "creating lock file")
return 0, errors.Wrap(err, "creating lock file")
}
f.Close()
break

View File

@ -662,12 +662,12 @@ func (r *Runtime) setupNetNS(ctr *Container) error {
nsPath := fmt.Sprintf("/var/run/netns/cni-%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:])
if err := os.MkdirAll(filepath.Dir(nsPath), 0711); err != nil {
return errors.Wrapf(err, "cannot create %s", filepath.Dir(nsPath))
return err
}
mountPointFd, err := os.Create(nsPath)
if err != nil {
return errors.Wrapf(err, "cannot open %s", nsPath)
return err
}
if err := mountPointFd.Close(); err != nil {
return err

View File

@ -157,15 +157,13 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
if err := os.MkdirAll(runtime.exitsDir, 0750); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
return nil, errors.Wrapf(err, "error creating OCI runtime exit files directory %s",
runtime.exitsDir)
return nil, errors.Wrapf(err, "error creating OCI runtime exit files directory")
}
}
if err := os.MkdirAll(runtime.socketsDir, 0750); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
return nil, errors.Wrapf(err, "error creating OCI runtime attach sockets directory %s",
runtime.socketsDir)
return nil, errors.Wrap(err, "error creating OCI runtime attach sockets directory")
}
}

View File

@ -251,8 +251,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
if err := os.MkdirAll(runtime.config.Engine.StaticDir, 0700); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
return errors.Wrapf(err, "error creating runtime static files directory %s",
runtime.config.Engine.StaticDir)
return errors.Wrap(err, "error creating runtime static files directory")
}
}
@ -348,7 +347,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0751); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
return errors.Wrapf(err, "error creating tmpdir %s", runtime.config.Engine.TmpDir)
return errors.Wrap(err, "error creating tmpdir")
}
}
@ -356,7 +355,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
if err := os.MkdirAll(filepath.Dir(runtime.config.Engine.EventsLogFilePath), 0700); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
return errors.Wrapf(err, "error creating events dirs %s", filepath.Dir(runtime.config.Engine.EventsLogFilePath))
return errors.Wrap(err, "error creating events dirs")
}
}
@ -416,8 +415,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0755); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
return errors.Wrapf(err, "error creating runtime temporary files directory %s",
runtime.config.Engine.TmpDir)
return errors.Wrapf(err, "error creating runtime temporary files directory")
}
}
@ -649,7 +647,7 @@ func (r *Runtime) refresh(alivePath string) error {
// Create a file indicating the runtime is alive and ready
file, err := os.OpenFile(alivePath, os.O_RDONLY|os.O_CREATE, 0644)
if err != nil {
return errors.Wrapf(err, "error creating runtime status file %s", alivePath)
return errors.Wrap(err, "error creating runtime status file")
}
defer file.Close()

View File

@ -331,7 +331,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
ctr.config.ShmDir = filepath.Join(ctr.bundlePath(), "shm")
if err := os.MkdirAll(ctr.config.ShmDir, 0700); err != nil {
if !os.IsExist(err) {
return nil, errors.Wrapf(err, "unable to create shm %q dir", ctr.config.ShmDir)
return nil, errors.Wrap(err, "unable to create shm dir")
}
}
ctr.config.Mounts = append(ctr.config.Mounts, ctr.config.ShmDir)

View File

@ -131,7 +131,7 @@ func getAvailableControllers(exclude map[string]controllerHandler, cgroup2 bool)
infos, err := ioutil.ReadDir(cgroupRoot)
if err != nil {
return nil, errors.Wrapf(err, "read directory %s", cgroupRoot)
return nil, err
}
controllers := []controller{}
for _, i := range infos {
@ -157,7 +157,7 @@ func (c *CgroupControl) getCgroupv1Path(name string) string {
func createCgroupv2Path(path string) (deferredError error) {
content, err := ioutil.ReadFile("/sys/fs/cgroup/cgroup.controllers")
if err != nil {
return errors.Wrapf(err, "read /sys/fs/cgroup/cgroup.controllers")
return err
}
if !strings.HasPrefix(path, "/sys/fs/cgroup/") {
return fmt.Errorf("invalid cgroup path %s", path)
@ -180,7 +180,7 @@ func createCgroupv2Path(path string) (deferredError error) {
if i > 0 {
if err := os.Mkdir(current, 0755); err != nil {
if !os.IsExist(err) {
return errors.Wrapf(err, "mkdir %s", path)
return err
}
} else {
// If the directory was created, be sure it is not left around on errors.
@ -237,7 +237,7 @@ func (c *CgroupControl) initialize() (err error) {
}
path := c.getCgroupv1Path(ctr.name)
if err := os.MkdirAll(path, 0755); err != nil {
return errors.Wrapf(err, "error creating cgroup path %s for %s", path, ctr.name)
return errors.Wrapf(err, "error creating cgroup path for %s", ctr.name)
}
}
}
@ -265,7 +265,7 @@ func (c *CgroupControl) createCgroupDirectory(controller string) (bool, error) {
func readFileAsUint64(path string) (uint64, error) {
data, err := ioutil.ReadFile(path)
if err != nil {
return 0, errors.Wrapf(err, "open %s", path)
return 0, err
}
v := cleanString(string(data))
if v == "max" {
@ -425,7 +425,7 @@ func rmDirRecursively(path string) error {
}
entries, err := ioutil.ReadDir(path)
if err != nil {
return errors.Wrapf(err, "read %s", path)
return err
}
for _, i := range entries {
if i.IsDir() {

View File

@ -46,7 +46,7 @@ func UserOwnsCurrentSystemdCgroup() (bool, error) {
f, err := os.Open("/proc/self/cgroup")
if err != nil {
return false, errors.Wrapf(err, "open file /proc/self/cgroup")
return false, err
}
defer f.Close()

View File

@ -134,7 +134,7 @@ func GetSystemCPUUsage() (uint64, error) {
files, err := ioutil.ReadDir(cgroupRoot)
if err != nil {
return 0, errors.Wrapf(err, "read directory %q", cgroupRoot)
return 0, err
}
var total uint64
for _, file := range files {

View File

@ -22,15 +22,9 @@ import (
// crImportFromJSON imports the JSON files stored in the exported
// checkpoint tarball
func crImportFromJSON(filePath string, v interface{}) error {
jsonFile, err := os.Open(filePath)
content, err := ioutil.ReadFile(filePath)
if err != nil {
return errors.Wrapf(err, "Failed to open container definition %s for restore", filePath)
}
defer errorhandling.CloseQuiet(jsonFile)
content, err := ioutil.ReadAll(jsonFile)
if err != nil {
return errors.Wrapf(err, "Failed to read container definition %s for restore", filePath)
return errors.Wrap(err, "Failed to read container definition for restore")
}
json := jsoniter.ConfigCompatibleWithStandardLibrary
if err = json.Unmarshal(content, v); err != nil {
@ -47,7 +41,7 @@ func CRImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, input stri
// tarball to a temporary directory
archiveFile, err := os.Open(input)
if err != nil {
return nil, errors.Wrapf(err, "Failed to open checkpoint archive %s for import", input)
return nil, errors.Wrap(err, "Failed to open checkpoint archive for import")
}
defer errorhandling.CloseQuiet(archiveFile)
options := &archive.TarOptions{

View File

@ -604,7 +604,7 @@ func checkExecPreserveFDs(options entities.ExecOptions) error {
if options.PreserveFDs > 0 {
entries, err := ioutil.ReadDir("/proc/self/fd")
if err != nil {
return errors.Wrapf(err, "unable to read /proc/self/fd")
return err
}
m := make(map[int]bool)

View File

@ -115,7 +115,7 @@ func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string,
return nil, err
}
if err = idtools.MkdirAllAndChownNew(ctrWorkDir, 0755, hostOwner); err != nil {
return nil, errors.Wrapf(err, "error creating directory %q", destPath)
return nil, err
}
cleanedPath, err := securejoin.SecureJoin(mountPoint, filepath.Join(ctr.WorkingDir(), destPath))
if err != nil {
@ -249,7 +249,7 @@ func containerCopy(srcPath, destPath, src, dest string, idMappingOpts storage.ID
}
destDirIsExist := err == nil
if err = os.MkdirAll(destdir, 0755); err != nil {
return errors.Wrapf(err, "error creating directory %q", destdir)
return err
}
// return functions for copying items
@ -351,7 +351,7 @@ func streamFileToStdout(srcPath string, srcfi os.FileInfo) error {
file, err := os.Open(srcPath)
if err != nil {
return errors.Wrapf(err, "error opening file %s", srcPath)
return err
}
defer file.Close()
if !archive.IsArchivePath(srcPath) {

View File

@ -770,7 +770,7 @@ func (ir *ImageEngine) Sign(ctx context.Context, names []string, options entitie
if err := os.MkdirAll(signatureDir, 0751); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
logrus.Errorf("error creating directory %s: %s", signatureDir, err)
logrus.Error(err)
continue
}
}

View File

@ -248,7 +248,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
case v1.HostPathDirectoryOrCreate:
if _, err := os.Stat(hostPath.Path); os.IsNotExist(err) {
if err := os.Mkdir(hostPath.Path, kubeDirectoryPermission); err != nil {
return nil, errors.Errorf("Error creating HostPath %s at %s", volume.Name, hostPath.Path)
return nil, errors.Errorf("Error creating HostPath %s", volume.Name)
}
}
// Label a newly created volume
@ -259,7 +259,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
if _, err := os.Stat(hostPath.Path); os.IsNotExist(err) {
f, err := os.OpenFile(hostPath.Path, os.O_RDONLY|os.O_CREATE, kubeFilePermission)
if err != nil {
return nil, errors.Errorf("Error creating HostPath %s at %s", volume.Name, hostPath.Path)
return nil, errors.Errorf("Error creating HostPath %s", volume.Name)
}
if err := f.Close(); err != nil {
logrus.Warnf("Error in closing newly created HostPath file: %v", err)
@ -272,7 +272,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
case v1.HostPathSocket:
st, err := os.Stat(hostPath.Path)
if err != nil {
return nil, errors.Wrapf(err, "Error checking HostPathSocket")
return nil, errors.Wrap(err, "Error checking HostPathSocket")
}
if st.Mode()&os.ModeSocket != os.ModeSocket {
return nil, errors.Errorf("Error checking HostPathSocket: path %s is not a socket", hostPath.Path)

View File

@ -24,7 +24,7 @@ func (ir *ImageEngine) ShowTrust(ctx context.Context, args []string, options ent
}
report.Raw, err = ioutil.ReadFile(policyPath)
if err != nil {
return nil, errors.Wrapf(err, "unable to read %s", policyPath)
return nil, err
}
if options.Raw {
return &report, nil
@ -67,7 +67,7 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti
if !os.IsNotExist(err) {
policyContent, err := ioutil.ReadFile(policyPath)
if err != nil {
return errors.Wrapf(err, "unable to read %s", policyPath)
return err
}
if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil {
return errors.Errorf("could not read trust policies")

View File

@ -84,7 +84,7 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin
for _, cidFile := range options.CIDFiles {
content, err := ioutil.ReadFile(cidFile)
if err != nil {
return nil, errors.Wrapf(err, "error reading CIDFile %s", cidFile)
return nil, errors.Wrap(err, "error reading CIDFile")
}
id := strings.Split(string(content), "\n")[0]
namesOrIds = append(namesOrIds, id)
@ -164,7 +164,7 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string,
for _, cidFile := range options.CIDFiles {
content, err := ioutil.ReadFile(cidFile)
if err != nil {
return nil, errors.Wrapf(err, "error reading CIDFile %s", cidFile)
return nil, errors.Wrap(err, "error reading CIDFile")
}
id := strings.Split(string(content), "\n")[0]
namesOrIds = append(namesOrIds, id)

View File

@ -453,7 +453,7 @@ func TryJoinFromFilePaths(pausePidPath string, needNewNamespace bool, paths []st
func ReadMappingsProc(path string) ([]idtools.IDMap, error) {
file, err := os.Open(path)
if err != nil {
return nil, errors.Wrapf(err, "cannot open %s", path)
return nil, err
}
defer file.Close()

View File

@ -29,7 +29,7 @@ func getSeccompConfig(config *SecurityConfig, configSpec *spec.Spec) (*spec.Linu
logrus.Debugf("Loading seccomp profile from %q", config.SeccompProfilePath)
seccompProfile, err := ioutil.ReadFile(config.SeccompProfilePath)
if err != nil {
return nil, errors.Wrapf(err, "opening seccomp profile (%s) failed", config.SeccompProfilePath)
return nil, errors.Wrap(err, "opening seccomp profile failed")
}
seccompConfig, err = goSeccomp.LoadProfile(string(seccompProfile), configSpec)
if err != nil {

View File

@ -47,7 +47,7 @@ func getSeccompConfig(s *specgen.SpecGenerator, configSpec *spec.Spec, img *imag
logrus.Debugf("Loading seccomp profile from %q", s.SeccompProfilePath)
seccompProfile, err := ioutil.ReadFile(s.SeccompProfilePath)
if err != nil {
return nil, errors.Wrapf(err, "opening seccomp profile (%s) failed", s.SeccompProfilePath)
return nil, errors.Wrap(err, "opening seccomp profile failed")
}
seccompConfig, err = goSeccomp.LoadProfile(string(seccompProfile), configSpec)
if err != nil {

View File

@ -226,10 +226,10 @@ func GetPolicy(policyPath string) (PolicyContent, error) {
var policyContentStruct PolicyContent
policyContent, err := ioutil.ReadFile(policyPath)
if err != nil {
return policyContentStruct, errors.Wrapf(err, "unable to read policy file %s", policyPath)
return policyContentStruct, errors.Wrap(err, "unable to read policy file")
}
if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil {
return policyContentStruct, errors.Wrapf(err, "could not parse trust policies")
return policyContentStruct, errors.Wrapf(err, "could not parse trust policies from %s", policyPath)
}
return policyContentStruct, nil
}

View File

@ -490,14 +490,14 @@ func WriteStorageConfigFile(storageOpts *storage.StoreOptions, storageConf strin
}
storageFile, err := os.OpenFile(storageConf, os.O_RDWR|os.O_TRUNC, 0600)
if err != nil {
return errors.Wrapf(err, "cannot open %s", storageConf)
return err
}
tomlConfiguration := getTomlStorage(storageOpts)
defer errorhandling.CloseQuiet(storageFile)
enc := toml.NewEncoder(storageFile)
if err := enc.Encode(tomlConfiguration); err != nil {
if err := os.Remove(storageConf); err != nil {
logrus.Errorf("unable to remove file %s", storageConf)
logrus.Error(err)
}
return err
}

View File

@ -30,7 +30,7 @@ func GetRuntimeDir() (string, error) {
if runtimeDir == "" {
tmpDir := filepath.Join("/run", "user", uid)
if err := os.MkdirAll(tmpDir, 0700); err != nil {
logrus.Debugf("unable to make temp dir %s", tmpDir)
logrus.Debug(err)
}
st, err := os.Stat(tmpDir)
if err == nil && int(st.Sys().(*syscall.Stat_t).Uid) == os.Geteuid() && (st.Mode().Perm()&0700 == 0700) {
@ -40,7 +40,7 @@ func GetRuntimeDir() (string, error) {
if runtimeDir == "" {
tmpDir := filepath.Join(os.TempDir(), fmt.Sprintf("run-%s", uid))
if err := os.MkdirAll(tmpDir, 0700); err != nil {
logrus.Debugf("unable to make temp dir %s", tmpDir)
logrus.Debug(err)
}
st, err := os.Stat(tmpDir)
if err == nil && int(st.Sys().(*syscall.Stat_t).Uid) == os.Geteuid() && (st.Mode().Perm()&0700 == 0700) {

View File

@ -155,7 +155,7 @@ func (i *VarlinkAPI) BuildImage(call iopodman.VarlinkCall, config iopodman.Build
reader, err := os.Open(contextDir)
if err != nil {
logrus.Errorf("failed to open the context dir tar file %s", contextDir)
logrus.Errorf("failed to open the context dir tar file")
return call.ReplyErrorOccurred(fmt.Sprintf("unable to open context dir tar file %s", contextDir))
}
defer reader.Close()
@ -166,7 +166,7 @@ func (i *VarlinkAPI) BuildImage(call iopodman.VarlinkCall, config iopodman.Build
logrus.Debugf("untar of %s successful", contextDir)
defer func() {
if err := os.Remove(contextDir); err != nil {
logrus.Errorf("unable to delete file '%s': %q", contextDir, err)
logrus.Error(err)
}
if err := os.RemoveAll(newContextDir); err != nil {
logrus.Errorf("unable to delete directory '%s': %q", newContextDir, err)

View File

@ -56,7 +56,7 @@ func RunUnderSystemdScope(pid int, slice string, unitName string) error {
func getCgroupProcess(procFile string) (string, error) {
f, err := os.Open(procFile)
if err != nil {
return "", errors.Wrapf(err, "open file %q", procFile)
return "", err
}
defer f.Close()
@ -104,7 +104,7 @@ func MoveUnderCgroupSubtree(subtree string) error {
procFile := "/proc/self/cgroup"
f, err := os.Open(procFile)
if err != nil {
return errors.Wrapf(err, "open file %q", procFile)
return err
}
defer f.Close()