diff --git a/cmd/podman/shared/container.go b/cmd/podman/shared/container.go index bc64d63a95..cdc1b2e42e 100644 --- a/cmd/podman/shared/container.go +++ b/cmd/podman/shared/container.go @@ -195,6 +195,8 @@ func NewBatchContainer(ctr *libpod.Container, opts PsOptions) (PsContainerOutput status = "Paused" case define.ContainerStateCreated.String(), define.ContainerStateConfigured.String(): status = "Created" + case define.ContainerStateRemoving.String(): + status = "Removing" default: status = "Error" } diff --git a/libpod/container_api.go b/libpod/container_api.go index 0c57d56781..9689502ffa 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -395,6 +395,11 @@ func (c *Container) Mount() (string, error) { return "", err } } + + if c.state.State == define.ContainerStateRemoving { + return "", errors.Wrapf(define.ErrCtrStateInvalid, "cannot mount container %s as it is being removed", c.ID()) + } + defer c.newContainerEvent(events.Mount) return c.mount() } @@ -479,7 +484,12 @@ func (c *Container) Export(path string) error { return err } } - defer c.newContainerEvent(events.Export) + + if c.state.State == define.ContainerStateRemoving { + return errors.Wrapf(define.ErrCtrStateInvalid, "cannot mount container %s as it is being removed", c.ID()) + } + + defer c.newContainerEvent(events.Mount) return c.export(path) } @@ -670,6 +680,10 @@ func (c *Container) Refresh(ctx context.Context) error { } } + if c.state.State == define.ContainerStateRemoving { + return errors.Wrapf(define.ErrCtrStateInvalid, "cannot refresh containers that are being removed") + } + wasCreated := false if c.state.State == define.ContainerStateCreated { wasCreated = true @@ -810,7 +824,6 @@ func (c *Container) Checkpoint(ctx context.Context, options ContainerCheckpointO return err } } - defer c.newContainerEvent(events.Checkpoint) return c.checkpoint(ctx, options) } diff --git a/libpod/container_internal.go b/libpod/container_internal.go index cf610b19a5..c478012491 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -719,7 +719,8 @@ func (c *Container) isStopped() (bool, error) { if err != nil { return true, err } - return c.state.State != define.ContainerStateRunning && c.state.State != define.ContainerStatePaused, nil + + return !c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused), nil } // save container state to the database @@ -1050,6 +1051,8 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { // If we are ContainerStateUnknown, throw an error if c.state.State == define.ContainerStateUnknown { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is in an unknown state", c.ID()) + } else if c.state.State == define.ContainerStateRemoving { + return errors.Wrapf(define.ErrCtrStateInvalid, "cannot start container %s as it is being removed", c.ID()) } // If we are running, do nothing diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 1cc34bbb3b..070b6d67c1 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -21,6 +21,7 @@ import ( "github.com/containernetworking/plugins/pkg/ns" "github.com/containers/buildah/pkg/secrets" "github.com/containers/libpod/libpod/define" + "github.com/containers/libpod/libpod/events" "github.com/containers/libpod/pkg/annotations" "github.com/containers/libpod/pkg/apparmor" "github.com/containers/libpod/pkg/cgroups" @@ -698,6 +699,8 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO return err } + defer c.newContainerEvent(events.Checkpoint) + if options.TargetFile != "" { if err = c.exportCheckpoint(options.TargetFile, options.IgnoreRootfs); err != nil { return err @@ -769,7 +772,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti return err } - if (c.state.State != define.ContainerStateConfigured) && (c.state.State != define.ContainerStateExited) { + if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited) { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is running or paused, cannot restore", c.ID()) } diff --git a/libpod/define/containerstate.go b/libpod/define/containerstate.go index ab2527b3ee..e7d258e214 100644 --- a/libpod/define/containerstate.go +++ b/libpod/define/containerstate.go @@ -25,6 +25,9 @@ const ( // ContainerStateExited indicates the the container has stopped and been // cleaned up ContainerStateExited ContainerStatus = iota + // ContainerStateRemoving indicates the container is in the process of + // being removed. + ContainerStateRemoving ContainerStatus = iota ) // ContainerStatus returns a string representation for users @@ -45,6 +48,8 @@ func (t ContainerStatus) String() string { return "paused" case ContainerStateExited: return "exited" + case ContainerStateRemoving: + return "removing" } return "bad state" } @@ -67,6 +72,8 @@ func StringToContainerStatus(status string) (ContainerStatus, error) { return ContainerStatePaused, nil case ContainerStateExited.String(): return ContainerStateExited, nil + case ContainerStateRemoving.String(): + return ContainerStateRemoving, nil default: return ContainerStateUnknown, errors.Wrapf(ErrInvalidArg, "unknown container state: %s", status) } diff --git a/libpod/options.go b/libpod/options.go index 66e8ef93cd..f275919026 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -769,16 +769,8 @@ func WithIPCNSFrom(nsCtr *Container) CtrCreateOption { return define.ErrCtrFinalized } - if !nsCtr.valid { - return define.ErrCtrRemoved - } - - if nsCtr.ID() == ctr.ID() { - return errors.Wrapf(define.ErrInvalidArg, "must specify another container") - } - - if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod { - return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID()) + if err := checkDependencyContainer(nsCtr, ctr); err != nil { + return err } ctr.config.IPCNsCtr = nsCtr.ID() @@ -797,16 +789,8 @@ func WithMountNSFrom(nsCtr *Container) CtrCreateOption { return define.ErrCtrFinalized } - if !nsCtr.valid { - return define.ErrCtrRemoved - } - - if nsCtr.ID() == ctr.ID() { - return errors.Wrapf(define.ErrInvalidArg, "must specify another container") - } - - if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod { - return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID()) + if err := checkDependencyContainer(nsCtr, ctr); err != nil { + return err } ctr.config.MountNsCtr = nsCtr.ID() @@ -825,22 +809,14 @@ func WithNetNSFrom(nsCtr *Container) CtrCreateOption { return define.ErrCtrFinalized } - if !nsCtr.valid { - return define.ErrCtrRemoved - } - - if nsCtr.ID() == ctr.ID() { - return errors.Wrapf(define.ErrInvalidArg, "must specify another container") + if err := checkDependencyContainer(nsCtr, ctr); err != nil { + return err } if ctr.config.CreateNetNS { return errors.Wrapf(define.ErrInvalidArg, "cannot join another container's net ns as we are making a new net ns") } - if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod { - return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID()) - } - ctr.config.NetNsCtr = nsCtr.ID() return nil @@ -857,16 +833,8 @@ func WithPIDNSFrom(nsCtr *Container) CtrCreateOption { return define.ErrCtrFinalized } - if !nsCtr.valid { - return define.ErrCtrRemoved - } - - if nsCtr.ID() == ctr.ID() { - return errors.Wrapf(define.ErrInvalidArg, "must specify another container") - } - - if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod { - return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID()) + if err := checkDependencyContainer(nsCtr, ctr); err != nil { + return err } if ctr.config.NoCgroups { @@ -889,16 +857,8 @@ func WithUserNSFrom(nsCtr *Container) CtrCreateOption { return define.ErrCtrFinalized } - if !nsCtr.valid { - return define.ErrCtrRemoved - } - - if nsCtr.ID() == ctr.ID() { - return errors.Wrapf(define.ErrInvalidArg, "must specify another container") - } - - if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod { - return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID()) + if err := checkDependencyContainer(nsCtr, ctr); err != nil { + return err } ctr.config.UserNsCtr = nsCtr.ID() @@ -918,16 +878,8 @@ func WithUTSNSFrom(nsCtr *Container) CtrCreateOption { return define.ErrCtrFinalized } - if !nsCtr.valid { - return define.ErrCtrRemoved - } - - if nsCtr.ID() == ctr.ID() { - return errors.Wrapf(define.ErrInvalidArg, "must specify another container") - } - - if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod { - return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID()) + if err := checkDependencyContainer(nsCtr, ctr); err != nil { + return err } ctr.config.UTSNsCtr = nsCtr.ID() @@ -946,16 +898,8 @@ func WithCgroupNSFrom(nsCtr *Container) CtrCreateOption { return define.ErrCtrFinalized } - if !nsCtr.valid { - return define.ErrCtrRemoved - } - - if nsCtr.ID() == ctr.ID() { - return errors.Wrapf(define.ErrInvalidArg, "must specify another container") - } - - if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod { - return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID()) + if err := checkDependencyContainer(nsCtr, ctr); err != nil { + return err } ctr.config.CgroupNsCtr = nsCtr.ID() diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 7069d34940..ae401013c8 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -489,32 +489,19 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, } } - var cleanupErr error - // Remove the container from the state - if c.config.Pod != "" { - // If we're removing the pod, the container will be evicted - // from the state elsewhere - if !removePod { - if err := r.state.RemoveContainerFromPod(pod, c); err != nil { - cleanupErr = err - } - } - } else { - if err := r.state.RemoveContainer(c); err != nil { - cleanupErr = err - } + // Set ContainerStateRemoving and remove exec sessions + c.state.State = define.ContainerStateRemoving + c.state.ExecSessions = nil + + if err := c.save(); err != nil { + return errors.Wrapf(err, "unable to set container %s removing state in database", c.ID()) } - // Set container as invalid so it can no longer be used - c.valid = false + var cleanupErr error // Clean up network namespace, cgroups, mounts if err := c.cleanup(ctx); err != nil { - if cleanupErr == nil { - cleanupErr = errors.Wrapf(err, "error cleaning up container %s", c.ID()) - } else { - logrus.Errorf("cleanup network, cgroups, mounts: %v", err) - } + cleanupErr = errors.Wrapf(err, "error cleaning up container %s", c.ID()) } // Stop the container's storage @@ -540,6 +527,29 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, } } + // Remove the container from the state + if c.config.Pod != "" { + // If we're removing the pod, the container will be evicted + // from the state elsewhere + if !removePod { + if err := r.state.RemoveContainerFromPod(pod, c); err != nil { + if cleanupErr == nil { + cleanupErr = err + } else { + logrus.Errorf("Error removing container %s from database: %v", c.ID(), err) + } + } + } + } else { + if err := r.state.RemoveContainer(c); err != nil { + if cleanupErr == nil { + cleanupErr = err + } else { + logrus.Errorf("Error removing container %s from database: %v", c.ID(), err) + } + } + } + // Deallocate the container's lock if err := c.lock.Free(); err != nil { if cleanupErr == nil { @@ -549,6 +559,9 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, } } + // Set container as invalid so it can no longer be used + c.valid = false + c.newContainerEvent(events.Remove) if !removeVolume { diff --git a/libpod/util.go b/libpod/util.go index 7bd834e302..3cdb2c3632 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -203,3 +203,28 @@ func DefaultSeccompPath() (string, error) { } return config.SeccompDefaultPath, nil } + +// CheckDependencyContainer verifies the given container can be used as a +// dependency of another container. +// Both the dependency to check and the container that will be using the +// dependency must be passed in. +// It is assumed that ctr is locked, and depCtr is unlocked. +func checkDependencyContainer(depCtr, ctr *Container) error { + state, err := depCtr.State() + if err != nil { + return errors.Wrapf(err, "error accessing dependency container %s state", depCtr.ID()) + } + if state == define.ContainerStateRemoving { + return errors.Wrapf(define.ErrCtrStateInvalid, "cannot use container %s as a dependency as it is being removed", depCtr.ID()) + } + + if depCtr.ID() == ctr.ID() { + return errors.Wrapf(define.ErrInvalidArg, "must specify another container") + } + + if ctr.config.Pod != "" && depCtr.PodID() != ctr.config.Pod { + return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, depCtr.ID()) + } + + return nil +}