Skip to content

Commit

Permalink
Merge pull request #14828 from saschagrunert/errors-libpod
Browse files Browse the repository at this point in the history 
libpod: switch to golang native error wrapping
  • Loading branch information
@openshift-ci
openshift-ci[bot] authored Jul 5, 2022
2 parents 9539a89 + 251d916 commit 39fc5d1
Show file tree
Hide file tree
Showing 96 changed files with 1,564 additions and 1,542 deletions.
364 changes: 182 additions & 182 deletions libpod/boltdb_state.go
View file

Large diffs are not rendered by default.

180 changes: 90 additions & 90 deletions libpod/boltdb_state_internal.go
View file

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions libpod/boltdb_state_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
package libpod

import (
"fmt"

"github.com/containers/podman/v4/libpod/define"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand All @@ -29,7 +30,7 @@ func replaceNetNS(netNSPath string, ctr *Container, newState *ContainerState) er
newState.NetNS = ns
} else {
if ctr.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) {
return errors.Wrapf(err, "error joining network namespace of container %s", ctr.ID())
return fmt.Errorf("error joining network namespace of container %s: %w", ctr.ID(), err)
}

logrus.Errorf("Joining network namespace for container %s: %v", ctr.ID(), err)
Expand Down
45 changes: 22 additions & 23 deletions libpod/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/containers/podman/v4/libpod/lock"
"github.com/containers/storage"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -355,14 +354,14 @@ func (c *Container) specFromState() (*spec.Spec, error) {
returnSpec = new(spec.Spec)
content, err := ioutil.ReadAll(f)
if err != nil {
return nil, errors.Wrapf(err, "error reading container config")
return nil, fmt.Errorf("error reading container config: %w", err)
}
if err := json.Unmarshal(content, &returnSpec); err != nil {
return nil, errors.Wrapf(err, "error unmarshalling container config")
return nil, fmt.Errorf("error unmarshalling container config: %w", err)
}
} else if !os.IsNotExist(err) {
// ignore when the file does not exist
return nil, errors.Wrapf(err, "error opening container config")
return nil, fmt.Errorf("error opening container config: %w", err)
}

return returnSpec, nil
Expand Down Expand Up @@ -518,7 +517,7 @@ func (c *Container) PortMappings() ([]types.PortMapping, error) {
if len(c.config.NetNsCtr) > 0 {
netNsCtr, err := c.runtime.GetContainer(c.config.NetNsCtr)
if err != nil {
return nil, errors.Wrapf(err, "unable to look up network namespace for container %s", c.ID())
return nil, fmt.Errorf("unable to look up network namespace for container %s: %w", c.ID(), err)
}
return netNsCtr.PortMappings()
}
Expand Down Expand Up @@ -705,7 +704,7 @@ func (c *Container) Mounted() (bool, string, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return false, "", errors.Wrapf(err, "error updating container %s state", c.ID())
return false, "", fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
// We cannot directly return c.state.Mountpoint as it is not guaranteed
Expand Down Expand Up @@ -735,7 +734,7 @@ func (c *Container) StartedTime() (time.Time, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return time.Time{}, errors.Wrapf(err, "error updating container %s state", c.ID())
return time.Time{}, fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
return c.state.StartedTime, nil
Expand All @@ -747,7 +746,7 @@ func (c *Container) FinishedTime() (time.Time, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return time.Time{}, errors.Wrapf(err, "error updating container %s state", c.ID())
return time.Time{}, fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
return c.state.FinishedTime, nil
Expand All @@ -762,7 +761,7 @@ func (c *Container) ExitCode() (int32, bool, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return 0, false, errors.Wrapf(err, "error updating container %s state", c.ID())
return 0, false, fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
return c.state.ExitCode, c.state.Exited, nil
Expand All @@ -774,7 +773,7 @@ func (c *Container) OOMKilled() (bool, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return false, errors.Wrapf(err, "error updating container %s state", c.ID())
return false, fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
return c.state.OOMKilled, nil
Expand Down Expand Up @@ -845,7 +844,7 @@ func (c *Container) execSessionNoCopy(id string) (*ExecSession, error) {

session, ok := c.state.ExecSessions[id]
if !ok {
return nil, errors.Wrapf(define.ErrNoSuchExecSession, "no exec session with ID %s found in container %s", id, c.ID())
return nil, fmt.Errorf("no exec session with ID %s found in container %s: %w", id, c.ID(), define.ErrNoSuchExecSession)
}

return session, nil
Expand All @@ -861,7 +860,7 @@ func (c *Container) ExecSession(id string) (*ExecSession, error) {

returnSession := new(ExecSession)
if err := JSONDeepCopy(session, returnSession); err != nil {
return nil, errors.Wrapf(err, "error copying contents of container %s exec session %s", c.ID(), session.ID())
return nil, fmt.Errorf("error copying contents of container %s exec session %s: %w", c.ID(), session.ID(), err)
}

return returnSession, nil
Expand Down Expand Up @@ -921,7 +920,7 @@ func (c *Container) NamespacePath(linuxNS LinuxNS) (string, error) { //nolint:in
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return "", errors.Wrapf(err, "error updating container %s state", c.ID())
return "", fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}

Expand All @@ -932,11 +931,11 @@ func (c *Container) NamespacePath(linuxNS LinuxNS) (string, error) { //nolint:in
// If the container is not running, an error will be returned
func (c *Container) namespacePath(linuxNS LinuxNS) (string, error) { //nolint:interfacer
if c.state.State != define.ContainerStateRunning && c.state.State != define.ContainerStatePaused {
return "", errors.Wrapf(define.ErrCtrStopped, "cannot get namespace path unless container %s is running", c.ID())
return "", fmt.Errorf("cannot get namespace path unless container %s is running: %w", c.ID(), define.ErrCtrStopped)
}

if linuxNS == InvalidNS {
return "", errors.Wrapf(define.ErrInvalidArg, "invalid namespace requested from container %s", c.ID())
return "", fmt.Errorf("invalid namespace requested from container %s: %w", c.ID(), define.ErrInvalidArg)
}

return fmt.Sprintf("/proc/%d/ns/%s", c.state.PID, linuxNS.String()), nil
Expand All @@ -959,7 +958,7 @@ func (c *Container) CgroupPath() (string, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return "", errors.Wrapf(err, "error updating container %s state", c.ID())
return "", fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
return c.cGroupPath()
Expand All @@ -971,10 +970,10 @@ func (c *Container) CgroupPath() (string, error) {
// NOTE: only call this when owning the container's lock.
func (c *Container) cGroupPath() (string, error) {
if c.config.NoCgroups || c.config.CgroupsMode == "disabled" {
return "", errors.Wrapf(define.ErrNoCgroups, "this container is not creating cgroups")
return "", fmt.Errorf("this container is not creating cgroups: %w", define.ErrNoCgroups)
}
if c.state.State != define.ContainerStateRunning && c.state.State != define.ContainerStatePaused {
return "", errors.Wrapf(define.ErrCtrStopped, "cannot get cgroup path unless container %s is running", c.ID())
return "", fmt.Errorf("cannot get cgroup path unless container %s is running: %w", c.ID(), define.ErrCtrStopped)
}

// Read /proc/{PID}/cgroup and find the *longest* cgroup entry. That's
Expand All @@ -995,7 +994,7 @@ func (c *Container) cGroupPath() (string, error) {
// If the file doesn't exist, it means the container could have been terminated
// so report it.
if os.IsNotExist(err) {
return "", errors.Wrapf(define.ErrCtrStopped, "cannot get cgroup path unless container %s is running", c.ID())
return "", fmt.Errorf("cannot get cgroup path unless container %s is running: %w", c.ID(), define.ErrCtrStopped)
}
return "", err
}
Expand Down Expand Up @@ -1024,7 +1023,7 @@ func (c *Container) cGroupPath() (string, error) {
}

if len(cgroupPath) == 0 {
return "", errors.Errorf("could not find any cgroup in %q", procPath)
return "", fmt.Errorf("could not find any cgroup in %q", procPath)
}

cgroupManager := c.CgroupManager()
Expand Down Expand Up @@ -1059,7 +1058,7 @@ func (c *Container) RootFsSize() (int64, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return -1, errors.Wrapf(err, "error updating container %s state", c.ID())
return -1, fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
return c.rootFsSize()
Expand All @@ -1071,7 +1070,7 @@ func (c *Container) RWSize() (int64, error) {
c.lock.Lock()
defer c.lock.Unlock()
if err := c.syncContainer(); err != nil {
return -1, errors.Wrapf(err, "error updating container %s state", c.ID())
return -1, fmt.Errorf("error updating container %s state: %w", c.ID(), err)
}
}
return c.rwSize()
Expand Down Expand Up @@ -1173,7 +1172,7 @@ func (c *Container) ContainerState() (*ContainerState, error) {
}
returnConfig := new(ContainerState)
if err := JSONDeepCopy(c.state, returnConfig); err != nil {
return nil, errors.Wrapf(err, "error copying container %s state", c.ID())
return nil, fmt.Errorf("error copying container %s state: %w", c.ID(), err)
}
return c.state, nil
}
Expand Down
49 changes: 24 additions & 25 deletions libpod/container_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package libpod

import (
"context"
"errors"
"fmt"
"io"
"io/ioutil"
Expand All @@ -14,7 +15,6 @@ import (
"github.com/containers/podman/v4/libpod/events"
"github.com/containers/podman/v4/pkg/signal"
"github.com/containers/storage/pkg/archive"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand All @@ -39,7 +39,7 @@ func (c *Container) Init(ctx context.Context, recursive bool) error {
}

if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateStopped, define.ContainerStateExited) {
return errors.Wrapf(define.ErrCtrStateInvalid, "container %s has already been created in runtime", c.ID())
return fmt.Errorf("container %s has already been created in runtime: %w", c.ID(), define.ErrCtrStateInvalid)
}

if !recursive {
Expand Down Expand Up @@ -197,7 +197,7 @@ func (c *Container) StopWithTimeout(timeout uint) error {
}

if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopping) {
return errors.Wrapf(define.ErrCtrStateInvalid, "can only stop created or running containers. %s is in state %s", c.ID(), c.state.State.String())
return fmt.Errorf("can only stop created or running containers. %s is in state %s: %w", c.ID(), c.state.State.String(), define.ErrCtrStateInvalid)
}

return c.stop(timeout)
Expand All @@ -221,7 +221,7 @@ func (c *Container) Kill(signal uint) error {
// stop the container and if that is taking too long, a user
// may have decided to kill the container after all.
default:
return errors.Wrapf(define.ErrCtrStateInvalid, "can only kill running containers. %s is in state %s", c.ID(), c.state.State.String())
return fmt.Errorf("can only kill running containers. %s is in state %s: %w", c.ID(), c.state.State.String(), define.ErrCtrStateInvalid)
}

// Hardcode all = false, we only use all when removing.
Expand All @@ -241,7 +241,7 @@ func (c *Container) Kill(signal uint) error {
// the duration of its runtime, only using it at the beginning to verify state.
func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-chan define.TerminalSize) error {
if c.LogDriver() == define.PassthroughLogging {
return errors.Wrapf(define.ErrNoLogs, "this container is using the 'passthrough' log driver, cannot attach")
return fmt.Errorf("this container is using the 'passthrough' log driver, cannot attach: %w", define.ErrNoLogs)
}
if !c.batched {
c.lock.Lock()
Expand All @@ -254,7 +254,7 @@ func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-
}

if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) {
return errors.Wrapf(define.ErrCtrStateInvalid, "can only attach to created or running containers")
return fmt.Errorf("can only attach to created or running containers: %w", define.ErrCtrStateInvalid)
}

// HACK: This is really gross, but there isn't a better way without
Expand Down Expand Up @@ -320,11 +320,11 @@ func (c *Container) HTTPAttach(r *http.Request, w http.ResponseWriter, streams *
}

if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) {
return errors.Wrapf(define.ErrCtrStateInvalid, "can only attach to created or running containers")
return fmt.Errorf("can only attach to created or running containers: %w", define.ErrCtrStateInvalid)
}

if !streamAttach && !streamLogs {
return errors.Wrapf(define.ErrInvalidArg, "must specify at least one of stream or logs")
return fmt.Errorf("must specify at least one of stream or logs: %w", define.ErrInvalidArg)
}

logrus.Infof("Performing HTTP Hijack attach to container %s", c.ID())
Expand All @@ -346,7 +346,7 @@ func (c *Container) AttachResize(newSize define.TerminalSize) error {
}

if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) {
return errors.Wrapf(define.ErrCtrStateInvalid, "can only resize created or running containers")
return fmt.Errorf("can only resize created or running containers: %w", define.ErrCtrStateInvalid)
}

logrus.Infof("Resizing TTY of container %s", c.ID())
Expand Down Expand Up @@ -383,20 +383,20 @@ func (c *Container) Unmount(force bool) error {
if c.state.Mounted {
mounted, err := c.runtime.storageService.MountedContainerImage(c.ID())
if err != nil {
return errors.Wrapf(err, "can't determine how many times %s is mounted, refusing to unmount", c.ID())
return fmt.Errorf("can't determine how many times %s is mounted, refusing to unmount: %w", c.ID(), err)
}
if mounted == 1 {
if c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) {
return errors.Wrapf(define.ErrCtrStateInvalid, "cannot unmount storage for container %s as it is running or paused", c.ID())
return fmt.Errorf("cannot unmount storage for container %s as it is running or paused: %w", c.ID(), define.ErrCtrStateInvalid)
}
execSessions, err := c.getActiveExecSessions()
if err != nil {
return err
}
if len(execSessions) != 0 {
return errors.Wrapf(define.ErrCtrStateInvalid, "container %s has active exec sessions, refusing to unmount", c.ID())
return fmt.Errorf("container %s has active exec sessions, refusing to unmount: %w", c.ID(), define.ErrCtrStateInvalid)
}
return errors.Wrapf(define.ErrInternal, "can't unmount %s last mount, it is still in use", c.ID())
return fmt.Errorf("can't unmount %s last mount, it is still in use: %w", c.ID(), define.ErrInternal)
}
}
defer c.newContainerEvent(events.Unmount)
Expand All @@ -415,10 +415,10 @@ func (c *Container) Pause() error {
}

if c.state.State == define.ContainerStatePaused {
return errors.Wrapf(define.ErrCtrStateInvalid, "%q is already paused", c.ID())
return fmt.Errorf("%q is already paused: %w", c.ID(), define.ErrCtrStateInvalid)
}
if c.state.State != define.ContainerStateRunning {
return errors.Wrapf(define.ErrCtrStateInvalid, "%q is not running, can't pause", c.state.State)
return fmt.Errorf("%q is not running, can't pause: %w", c.state.State, define.ErrCtrStateInvalid)
}
defer c.newContainerEvent(events.Pause)
return c.pause()
Expand All @@ -436,7 +436,7 @@ func (c *Container) Unpause() error {
}

if c.state.State != define.ContainerStatePaused {
return errors.Wrapf(define.ErrCtrStateInvalid, "%q is not paused, can't unpause", c.ID())
return fmt.Errorf("%q is not paused, can't unpause: %w", c.ID(), define.ErrCtrStateInvalid)
}
defer c.newContainerEvent(events.Unpause)
return c.unpause()
Expand All @@ -455,7 +455,7 @@ func (c *Container) Export(path string) error {
}

if c.state.State == define.ContainerStateRemoving {
return errors.Wrapf(define.ErrCtrStateInvalid, "cannot mount container %s as it is being removed", c.ID())
return fmt.Errorf("cannot mount container %s as it is being removed: %w", c.ID(), define.ErrCtrStateInvalid)
}

defer c.newContainerEvent(events.Mount)
Expand Down Expand Up @@ -666,22 +666,21 @@ func (c *Container) Cleanup(ctx context.Context) error {
defer c.lock.Unlock()

if err := c.syncContainer(); err != nil {
switch errors.Cause(err) {
// When the container has already been removed, the OCI runtime directory remain.
case define.ErrNoSuchCtr, define.ErrCtrRemoved:
if errors.Is(err, define.ErrNoSuchCtr) || errors.Is(err, define.ErrCtrRemoved) {
if err := c.cleanupRuntime(ctx); err != nil {
return errors.Wrapf(err, "error cleaning up container %s from OCI runtime", c.ID())
return fmt.Errorf("error cleaning up container %s from OCI runtime: %w", c.ID(), err)
}
default:
logrus.Errorf("Syncing container %s status: %v", c.ID(), err)
return nil
}
logrus.Errorf("Syncing container %s status: %v", c.ID(), err)
return err
}
}

// Check if state is good
if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateStopped, define.ContainerStateStopping, define.ContainerStateExited) {
return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is running or paused, refusing to clean up", c.ID())
return fmt.Errorf("container %s is running or paused, refusing to clean up: %w", c.ID(), define.ErrCtrStateInvalid)
}

// Handle restart policy.
Expand All @@ -703,7 +702,7 @@ func (c *Container) Cleanup(ctx context.Context) error {
return err
}
if len(sessions) > 0 {
return errors.Wrapf(define.ErrCtrStateInvalid, "container %s has active exec sessions, refusing to clean up", c.ID())
return fmt.Errorf("container %s has active exec sessions, refusing to clean up: %w", c.ID(), define.ErrCtrStateInvalid)
}

defer c.newContainerEvent(events.Cleanup)
Expand Down Expand Up @@ -789,7 +788,7 @@ func (c *Container) ReloadNetwork() error {
}

if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) {
return errors.Wrapf(define.ErrCtrStateInvalid, "cannot reload network unless container network has been configured")
return fmt.Errorf("cannot reload network unless container network has been configured: %w", define.ErrCtrStateInvalid)
}

return c.reloadNetwork()
Expand Down
Loading

0 comments on commit 39fc5d1

Please sign in to comment.