Update libpod logic for placing containers in pods

Signed-off-by: Matthew Heon <mheon@redhat.com>

libpod/container.go

@@ -100,7 +100,7 @@ type containerConfig struct {
 	// reboot
 	StaticDir string `json:"staticDir"`
 	// Pod the container belongs to
-	Pod *string `json:"pod,omitempty"`
+	Pod string `json:"pod,omitempty"`
 	// Shared namespaces with container
 	SharedNamespaceCtr *string           `json:"shareNamespacesWith,omitempty"`
 	SharedNamespaceMap map[string]string `json:"sharedNamespaces"`
@@ -218,6 +218,44 @@ func (c *Container) setupImageRootfs() error {
 	return nil
 }
 
+// Tear down a container's storage prior to removal
+func (c *Container) teardownStorage() error {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	if !c.valid {
+		return errors.Wrapf(ErrCtrRemoved, "container %s is not valid", c.ID())
+	}
+
+	if c.state.State == ContainerStateRunning || c.state.State == ContainerStatePaused {
+		return errors.Wrapf(ErrCtrStateInvalid, "cannot remove storage for container %s as it is running or paused", c.ID())
+	}
+
+	if !c.config.RootfsFromImage {
+		// TODO implement directory-based root filesystems
+		return ErrNotImplemented
+	}
+
+	return c.teardownImageRootfs()
+}
+
+// Completely remove image-based root filesystem for a container
+func (c *Container) teardownImageRootfs() error {
+	if c.state.Mounted {
+		if err := c.runtime.storageService.StopContainer(c.ID()); err != nil {
+			return errors.Wrapf(err, "error unmounting container %s root filesystem", c.ID())
+		}
+
+		c.state.Mounted = false
+	}
+
+	if err := c.runtime.storageService.DeleteContainer(c.ID()); err != nil {
+		return errors.Wrapf(err, "error removing container %s root filesystem", c.ID())
+	}
+
+	return nil
+}
+
 // Create creates a container in the OCI runtime
 func (c *Container) Create() (err error) {
 	c.lock.Lock()

libpod/options.go

@@ -245,7 +245,7 @@ func WithRootFSFromPath(path string) CtrCreateOption {
 		}
 
 		if ctr.config.RootfsDir != "" || ctr.config.RootfsImageID != "" || ctr.config.RootfsImageName != "" {
-			return fmt.Errorf("container already configured to with rootfs")
+			return errors.Wrapf(ErrInvalidArg, "container already configured with root filesystem")
 		}
 
 		ctr.config.RootfsDir = path
@@ -266,7 +266,7 @@ func WithRootFSFromImage(imageID string, imageName string, useImageConfig bool)
 		}
 
 		if ctr.config.RootfsDir != "" || ctr.config.RootfsImageID != "" || ctr.config.RootfsImageName != "" {
-			return fmt.Errorf("container already configured to with rootfs")
+			return errors.Wrapf(ErrInvalidArg, "container already configured with root filesystem")
 		}
 
 		ctr.config.RootfsImageID = imageID
@@ -307,21 +307,11 @@ func (r *Runtime) WithPod(pod *Pod) CtrCreateOption {
 			return ErrCtrFinalized
 		}
 
-		if ctr.pod != nil {
+		if pod == nil {
-			return fmt.Errorf("container has already been added to a pod")
+			return ErrInvalidArg
-		}
-
-		exists, err := r.state.HasPod(pod.ID())
-		if err != nil {
-			return errors.Wrapf(err, "error searching state for pod %s", pod.ID())
-		} else if !exists {
-			return errors.Wrapf(ErrNoSuchPod, "pod %s cannot be found in state", pod.ID())
-		}
-
-		if err := pod.addContainer(ctr); err != nil {
-			return errors.Wrapf(err, "error adding container to pod")
 		}
 
+		ctr.config.Pod = pod.ID()
 		ctr.pod = pod
 
 		return nil

libpod/pod.go

@@ -45,11 +45,17 @@ func newPod() (*Pod, error) {
 func (p *Pod) addContainer(ctr *Container) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
+	ctr.lock.Lock()
+	defer ctr.lock.Unlock()
 
 	if !p.valid {
 		return ErrPodRemoved
 	}
 
+	if !ctr.valid {
+		return ErrCtrRemoved
+	}
+
 	if _, ok := p.containers[ctr.ID()]; ok {
 		return errors.Wrapf(ErrCtrExists, "container with ID %s already exists in pod %s", ctr.ID(), p.id)
 	}

libpod/runtime_ctr.go

@@ -4,6 +4,7 @@ import (
 	"github.com/containers/storage"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // Contains the public Runtime API for containers
@@ -18,7 +19,7 @@ type CtrCreateOption func(*Container) error
 type ContainerFilter func(*Container) bool
 
 // NewContainer creates a new container from a given OCI config
-func (r *Runtime) NewContainer(spec *spec.Spec, options ...CtrCreateOption) (*Container, error) {
+func (r *Runtime) NewContainer(spec *spec.Spec, options ...CtrCreateOption) (ctr *Container, err error) {
 	r.lock.Lock()
 	defer r.lock.Unlock()
 
@@ -26,7 +27,7 @@ func (r *Runtime) NewContainer(spec *spec.Spec, options ...CtrCreateOption) (*Co
 		return nil, ErrRuntimeStopped
 	}
 
-	ctr, err := newContainer(spec)
+	ctr, err = newContainer(spec)
 	if err != nil {
 		return nil, err
 	}
@@ -41,19 +42,33 @@ func (r *Runtime) NewContainer(spec *spec.Spec, options ...CtrCreateOption) (*Co
 	ctr.state.State = ContainerStateConfigured
 	ctr.runtime = r
 
+	// Set up storage for the container
 	if err := ctr.setupStorage(); err != nil {
 		return nil, errors.Wrapf(err, "error configuring storage for container")
 	}
-	// TODO: once teardownStorage is implemented, do a defer here that tears down storage is AddContainer fails
+	defer func() {
-
+		if err != nil {
-	if err := r.state.AddContainer(ctr); err != nil {
+			if err2 := ctr.teardownStorage(); err2 != nil {
-		// If we joined a pod, remove ourself from it
+				logrus.Errorf("Error removing partially-created container root filesystem: %s", err2)
-		if ctr.pod != nil {
-			if err2 := ctr.pod.removeContainer(ctr); err2 != nil {
-				return nil, errors.Wrapf(err, "error adding new container to state, container could not be removed from pod %s", ctr.pod.ID())
 			}
 		}
+	}()
 
+	// If the container is in a pod, add it to the pod
+	if ctr.pod != nil {
+		if err := ctr.pod.addContainer(ctr); err != nil {
+			return nil, errors.Wrapf(err, "error adding new container to pod %s", ctr.pod.ID())
+		}
+	}
+	defer func() {
+		if err != nil {
+			if err2 := ctr.pod.removeContainer(ctr); err2 != nil {
+				logrus.Errorf("Error removing partially-created container from pod %s: %s", ctr.pod.ID(), err2)
+			}
+		}
+	}()
+
+	if err := r.state.AddContainer(ctr); err != nil {
 		// TODO: Might be worth making an effort to detect duplicate IDs
 		// We can recover from that by generating a new ID for the
 		// container