opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-07-15 03:02:52 +08:00
Code Issues Packages Projects Releases Wiki Activity

Refactor dependency checks from init() into public API

Browse Source 
Instead of checking during init(), which could result in major
locking issues when used with pods, make our dependency checks in
the public API instead. This avoids doing them when we start pods
(where, because of the dependency graph, we can reasonably say
all dependencies are up before we start a container).

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #577
Approved by: rhatdan
This commit is contained in:
Matthew Heon
2018-04-02 12:23:19 -04:00
committed by Atomic Bot
parent 4d4646d09b
commit 98b19aeb0c
3 changed files with 90 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
libpod/container_api.go
View File

@ -5,6 +5,7 @@ import (
"io/ioutil"
"os"
"strconv"
"strings"
"time"
"github.com/docker/docker/daemon/caps"
@ -33,6 +34,15 @@ func (c *Container) Init() (err error) {
return errors.Wrapf(ErrCtrExists, "container %s has already been created in runtime", c.ID())
}
notRunning, err := c.checkDependenciesRunning()
if err != nil {
return errors.Wrapf(err, "error checking dependencies for container %s")
}
if len(notRunning) > 0 {
depString := strings.Join(notRunning, ",")
return errors.Wrapf(ErrCtrStateInvalid, "some dependencies of container %s are not started: %s", c.ID(), depString)
}
if err := c.prepare(); err != nil {
return err
}
@ -70,6 +80,15 @@ func (c *Container) Start() (err error) {
return errors.Wrapf(ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID())
}
notRunning, err := c.checkDependenciesRunning()
if err != nil {
return errors.Wrapf(err, "error checking dependencies for container %s")
}
if len(notRunning) > 0 {
depString := strings.Join(notRunning, ",")
return errors.Wrapf(ErrCtrStateInvalid, "some dependencies of container %s are not started: %s", c.ID(), depString)
}
if err := c.prepare(); err != nil {
return err
}
@ -124,6 +143,15 @@ func (c *Container) StartAndAttach(noStdin bool, keys string) (attachResChan <-c
return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID())
}
notRunning, err := c.checkDependenciesRunning()
if err != nil {
return nil, errors.Wrapf(err, "error checking dependencies for container %s")
}
if len(notRunning) > 0 {
depString := strings.Join(notRunning, ",")
return nil, errors.Wrapf(ErrCtrStateInvalid, "some dependencies of container %s are not started: %s", c.ID(), depString)
}
if err := c.prepare(); err != nil {
return nil, err
}

80
libpod/container_internal.go
View File

@ -350,40 +350,72 @@ func (c *Container) save() error {
return nil
}
// Initialize a container, creating it in the runtime
func (c *Container) init() error {
// Check if a container's dependencies are running
// Returns a []string containing the IDs of dependencies that are not running
func (c *Container) checkDependenciesRunning() ([]string, error) {
deps := c.Dependencies()
notRunning := []string{}
// We were not passed a set of dependency containers
// Make it ourselves
depCtrs := make(map[string]*Container, len(deps))
// Ensure that dependencies are up before we begin
for _, dep := range deps {
// Get the dependency container
depCtr, err := c.runtime.state.Container(dep)
if err != nil {
return errors.Wrapf(err, "error retrieving dependency %s of container %s from state", dep, c.ID())
return nil, errors.Wrapf(err, "error retrieving dependency %s of container %s from state", dep, c.ID())
}
// Check the status
state, err := depCtr.State()
if err != nil {
return errors.Wrapf(err, "error retrieving state of dependency %s of container %s", dep, c.ID())
return nil, errors.Wrapf(err, "error retrieving state of dependency %s of container %s", dep, c.ID())
}
if state != ContainerStateRunning {
return errors.Wrapf(ErrCtrStateInvalid, "dependency %s of container %s is not running, cannot initialize container", dep, c.ID())
notRunning = append(notRunning, dep)
}
depCtrs[dep] = depCtr
// There is potential race here
// A dep container may stop before this container starts
// We can't really control this, but it probably can't cause
// anything insidious.
}
return notRunning, nil
}
// Check if a container's dependencies are running
// Returns a []string containing the IDs of dependencies that are not running
// Assumes depencies are already locked, and will be passed in
// Accepts a map[string]*Container containing, at a minimum, the locked
// dependency containers
// (This must be a map from container ID to container)
func (c *Container) checkDependenciesRunningLocked(depCtrs map[string]*Container) ([]string, error) {
deps := c.Dependencies()
notRunning := []string{}
for _, dep := range deps {
depCtr, ok := depCtrs[dep]
if !ok {
return nil, errors.Wrapf(ErrNoSuchCtr, "container %s depends on container %s but it is not on containers passed to checkDependenciesRunning", c.ID(), dep)
}
if err := c.syncContainer(); err != nil {
return nil, err
}
if depCtr.state.State != ContainerStateRunning {
notRunning = append(notRunning, dep)
}
}
return notRunning, nil
}
// Initialize a container, creating it in the runtime
func (c *Container) init() error {
if err := c.makeBindMounts(); err != nil {
return err
}
// Generate the OCI spec
spec, err := c.generateSpec(depCtrs)
spec, err := c.generateSpec()
if err != nil {
return err
}
@ -875,7 +907,7 @@ func (c *Container) generateHosts() (string, error) {
// Generate spec for a container
// Accepts a map of the container's dependencies
func (c *Container) generateSpec(deps map[string]*Container) (*spec.Spec, error) {
func (c *Container) generateSpec() (*spec.Spec, error) {
g := generate.NewFromSpec(c.config.Spec)
// If network namespace was requested, add it now
@ -921,37 +953,37 @@ func (c *Container) generateSpec(deps map[string]*Container) (*spec.Spec, error)
// Add shared namespaces from other containers
if c.config.IPCNsCtr != "" {
if err := c.addNamespaceContainer(&g, IPCNS, deps[c.config.IPCNsCtr], spec.IPCNamespace); err != nil {
if err := c.addNamespaceContainer(&g, IPCNS, c.config.IPCNsCtr, spec.IPCNamespace); err != nil {
return nil, err
}
}
if c.config.MountNsCtr != "" {
if err := c.addNamespaceContainer(&g, MountNS, deps[c.config.MountNsCtr], spec.MountNamespace); err != nil {
if err := c.addNamespaceContainer(&g, MountNS, c.config.MountNsCtr, spec.MountNamespace); err != nil {
return nil, err
}
}
if c.config.NetNsCtr != "" {
if err := c.addNamespaceContainer(&g, NetNS, deps[c.config.NetNsCtr], spec.NetworkNamespace); err != nil {
if err := c.addNamespaceContainer(&g, NetNS, c.config.NetNsCtr, spec.NetworkNamespace); err != nil {
return nil, err
}
}
if c.config.PIDNsCtr != "" {
if err := c.addNamespaceContainer(&g, PIDNS, deps[c.config.PIDNsCtr], string(spec.PIDNamespace)); err != nil {
if err := c.addNamespaceContainer(&g, PIDNS, c.config.PIDNsCtr, string(spec.PIDNamespace)); err != nil {
return nil, err
}
}
if c.config.UserNsCtr != "" {
if err := c.addNamespaceContainer(&g, UserNS, deps[c.config.UserNsCtr], spec.UserNamespace); err != nil {
if err := c.addNamespaceContainer(&g, UserNS, c.config.UserNsCtr, spec.UserNamespace); err != nil {
return nil, err
}
}
if c.config.UTSNsCtr != "" {
if err := c.addNamespaceContainer(&g, UTSNS, deps[c.config.UTSNsCtr], spec.UTSNamespace); err != nil {
if err := c.addNamespaceContainer(&g, UTSNS, c.config.UTSNsCtr, spec.UTSNamespace); err != nil {
return nil, err
}
}
if c.config.CgroupNsCtr != "" {
if err := c.addNamespaceContainer(&g, CgroupNS, deps[c.config.CgroupNsCtr], spec.CgroupNamespace); err != nil {
if err := c.addNamespaceContainer(&g, CgroupNS, c.config.CgroupNsCtr, spec.CgroupNamespace); err != nil {
return nil, err
}
}
@ -979,11 +1011,13 @@ func (c *Container) generateSpec(deps map[string]*Container) (*spec.Spec, error)
}
// Add an existing container's namespace to the spec
func (c *Container) addNamespaceContainer(g *generate.Generator, ns LinuxNS, nsCtr *Container, specNS string) error {
if nsCtr == nil {
return errors.Wrapf(ErrNoSuchCtr, "nil container passed to addNamespaceContainer")
func (c *Container) addNamespaceContainer(g *generate.Generator, ns LinuxNS, ctr string, specNS string) error {
nsCtr, err := c.runtime.state.Container(ctr)
if err != nil {
return errors.Wrapf(err, "error retrieving dependency %s of container %s from state", ctr, c.ID())
}
// TODO need unlocked version of this for use in pods
nsPath, err := nsCtr.NamespacePath(ns)
if err != nil {
return err

5
libpod/pod.go
View File

@ -165,6 +165,11 @@ func startNode(node *containerNode, setError bool, ctrErrors map[string]error, c
// Going to start the container, mark us as visited
ctrsVisited[node.id] = true
// TODO: Maybe have a checkDependenciesRunningLocked here?
// Graph traversal should ensure our deps have been started, but some
// might have stopped since?
// Potentially will hurt our perf, though
// Start the container (only if it is not running)
ctrErrored := false
if node.container.state.State != ContainerStateRunning {