Merge pull request #19671 from calavera/volume-dangling

Make volume dangling filter return only used volumes with `dangling=false`.

contrib/completion/bash/docker

@@ -1998,7 +1998,14 @@ _docker_volume_inspect() {
 _docker_volume_ls() {
 	case "$prev" in
 		--filter|-f)
-			COMPREPLY=( $( compgen -W "dangling=true" -- "$cur" ) )
+			COMPREPLY=( $( compgen -W "dangling" -- "$cur" ) )
+			return
+			;;
+	esac
+
+	case "${words[$cword-2]}$prev=" in
+		*dangling=*)
+			COMPREPLY=( $( compgen -W "true false" -- "${cur#=}" ) )
 			return
 			;;
 	esac

daemon/list.go

@@ -439,8 +439,8 @@ func (daemon *Daemon) Volumes(filter string) ([]*types.Volume, []string, error)
 	if err != nil {
 		return nil, nil, err
 	}
-	if danglingOnly {
-		volumes = daemon.volumes.FilterByUsed(volumes)
+	if volFilters.Include("dangling") {
+		volumes = daemon.volumes.FilterByUsed(volumes, !danglingOnly)
 	}
 	for _, v := range volumes {
 		volumesOut = append(volumesOut, volumeToAPIType(v))

integration-cli/docker_cli_volume_test.go

@@ -106,8 +106,8 @@ func (s *DockerSuite) TestVolumeCliLsFilterDangling(c *check.C) {
 
 	out, _ = dockerCmd(c, "volume", "ls", "--filter", "dangling=false")
 
-	// Same as above, but explicitly disabling dangling
-	c.Assert(out, checker.Contains, "testnotinuse1\n", check.Commentf("expected volume 'testnotinuse1' in output"))
+	// Explicitly disabling dangling
+	c.Assert(out, check.Not(checker.Contains), "testnotinuse1\n", check.Commentf("expected volume 'testnotinuse1' in output"))
 	c.Assert(out, checker.Contains, "testisinuse1\n", check.Commentf("expected volume 'testisinuse1' in output"))
 	c.Assert(out, checker.Contains, "testisinuse2\n", check.Commentf("expected volume 'testisinuse2' in output"))
 
@@ -126,7 +126,7 @@ func (s *DockerSuite) TestVolumeCliLsFilterDangling(c *check.C) {
 
 	out, _ = dockerCmd(c, "volume", "ls", "--filter", "dangling=0")
 	// dangling=0 is same as dangling=false case
-	c.Assert(out, checker.Contains, "testnotinuse1\n", check.Commentf("expected volume 'testnotinuse1' in output"))
+	c.Assert(out, check.Not(checker.Contains), "testnotinuse1\n", check.Commentf("expected volume 'testnotinuse1' in output"))
 	c.Assert(out, checker.Contains, "testisinuse1\n", check.Commentf("expected volume 'testisinuse1' in output"))
 	c.Assert(out, checker.Contains, "testisinuse2\n", check.Commentf("expected volume 'testisinuse2' in output"))
 }

volume/store/store.go

@@ -336,12 +336,18 @@ func (s *VolumeStore) FilterByDriver(name string) ([]volume.Volume, error) {
 	return ls, nil
 }
 
-// FilterByUsed returns the available volumes filtered by if they are not in use
-func (s *VolumeStore) FilterByUsed(vols []volume.Volume) []volume.Volume {
+// FilterByUsed returns the available volumes filtered by if they are in use or not.
+// `used=true` returns only volumes that are being used, while `used=false` returns
+// only volumes that are not being used.
+func (s *VolumeStore) FilterByUsed(vols []volume.Volume, used bool) []volume.Volume {
 	return s.filter(vols, func(v volume.Volume) bool {
 		s.locks.Lock(v.Name())
-		defer s.locks.Unlock(v.Name())
-		return len(s.refs[v.Name()]) == 0
+		l := len(s.refs[v.Name()])
+		s.locks.Unlock(v.Name())
+		if (used && l > 0) || (!used && l == 0) {
+			return true
+		}
+		return false
 	})
 }
 

volume/store/store_test.go

@@ -123,3 +123,37 @@ func TestFilterByDriver(t *testing.T) {
 		t.Fatalf("Expected 1 volume, got %v, %v", len(l), l)
 	}
 }
+
+func TestFilterByUsed(t *testing.T) {
+	volumedrivers.Register(vt.NewFakeDriver("fake"), "fake")
+	volumedrivers.Register(vt.NewFakeDriver("noop"), "noop")
+
+	s := New()
+	if _, err := s.CreateWithRef("fake1", "fake", "volReference", nil); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Create("fake2", "fake", nil); err != nil {
+		t.Fatal(err)
+	}
+
+	vols, _, err := s.List()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dangling := s.FilterByUsed(vols, false)
+	if len(dangling) != 1 {
+		t.Fatalf("expected 1 danging volume, got %v", len(dangling))
+	}
+	if dangling[0].Name() != "fake2" {
+		t.Fatalf("expected danging volume fake2, got %s", dangling[0].Name())
+	}
+
+	used := s.FilterByUsed(vols, true)
+	if len(used) != 1 {
+		t.Fatalf("expected 1 used volume, got %v", len(used))
+	}
+	if used[0].Name() != "fake1" {
+		t.Fatalf("expected used volume fake1, got %s", used[0].Name())
+	}
+}