From 2c7f97d5a782d35efc195baf7a7ca9016ca05409 Mon Sep 17 00:00:00 2001
From: Matthew Heon <matthew.heon@gmail.com>
Date: Sun, 23 Sep 2018 18:04:29 -0400
Subject: Add ContainerStateExited and OCI delete() in cleanup()
To work better with Kata containers, we need to delete() from the
OCI runtime as a part of cleanup, to ensure resources aren't
retained longer than they need to be.
To enable this, we need to add a new state to containers,
ContainerStateExited. Containers transition from
ContainerStateStopped to ContainerStateExited via cleanupRuntime
which is invoked as part of cleanup(). A container in the Exited
state is identical to Stopped, except it has been removed from
the OCI runtime and thus will be handled differently when
initializing the container.
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
---
cmd/podman/cleanup.go | 4 ++-
cmd/podman/pod_stop.go | 4 ++-
cmd/podman/run.go | 2 +-
cmd/podman/start.go | 7 ++--
libpod/container.go | 7 +++-
libpod/container_api.go | 59 +++++++++++++-----------------
libpod/container_internal.go | 86 ++++++++++++++++++++++++--------------------
libpod/pod_api.go | 4 +--
libpod/runtime_ctr.go | 5 +--
libpod/runtime_pod_linux.go | 5 +--
pkg/varlinkapi/pods.go | 2 +-
11 files changed, 99 insertions(+), 86 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/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/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 0642c5aad..60e83576a 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 {
@@ -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,30 @@ 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
+ // Our state is now Exited, as we've removed ourself from
+ // the runtime.
+ c.state.State = ContainerStateExited
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 +609,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 +620,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 +705,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 +716,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 +827,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 +846,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/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..b8a8b6c20 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -311,7 +311,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 {
@@ -335,7 +335,8 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool)
// 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 {
+ 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
--
cgit v1.2.3-54-g00ecf