Merge pull request #2774 from mheon/db_rework_named_volume

Rework named volumes in DB

2 files changed, 112 insertions, 55 deletions

diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go
index a433fc16d..e71d9d3db 100644
--- a/pkg/spec/createconfig.go
+++ b/pkg/spec/createconfig.go
@@ -1,7 +1,6 @@
 package createconfig
 
 import (
-	"encoding/json"
 	"fmt"
 	"net"
 	"os"
@@ -23,18 +22,16 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-type mountType string
-
 // Type constants
 const (
 	bps = iota
 	iops
 	// TypeBind is the type for mounting host dir
-	TypeBind mountType = "bind"
+	TypeBind = "bind"
 	// TypeVolume is the type for remote storage volumes
-	// TypeVolume mountType = "volume"  // re-enable upon use
+	// TypeVolume = "volume"  // re-enable upon use
 	// TypeTmpfs is the type for mounting tmpfs
-	TypeTmpfs mountType = "tmpfs"
+	TypeTmpfs = "tmpfs"
 )
 
 // CreateResourceConfig represents resource elements in CreateConfig
@@ -130,15 +127,15 @@ type CreateConfig struct {
 	Mounts             []spec.Mount          //mounts
 	Volumes            []string              //volume
 	VolumesFrom        []string
-	WorkDir            string   //workdir
-	LabelOpts          []string //SecurityOpts
-	NoNewPrivs         bool     //SecurityOpts
-	ApparmorProfile    string   //SecurityOpts
-	SeccompProfilePath string   //SecurityOpts
+	NamedVolumes       []*libpod.ContainerNamedVolume // Filled in by CreateConfigToOCISpec
+	WorkDir            string                         //workdir
+	LabelOpts          []string                       //SecurityOpts
+	NoNewPrivs         bool                           //SecurityOpts
+	ApparmorProfile    string                         //SecurityOpts
+	SeccompProfilePath string                         //SecurityOpts
 	SecurityOpts       []string
 	Rootfs             string
-	LocalVolumes       []spec.Mount //Keeps track of the built-in volumes of container used in the --volumes-from flag
-	Syslog             bool         // Whether to enable syslog on exit commands
+	Syslog             bool // Whether to enable syslog on exit commands
 }
 
 func u32Ptr(i int64) *uint32     { u := uint32(i); return &u }
@@ -172,9 +169,9 @@ func (c *CreateConfig) AddContainerInitBinary(path string) error {
 	c.Command = append([]string{"/dev/init", "--"}, c.Command...)
 	c.Mounts = append(c.Mounts, spec.Mount{
 		Destination: "/dev/init",
-		Type:        "bind",
+		Type:        TypeBind,
 		Source:      path,
-		Options:     []string{"bind", "ro"},
+		Options:     []string{TypeBind, "ro"},
 	})
 	return nil
 }
@@ -217,9 +214,9 @@ func (c *CreateConfig) initFSMounts() []spec.Mount {
 	return mounts
 }
 
-//GetVolumeMounts takes user provided input for bind mounts and creates Mount structs
+// GetVolumeMounts takes user provided input for bind mounts and creates Mount structs
 func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, error) {
-	m := c.LocalVolumes
+	m := []spec.Mount{}
 	for _, i := range c.Volumes {
 		var options []string
 		spliti := strings.Split(i, ":")
@@ -255,9 +252,11 @@ func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, e
 			mount.Source = "tmpfs"
 			mount.Options = append(mount.Options, "tmpcopyup")
 		} else {
+			// TODO: Move support for this and tmpfs into libpod
+			// Should tmpfs also be handled as named volumes? Wouldn't be hard
 			// This will cause a new local Volume to be created on your system
 			mount.Source = stringid.GenerateNonCryptoID()
-			mount.Options = append(mount.Options, "bind")
+			mount.Options = append(mount.Options, TypeBind)
 		}
 		m = append(m, mount)
 	}
@@ -268,13 +267,12 @@ func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, e
 // GetVolumesFrom reads the create-config artifact of the container to get volumes from
 // and adds it to c.Volumes of the current container.
 func (c *CreateConfig) GetVolumesFrom() error {
-	var options string
-
 	if os.Geteuid() != 0 {
 		return nil
 	}
 
 	for _, vol := range c.VolumesFrom {
+		options := ""
 		splitVol := strings.SplitN(vol, ":", 2)
 		if len(splitVol) == 2 {
 			options = splitVol[1]
@@ -283,41 +281,60 @@ func (c *CreateConfig) GetVolumesFrom() error {
 		if err != nil {
 			return errors.Wrapf(err, "error looking up container %q", splitVol[0])
 		}
-		inspect, err := ctr.Inspect(false)
-		if err != nil {
-			return errors.Wrapf(err, "error inspecting %q", splitVol[0])
-		}
-		var createArtifact CreateConfig
-		artifact, err := ctr.GetArtifact("create-config")
-		if err != nil {
-			return errors.Wrapf(err, "error getting create-config artifact for %q", splitVol[0])
+
+		logrus.Debugf("Adding volumes from container %s", ctr.ID())
+
+		// Look up the container's user volumes. This gets us the
+		// destinations of all mounts the user added to the container.
+		userVolumesArr := ctr.UserVolumes()
+
+		// We're going to need to access them a lot, so convert to a map
+		// to reduce looping.
+		// We'll also use the map to indicate if we missed any volumes along the way.
+		userVolumes := make(map[string]bool)
+		for _, dest := range userVolumesArr {
+			userVolumes[dest] = false
 		}
-		if err := json.Unmarshal(artifact, &createArtifact); err != nil {
-			return err
+
+		// Now we get the container's spec and loop through its volumes
+		// and append them in if we can find them.
+		spec := ctr.Spec()
+		if spec == nil {
+			return errors.Errorf("error retrieving container %s spec", ctr.ID())
 		}
-		for key := range createArtifact.BuiltinImgVolumes {
-			for _, m := range inspect.Mounts {
-				if m.Destination == key {
-					c.LocalVolumes = append(c.LocalVolumes, m)
-					break
+		for _, mnt := range spec.Mounts {
+			if mnt.Type != TypeBind {
+				continue
+			}
+			if _, exists := userVolumes[mnt.Destination]; exists {
+				userVolumes[mnt.Destination] = true
+				localOptions := options
+				if localOptions == "" {
+					localOptions = strings.Join(mnt.Options, ",")
 				}
+				c.Volumes = append(c.Volumes, fmt.Sprintf("%s:%s:%s", mnt.Source, mnt.Destination, localOptions))
 			}
 		}
 
-		for _, i := range createArtifact.Volumes {
-			// Volumes format is host-dir:ctr-dir[:options], so get the host and ctr dir
-			// and add on the options given by the user to the flag.
-			spliti := strings.SplitN(i, ":", 3)
-			// Throw error if mounting volume from container with Z option (private label)
-			// Override this by adding 'z' to options.
-			if len(spliti) > 2 && strings.Contains(spliti[2], "Z") && !strings.Contains(options, "z") {
-				return errors.Errorf("volume mounted with private option 'Z' in %q. Use option 'z' to mount in current container", ctr.ID())
+		// We're done with the spec mounts. Add named volumes.
+		// Add these unconditionally - none of them are automatically
+		// part of the container, as some spec mounts are.
+		namedVolumes := ctr.NamedVolumes()
+		for _, namedVol := range namedVolumes {
+			if _, exists := userVolumes[namedVol.Dest]; exists {
+				userVolumes[namedVol.Dest] = true
 			}
-			if options == "" {
-				// Mount the volumes with the default options
-				c.Volumes = append(c.Volumes, createArtifact.Volumes...)
-			} else {
-				c.Volumes = append(c.Volumes, spliti[0]+":"+spliti[1]+":"+options)
+			localOptions := options
+			if localOptions == "" {
+				localOptions = strings.Join(namedVol.Options, ",")
+			}
+			c.Volumes = append(c.Volumes, fmt.Sprintf("%s:%s:%s", namedVol.Name, namedVol.Dest, localOptions))
+		}
+
+		// Check if we missed any volumes
+		for volDest, found := range userVolumes {
+			if !found {
+				logrus.Warnf("Unable to match volume %s from container %s for volumes-from", volDest, ctr.ID())
 			}
 		}
 	}
@@ -417,14 +434,20 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 		// others, if they are included
 		volumes := make([]string, 0, len(c.Volumes))
 		for _, vol := range c.Volumes {
-			volumes = append(volumes, strings.SplitN(vol, ":", 2)[0])
+			// We always want the volume destination
+			splitVol := strings.SplitN(vol, ":", 3)
+			if len(splitVol) > 1 {
+				volumes = append(volumes, splitVol[1])
+			} else {
+				volumes = append(volumes, splitVol[0])
+			}
 		}
 
 		options = append(options, libpod.WithUserVolumes(volumes))
 	}
 
-	if len(c.LocalVolumes) != 0 {
-		options = append(options, libpod.WithLocalVolumes(c.LocalVolumes))
+	if len(c.NamedVolumes) != 0 {
+		options = append(options, libpod.WithNamedVolumes(c.NamedVolumes))
 	}
 
 	if len(c.Command) != 0 {
@@ -538,7 +561,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 
 	options = append(options, libpod.WithPrivileged(c.Privileged))
 
-	useImageVolumes := c.ImageVolumeType == "bind"
+	useImageVolumes := c.ImageVolumeType == TypeBind
 	// Gather up the options for NewContainer which consist of With... funcs
 	options = append(options, libpod.WithRootFSFromImage(c.ImageID, c.Image, useImageVolumes))
 	options = append(options, libpod.WithSecLabels(c.LabelOpts))
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
index a61741f73..9b6bd089e 100644
--- a/pkg/spec/spec.go
+++ b/pkg/spec/spec.go
@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/storage/pkg/mount"
 	pmount "github.com/containers/storage/pkg/mount"
@@ -48,6 +49,33 @@ func supercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.M
 	return configMount
 }
 
+// Split named volumes from normal volumes
+func splitNamedVolumes(mounts []spec.Mount) ([]spec.Mount, []*libpod.ContainerNamedVolume) {
+	newMounts := make([]spec.Mount, 0)
+	namedVolumes := make([]*libpod.ContainerNamedVolume, 0)
+	for _, mount := range mounts {
+		// If it's not a named volume, append unconditionally
+		if mount.Type != TypeBind {
+			newMounts = append(newMounts, mount)
+			continue
+		}
+		// Volumes that are not named volumes must be an absolute or
+		// relative path.
+		// Volume names may not begin with a non-alphanumeric character
+		// so the HasPrefix() check is safe here.
+		if strings.HasPrefix(mount.Source, "/") || strings.HasPrefix(mount.Source, ".") {
+			newMounts = append(newMounts, mount)
+		} else {
+			namedVolume := new(libpod.ContainerNamedVolume)
+			namedVolume.Name = mount.Source
+			namedVolume.Dest = mount.Destination
+			namedVolume.Options = mount.Options
+			namedVolumes = append(namedVolumes, namedVolume)
+		}
+	}
+	return newMounts, namedVolumes
+}
+
 func getAvailableGids() (int64, error) {
 	idMap, err := user.ParseIDMapFile("/proc/self/gid_map")
 	if err != nil {
@@ -99,7 +127,7 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 		}
 		sysMnt := spec.Mount{
 			Destination: "/sys",
-			Type:        "bind",
+			Type:        TypeBind,
 			Source:      "/sys",
 			Options:     []string{"rprivate", "nosuid", "noexec", "nodev", r, "rbind"},
 		}
@@ -126,7 +154,7 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 		g.RemoveMount("/dev/mqueue")
 		devMqueue := spec.Mount{
 			Destination: "/dev/mqueue",
-			Type:        "bind",
+			Type:        TypeBind,
 			Source:      "/dev/mqueue",
 			Options:     []string{"bind", "nosuid", "noexec", "nodev"},
 		}
@@ -136,7 +164,7 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 		g.RemoveMount("/proc")
 		procMount := spec.Mount{
 			Destination: "/proc",
-			Type:        "bind",
+			Type:        TypeBind,
 			Source:      "/proc",
 			Options:     []string{"rbind", "nosuid", "noexec", "nodev"},
 		}
@@ -377,6 +405,12 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 	configSpec.Mounts = supercedeUserMounts(volumeMounts, configSpec.Mounts)
 	//--mount
 	configSpec.Mounts = supercedeUserMounts(config.initFSMounts(), configSpec.Mounts)
+
+	// Split normal mounts and named volumes
+	newMounts, namedVolumes := splitNamedVolumes(configSpec.Mounts)
+	configSpec.Mounts = newMounts
+	config.NamedVolumes = namedVolumes
+
 	// BLOCK IO
 	blkio, err := config.CreateBlockIO()
 	if err != nil {