diff options
-rw-r--r-- | cmd/podman/cleanup.go | 4 | ||||
-rw-r--r-- | cmd/podman/pod_stop.go | 4 | ||||
-rw-r--r-- | cmd/podman/ps.go | 2 | ||||
-rw-r--r-- | cmd/podman/run.go | 2 | ||||
-rw-r--r-- | cmd/podman/start.go | 7 | ||||
-rw-r--r-- | contrib/python/podman/test/test_pods_ctnrs.py | 4 | ||||
-rw-r--r-- | libpod/container.go | 7 | ||||
-rw-r--r-- | libpod/container_api.go | 59 | ||||
-rw-r--r-- | libpod/container_internal.go | 95 | ||||
-rw-r--r-- | libpod/oci.go | 2 | ||||
-rw-r--r-- | libpod/pod_api.go | 4 | ||||
-rw-r--r-- | libpod/runtime_ctr.go | 14 | ||||
-rw-r--r-- | libpod/runtime_pod_linux.go | 5 | ||||
-rw-r--r-- | pkg/varlinkapi/pods.go | 2 |
14 files changed, 115 insertions, 96 deletions
diff --git a/cmd/podman/cleanup.go b/cmd/podman/cleanup.go index 6ebb682ed..316704f91 100644 --- a/cmd/podman/cleanup.go +++ b/cmd/podman/cleanup.go @@ -46,6 +46,8 @@ func cleanupCmd(c *cli.Context) error { args := c.Args() + ctx := getContext() + var lastError error var cleanupContainers []*libpod.Container if c.Bool("all") { @@ -80,7 +82,7 @@ func cleanupCmd(c *cli.Context) error { } } for _, ctr := range cleanupContainers { - if err = ctr.Cleanup(); err != nil { + if err = ctr.Cleanup(ctx); err != nil { if lastError != nil { fmt.Fprintln(os.Stderr, lastError) } diff --git a/cmd/podman/pod_stop.go b/cmd/podman/pod_stop.go index 03d04a3ec..6dc6a2b2d 100644 --- a/cmd/podman/pod_stop.go +++ b/cmd/podman/pod_stop.go @@ -50,9 +50,11 @@ func podStopCmd(c *cli.Context) error { // in which case the following loop will be skipped. pods, lastError := getPodsFromContext(c, runtime) + ctx := getContext() + for _, pod := range pods { // set cleanup to true to clean mounts and namespaces - ctr_errs, err := pod.Stop(true) + ctr_errs, err := pod.Stop(ctx, true) if ctr_errs != nil { for ctr, err := range ctr_errs { if lastError != nil { diff --git a/cmd/podman/ps.go b/cmd/podman/ps.go index d36c929e8..e53afe1bf 100644 --- a/cmd/podman/ps.go +++ b/cmd/podman/ps.go @@ -548,6 +548,8 @@ func getTemplateOutput(psParams []psJSONParams, opts shared.PsOptions) ([]psTemp labels := formatLabels(psParam.Labels) switch psParam.Status { + case libpod.ContainerStateExited.String(): + fallthrough case libpod.ContainerStateStopped.String(): exitedSince := units.HumanDuration(time.Since(psParam.ExitedAt)) status = fmt.Sprintf("Exited (%d) %s ago", psParam.ExitCode, exitedSince) diff --git a/cmd/podman/run.go b/cmd/podman/run.go index f9a96e4a6..fbad4237d 100644 --- a/cmd/podman/run.go +++ b/cmd/podman/run.go @@ -140,7 +140,7 @@ func runCmd(c *cli.Context) error { return runtime.RemoveContainer(ctx, ctr, true) } - if err := ctr.Cleanup(); err != nil { + if err := ctr.Cleanup(ctx); err != nil { // If the container has been removed already, no need to error on cleanup // Also, if it was restarted, don't error either if errors.Cause(err) == libpod.ErrNoSuchCtr || diff --git a/cmd/podman/start.go b/cmd/podman/start.go index cb65ec6d4..a34f6df5d 100644 --- a/cmd/podman/start.go +++ b/cmd/podman/start.go @@ -81,6 +81,9 @@ func startCmd(c *cli.Context) error { } args = append(args, lastCtr.ID()) } + + ctx := getContext() + var lastError error for _, container := range args { ctr, err := runtime.LookupContainer(container) @@ -121,14 +124,14 @@ func startCmd(c *cli.Context) error { exitCode = int(ecode) } - return ctr.Cleanup() + return ctr.Cleanup(ctx) } if ctrRunning { fmt.Println(ctr.ID()) continue } // Handle non-attach start - if err := ctr.Start(getContext()); err != nil { + if err := ctr.Start(ctx); err != nil { if lastError != nil { fmt.Fprintln(os.Stderr, lastError) } diff --git a/contrib/python/podman/test/test_pods_ctnrs.py b/contrib/python/podman/test/test_pods_ctnrs.py index c5733091c..14ce95c8a 100644 --- a/contrib/python/podman/test/test_pods_ctnrs.py +++ b/contrib/python/podman/test/test_pods_ctnrs.py @@ -46,11 +46,11 @@ class TestPodsCtnrs(PodmanTestCase): pod = pod.start() status = FoldedString(pod.containersinfo[0]['status']) # Race on whether container is still running or finished - self.assertIn(status, ('exited', 'running')) + self.assertIn(status, ('stopped', 'exited', 'running')) pod = pod.restart() status = FoldedString(pod.containersinfo[0]['status']) - self.assertIn(status, ('exited', 'running')) + self.assertIn(status, ('stopped', 'exited', 'running')) killed = pod.kill() self.assertEqual(pod, killed) diff --git a/libpod/container.go b/libpod/container.go index 0b1879208..55a0f3a2c 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -36,6 +36,9 @@ const ( ContainerStateStopped ContainerStatus = iota // ContainerStatePaused indicates that the container has been paused ContainerStatePaused ContainerStatus = iota + // ContainerStateExited indicates the the container has stopped and been + // cleaned up + ContainerStateExited ContainerStatus = iota ) // CgroupfsDefaultCgroupParent is the cgroup parent for CGroupFS in libpod @@ -354,9 +357,11 @@ func (t ContainerStatus) String() string { case ContainerStateRunning: return "running" case ContainerStateStopped: - return "exited" + return "stopped" case ContainerStatePaused: return "paused" + case ContainerStateExited: + return "exited" } return "bad state" } diff --git a/libpod/container_api.go b/libpod/container_api.go index fc2058de6..192ccd347 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -32,7 +32,8 @@ func (c *Container) Init(ctx context.Context) (err error) { } if !(c.state.State == ContainerStateConfigured || - c.state.State == ContainerStateStopped) { + c.state.State == ContainerStateStopped || + c.state.State == ContainerStateExited) { return errors.Wrapf(ErrCtrExists, "container %s has already been created in runtime", c.ID()) } @@ -50,7 +51,7 @@ func (c *Container) Init(ctx context.Context) (err error) { } defer func() { if err != nil { - if err2 := c.cleanup(); err2 != nil { + if err2 := c.cleanup(ctx); err2 != nil { logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) } } @@ -84,7 +85,8 @@ func (c *Container) Start(ctx context.Context) (err error) { // Container must be created or stopped to be started if !(c.state.State == ContainerStateConfigured || c.state.State == ContainerStateCreated || - c.state.State == ContainerStateStopped) { + c.state.State == ContainerStateStopped || + c.state.State == ContainerStateExited) { return errors.Wrapf(ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID()) } @@ -102,7 +104,7 @@ func (c *Container) Start(ctx context.Context) (err error) { } defer func() { if err != nil { - if err2 := c.cleanup(); err2 != nil { + if err2 := c.cleanup(ctx); err2 != nil { logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) } } @@ -113,8 +115,9 @@ func (c *Container) Start(ctx context.Context) (err error) { if err := c.reinit(ctx); err != nil { return err } - } else if c.state.State == ContainerStateConfigured { - // Or initialize it for the first time if necessary + } else if c.state.State == ContainerStateConfigured || + c.state.State == ContainerStateExited { + // Or initialize it if necessary if err := c.init(ctx); err != nil { return err } @@ -147,7 +150,8 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams, // Container must be created or stopped to be started if !(c.state.State == ContainerStateConfigured || c.state.State == ContainerStateCreated || - c.state.State == ContainerStateStopped) { + c.state.State == ContainerStateStopped || + c.state.State == ContainerStateExited) { return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID()) } @@ -165,7 +169,7 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams, } defer func() { if err != nil { - if err2 := c.cleanup(); err2 != nil { + if err2 := c.cleanup(ctx); err2 != nil { logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) } } @@ -176,8 +180,9 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams, if err := c.reinit(ctx); err != nil { return nil, err } - } else if c.state.State == ContainerStateConfigured { - // Or initialize it for the first time if necessary + } else if c.state.State == ContainerStateConfigured || + c.state.State == ContainerStateExited { + // Or initialize it if necessary if err := c.init(ctx); err != nil { return nil, err } @@ -202,26 +207,8 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams, // Default stop timeout is 10 seconds, but can be overridden when the container // is created func (c *Container) Stop() error { - if !c.batched { - c.lock.Lock() - defer c.lock.Unlock() - - if err := c.syncContainer(); err != nil { - return err - } - } - - if c.state.State == ContainerStateConfigured || - c.state.State == ContainerStateUnknown || - c.state.State == ContainerStatePaused { - return errors.Wrapf(ErrCtrStateInvalid, "can only stop created, running, or stopped containers") - } - - if c.state.State == ContainerStateStopped { - return ErrCtrStopped - } - - return c.stop(c.config.StopTimeout) + // Stop with the container's given timeout + return c.StopWithTimeout(c.config.StopTimeout) } // StopWithTimeout is a version of Stop that allows a timeout to be specified @@ -243,7 +230,8 @@ func (c *Container) StopWithTimeout(timeout uint) error { return errors.Wrapf(ErrCtrStateInvalid, "can only stop created, running, or stopped containers") } - if c.state.State == ContainerStateStopped { + if c.state.State == ContainerStateStopped || + c.state.State == ContainerStateExited { return ErrCtrStopped } @@ -431,7 +419,8 @@ func (c *Container) Attach(streams *AttachStreams, keys string, resize <-chan re } if c.state.State != ContainerStateCreated && - c.state.State != ContainerStateRunning { + c.state.State != ContainerStateRunning && + c.state.State != ContainerStateExited { return errors.Wrapf(ErrCtrStateInvalid, "can only attach to created or running containers") } @@ -626,7 +615,7 @@ func (c *Container) WaitWithInterval(waitTimeout time.Duration) (int32, error) { // Cleanup unmounts all mount points in container and cleans up container storage // It also cleans up the network stack -func (c *Container) Cleanup() error { +func (c *Container) Cleanup(ctx context.Context) error { if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -645,7 +634,7 @@ func (c *Container) Cleanup() error { return errors.Wrapf(ErrCtrStateInvalid, "container %s has active exec sessions, refusing to clean up", c.ID()) } - return c.cleanup() + return c.cleanup(ctx) } // Batch starts a batch operation on the given container @@ -800,7 +789,7 @@ func (c *Container) Refresh(ctx context.Context) error { // Fire cleanup code one more time unconditionally to ensure we are good // to refresh - if err := c.cleanup(); err != nil { + if err := c.cleanup(ctx); err != nil { return err } diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d909c5d17..1e49673bf 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -150,7 +150,8 @@ func (c *Container) syncContainer() error { // If runtime knows about the container, update its status in runtime // And then save back to disk if (c.state.State != ContainerStateUnknown) && - (c.state.State != ContainerStateConfigured) { + (c.state.State != ContainerStateConfigured) && + (c.state.State != ContainerStateExited) { oldState := c.state.State // TODO: optionally replace this with a stat for the exit file if err := c.runtime.ociRuntime.updateContainerStatus(c); err != nil { @@ -422,7 +423,7 @@ func (c *Container) isStopped() (bool, error) { if err != nil { return true, err } - return c.state.State == ContainerStateStopped, nil + return (c.state.State == ContainerStateStopped || c.state.State == ContainerStateExited), nil } // save container state to the database @@ -528,6 +529,8 @@ func (c *Container) init(ctx context.Context) error { logrus.Debugf("Created container %s in OCI runtime", c.ID()) + c.state.ExitCode = 0 + c.state.Exited = false c.state.State = ContainerStateCreated if err := c.save(); err != nil { @@ -537,11 +540,14 @@ func (c *Container) init(ctx context.Context) error { return c.completeNetworkSetup() } -// Reinitialize a container -// Deletes and recreates a container in the runtime -// Should only be done on ContainerStateStopped containers -func (c *Container) reinit(ctx context.Context) error { - logrus.Debugf("Recreating container %s in OCI runtime", c.ID()) +// Clean up a container in the OCI runtime. +// Deletes the container in the runtime, and resets its state to Exited. +// The container can be restarted cleanly after this. +func (c *Container) cleanupRuntime(ctx context.Context) error { + // If the container is not ContainerStateStopped, do nothing + if c.state.State != ContainerStateStopped { + return nil + } // If necessary, delete attach and ctl files if err := c.removeConmonFiles(); err != nil { @@ -552,19 +558,33 @@ func (c *Container) reinit(ctx context.Context) error { return err } - // Our state is now Configured, as we've removed ourself from - // the runtime - // Set and save now to make sure that, if the init() below fails - // we still have a valid state - c.state.State = ContainerStateConfigured - c.state.ExitCode = 0 - c.state.Exited = false - if err := c.save(); err != nil { - return err + // Our state is now Exited, as we've removed ourself from + // the runtime. + c.state.State = ContainerStateExited + + if c.valid { + if err := c.save(); err != nil { + return err + } } logrus.Debugf("Successfully cleaned up container %s", c.ID()) + return nil +} + +// Reinitialize a container. +// Deletes and recreates a container in the runtime. +// Should only be done on ContainerStateStopped containers. +// Not necessary for ContainerStateExited - the container has already been +// removed from the runtime, so init() can proceed freely. +func (c *Container) reinit(ctx context.Context) error { + logrus.Debugf("Recreating container %s in OCI runtime", c.ID()) + + if err := c.cleanupRuntime(ctx); err != nil { + return err + } + // Initialize the container again return c.init(ctx) } @@ -592,7 +612,7 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { } defer func() { if err != nil { - if err2 := c.cleanup(); err2 != nil { + if err2 := c.cleanup(ctx); err2 != nil { logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) } } @@ -603,28 +623,11 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { if c.state.State == ContainerStateStopped { logrus.Debugf("Recreating container %s in OCI runtime", c.ID()) - // If necessary, delete attach and ctl files - if err := c.removeConmonFiles(); err != nil { - return err - } - - // Delete the container in the runtime - if err := c.runtime.ociRuntime.deleteContainer(c); err != nil { - return errors.Wrapf(err, "error removing container %s from runtime", c.ID()) - } - - // Our state is now Configured, as we've removed ourself from - // the runtime - // Set and save now to make sure that, if the init() below fails - // we still have a valid state - c.state.State = ContainerStateConfigured - if err := c.save(); err != nil { + if err := c.reinit(ctx); err != nil { return err } - } - - // If we are ContainerStateConfigured we need to init() - if c.state.State == ContainerStateConfigured { + } else if c.state.State == ContainerStateConfigured || + c.state.State == ContainerStateExited { if err := c.init(ctx); err != nil { return err } @@ -705,7 +708,7 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e } defer func() { if err != nil { - if err2 := c.cleanup(); err2 != nil { + if err2 := c.cleanup(ctx); err2 != nil { logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) } } @@ -716,8 +719,9 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e if err := c.reinit(ctx); err != nil { return err } - } else if c.state.State == ContainerStateConfigured { - // Initialize the container if it has never been initialized + } else if c.state.State == ContainerStateConfigured || + c.state.State == ContainerStateExited { + // Initialize the container if err := c.init(ctx); err != nil { return err } @@ -826,7 +830,7 @@ func (c *Container) cleanupStorage() error { } // Unmount the a container and free its resources -func (c *Container) cleanup() error { +func (c *Container) cleanup(ctx context.Context) error { var lastError error logrus.Debugf("Cleaning up container %s", c.ID()) @@ -845,6 +849,15 @@ func (c *Container) cleanup() error { } } + // Remove the container from the runtime, if necessary + if err := c.cleanupRuntime(ctx); err != nil { + if lastError != nil { + logrus.Errorf("Error removing container %s from OCI runtime: %v", c.ID(), err) + } else { + lastError = err + } + } + return lastError } diff --git a/libpod/oci.go b/libpod/oci.go index 17f0630c4..e5db06540 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -457,7 +457,7 @@ func (r *OCIRuntime) updateContainerStatus(ctr *Container) error { if err != nil { if strings.Contains(string(out), "does not exist") { ctr.removeConmonFiles() - ctr.state.State = ContainerStateConfigured + ctr.state.State = ContainerStateExited return nil } return errors.Wrapf(err, "error getting container %s state. stderr/out: %s", ctr.ID(), out) diff --git a/libpod/pod_api.go b/libpod/pod_api.go index 0c518da0d..3d5512e8c 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -77,7 +77,7 @@ func (p *Pod) Start(ctx context.Context) (map[string]error, error) { // containers. The container ID is mapped to the error encountered. The error is // set to ErrCtrExists // If both error and the map are nil, all containers were stopped without error -func (p *Pod) Stop(cleanup bool) (map[string]error, error) { +func (p *Pod) Stop(ctx context.Context, cleanup bool) (map[string]error, error) { p.lock.Lock() defer p.lock.Unlock() @@ -118,7 +118,7 @@ func (p *Pod) Stop(cleanup bool) (map[string]error, error) { } if cleanup { - if err := ctr.cleanup(); err != nil { + if err := ctr.cleanup(ctx); err != nil { ctrErrors[ctr.ID()] = err } } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 6c487e367..4256a84a0 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -262,7 +262,8 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) } } else if !(c.state.State == ContainerStateConfigured || c.state.State == ContainerStateCreated || - c.state.State == ContainerStateStopped) { + c.state.State == ContainerStateStopped || + c.state.State == ContainerStateExited) { return errors.Wrapf(ErrCtrStateInvalid, "cannot remove container %s as it is %s - running or paused containers cannot be removed", c.ID(), c.state.State.String()) } @@ -311,7 +312,7 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) c.valid = false // Clean up network namespace, cgroups, mounts - if err := c.cleanup(); err != nil { + if err := c.cleanup(ctx); err != nil { if cleanupErr == nil { cleanupErr = err } else { @@ -332,10 +333,11 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) label.ReleaseLabel(c.ProcessLabel()) r.reserveLabels() } - // Delete the container - // Only do this if we're not ContainerStateConfigured - if we are, - // we haven't been created in the runtime yet - if c.state.State != ContainerStateConfigured { + // Delete the container. + // Not needed in Configured and Exited states, where the container + // doesn't exist in the runtime + if c.state.State != ContainerStateConfigured && + c.state.State != ContainerStateExited { if err := c.delete(ctx); err != nil { if cleanupErr == nil { cleanupErr = err diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index dd57007e0..974cd2b68 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -222,7 +222,7 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool) // As we have guaranteed their dependencies are in the pod for _, ctr := range ctrs { // Clean up network namespace, cgroups, mounts - if err := ctr.cleanup(); err != nil { + if err := ctr.cleanup(ctx); err != nil { return err } @@ -233,7 +233,8 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool) // Delete the container from runtime (only if we are not // ContainerStateConfigured) - if ctr.state.State != ContainerStateConfigured { + if ctr.state.State != ContainerStateConfigured && + ctr.state.State != ContainerStateExited { if err := ctr.delete(ctx); err != nil { return err } diff --git a/pkg/varlinkapi/pods.go b/pkg/varlinkapi/pods.go index 8b72492b0..7930a956f 100644 --- a/pkg/varlinkapi/pods.go +++ b/pkg/varlinkapi/pods.go @@ -125,7 +125,7 @@ func (i *LibpodAPI) StopPod(call iopodman.VarlinkCall, name string) error { if err != nil { return call.ReplyPodNotFound(name) } - ctrErrs, err := pod.Stop(true) + ctrErrs, err := pod.Stop(getContext(), true) callErr := handlePodCall(call, pod, ctrErrs, err) if callErr != nil { return err |