Fix volume handling in podman

iFix builtin volumes to work with podman volume

Currently builtin volumes are not recored in podman volumes when
they are created automatically. This patch fixes this.

Remove container volumes when requested

Currently the --volume option on podman remove does nothing.
This will implement the changes needed to remove the volumes
if the user requests it.

When removing a volume make sure that no container uses the volume.

Signed-off-by: Daniel J Walsh dwalsh@redhat.com
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

4 files changed, 47 insertions, 20 deletions

diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go
index 8edab831f..8da44a2f0 100644
--- a/pkg/spec/createconfig.go
+++ b/pkg/spec/createconfig.go
@@ -11,7 +11,9 @@ import (
 
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/namespaces"
+	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/storage"
+	"github.com/containers/storage/pkg/stringid"
 	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/docker/go-connections/nat"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
@@ -133,8 +135,8 @@ type CreateConfig struct {
 	SeccompProfilePath string   //SecurityOpts
 	SecurityOpts       []string
 	Rootfs             string
-	LocalVolumes       []string //Keeps track of the built-in volumes of container used in the --volumes-from flag
-	Syslog             bool     // Whether to enable syslog on exit commands
+	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
 }
 
 func u32Ptr(i int64) *uint32     { u := uint32(i); return &u }
@@ -215,7 +217,7 @@ func (c *CreateConfig) initFSMounts() []spec.Mount {
 
 //GetVolumeMounts takes user provided input for bind mounts and creates Mount structs
 func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, error) {
-	var m []spec.Mount
+	m := c.LocalVolumes
 	for _, i := range c.Volumes {
 		var options []string
 		spliti := strings.Split(i, ":")
@@ -233,22 +235,31 @@ func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, e
 		logrus.Debugf("User mount %s:%s options %v", spliti[0], spliti[1], options)
 	}
 
-	// volumes from image config
-	if c.ImageVolumeType != "tmpfs" {
+	if c.ImageVolumeType == "ignore" {
 		return m, nil
 	}
+
 	for vol := range c.BuiltinImgVolumes {
 		if libpod.MountExists(specMounts, vol) {
 			continue
 		}
+
 		mount := spec.Mount{
 			Destination: vol,
-			Type:        string(TypeTmpfs),
-			Source:      string(TypeTmpfs),
-			Options:     []string{"rprivate", "rw", "noexec", "nosuid", "nodev", "tmpcopyup"},
+			Type:        c.ImageVolumeType,
+			Options:     []string{"rprivate", "rw", "nodev"},
+		}
+		if c.ImageVolumeType == "tmpfs" {
+			mount.Source = "tmpfs"
+			mount.Options = append(mount.Options, "tmpcopyup")
+		} else {
+			// This will cause a new local Volume to be created on your system
+			mount.Source = stringid.GenerateNonCryptoID()
+			mount.Options = append(mount.Options, "bind")
 		}
 		m = append(m, mount)
 	}
+
 	return m, nil
 }
 
@@ -256,6 +267,11 @@ func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, e
 // and adds it to c.Volumes of the current container.
 func (c *CreateConfig) GetVolumesFrom() error {
 	var options string
+
+	if rootless.SkipStorageSetup() {
+		return nil
+	}
+
 	for _, vol := range c.VolumesFrom {
 		splitVol := strings.SplitN(vol, ":", 2)
 		if len(splitVol) == 2 {
@@ -265,6 +281,10 @@ 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 {
@@ -273,9 +293,13 @@ func (c *CreateConfig) GetVolumesFrom() error {
 		if err := json.Unmarshal(artifact, &createArtifact); err != nil {
 			return err
 		}
-
 		for key := range createArtifact.BuiltinImgVolumes {
-			c.LocalVolumes = append(c.LocalVolumes, key)
+			for _, m := range inspect.Mounts {
+				if m.Destination == key {
+					c.LocalVolumes = append(c.LocalVolumes, m)
+					break
+				}
+			}
 		}
 
 		for _, i := range createArtifact.Volumes {
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 52f431881..db8a3d5bb 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -259,8 +259,8 @@ func GetRootlessStorageOpts() (storage.StoreOptions, error) {
 	return opts, nil
 }
 
-// GetRootlessVolumeInfo returns where all the name volumes will be created in rootless mode
-func GetRootlessVolumeInfo() (string, error) {
+// GetRootlessVolumePath returns where all the name volumes will be created in rootless mode
+func GetRootlessVolumePath() (string, error) {
 	dataDir, _, err := GetRootlessDirInfo()
 	if err != nil {
 		return "", err
@@ -307,15 +307,13 @@ func GetDefaultStoreOptions() (storage.StoreOptions, string, error) {
 		err                      error
 	)
 	storageOpts := storage.DefaultStoreOptions
-	volumePath := "/var/lib/containers/storage"
-
+	volumePath := filepath.Join(storageOpts.GraphRoot, "volumes")
 	if rootless.IsRootless() {
 		storageOpts, err = GetRootlessStorageOpts()
 		if err != nil {
 			return storageOpts, volumePath, err
 		}
-
-		volumePath, err = GetRootlessVolumeInfo()
+		volumePath, err = GetRootlessVolumePath()
 		if err != nil {
 			return storageOpts, volumePath, err
 		}
diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go
index 2b2832838..8a52efa61 100644
--- a/pkg/varlinkapi/containers.go
+++ b/pkg/varlinkapi/containers.go
@@ -358,13 +358,13 @@ func (i *LibpodAPI) WaitContainer(call iopodman.VarlinkCall, name string) error
 }
 
 // RemoveContainer ...
-func (i *LibpodAPI) RemoveContainer(call iopodman.VarlinkCall, name string, force bool) error {
+func (i *LibpodAPI) RemoveContainer(call iopodman.VarlinkCall, name string, force bool, removeVolumes bool) error {
 	ctx := getContext()
 	ctr, err := i.Runtime.LookupContainer(name)
 	if err != nil {
 		return call.ReplyContainerNotFound(name)
 	}
-	if err := i.Runtime.RemoveContainer(ctx, ctr, force); err != nil {
+	if err := i.Runtime.RemoveContainer(ctx, ctr, force, removeVolumes); err != nil {
 		return call.ReplyErrorOccurred(err.Error())
 	}
 	return call.ReplyRemoveContainer(ctr.ID())
@@ -385,7 +385,7 @@ func (i *LibpodAPI) DeleteStoppedContainers(call iopodman.VarlinkCall) error {
 			return call.ReplyErrorOccurred(err.Error())
 		}
 		if state != libpod.ContainerStateRunning {
-			if err := i.Runtime.RemoveContainer(ctx, ctr, false); err != nil {
+			if err := i.Runtime.RemoveContainer(ctx, ctr, false, false); err != nil {
 				return call.ReplyErrorOccurred(err.Error())
 			}
 			deletedContainers = append(deletedContainers, ctr.ID())
diff --git a/pkg/varlinkapi/containers_create.go b/pkg/varlinkapi/containers_create.go
index f1835a189..6b53b22c6 100644
--- a/pkg/varlinkapi/containers_create.go
+++ b/pkg/varlinkapi/containers_create.go
@@ -131,9 +131,14 @@ func varlinkCreateToCreateConfig(ctx context.Context, create iopodman.Create, ru
 	}
 
 	imageID := data.ID
+	var ImageVolumes map[string]struct{}
+	if data != nil && create.Image_volume_type != "ignore" {
+		ImageVolumes = data.Config.Volumes
+	}
+
 	config := &cc.CreateConfig{
 		Runtime:           runtime,
-		BuiltinImgVolumes: data.Config.Volumes,
+		BuiltinImgVolumes: ImageVolumes,
 		ConmonPidFile:     create.Conmon_pidfile,
 		ImageVolumeType:   create.Image_volume_type,
 		CapAdd:            create.Cap_add,