Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jan 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion contrib/completion/bash/docker
Original file line number Diff line number Diff line change
Expand Up @@ -1988,7 +1988,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
Expand Down
4 changes: 2 additions & 2 deletions daemon/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions integration-cli/docker_cli_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess what I don't quite understand is this change in case.... I suppose this false wasn't working before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before this change false meant all volumes, dangling and undangling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, makes sense.

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"))

Expand All @@ -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"))
}
Expand Down
14 changes: 10 additions & 4 deletions volume/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}

Expand Down
34 changes: 34 additions & 0 deletions volume/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}