Merge pull request #44216 from cpuguy83/volume_unnamed_label

Volume prune: only prune anonymous volumes by default
This commit is contained in:
Sebastiaan van Stijn 2022-10-05 19:34:47 +02:00 committed by GitHub
commit 49940ab5ee
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 179 additions and 9 deletions

View file

@ -187,6 +187,12 @@ func (v *volumeRouter) postVolumesPrune(ctx context.Context, w http.ResponseWrit
return err
}
// API version 1.42 changes behavior where prune should only prune anonymous volumes.
// To keep older API behavior working, we need to add this filter option to consider all (local) volumes for pruning, not just anonymous ones.
if versions.LessThan(httputils.VersionFromContext(ctx), "1.42") {
pruneFilters.Add("all", "true")
}
pruneReport, err := v.backend.Prune(ctx, pruneFilters)
if err != nil {
return err

View file

@ -9700,6 +9700,7 @@ paths:
Available filters:
- `label` (`label=<key>`, `label=<key>=<value>`, `label!=<key>`, or `label!=<key>=<value>`) Prune volumes with (or without, in case `label!=...` is used) the specified labels.
- `all` (`all=true`) - Consider all (local) volumes for pruning and not just anonymous volumes.
type: "string"
responses:
200:

View file

@ -9699,6 +9699,7 @@ paths:
Available filters:
- `label` (`label=<key>`, `label=<key>=<value>`, `label!=<key>`, or `label!=<key>=<value>`) Prune volumes with (or without, in case `label!=...` is used) the specified labels.
- `all` (`all=true`) - Consider all (local) volumes for pruning and not just anonymous volumes.
type: "string"
responses:
200:

View file

@ -125,6 +125,7 @@ keywords: "API, Docker, rcli, REST, documentation"
is set with a non-matching mount Type.
* `POST /containers/{id}/exec` now accepts an optional `ConsoleSize` parameter.
It allows to set the console size of the executed process immediately when it's created.
* `POST /volumes/prune` will now only prune "anonymous" volumes (volumes which were not given a name) by default. A new filter parameter `all` can be set to a truth-y value (`true`, `1`) to get the old behavior.
## v1.41 API changes

View file

@ -16,7 +16,8 @@ source hack/make/.integration-test-helpers
# --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream
# TODO re-enable test_attach_no_stream after https://github.com/docker/docker-py/issues/2513 is resolved
# TODO re-enable test_create_with_device_cgroup_rules after https://github.com/docker/docker-py/issues/2939 is resolved
: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::CreateContainerTest::test_create_with_device_cgroup_rules}"
# TODO re-enable test_prune_volumes after https://github.com/docker/docker-py/pull/3051 is resolved
: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::CreateContainerTest::test_create_with_device_cgroup_rules --deselect=tests/integration/api_volume_test.py::TestVolumes::test_prune_volumes}"
(
bundle .integration-daemon-start

View file

@ -9,11 +9,14 @@ import (
"time"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/volume"
clientpkg "github.com/docker/docker/client"
"github.com/docker/docker/integration/internal/container"
"github.com/docker/docker/testutil/request"
"github.com/google/go-cmp/cmp/cmpopts"
"gotest.tools/v3/assert"
"gotest.tools/v3/assert/cmp"
is "gotest.tools/v3/assert/cmp"
)
@ -164,3 +167,54 @@ func getPrefixAndSlashFromDaemonPlatform() (prefix, slash string) {
}
return "", "/"
}
func TestVolumePruneAnonymous(t *testing.T) {
defer setupTest(t)()
client := testEnv.APIClient()
ctx := context.Background()
// Create an anonymous volume
v, err := client.VolumeCreate(ctx, volume.CreateOptions{})
assert.NilError(t, err)
// Create a named volume
vNamed, err := client.VolumeCreate(ctx, volume.CreateOptions{
Name: "test",
})
assert.NilError(t, err)
// Prune anonymous volumes
pruneReport, err := client.VolumesPrune(ctx, filters.Args{})
assert.NilError(t, err)
assert.Check(t, is.Equal(len(pruneReport.VolumesDeleted), 1))
assert.Check(t, is.Equal(pruneReport.VolumesDeleted[0], v.Name))
_, err = client.VolumeInspect(ctx, vNamed.Name)
assert.NilError(t, err)
// Prune all volumes
_, err = client.VolumeCreate(ctx, volume.CreateOptions{})
assert.NilError(t, err)
pruneReport, err = client.VolumesPrune(ctx, filters.NewArgs(filters.Arg("all", "1")))
assert.NilError(t, err)
assert.Check(t, is.Equal(len(pruneReport.VolumesDeleted), 2))
// Validate that older API versions still have the old behavior of pruning all local volumes
clientOld, err := clientpkg.NewClientWithOpts(clientpkg.FromEnv, clientpkg.WithVersion("1.41"))
assert.NilError(t, err)
defer clientOld.Close()
assert.Equal(t, clientOld.ClientVersion(), "1.41")
v, err = client.VolumeCreate(ctx, volume.CreateOptions{})
assert.NilError(t, err)
vNamed, err = client.VolumeCreate(ctx, volume.CreateOptions{Name: "test-api141"})
assert.NilError(t, err)
pruneReport, err = clientOld.VolumesPrune(ctx, filters.Args{})
assert.NilError(t, err)
assert.Check(t, is.Equal(len(pruneReport.VolumesDeleted), 2))
assert.Check(t, cmp.Contains(pruneReport.VolumesDeleted, v.Name))
assert.Check(t, cmp.Contains(pruneReport.VolumesDeleted, vNamed.Name))
}

View file

@ -2,10 +2,13 @@ package service
import (
"context"
"fmt"
"strconv"
"time"
"github.com/docker/docker/api/types/filters"
volumetypes "github.com/docker/docker/api/types/volume"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/pkg/directory"
"github.com/docker/docker/volume"
"github.com/sirupsen/logrus"
@ -130,3 +133,22 @@ func filtersToBy(filter filters.Args, acceptedFilters map[string]bool) (By, erro
}
return by, nil
}
func withPrune(filter filters.Args) error {
all := filter.Get("all")
switch {
case len(all) > 1:
return invalidFilter{"all", all}
case len(all) == 1:
ok, err := strconv.ParseBool(all[0])
if err != nil {
return errdefs.InvalidParameter(fmt.Errorf("invalid filter 'all': %w", err))
}
if ok {
return nil
}
}
filter.Add("label", AnonymousLabel)
return nil
}

View file

@ -0,0 +1,63 @@
package service
import (
"testing"
"github.com/docker/docker/api/types/filters"
"gotest.tools/v3/assert"
"gotest.tools/v3/assert/cmp"
)
func TestFilterWithPrune(t *testing.T) {
f := filters.NewArgs()
assert.NilError(t, withPrune(f))
assert.Check(t, cmp.Len(f.Get("label"), 1))
assert.Check(t, f.Match("label", AnonymousLabel))
f = filters.NewArgs()
f.Add("label", "foo=bar")
f.Add("label", "bar=baz")
assert.NilError(t, withPrune(f))
assert.Check(t, cmp.Len(f.Get("label"), 3))
assert.Check(t, f.Match("label", AnonymousLabel))
assert.Check(t, f.Match("label", "foo=bar"))
assert.Check(t, f.Match("label", "bar=baz"))
f = filters.NewArgs()
f.Add("label", "foo=bar")
f.Add("all", "1")
assert.NilError(t, withPrune(f))
assert.Check(t, cmp.Len(f.Get("label"), 1))
assert.Check(t, f.Match("label", "foo=bar"))
f = filters.NewArgs()
f.Add("label", "foo=bar")
f.Add("all", "true")
assert.NilError(t, withPrune(f))
assert.Check(t, cmp.Len(f.Get("label"), 1))
assert.Check(t, f.Match("label", "foo=bar"))
f = filters.NewArgs()
f.Add("all", "0")
assert.NilError(t, withPrune(f))
assert.Check(t, cmp.Len(f.Get("label"), 1))
assert.Check(t, f.Match("label", AnonymousLabel))
f = filters.NewArgs()
f.Add("all", "false")
assert.NilError(t, withPrune(f))
assert.Check(t, cmp.Len(f.Get("label"), 1))
assert.Check(t, f.Match("label", AnonymousLabel))
f = filters.NewArgs()
f.Add("all", "")
assert.ErrorContains(t, withPrune(f), "invalid filter 'all'")
f = filters.NewArgs()
f.Add("all", "1")
f.Add("all", "0")
assert.ErrorContains(t, withPrune(f), "invalid filter 'all")
}

View file

@ -11,6 +11,16 @@ type CreateConfig struct {
Reference string
}
// WithCreateLabel creates a CreateOption which adds a label with the given key/value pair
func WithCreateLabel(key, value string) CreateOption {
return func(cfg *CreateConfig) {
if cfg.Labels == nil {
cfg.Labels = map[string]string{}
}
cfg.Labels[key] = value
}
}
// WithCreateLabels creates a CreateOption which sets the labels to the
// passed in value
func WithCreateLabels(labels map[string]string) CreateOption {

View file

@ -60,6 +60,10 @@ func (s *VolumesService) GetDriverList() []string {
return s.ds.GetDriverList()
}
// AnonymousLabel is the label used to indicate that a volume is anonymous
// This is set automatically on a volume when a volume is created without a name specified, and as such an id is generated for it.
const AnonymousLabel = "com.docker.volume.anonymous"
// Create creates a volume
// If the caller is creating this volume to be consumed immediately, it is
// expected that the caller specifies a reference ID.
@ -67,11 +71,12 @@ func (s *VolumesService) GetDriverList() []string {
//
// A good example for a reference ID is a container's ID.
// When whatever is going to reference this volume is removed the caller should defeference the volume by calling `Release`.
func (s *VolumesService) Create(ctx context.Context, name, driverName string, opts ...opts.CreateOption) (*volumetypes.Volume, error) {
func (s *VolumesService) Create(ctx context.Context, name, driverName string, options ...opts.CreateOption) (*volumetypes.Volume, error) {
if name == "" {
name = stringid.GenerateRandomID()
options = append(options, opts.WithCreateLabel(AnonymousLabel, ""))
}
v, err := s.vs.Create(ctx, name, driverName, opts...)
v, err := s.vs.Create(ctx, name, driverName, options...)
if err != nil {
return nil, err
}
@ -171,6 +176,8 @@ func (s *VolumesService) Remove(ctx context.Context, name string, rmOpts ...opts
var acceptedPruneFilters = map[string]bool{
"label": true,
"label!": true,
// All tells the filter to consider all volumes not just anonymous ones.
"all": true,
}
var acceptedListFilters = map[string]bool{
@ -215,6 +222,10 @@ func (s *VolumesService) Prune(ctx context.Context, filter filters.Args) (*types
}
defer atomic.StoreInt32(&s.pruneRunning, 0)
if err := withPrune(filter); err != nil {
return nil, err
}
by, err := filtersToBy(filter, acceptedPruneFilters)
if err != nil {
return nil, err

View file

@ -172,11 +172,11 @@ func TestServicePrune(t *testing.T) {
_, err = service.Create(ctx, "test2", "other")
assert.NilError(t, err)
pr, err := service.Prune(ctx, filters.NewArgs(filters.Arg("label", "banana")))
pr, err := service.Prune(ctx, filters.NewArgs(filters.Arg("label", "banana"), filters.Arg("all", "true")))
assert.NilError(t, err)
assert.Assert(t, is.Len(pr.VolumesDeleted, 0))
pr, err = service.Prune(ctx, filters.NewArgs())
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("all", "true")))
assert.NilError(t, err)
assert.Assert(t, is.Len(pr.VolumesDeleted, 1))
assert.Assert(t, is.Equal(pr.VolumesDeleted[0], "test"))
@ -191,7 +191,7 @@ func TestServicePrune(t *testing.T) {
_, err = service.Create(ctx, "test", volume.DefaultDriverName)
assert.NilError(t, err)
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("label!", "banana")))
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("label!", "banana"), filters.Arg("all", "true")))
assert.NilError(t, err)
assert.Assert(t, is.Len(pr.VolumesDeleted, 1))
assert.Assert(t, is.Equal(pr.VolumesDeleted[0], "test"))
@ -207,12 +207,12 @@ func TestServicePrune(t *testing.T) {
_, err = service.Create(ctx, "test3", volume.DefaultDriverName, opts.WithCreateLabels(map[string]string{"banana": "split"}))
assert.NilError(t, err)
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("label!", "banana=split")))
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("label!", "banana=split"), filters.Arg("all", "true")))
assert.NilError(t, err)
assert.Assert(t, is.Len(pr.VolumesDeleted, 1))
assert.Assert(t, is.Equal(pr.VolumesDeleted[0], "test"))
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("label", "banana=split")))
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("label", "banana=split"), filters.Arg("all", "true")))
assert.NilError(t, err)
assert.Assert(t, is.Len(pr.VolumesDeleted, 1))
assert.Assert(t, is.Equal(pr.VolumesDeleted[0], "test3"))
@ -225,7 +225,7 @@ func TestServicePrune(t *testing.T) {
assert.Assert(t, is.Len(pr.VolumesDeleted, 0))
assert.Assert(t, service.Release(ctx, v.Name, t.Name()))
pr, err = service.Prune(ctx, filters.NewArgs())
pr, err = service.Prune(ctx, filters.NewArgs(filters.Arg("all", "true")))
assert.NilError(t, err)
assert.Assert(t, is.Len(pr.VolumesDeleted, 1))
assert.Assert(t, is.Equal(pr.VolumesDeleted[0], "test"))