From 1431b623a4809ec3992a357387037366ab083548 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Mon, 25 Jan 2016 14:39:41 -0500 Subject: [PATCH] Make volume dangling filter return only used volumes with `dangling=false`. Signed-off-by: David Calavera --- daemon/list.go | 4 +-- integration-cli/docker_cli_volume_test.go | 6 ++-- volume/store/store.go | 14 +++++++--- volume/store/store_test.go | 34 +++++++++++++++++++++++ 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/daemon/list.go b/daemon/list.go index 4bc844a4cb..5aebb7472d 100644 --- a/daemon/list.go +++ b/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)) diff --git a/integration-cli/docker_cli_volume_test.go b/integration-cli/docker_cli_volume_test.go index 7c08e393aa..5a19ba539e 100644 --- a/integration-cli/docker_cli_volume_test.go +++ b/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")) } diff --git a/volume/store/store.go b/volume/store/store.go index 9811fd904a..dace845ad8 100644 --- a/volume/store/store.go +++ b/volume/store/store.go @@ -326,12 +326,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 }) } diff --git a/volume/store/store_test.go b/volume/store/store_test.go index 652feaa594..83d49821be 100644 --- a/volume/store/store_test.go +++ b/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()) + } +}