From 65d643caeb31364b59612f0f91be90894c65d703 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 13 Dec 2017 23:12:32 -0500 Subject: Change handling for pods in libpod state Add new functions to update pods and add/remove containers from them Use these new functions in place of manually modifying pods Signed-off-by: Matthew Heon Closes: #229 Approved by: rhatdan --- libpod/container.go | 1 - libpod/in_memory_state.go | 96 ++++++++++++++++++++++++++++++++++++++++------- libpod/options.go | 1 - libpod/runtime_ctr.go | 62 ++++++++++++++++-------------- libpod/sql_state.go | 15 ++++++++ libpod/state.go | 12 ++++-- 6 files changed, 141 insertions(+), 46 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index 0731babb5..95c7eecb4 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -114,7 +114,6 @@ func (ns LinuxNS) String() string { type Container struct { config *ContainerConfig - pod *Pod runningSpec *spec.Spec state *containerRuntimeInfo diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index 93c61537f..244a1ab25 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -104,8 +104,7 @@ func (s *InMemoryState) HasContainer(id string) (bool, error) { } // AddContainer adds a container to the state -// If the container belongs to a pod, the pod must already be present when the -// container is added, and the container must be present in the pod +// Containers in a pod cannot be added to the state func (s *InMemoryState) AddContainer(ctr *Container) error { if !ctr.valid { return errors.Wrapf(ErrCtrRemoved, "container with ID %s is not valid", ctr.ID()) @@ -116,17 +115,8 @@ func (s *InMemoryState) AddContainer(ctr *Container) error { return errors.Wrapf(ErrCtrExists, "container with ID %s already exists in state", ctr.ID()) } - if ctr.pod != nil { - if _, ok := s.pods[ctr.pod.ID()]; !ok { - return errors.Wrapf(ErrNoSuchPod, "pod %s does not exist, cannot add container %s", ctr.pod.ID(), ctr.ID()) - } - - hasCtr, err := ctr.pod.HasContainer(ctr.ID()) - if err != nil { - return errors.Wrapf(err, "error checking if container %s is present in pod %s", ctr.ID(), ctr.pod.ID()) - } else if !hasCtr { - return errors.Wrapf(ErrNoSuchCtr, "container %s is not present in pod %s", ctr.ID(), ctr.pod.ID()) - } + if ctr.config.Pod != "" { + return errors.Wrapf(ErrInvalidArg, "cannot add a container that is in a pod with AddContainer, use AddContainerToPod") } if err := s.ctrNameIndex.Reserve(ctr.Name(), ctr.ID()); err != nil { @@ -346,6 +336,86 @@ func (s *InMemoryState) RemovePod(pod *Pod) error { return nil } +// UpdatePod updates a pod's state from the backing database +// As in-memory states have no database this is a no-op +func (s *InMemoryState) UpdatePod(pod *Pod) error { + return nil +} + +// AddContainerToPod adds a container to the given pod, also adding it to the +// state +func (s *InMemoryState) AddContainerToPod(pod *Pod, ctr *Container) error { + if !pod.valid { + return errors.Wrapf(ErrPodRemoved, "pod %s is not valid and cannot be added to", pod.ID()) + } else if !ctr.valid { + return errors.Wrapf(ErrCtrRemoved, "container %s is not valid and cannot be added to the pod", ctr.ID()) + } + + if ctr.config.Pod != pod.ID() { + return errors.Wrapf(ErrInvalidArg, "container %s is not in pod %s", ctr.ID(), pod.ID()) + } + + // Add container to pod + if err := pod.addContainer(ctr); err != nil { + return err + } + + // Add container to state + _, ok := s.containers[ctr.ID()] + if ok { + return errors.Wrapf(ErrCtrExists, "container with ID %s already exists in state", ctr.ID()) + } + + if err := s.ctrNameIndex.Reserve(ctr.Name(), ctr.ID()); err != nil { + return errors.Wrapf(err, "error registering container name %s", ctr.Name()) + } + + if err := s.ctrIDIndex.Add(ctr.ID()); err != nil { + s.ctrNameIndex.Release(ctr.Name()) + return errors.Wrapf(err, "error registering container ID %s", ctr.ID()) + } + + s.containers[ctr.ID()] = ctr + + return nil +} + +// RemoveContainerFromPod removes the given container from the given pod +// The container is also removed from the state +func (s *InMemoryState) RemoveContainerFromPod(pod *Pod, ctr *Container) error { + if !pod.valid { + return errors.Wrapf(ErrPodRemoved, "pod %s is not valid and containers cannot be removed", pod.ID()) + } else if !ctr.valid { + return errors.Wrapf(ErrCtrRemoved, "container %s is not valid and cannot be removed from the pod", ctr.ID()) + } + + // Is the container in the pod? + exists, err := pod.HasContainer(ctr.ID()) + if err != nil { + return errors.Wrapf(err, "error checking for container %s in pod %s", ctr.ID(), pod.ID()) + } else if !exists { + return errors.Wrapf(ErrNoSuchCtr, "no container %s in pod %s", ctr.ID(), pod.ID()) + } + + // Remove container from pod + if err := pod.removeContainer(ctr); err != nil { + return err + } + + // Remove container from state + if _, ok := s.containers[ctr.ID()]; !ok { + return errors.Wrapf(ErrNoSuchCtr, "no container exists in state with ID %s", ctr.ID()) + } + + if err := s.ctrIDIndex.Delete(ctr.ID()); err != nil { + return errors.Wrapf(err, "error removing container ID from index") + } + delete(s.containers, ctr.ID()) + s.ctrNameIndex.Release(ctr.Name()) + + return nil +} + // AllPods retrieves all pods currently in the state func (s *InMemoryState) AllPods() ([]*Pod, error) { pods := make([]*Pod, 0, len(s.pods)) diff --git a/libpod/options.go b/libpod/options.go index a62a13053..4890c71ff 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -334,7 +334,6 @@ func (r *Runtime) WithPod(pod *Pod) CtrCreateOption { } ctr.config.Pod = pod.ID() - ctr.pod = pod return nil } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index c1078e504..bc1b6bc2f 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -72,25 +72,22 @@ func (r *Runtime) NewContainer(rSpec *spec.Spec, options ...CtrCreateOption) (c ctr.config.Mounts = append(ctr.config.Mounts, ctr.config.ShmDir) } - // 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 && ctr.pod != nil { - if err2 := ctr.pod.removeContainer(ctr); err2 != nil { - logrus.Errorf("Error removing partially-created container from pod %s: %s", ctr.pod.ID(), err2) - } + // Add the container to the state + // TODO: May be worth looking into recovering from name/ID collisions here + if ctr.config.Pod != "" { + // Get the pod from state + pod, err := r.state.Pod(ctr.config.Pod) + if err != nil { + return nil, errors.Wrapf(err, "cannot add container %s to pod %s", ctr.ID(), ctr.config.Pod) } - }() - if err := r.state.AddContainer(ctr); err != nil { - // TODO: Might be worth making an effort to detect duplicate IDs and names - // We can recover from that by generating a new ID for the - // container - return nil, errors.Wrapf(err, "error adding new container to state") + if err := r.state.AddContainerToPod(pod, ctr); err != nil { + return nil, errors.Wrapf(err, "error adding new container to state") + } + } else { + if err := r.state.AddContainer(ctr); err != nil { + return nil, errors.Wrapf(err, "error adding new container to state") + } } return ctr, nil @@ -161,25 +158,34 @@ func (r *Runtime) removeContainer(c *Container, force bool) error { return err } - if err := r.state.RemoveContainer(c); err != nil { - return errors.Wrapf(err, "error removing container from state") + // Remove the container from the state + if c.config.Pod != "" { + pod, err := r.state.Pod(c.config.Pod) + if err != nil { + return errors.Wrapf(err, "container %s is in pod %s, but pod cannot be retrieved", c.ID(), pod.ID()) + } + + if err := r.state.RemoveContainerFromPod(pod, c); err != nil { + return errors.Wrapf(err, "error removing container %s from state", c.ID()) + } + } else { + if err := r.state.RemoveContainer(c); err != nil { + return errors.Wrapf(err, "error removing container from state") + } } // Delete the container - if err := r.ociRuntime.deleteContainer(c); err != nil { - return errors.Wrapf(err, "error removing container %s from runc", c.ID()) + // Only do this if we're not ContainerStateConfigured - if we are, + // we haven't been created in runc yet + if c.state.State == ContainerStateConfigured { + if err := r.ociRuntime.deleteContainer(c); err != nil { + return errors.Wrapf(err, "error removing container %s from runc", c.ID()) + } } // Set container as invalid so it can no longer be used c.valid = false - // Remove container from pod, if it joined one - if c.pod != nil { - if err := c.pod.removeContainer(c); err != nil { - return errors.Wrapf(err, "error removing container from pod %s", c.pod.ID()) - } - } - return nil } diff --git a/libpod/sql_state.go b/libpod/sql_state.go index f67fc0e2c..3bdca0e63 100644 --- a/libpod/sql_state.go +++ b/libpod/sql_state.go @@ -853,6 +853,21 @@ func (s *SQLState) RemovePod(pod *Pod) error { return ErrNotImplemented } +// UpdatePod updates a pod from the database +func (s *SQLState) UpdatePod(pod *Pod) error { + return ErrNotImplemented +} + +// AddContainerToPod adds a container to the given pod +func (s *SQLState) AddContainerToPod(pod *Pod, ctr *Container) error { + return ErrNotImplemented +} + +// RemoveContainerFromPods removes a container from the given pod +func (s *SQLState) RemoveContainerFromPod(pod *Pod, ctr *Container) error { + return ErrNotImplemented +} + // AllPods retrieves all pods presently in the state func (s *SQLState) AllPods() ([]*Pod, error) { return nil, ErrNotImplemented diff --git a/libpod/state.go b/libpod/state.go index 95012e586..01ae58bd1 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -16,9 +16,7 @@ type State interface { // Checks if a container with the given ID is present in the state HasContainer(id string) (bool, error) // Adds container to state - // If the container belongs to a pod, that pod must already be present - // in the state when the container is added, and the container must be - // present in the pod + // The container cannot be part of a pod AddContainer(ctr *Container) error // Removes container from state // The container will only be removed from the state, not from the pod @@ -53,6 +51,14 @@ type State interface { // Containers within a pod will not be removed from the state, and will // not be changed to remove them from the now-removed pod RemovePod(pod *Pod) error + // UpdatePod updates a pod's state from the backing store + UpdatePod(pod *Pod) error + // AddContainerToPod adds a container to an existing pod + // The container given will be added to the state and the pod + AddContainerToPod(pod *Pod, ctr *Container) error + // RemoveContainerFromPod removes a container from an existing pod + // The container will also be removed from the state + RemoveContainerFromPod(pod *Pod, ctr *Container) error // Retrieves all pods presently in state AllPods() ([]*Pod, error) } -- cgit v1.2.3-54-g00ecf