Revert "Merge pull request #2280 from kolyshkin/errors-unwrap"

Using errors.Unwrap() is not the best thing to do, since it returns
nil in case of an error which was not wrapped. More to say,
errors package provides more elegant ways to check for underlying
errors, such as errors.As() and errors.Is().

This reverts commit f8e138855d, reversing
changes made to 6ca9d8e6da.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin 2020-04-02 19:38:25 -07:00
parent 0c6659ac8e
commit c39f87a47a
6 changed files with 56 additions and 10 deletions

View File

@ -110,13 +110,21 @@ func isIgnorableError(rootless bool, err error) bool {
if !rootless { if !rootless {
return false return false
} }
err = errors.Cause(err)
// Is it an ordinary EPERM? // Is it an ordinary EPERM?
if os.IsPermission(err) { if os.IsPermission(errors.Cause(err)) {
return true return true
} }
// Handle some specific syscall errors.
errno := errors.Unwrap(err) // Try to handle other errnos.
var errno error
switch err := errors.Cause(err).(type) {
case *os.PathError:
errno = err.Err
case *os.LinkError:
errno = err.Err
case *os.SyscallError:
errno = err.Err
}
return errno == unix.EROFS || errno == unix.EPERM || errno == unix.EACCES return errno == unix.EROFS || errno == unix.EPERM || errno == unix.EACCES
} }

View File

@ -6,8 +6,10 @@ import (
"errors" "errors"
"fmt" "fmt"
"io/ioutil" "io/ioutil"
"os"
"path/filepath" "path/filepath"
"strconv" "strconv"
"syscall" // for Errno type only
"github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups"
"golang.org/x/sys/unix" "golang.org/x/sys/unix"
@ -47,8 +49,12 @@ func setKernelMemory(path string, kernelMemoryLimit int64) error {
// The EBUSY signal is returned on attempts to write to the // The EBUSY signal is returned on attempts to write to the
// memory.kmem.limit_in_bytes file if the cgroup has children or // memory.kmem.limit_in_bytes file if the cgroup has children or
// once tasks have been attached to the cgroup // once tasks have been attached to the cgroup
if errors.Unwrap(err) == unix.EBUSY { if pathErr, ok := err.(*os.PathError); ok {
return fmt.Errorf("failed to set %s, because either tasks have already joined this cgroup or it has children", cgroupKernelMemoryLimit) if errNo, ok := pathErr.Err.(syscall.Errno); ok {
if errNo == unix.EBUSY {
return fmt.Errorf("failed to set %s, because either tasks have already joined this cgroup or it has children", cgroupKernelMemoryLimit)
}
}
} }
return fmt.Errorf("failed to write %v to %v: %v", kernelMemoryLimit, cgroupKernelMemoryLimit, err) return fmt.Errorf("failed to write %v to %v: %v", kernelMemoryLimit, cgroupKernelMemoryLimit, err)
} }

View File

@ -4,6 +4,7 @@ package fs2
import ( import (
"io/ioutil" "io/ioutil"
"os"
"path/filepath" "path/filepath"
"strings" "strings"
@ -24,11 +25,20 @@ func setPids(dirPath string, cgroup *configs.Cgroup) error {
return nil return nil
} }
func isNOTSUP(err error) bool {
switch err := err.(type) {
case *os.PathError:
return err.Err == unix.ENOTSUP
default:
return false
}
}
func statPidsWithoutController(dirPath string, stats *cgroups.Stats) error { func statPidsWithoutController(dirPath string, stats *cgroups.Stats) error {
// if the controller is not enabled, let's read PIDS from cgroups.procs // if the controller is not enabled, let's read PIDS from cgroups.procs
// (or threads if cgroup.threads is enabled) // (or threads if cgroup.threads is enabled)
contents, err := ioutil.ReadFile(filepath.Join(dirPath, "cgroup.procs")) contents, err := ioutil.ReadFile(filepath.Join(dirPath, "cgroup.procs"))
if err != nil && errors.Unwrap(err) == unix.ENOTSUP { if err != nil && isNOTSUP(err) {
contents, err = ioutil.ReadFile(filepath.Join(dirPath, "cgroup.threads")) contents, err = ioutil.ReadFile(filepath.Join(dirPath, "cgroup.threads"))
} }
if err != nil { if err != nil {

View File

@ -41,10 +41,20 @@ func ReadFile(dir, file string) (string, error) {
func retryingWriteFile(filename string, data []byte, perm os.FileMode) error { func retryingWriteFile(filename string, data []byte, perm os.FileMode) error {
for { for {
err := ioutil.WriteFile(filename, data, perm) err := ioutil.WriteFile(filename, data, perm)
if errors.Unwrap(err) == syscall.EINTR { if isInterruptedWriteFile(err) {
logrus.Infof("interrupted while writing %s to %s", string(data), filename) logrus.Infof("interrupted while writing %s to %s", string(data), filename)
continue continue
} }
return err return err
} }
} }
func isInterruptedWriteFile(err error) bool {
if patherr, ok := err.(*os.PathError); ok {
errno, ok2 := patherr.Err.(syscall.Errno)
if ok2 && errno == syscall.EINTR {
return true
}
}
return false
}

View File

@ -576,7 +576,7 @@ func WriteCgroupProc(dir string, pid int) error {
// EINVAL might mean that the task being added to cgroup.procs is in state // EINVAL might mean that the task being added to cgroup.procs is in state
// TASK_NEW. We should attempt to do so again. // TASK_NEW. We should attempt to do so again.
if errors.Unwrap(err) == unix.EINVAL { if isEINVAL(err) {
time.Sleep(30 * time.Millisecond) time.Sleep(30 * time.Millisecond)
continue continue
} }
@ -586,6 +586,15 @@ func WriteCgroupProc(dir string, pid int) error {
return err return err
} }
func isEINVAL(err error) bool {
switch err := err.(type) {
case *os.PathError:
return err.Err == unix.EINVAL
default:
return false
}
}
// Since the OCI spec is designed for cgroup v1, in some cases // Since the OCI spec is designed for cgroup v1, in some cases
// there is need to convert from the cgroup v1 configuration to cgroup v2 // there is need to convert from the cgroup v1 configuration to cgroup v2
// the formula for BlkIOWeight is y = (1 + (x - 10) * 9999 / 990) // the formula for BlkIOWeight is y = (1 + (x - 10) * 9999 / 990)

View File

@ -1855,7 +1855,10 @@ func (c *linuxContainer) isPaused() (bool, error) {
data, err := ioutil.ReadFile(filepath.Join(fcg, filename)) data, err := ioutil.ReadFile(filepath.Join(fcg, filename))
if err != nil { if err != nil {
// If freezer cgroup is not mounted, the container would just be not paused. // If freezer cgroup is not mounted, the container would just be not paused.
if os.IsNotExist(err) || errors.Unwrap(err) == syscall.ENODEV { if os.IsNotExist(err) {
return false, nil
}
if pathError, isPathError := err.(*os.PathError); isPathError && pathError.Err == syscall.ENODEV {
return false, nil return false, nil
} }
return false, newSystemErrorWithCause(err, "checking if container is paused") return false, newSystemErrorWithCause(err, "checking if container is paused")