Merge pull request #3896 from mheon/volume_lookup

Add ability to look up volumes by unambiguous partial name

14 files changed, 231 insertions, 60 deletions

diff --git a/API.md b/API.md
index 4e61b7c9f..b9f3bd3d5 100755
--- a/API.md
+++ b/API.md
@@ -177,7 +177,7 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in
 
 [func VolumeCreate(options: VolumeCreateOpts) string](#VolumeCreate)
 
-[func VolumeRemove(options: VolumeRemoveOpts) []string](#VolumeRemove)
+[func VolumeRemove(options: VolumeRemoveOpts) []string, map[string]](#VolumeRemove)
 
 [func VolumesPrune() []string, []string](#VolumesPrune)
 
@@ -1151,7 +1151,7 @@ VolumeCreate creates a volume on a remote host
 ### <a name="VolumeRemove"></a>func VolumeRemove
 <div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
 
-method VolumeRemove(options: [VolumeRemoveOpts](#VolumeRemoveOpts)) [[]string](#[]string)</div>
+method VolumeRemove(options: [VolumeRemoveOpts](#VolumeRemoveOpts)) [[]string](#[]string), [map[string]](#map[string])</div>
 VolumeRemove removes a volume on a remote host
 ### <a name="VolumesPrune"></a>func VolumesPrune
 <div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
diff --git a/cmd/podman/shared/volumes_shared.go b/cmd/podman/shared/volumes_shared.go
new file mode 100644
index 000000000..912615cad
--- /dev/null
+++ b/cmd/podman/shared/volumes_shared.go
@@ -0,0 +1,47 @@
+package shared
+
+import (
+	"context"
+
+	"github.com/containers/libpod/libpod"
+)
+
+// Remove given set of volumes
+func SharedRemoveVolumes(ctx context.Context, runtime *libpod.Runtime, vols []string, all, force bool) ([]string, map[string]error, error) {
+	var (
+		toRemove []*libpod.Volume
+		success  []string
+		failed   map[string]error
+	)
+
+	failed = make(map[string]error)
+
+	if all {
+		vols, err := runtime.Volumes()
+		if err != nil {
+			return nil, nil, err
+		}
+		toRemove = vols
+	} else {
+		for _, v := range vols {
+			vol, err := runtime.LookupVolume(v)
+			if err != nil {
+				failed[v] = err
+				continue
+			}
+			toRemove = append(toRemove, vol)
+		}
+	}
+
+	// We could parallelize this, but I haven't heard anyone complain about
+	// performance here yet, so hold off.
+	for _, vol := range toRemove {
+		if err := runtime.RemoveVolume(ctx, vol, force); err != nil {
+			failed[vol.Name()] = err
+			continue
+		}
+		success = append(success, vol.Name())
+	}
+
+	return success, failed, nil
+}
diff --git a/cmd/podman/varlink/io.podman.varlink b/cmd/podman/varlink/io.podman.varlink
index 752e28256..2e46b31ce 100644
--- a/cmd/podman/varlink/io.podman.varlink
+++ b/cmd/podman/varlink/io.podman.varlink
@@ -1212,7 +1212,7 @@ method ReceiveFile(path: string, delete: bool) -> (len: int)
 method VolumeCreate(options: VolumeCreateOpts) -> (volumeName: string)
 
 # VolumeRemove removes a volume on a remote host
-method VolumeRemove(options: VolumeRemoveOpts) -> (volumeNames: []string)
+method VolumeRemove(options: VolumeRemoveOpts) -> (successes: []string, failures: [string]string)
 
 # GetVolumes gets slice of the volumes on a remote host
 method GetVolumes(args: []string, all: bool) -> (volumes: []Volume)
diff --git a/cmd/podman/volume_rm.go b/cmd/podman/volume_rm.go
index 0141d06da..2fa6a8d03 100644
--- a/cmd/podman/volume_rm.go
+++ b/cmd/podman/volume_rm.go
@@ -1,8 +1,6 @@
 package main
 
 import (
-	"fmt"
-
 	"github.com/containers/libpod/cmd/podman/cliconfig"
 	"github.com/containers/libpod/pkg/adapter"
 	"github.com/pkg/errors"
@@ -52,19 +50,9 @@ func volumeRmCmd(c *cliconfig.VolumeRmValues) error {
 		return errors.Wrapf(err, "error creating libpod runtime")
 	}
 	defer runtime.DeferredShutdown(false)
-	deletedVolumeNames, err := runtime.RemoveVolumes(getContext(), c)
+	deletedVolumeNames, deletedVolumeErrors, err := runtime.RemoveVolumes(getContext(), c)
 	if err != nil {
-		if len(deletedVolumeNames) > 0 {
-			printDeleteVolumes(deletedVolumeNames)
-			return err
-		}
-	}
-	printDeleteVolumes(deletedVolumeNames)
-	return err
-}
-
-func printDeleteVolumes(volumes []string) {
-	for _, v := range volumes {
-		fmt.Println(v)
+		return err
 	}
+	return printCmdResults(deletedVolumeNames, deletedVolumeErrors)
 }
diff --git a/docs/podman-volume-inspect.1.md b/docs/podman-volume-inspect.1.md
index a6c99f6c8..ac5b6c977 100644
--- a/docs/podman-volume-inspect.1.md
+++ b/docs/podman-volume-inspect.1.md
@@ -11,6 +11,7 @@ podman\-volume\-inspect - Get detailed information on one or more volumes
 Display detailed information on one or more volumes. The output can be formatted using
 the **--format** flag and a Go template. To get detailed information about all the
 existing volumes, use the **--all** flag.
+Volumes can be queried individually by providing their full name or a unique partial name.
 
 
 ## OPTIONS
diff --git a/docs/podman-volume-rm.1.md b/docs/podman-volume-rm.1.md
index fe047e7da..9a2fe8c99 100644
--- a/docs/podman-volume-rm.1.md
+++ b/docs/podman-volume-rm.1.md
@@ -4,14 +4,14 @@
 podman\-volume\-rm - Remove one or more volumes
 
 ## SYNOPSIS
-**podman volume rm** [*options*]
+**podman volume rm** [*options*] *volume* [...]
 
 ## DESCRIPTION
 
-Removes one ore more volumes. Only volumes that are not being used will be removed.
+Removes one or more volumes. Only volumes that are not being used will be removed.
 If a volume is being used by a container, an error will be returned unless the **--force**
-flag is being used. To remove all the volumes, use the **--all** flag.
-
+flag is being used. To remove all volumes, use the **--all** flag.
+Volumes can be removed individually by providing their full name or a unique partial name.
 
 ## OPTIONS
 
diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go
index 31b551b36..4e7f78f13 100644
--- a/libpod/boltdb_state.go
+++ b/libpod/boltdb_state.go
@@ -1735,6 +1735,74 @@ func (s *BoltState) Volume(name string) (*Volume, error) {
 	return volume, nil
 }
 
+// LookupVolume locates a volume from a partial name.
+func (s *BoltState) LookupVolume(name string) (*Volume, error) {
+	if name == "" {
+		return nil, define.ErrEmptyID
+	}
+
+	if !s.valid {
+		return nil, define.ErrDBClosed
+	}
+
+	volName := []byte(name)
+
+	volume := new(Volume)
+	volume.config = new(VolumeConfig)
+
+	db, err := s.getDBCon()
+	if err != nil {
+		return nil, err
+	}
+	defer s.deferredCloseDBCon(db)
+
+	err = db.View(func(tx *bolt.Tx) error {
+		volBkt, err := getVolBucket(tx)
+		if err != nil {
+			return err
+		}
+
+		allVolsBkt, err := getAllVolsBucket(tx)
+		if err != nil {
+			return err
+		}
+
+		// Check for exact match on name
+		volDB := volBkt.Bucket(volName)
+		if volDB != nil {
+			return s.getVolumeFromDB(volName, volume, volBkt)
+		}
+
+		// No exact match. Search all names.
+		foundMatch := false
+		err = allVolsBkt.ForEach(func(checkName, checkName2 []byte) error {
+			if strings.HasPrefix(string(checkName), name) {
+				if foundMatch {
+					return errors.Wrapf(define.ErrVolumeExists, "more than one result for volume name %q", name)
+				}
+				foundMatch = true
+				volName = checkName
+			}
+			return nil
+		})
+		if err != nil {
+			return err
+		}
+
+		if !foundMatch {
+			return errors.Wrapf(define.ErrNoSuchVolume, "no volume with name %q found", name)
+		}
+
+		return s.getVolumeFromDB(volName, volume, volBkt)
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return volume, nil
+
+}
+
 // HasVolume returns true if the given volume exists in the state, otherwise it returns false
 func (s *BoltState) HasVolume(name string) (bool, error) {
 	if name == "" {
diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go
index 280ae5f5c..a008fcb39 100644
--- a/libpod/in_memory_state.go
+++ b/libpod/in_memory_state.go
@@ -459,6 +459,41 @@ func (s *InMemoryState) Volume(name string) (*Volume, error) {
 	return vol, nil
 }
 
+// LookupVolume finds a volume from an unambiguous partial ID.
+func (s *InMemoryState) LookupVolume(name string) (*Volume, error) {
+	if name == "" {
+		return nil, define.ErrEmptyID
+	}
+
+	vol, ok := s.volumes[name]
+	if ok {
+		return vol, nil
+	}
+
+	// Alright, we've failed to find by full name. Now comes the expensive
+	// part.
+	// Loop through all volumes and look for matches.
+	var (
+		foundMatch bool
+		candidate  *Volume
+	)
+	for volName, vol := range s.volumes {
+		if strings.HasPrefix(volName, name) {
+			if foundMatch {
+				return nil, errors.Wrapf(define.ErrVolumeExists, "more than one result for volume name %q", name)
+			}
+			candidate = vol
+			foundMatch = true
+		}
+	}
+
+	if !foundMatch {
+		return nil, errors.Wrapf(define.ErrNoSuchVolume, "no volume with name %q found", name)
+	}
+
+	return candidate, nil
+}
+
 // HasVolume checks if a volume with the given name is present in the state
 func (s *InMemoryState) HasVolume(name string) (bool, error) {
 	if name == "" {
diff --git a/libpod/runtime_volume.go b/libpod/runtime_volume.go
index 512e778a1..a6ab748e5 100644
--- a/libpod/runtime_volume.go
+++ b/libpod/runtime_volume.go
@@ -6,7 +6,6 @@ import (
 	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/libpod/events"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 // Contains the public Runtime API for volumes
@@ -43,40 +42,25 @@ func (r *Runtime) RemoveVolume(ctx context.Context, v *Volume, force bool) error
 	return r.removeVolume(ctx, v, force)
 }
 
-// RemoveVolumes removes a slice of volumes or all with a force bool
-func (r *Runtime) RemoveVolumes(ctx context.Context, volumes []string, all, force bool) ([]string, error) {
-	var (
-		vols        []*Volume
-		err         error
-		deletedVols []string
-	)
-	if all {
-		vols, err = r.Volumes()
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to get all volumes")
-		}
-	} else {
-		for _, i := range volumes {
-			vol, err := r.GetVolume(i)
-			if err != nil {
-				return nil, err
-			}
-			vols = append(vols, vol)
-		}
+// GetVolume retrieves a volume given its full name.
+func (r *Runtime) GetVolume(name string) (*Volume, error) {
+	r.lock.RLock()
+	defer r.lock.RUnlock()
+
+	if !r.valid {
+		return nil, define.ErrRuntimeStopped
 	}
 
-	for _, vol := range vols {
-		if err := r.RemoveVolume(ctx, vol, force); err != nil {
-			return deletedVols, err
-		}
-		logrus.Debugf("removed volume %s", vol.Name())
-		deletedVols = append(deletedVols, vol.Name())
+	vol, err := r.state.Volume(name)
+	if err != nil {
+		return nil, err
 	}
-	return deletedVols, nil
+
+	return vol, nil
 }
 
-// GetVolume retrieves a volume given its full name.
-func (r *Runtime) GetVolume(name string) (*Volume, error) {
+// LookupVolume retrieves a volume by unambigious partial name.
+func (r *Runtime) LookupVolume(name string) (*Volume, error) {
 	r.lock.RLock()
 	defer r.lock.RUnlock()
 
@@ -84,7 +68,7 @@ func (r *Runtime) GetVolume(name string) (*Volume, error) {
 		return nil, define.ErrRuntimeStopped
 	}
 
-	vol, err := r.state.Volume(name)
+	vol, err := r.state.LookupVolume(name)
 	if err != nil {
 		return nil, err
 	}
diff --git a/libpod/state.go b/libpod/state.go
index db4667ad6..40080d2cc 100644
--- a/libpod/state.go
+++ b/libpod/state.go
@@ -190,6 +190,9 @@ type State interface {
 	// Volume accepts full name of volume
 	// If the volume doesn't exist, an error will be returned
 	Volume(volName string) (*Volume, error)
+	// LookupVolume accepts an unambiguous partial name or full name of a
+	// volume. Ambiguous names will result in an error.
+	LookupVolume(name string) (*Volume, error)
 	// HasVolume returns true if volName exists in the state,
 	// otherwise it returns false
 	HasVolume(volName string) (bool, error)
diff --git a/pkg/adapter/runtime.go b/pkg/adapter/runtime.go
index dd15e1d15..fd6587505 100644
--- a/pkg/adapter/runtime.go
+++ b/pkg/adapter/runtime.go
@@ -196,8 +196,8 @@ func (r *LocalRuntime) CreateVolume(ctx context.Context, c *cliconfig.VolumeCrea
 }
 
 // RemoveVolumes is a wrapper to remove volumes
-func (r *LocalRuntime) RemoveVolumes(ctx context.Context, c *cliconfig.VolumeRmValues) ([]string, error) {
-	return r.Runtime.RemoveVolumes(ctx, c.InputArgs, c.All, c.Force)
+func (r *LocalRuntime) RemoveVolumes(ctx context.Context, c *cliconfig.VolumeRmValues) ([]string, map[string]error, error) {
+	return shared.SharedRemoveVolumes(ctx, r.Runtime, c.InputArgs, c.All, c.Force)
 }
 
 // Push is a wrapper to push an image to a registry
@@ -220,7 +220,7 @@ func (r *LocalRuntime) InspectVolumes(ctx context.Context, c *cliconfig.VolumeIn
 		volumes, err = r.GetAllVolumes()
 	} else {
 		for _, v := range c.InputArgs {
-			vol, err := r.GetVolume(v)
+			vol, err := r.LookupVolume(v)
 			if err != nil {
 				return nil, err
 			}
diff --git a/pkg/adapter/runtime_remote.go b/pkg/adapter/runtime_remote.go
index 718a6d542..f079b914a 100644
--- a/pkg/adapter/runtime_remote.go
+++ b/pkg/adapter/runtime_remote.go
@@ -622,13 +622,18 @@ func (r *LocalRuntime) CreateVolume(ctx context.Context, c *cliconfig.VolumeCrea
 }
 
 // RemoveVolumes removes volumes over a varlink connection for the remote client
-func (r *LocalRuntime) RemoveVolumes(ctx context.Context, c *cliconfig.VolumeRmValues) ([]string, error) {
+func (r *LocalRuntime) RemoveVolumes(ctx context.Context, c *cliconfig.VolumeRmValues) ([]string, map[string]error, error) {
 	rmOpts := iopodman.VolumeRemoveOpts{
 		All:     c.All,
 		Force:   c.Force,
 		Volumes: c.InputArgs,
 	}
-	return iopodman.VolumeRemove().Call(r.Conn, rmOpts)
+	success, failures, err := iopodman.VolumeRemove().Call(r.Conn, rmOpts)
+	stringsToErrors := make(map[string]error)
+	for k, v := range failures {
+		stringsToErrors[k] = errors.New(v)
+	}
+	return success, stringsToErrors, err
 }
 
 func (r *LocalRuntime) Push(ctx context.Context, srcName, destination, manifestMIMEType, authfile, digestfile, signaturePolicyPath string, writer io.Writer, forceCompress bool, signingOptions image.SigningOptions, dockerRegistryOptions *image.DockerRegistryOptions, additionalDockerArchiveTags []reference.NamedTagged) error {
diff --git a/pkg/varlinkapi/volumes.go b/pkg/varlinkapi/volumes.go
index 6dd86d831..b41eb5086 100644
--- a/pkg/varlinkapi/volumes.go
+++ b/pkg/varlinkapi/volumes.go
@@ -3,6 +3,7 @@
 package varlinkapi
 
 import (
+	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/cmd/podman/varlink"
 	"github.com/containers/libpod/libpod"
 )
@@ -32,11 +33,16 @@ func (i *LibpodAPI) VolumeCreate(call iopodman.VarlinkCall, options iopodman.Vol
 
 // VolumeRemove removes volumes by options.All or options.Volumes
 func (i *LibpodAPI) VolumeRemove(call iopodman.VarlinkCall, options iopodman.VolumeRemoveOpts) error {
-	deletedVolumes, err := i.Runtime.RemoveVolumes(getContext(), options.Volumes, options.All, options.Force)
+	success, failed, err := shared.SharedRemoveVolumes(getContext(), i.Runtime, options.Volumes, options.All, options.Force)
 	if err != nil {
 		return call.ReplyErrorOccurred(err.Error())
 	}
-	return call.ReplyVolumeRemove(deletedVolumes)
+	// Convert map[string]string to map[string]error
+	errStrings := make(map[string]string)
+	for k, v := range failed {
+		errStrings[k] = v.Error()
+	}
+	return call.ReplyVolumeRemove(success, errStrings)
 }
 
 // GetVolumes returns all the volumes known to the remote system
diff --git a/test/e2e/volume_rm_test.go b/test/e2e/volume_rm_test.go
index 5dcf51ccd..61cf9b893 100644
--- a/test/e2e/volume_rm_test.go
+++ b/test/e2e/volume_rm_test.go
@@ -89,4 +89,38 @@ var _ = Describe("Podman volume rm", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(len(session.OutputToStringArray())).To(Equal(0))
 	})
+
+	It("podman volume rm by partial name", func() {
+		session := podmanTest.Podman([]string{"volume", "create", "myvol"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"volume", "rm", "myv"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"volume", "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(len(session.OutputToStringArray())).To(Equal(0))
+	})
+
+	It("podman volume rm by nonunique partial name", func() {
+		session := podmanTest.Podman([]string{"volume", "create", "myvol1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"volume", "create", "myvol2"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"volume", "rm", "myv"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+
+		session = podmanTest.Podman([]string{"volume", "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(len(session.OutputToStringArray()) >= 2).To(BeTrue())
+	})
 })