From 9b563f7970c369cc89f52c1a5e519dda2e2404d0 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 25 Oct 2017 11:47:00 -0400 Subject: [PATCH] Update libpod logic for placing containers in pods Signed-off-by: Matthew Heon --- libpod/container.go | 40 +++++++++++++++++++++++++++++++++++++++- libpod/options.go | 20 +++++--------------- libpod/pod.go | 6 ++++++ libpod/runtime_ctr.go | 33 ++++++++++++++++++++++++--------- 4 files changed, 74 insertions(+), 25 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index f5c446d8..6b3e1484 100644 --- a/libpod/container.go +++ b/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() diff --git a/libpod/options.go b/libpod/options.go index a5305d7a..3d424bb7 100644 --- a/libpod/options.go +++ b/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 { - return fmt.Errorf("container has already been added to a pod") - } - - 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") + if pod == nil { + return ErrInvalidArg } + ctr.config.Pod = pod.ID() ctr.pod = pod return nil diff --git a/libpod/pod.go b/libpod/pod.go index 46b78909..48a761d5 100644 --- a/libpod/pod.go +++ b/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) } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 91e8adea..45990d2d 100644 --- a/libpod/runtime_ctr.go +++ b/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 - - if err := r.state.AddContainer(ctr); err != nil { - // If we joined a pod, remove ourself from it - 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()) + defer func() { + if err != nil { + if err2 := ctr.teardownStorage(); err2 != nil { + logrus.Errorf("Error removing partially-created container root filesystem: %s", err2) } } + }() + // 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