Skip to content

Commit

Permalink
Merge pull request #785 from vrothberg/prune
Browse files Browse the repository at this point in the history
libimage: prune: allow for removing external containers
  • Loading branch information
openshift-merge-robot authored Sep 28, 2021
2 parents ab0bafe + e392779 commit 32e2029
Show file tree
Hide file tree
Showing 63 changed files with 184 additions and 225 deletions.
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.0
github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635
github.com/vbauerster/mpb/v7 v7.1.4 // indirect
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/sys v0.0.0-20210820121016-41cdb8703e55
golang.org/x/sys v0.0.0-20210906170528-6f6e22806c34
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1
)
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,9 @@ github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtX
github.com/urfave/cli v1.22.4/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
github.com/vbatts/tar-split v0.11.2 h1:Via6XqJr0hceW4wff3QRzD5gAk/tatMw/4ZA7cTlIME=
github.com/vbatts/tar-split v0.11.2/go.mod h1:vV3ZuO2yWSVsz+pfFzDG/upWH1JhjOiEaWq6kXyQ3VI=
github.com/vbauerster/mpb/v7 v7.1.3 h1:VJkiLuuBs/re5SCHLVkYOPYAs+1jagk5QIDHgAXLVVA=
github.com/vbauerster/mpb/v7 v7.1.3/go.mod h1:X5GlohZw2fIpypMXWaKart+HGSAjpz49skxkDk+ZL7c=
github.com/vbauerster/mpb/v7 v7.1.4 h1:XGWpWEB8aWnvqSlAMA7F7kdeUGqcTujuVFvYj9+59Ww=
github.com/vbauerster/mpb/v7 v7.1.4/go.mod h1:4zulrZfvshMOnd2APiHgWS9Yrw08AzZVRr9G11tkpcQ=
github.com/vishvananda/netlink v0.0.0-20181108222139-023a6dafdcdf/go.mod h1:+SR5DhBJrl6ZM7CoCKvpw5BKroDKQ+PJqOg65H/2ktk=
github.com/vishvananda/netlink v1.1.0/go.mod h1:cTgwzPIzzgDAYoQrMm0EdrjRUBkTqKYppBueQtXaqoE=
github.com/vishvananda/netlink v1.1.1-0.20201029203352-d40f9887b852/go.mod h1:twkDnbuQxJYemMlGd4JFIcuhgX83tXhKS2B/PRMpOho=
Expand Down Expand Up @@ -1041,8 +1042,9 @@ golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210426230700-d19ff857e887/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210820121016-41cdb8703e55 h1:rw6UNGRMfarCepjI8qOepea/SXwIBVfTKjztZ5gBbq4=
golang.org/x/sys v0.0.0-20210820121016-41cdb8703e55/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210906170528-6f6e22806c34 h1:GkvMjFtXUmahfDtashnc1mnrCtuBVcwse5QV2lUk/tI=
golang.org/x/sys v0.0.0-20210906170528-6f6e22806c34/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
38 changes: 29 additions & 9 deletions libimage/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ func filterImages(images []*Image, filters []filterFunc) ([]*Image, error) {
// compileImageFilters creates `filterFunc`s for the specified filters. The
// required format is `key=value` with the following supported keys:
// after, since, before, containers, dangling, id, label, readonly, reference, intermediate
func (r *Runtime) compileImageFilters(ctx context.Context, filters []string) ([]filterFunc, error) {
logrus.Tracef("Parsing image filters %s", filters)
func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOptions) ([]filterFunc, error) {
logrus.Tracef("Parsing image filters %s", options.Filters)

filterFuncs := []filterFunc{}
for _, filter := range filters {
for _, filter := range options.Filters {
var key, value string
split := strings.SplitN(filter, "=", 2)
if len(split) != 2 {
Expand All @@ -77,11 +77,16 @@ func (r *Runtime) compileImageFilters(ctx context.Context, filters []string) ([]
filterFuncs = append(filterFuncs, filterBefore(img.Created()))

case "containers":
containers, err := strconv.ParseBool(value)
if err != nil {
return nil, errors.Wrapf(err, "non-boolean value %q for dangling filter", value)
switch value {
case "false", "true":
case "external":
if options.IsExternalContainerFunc == nil {
return nil, fmt.Errorf("libimage error: external containers filter without callback")
}
default:
return nil, fmt.Errorf("unsupported value %q for containers filter", value)
}
filterFuncs = append(filterFuncs, filterContainers(containers))
filterFuncs = append(filterFuncs, filterContainers(value, options.IsExternalContainerFunc))

case "dangling":
dangling, err := strconv.ParseBool(value)
Expand Down Expand Up @@ -190,13 +195,28 @@ func filterReadOnly(value bool) filterFunc {
}

// filterContainers creates a container filter for matching the specified value.
func filterContainers(value bool) filterFunc {
func filterContainers(value string, fn IsExternalContainerFunc) filterFunc {
return func(img *Image) (bool, error) {
ctrs, err := img.Containers()
if err != nil {
return false, err
}
return (len(ctrs) > 0) == value, nil
if value != "external" {
boolValue := value == "true"
return (len(ctrs) > 0) == boolValue, nil
}

// Check whether all associated containers are external ones.
for _, c := range ctrs {
isExternal, err := fn(c)
if err != nil {
return false, fmt.Errorf("checking if %s is an external container in filter: %w", c, err)
}
if !isExternal {
return isExternal, nil
}
}
return true, nil
}
}

Expand Down
34 changes: 25 additions & 9 deletions libimage/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package libimage

import (
"context"
"fmt"
"path/filepath"
"sort"
"strings"
Expand Down Expand Up @@ -232,11 +233,15 @@ func (i *Image) Containers() ([]string, error) {
}

// removeContainers removes all containers using the image.
func (i *Image) removeContainers(fn RemoveContainerFunc) error {
// Execute the custom removal func if specified.
if fn != nil {
func (i *Image) removeContainers(options *RemoveImagesOptions) error {
if !options.Force && !options.ExternalContainers {
// Nothing to do.
return nil
}

if options.Force && options.RemoveContainerFunc != nil {
logrus.Debugf("Removing containers of image %s with custom removal function", i.ID())
if err := fn(i.ID()); err != nil {
if err := options.RemoveContainerFunc(i.ID()); err != nil {
return err
}
}
Expand All @@ -246,6 +251,19 @@ func (i *Image) removeContainers(fn RemoveContainerFunc) error {
return err
}

if !options.Force && options.ExternalContainers {
// All containers must be external ones.
for _, cID := range containers {
isExternal, err := options.IsExternalContainerFunc(cID)
if err != nil {
return fmt.Errorf("checking if %s is an external container: %w", cID, err)
}
if !isExternal {
return fmt.Errorf("cannot remove container %s: not an external container", cID)
}
}
}

logrus.Debugf("Removing containers of image %s from the local containers storage", i.ID())
var multiE error
for _, cID := range containers {
Expand Down Expand Up @@ -392,11 +410,9 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma
return processedIDs, nil
}

// Perform the actual removal. First, remove containers if needed.
if options.Force {
if err := i.removeContainers(options.RemoveContainerFunc); err != nil {
return processedIDs, err
}
// Perform the container removal, if needed.
if err := i.removeContainers(options); err != nil {
return processedIDs, err
}

// Podman/Docker compat: we only report an image as removed if it has
Expand Down
11 changes: 7 additions & 4 deletions libimage/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,13 @@ func TestImageFunctions(t *testing.T) {
require.Nil(t, containers)

// Since we have no containers here, we can only smoke test.
require.NoError(t, image.removeContainers(nil))
require.Error(t, image.removeContainers(func(_ string) error {
return errors.New("TEST")
}))
rmOptions := &RemoveImagesOptions{
RemoveContainerFunc: func(_ string) error {
return errors.New("TEST")
},
Force: true,
}
require.Error(t, image.removeContainers(rmOptions))

// Two items since both names are "Named".
namedRepoTags, err := image.NamedRepoTags()
Expand Down
33 changes: 31 additions & 2 deletions libimage/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package libimage

import (
"context"
"fmt"
"os"
"strings"

Expand Down Expand Up @@ -484,17 +485,28 @@ func (r *Runtime) imageReferenceMatchesContext(ref types.ImageReference, options
return true, nil
}

// IsExternalContainerFunc allows for checking whether the specified container
// is an external one. The definition of an external container can be set by
// callers.
type IsExternalContainerFunc func(containerID string) (bool, error)

// ListImagesOptions allow for customizing listing images.
type ListImagesOptions struct {
// Filters to filter the listed images. Supported filters are
// * after,before,since=image
// * containers=true,false,external
// * dangling=true,false
// * intermediate=true,false (useful for pruning images)
// * id=id
// * label=key[=value]
// * readonly=true,false
// * reference=name[:tag] (wildcards allowed)
Filters []string
// IsExternalContainerFunc allows for checking whether the specified
// container is an external one (when containers=external filter is
// used). The definition of an external container can be set by
// callers.
IsExternalContainerFunc IsExternalContainerFunc
}

// ListImages lists images in the local container storage. If names are
Expand Down Expand Up @@ -525,7 +537,7 @@ func (r *Runtime) ListImages(ctx context.Context, names []string, options *ListI

var filters []filterFunc
if len(options.Filters) > 0 {
compiledFilters, err := r.compileImageFilters(ctx, options.Filters)
compiledFilters, err := r.compileImageFilters(ctx, options)
if err != nil {
return nil, err
}
Expand All @@ -550,8 +562,17 @@ type RemoveImagesOptions struct {
// containers using a specific image. By default, all containers in
// the local containers storage will be removed (if Force is set).
RemoveContainerFunc RemoveContainerFunc
// IsExternalContainerFunc allows for checking whether the specified
// container is an external one (when containers=external filter is
// used). The definition of an external container can be set by
// callers.
IsExternalContainerFunc IsExternalContainerFunc
// Remove external containers even when Force is false. Requires
// IsExternalContainerFunc to be specified.
ExternalContainers bool
// Filters to filter the removed images. Supported filters are
// * after,before,since=image
// * containers=true,false,external
// * dangling=true,false
// * intermediate=true,false (useful for pruning images)
// * id=id
Expand Down Expand Up @@ -581,6 +602,10 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem
options = &RemoveImagesOptions{}
}

if options.ExternalContainers && options.IsExternalContainerFunc == nil {
return nil, []error{fmt.Errorf("libimage error: cannot remove external containers without callback")}
}

// The logic here may require some explanation. Image removal is
// surprisingly complex since it is recursive (intermediate parents are
// removed) and since multiple items in `names` may resolve to the
Expand Down Expand Up @@ -635,7 +660,11 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem
}

default:
filteredImages, err := r.ListImages(ctx, nil, &ListImagesOptions{Filters: options.Filters})
options := &ListImagesOptions{
IsExternalContainerFunc: options.IsExternalContainerFunc,
Filters: options.Filters,
}
filteredImages, err := r.ListImages(ctx, nil, options)
if err != nil {
appendError(err)
return nil, rmErrors
Expand Down
36 changes: 19 additions & 17 deletions vendor/github.com/vbauerster/mpb/v7/bar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/github.com/vbauerster/mpb/v7/go.mod

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/github.com/vbauerster/mpb/v7/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions vendor/golang.org/x/sys/unix/syscall_illumos.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 5 additions & 25 deletions vendor/golang.org/x/sys/unix/syscall_linux.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 32e2029

Please sign in to comment.